Jens Lehmann wrote:
> To be able to access all commits of populated submodules referenced by the
> superproject it is sufficient to only then let "git fetch" recurse into a
> submodule when the new commits fetched in the superproject record new
> commits for it.
Exciting stuff. This would use the default refspec rather than fetching
the bare minimum of commits in submodules, right?
> There is currently no infrastructure for storing deleted and new
> submodules in the .git directory of the superproject. Thats why fetch and
> pull for now only fetch submodules that are already checked out and are
> not renamed.
Ah.
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -73,6 +73,14 @@ ifndef::git-pull[]
> Prepend <path> to paths printed in informative messages
> such as "Fetching submodule foo". This option is used
> internally when recursing over submodules.
> +
> +--submodule-default=[yes|on-demand]::
> + This option is used internally to set the submodule recursion default
> + to either a boolean configuration value representing "true" (for
> + unconditonal recursion) or to "on-demand" (when only those submodules
> + should be fetched of which new commits have been fetched in its
> + superproject).
> + Using the "--[no-]recurse-submodules" option ignores this setting.
Could this be combined with the --recurse-submodules, with a "last instance
of the option wins" rule? Something like:
--recurse-submodules[=(yes | no | changed)]::
--no-recurse-submodules::
> +++ b/submodule.c
[...]
> @@ -248,11 +263,73 @@ void set_config_fetch_recurse_submodules(int value)
[...]
> +static void submodule_collect_changed_cb(struct diff_queue_struct *q,
[...]
> + if (S_ISGITLINK(p->one->mode)) {
> + /* NEEDSWORK: We should honor the name configured in
> + * the .gitmodules file of the commit we are examining
> + * here to be able to correctly follow submodules
> + * being moved around. */
Hmm, a sort of variant on rename detection. Does "git submodule update" /
"git checkout --recurse-submodules" use .gitmodules to move pre-populated
subrepositories?
> + /* Submodule is new or was moved here */
> + /* NEEDSWORK: When the .git directories of submodules
> + * live inside the superprojects .git directory some
> + * day we should fetch new submodules directly into
> + * that location too when config or options request
> + * that so they can be checked out from there. */
> + continue;
Maybe this can be mentioned in a BUGS section on the git-fetch(1)
manpage to give readers a warning and clue about the intended
meaning of --recurse-submodules?
I'm afraid a certain kind of person might get used to the "don't fetch
new submodules" behavior (e.g., if upstream is including unneeded
convenience copies of libraries right and left in addition to some
useful submodules).
> +void check_for_new_submodule_commits(unsigned char new_sha1[20])
> +{
> + struct rev_info rev;
> + struct commit *commit;
> + int argc = 5;
> + const char *argv[] = {NULL, NULL, "--not", "--branches", "--remotes", NULL};
Nit: maybe
const char *argv[] = ...;
int argc = ARRAY_SIZE(argv) - 1;
?
> +
> + init_revisions(&rev, NULL);
> + argv[1] = xstrdup(sha1_to_hex(new_sha1));
Maybe:
char *new_rev;
...
argv[1] = new_rev = xstrdup(...);
...
free(new_rev);
> + setup_revisions(argc, argv, &rev, NULL);
> + if (prepare_revision_walk(&rev))
> + die("revision walk setup failed");
> +
Maybe a comment so the reader doesn't have to delve deeper?
/*
* Collect checked out submodules that have changed upstream
* in "changed_submodule_paths".
*/
> + while ((commit = get_revision(&rev))) {
[...]
> +++ b/t/t5526-fetch-submodules.sh
> @@ -192,4 +192,113 @@ test_expect_success "--no-recurse-submodules overrides config setting" '
[...]
> + echo "Fetching submodule submodule" > expect.out.sub &&
> + echo "From $pwd/." > expect.err.sub &&
> + echo " $head1..$head2 master -> origin/master" >> expect.err.sub
I wonder if we should be testing the output in this much detail.
Wouldn't it be nicer to check the remote-tracking refs to make sure
the lasting effects were correct, rather than the detailed output
format?
So: aside from the option name, nothing but nits. Thanks; that was
fun.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html