{"thread":{"id":"26584","subject":"[PATCH 0/6] Teach fetch/pull the on-demand mode and make it the default","startedAt":"2011-02-23T20:33:41Z","lastAt":"2011-02-24T20:44:30Z","messageCount":23,"participants":["Jens Lehmann","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"162052","messageId":"4D656F25.5090007@web.de","threadId":"26584","inReplyTo":null,"subject":"[PATCH 0/6] Teach fetch/pull the on-demand mode and make it the default","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T20:33:41Z","receivedAt":"2011-02-23T20:33:41Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"So here is my patch series to implement the new default \"on-demand\"\nfetch mode and let \"git submodule update\" only fetch submodules when\nthe commit to check out is not already present in the submodule.\nThis is only done for populated submodules and prepares them for\ndisconnected operation without having to think about doing a\nrecursive fetch before going on a plane and having to add the '-N'\noption to \"git submodule update\" while on it. Also merging submodule\ncommits in the superproject and \"git diff --submodule\" (which is\nused by git gui and gitk) depend on the presence of those commits\nreferenced by the superproject to work.\n\nThe last commit is slightly orthogonal as it would be useful without\non-demand fetch too. With it rewinding a submodule to a commit that\nis already present (e.g. because it was checked out earlier) will be\ndone without issuing a fetch even in current git. Together with the\nother patches \"git submodule update\" won't have to fetch a submodule\nat all anymore (at least unless the new \"on-demand\" default is\ndisabled through the also added configuration options).\n\nChanges since the last version:\n(see http://article.gmane.org/gmane.comp.version-control.git/163429)\n\n*) Added the \"on-demand\" value to the \"--recurse-submodules\" command\n   line option and the \"submodule.<name>.fetchRecurseSubmodules\" and\n   \"fetch.recurseSubmodules\" config options.\n*) The fetch is only done when the recorded submodule commit isn't\n   already present.\n*) Added test cases.\n\nThere are two things that could be optimized, but that can be the\ntopic of other patches:\n\n*) The submodule fetches could be done in parallel.\n*) It might be expensive to compute the newly fetched commits. If\n   the transport helper could be asked for the list commits it just\n   fetched that would go away.\n\nI tend to think that this is suited for 1.7.5 but don't have any\nobjections against holding it back until 1.8.0 either. What do\nothers think?\n\nJens Lehmann (6):\n  fetch/pull: recurse into submodules when necessary\n  fetch/pull: Add the 'on-demand' value to the --recurse-submodules\n    option\n  config: teach the fetch.recurseSubmodules option the 'on-demand'\n    value\n  Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule'\n    option\n  fetch/pull: Don't recurse into a submodule when commits are already\n    present\n  submodule update: Don't fetch when the submodule commit is already\n    present\n\n Documentation/config.txt        |   12 ++-\n Documentation/fetch-options.txt |   24 +++-\n Documentation/git-pull.txt      |    2 +-\n Documentation/gitmodules.txt    |    4 +-\n builtin/fetch.c                 |   49 ++++++--\n git-pull.sh                     |    3 +\n git-submodule.sh                |    2 +-\n submodule.c                     |  124 +++++++++++++++++--\n submodule.h                     |   11 ++-\n t/t5526-fetch-submodules.sh     |  255 +++++++++++++++++++++++++++++++++++++++\n t/t7403-submodule-sync.sh       |    2 +-\n t/t7406-submodule-update.sh     |   20 +++\n 12 files changed, 471 insertions(+), 37 deletions(-)\n\n-- \n1.7.4.1.190.g13e20\n"},{"id":"162053","messageId":"4D656F4D.6080401@web.de","threadId":"26584","inReplyTo":"4D656F25.5090007@web.de","subject":"[PATCH 1/6] fetch/pull: recurse into submodules when necessary","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T20:34:21Z","receivedAt":"2011-02-23T20:34:21Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"To be able to access all commits of populated submodules referenced by the\nsuperproject it is sufficient to only then let \"git fetch\" recurse into a\nsubmodule when the new commits fetched in the superproject record new\ncommits for it. Having these commits present is extremely useful when\nusing the \"--submodule\" option to \"git diff\" (which is what \"git gui\" and\n\"gitk\" do since 1.6.6), as all submodule commits needed for creating a\ndescriptive output can be accessed. Also merging submodule commits (added\nin 1.7.3) depends on the submodule commits in question being present to\nwork. Last but not least this enables disconnected operation when using\nsubmodules, as all commits necessary for a successful \"git submodule\nupdate -N\" will have been fetched automatically. So we choose this mode as\nthe default for fetch and pull.\n\nBefore a new or changed ref from upstream is updated in update_local_ref()\n\"git rev-list <new-sha1> --not --branches --remotes\" is used to determine\nall newly fetched commits. These are then walked and diffed against their\nparent(s) to see if a submodule has been changed. If that is the case, its\npath is stored to be fetched after the superproject fetch is completed.\n\nUsing the \"--recurse-submodules\" or the \"--no-recurse-submodules\" option\ndisables the examination of the fetched refs because the result will be\nignored anyway.\n\nThere is currently no infrastructure for storing deleted and new\nsubmodules in the .git directory of the superproject. Thats why fetch and\npull for now only fetch submodules that are already checked out and are\nnot renamed.\n\nIn t7403 the \"--no-recurse-submodules\" argument had to be added to \"git\npull\" to avoid failure because of the moved upstream submodule repo.\n\nThanks-To: Jonathan Nieder <jrnieder@gmail.com>\nThanks-To: Heiko Voigt <hvoigt@hvoigt.net>\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n Documentation/fetch-options.txt |    8 +++\n builtin/fetch.c                 |   25 ++++++---\n submodule.c                     |   98 +++++++++++++++++++++++++++++++++--\n submodule.h                     |    9 +++\n t/t5526-fetch-submodules.sh     |  109 +++++++++++++++++++++++++++++++++++++++\n t/t7403-submodule-sync.sh       |    2 +-\n 6 files changed, 235 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex f37276e..1d770d9 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -73,6 +73,14 @@ ifndef::git-pull[]\n \tPrepend <path> to paths printed in informative messages\n \tsuch as \"Fetching submodule foo\".  This option is used\n \tinternally when recursing over submodules.\n+\n+--submodule-default=[yes|on-demand]::\n+\tThis option is used internally to set the submodule recursion default\n+\tto either a boolean configuration value representing \"true\" (for\n+\tunconditonal recursion) or to \"on-demand\" (when only those submodules\n+\tshould be fetched of which new commits have been fetched in its\n+\tsuperproject).\n+\tUsing the \"--[no-]recurse-submodules\" option ignores this setting.\n endif::git-pull[]\n\n -u::\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 7efecfe..feb9392 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -28,12 +28,6 @@ enum {\n \tTAGS_SET = 2\n };\n\n-enum {\n-\tRECURSE_SUBMODULES_OFF = 0,\n-\tRECURSE_SUBMODULES_DEFAULT = 1,\n-\tRECURSE_SUBMODULES_ON = 2\n-};\n-\n static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;\n static int progress, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT;\n@@ -42,6 +36,7 @@ static const char *upload_pack;\n static struct strbuf default_rla = STRBUF_INIT;\n static struct transport *transport;\n static const char *submodule_prefix = \"\";\n+static const char *submodule_default;\n\n static struct option builtin_fetch_options[] = {\n \tOPT__VERBOSITY(&verbosity),\n@@ -73,6 +68,8 @@ static struct option builtin_fetch_options[] = {\n \t\t   \"deepen history of shallow clone\"),\n \t{ OPTION_STRING, 0, \"submodule-prefix\", &submodule_prefix, \"dir\",\n \t\t   \"prepend this to submodule path output\", PARSE_OPT_HIDDEN },\n+\t{ OPTION_STRING, 0, \"submodule-default\", &submodule_default, NULL,\n+\t\t   \"default mode for recursion\", PARSE_OPT_HIDDEN },\n \tOPT_END()\n };\n\n@@ -284,6 +281,9 @@ static int update_local_ref(struct ref *ref,\n \t\telse {\n \t\t\tmsg = \"storing head\";\n \t\t\twhat = \"[new branch]\";\n+\t\t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n+\t\t\t    (recurse_submodules != RECURSE_SUBMODULES_ON))\n+\t\t\t\tcheck_for_new_submodule_commits(ref->new_sha1);\n \t\t}\n\n \t\tr = s_update_ref(msg, ref, 0);\n@@ -299,6 +299,9 @@ static int update_local_ref(struct ref *ref,\n \t\tstrcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));\n \t\tstrcat(quickref, \"..\");\n \t\tstrcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));\n+\t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n+\t\t    (recurse_submodules != RECURSE_SUBMODULES_ON))\n+\t\t\tcheck_for_new_submodule_commits(ref->new_sha1);\n \t\tr = s_update_ref(\"fast-forward\", ref, 1);\n \t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' : ' ',\n \t\t\tTRANSPORT_SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,\n@@ -310,6 +313,9 @@ static int update_local_ref(struct ref *ref,\n \t\tstrcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));\n \t\tstrcat(quickref, \"...\");\n \t\tstrcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));\n+\t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n+\t\t    (recurse_submodules != RECURSE_SUBMODULES_ON))\n+\t\t\tcheck_for_new_submodule_commits(ref->new_sha1);\n \t\tr = s_update_ref(\"forced-update\", ref, 1);\n \t\tsprintf(display, \"%c %-*s %-*s -> %s  (%s)\", r ? '!' : '+',\n \t\t\tTRANSPORT_SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,\n@@ -949,9 +955,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tif (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {\n \t\tconst char *options[10];\n \t\tint num_options = 0;\n-\t\t/* Set recursion as default when we already are recursing */\n-\t\tif (submodule_prefix[0])\n-\t\t\tset_config_fetch_recurse_submodules(1);\n+\t\tif (submodule_default) {\n+\t\t\tint arg = parse_fetch_recurse_submodules_arg(submodule_default);\n+\t\t\tset_config_fetch_recurse_submodules(arg);\n+\t\t}\n \t\tgitmodules_config();\n \t\tgit_config(submodule_config, NULL);\n \t\tadd_options_to_argv(&num_options, options);\ndiff --git a/submodule.c b/submodule.c\nindex 6f1c107..c8c3a99 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -12,7 +12,8 @@\n struct string_list config_name_for_path;\n struct string_list config_fetch_recurse_submodules_for_name;\n struct string_list config_ignore_for_name;\n-static int config_fetch_recurse_submodules;\n+static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n+struct string_list changed_submodule_paths;\n\n static int add_submodule_odb(const char *path)\n {\n@@ -152,6 +153,20 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\tdie(\"bad --ignore-submodules argument: %s\", arg);\n }\n\n+int parse_fetch_recurse_submodules_arg(const char *arg)\n+{\n+\tswitch (git_config_maybe_bool(\"\", arg)) {\n+\tcase 1:\n+\t\treturn RECURSE_SUBMODULES_ON;\n+\tcase 0:\n+\t\treturn RECURSE_SUBMODULES_OFF;\n+\tdefault:\n+\t\tif (!strcmp(arg, \"on-demand\"))\n+\t\t\treturn RECURSE_SUBMODULES_ON_DEMAND;\n+\t\tdie(\"bad --recurse-submodules argument: %s\", arg);\n+\t}\n+}\n+\n void show_submodule_summary(FILE *f, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule,\n@@ -248,11 +263,73 @@ void set_config_fetch_recurse_submodules(int value)\n \tconfig_fetch_recurse_submodules = value;\n }\n\n+static void submodule_collect_changed_cb(struct diff_queue_struct *q,\n+\t\t\t\t\t struct diff_options *options,\n+\t\t\t\t\t void *data)\n+{\n+\tint i;\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filepair *p = q->queue[i];\n+\t\tif (!S_ISGITLINK(p->two->mode))\n+\t\t\tcontinue;\n+\n+\t\tif (S_ISGITLINK(p->one->mode)) {\n+\t\t\t/* NEEDSWORK: We should honor the name configured in\n+\t\t\t * the .gitmodules file of the commit we are examining\n+\t\t\t * here to be able to correctly follow submodules\n+\t\t\t * being moved around. */\n+\t\t\tstruct string_list_item *path;\n+\t\t\tpath = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);\n+\t\t\tif (!path)\n+\t\t\t\tstring_list_append(&changed_submodule_paths, xstrdup(p->two->path));\n+\t\t} else {\n+\t\t\t/* Submodule is new or was moved here */\n+\t\t\t/* NEEDSWORK: When the .git directories of submodules\n+\t\t\t * live inside the superprojects .git directory some\n+\t\t\t * day we should fetch new submodules directly into\n+\t\t\t * that location too when config or options request\n+\t\t\t * that so they can be checked out from there. */\n+\t\t\tcontinue;\n+\t\t}\n+\t}\n+}\n+\n+void check_for_new_submodule_commits(unsigned char new_sha1[20])\n+{\n+\tstruct rev_info rev;\n+\tstruct commit *commit;\n+\tint argc = 5;\n+\tconst char *argv[] = {NULL, NULL, \"--not\", \"--branches\", \"--remotes\", NULL};\n+\n+\tinit_revisions(&rev, NULL);\n+\targv[1] = xstrdup(sha1_to_hex(new_sha1));\n+\tsetup_revisions(argc, argv, &rev, NULL);\n+\tif (prepare_revision_walk(&rev))\n+\t\tdie(\"revision walk setup failed\");\n+\n+\twhile ((commit = get_revision(&rev))) {\n+\t\tstruct commit_list *parent = commit->parents;\n+\t\twhile (parent) {\n+\t\t\tstruct diff_options diff_opts;\n+\t\t\tdiff_setup(&diff_opts);\n+\t\t\tdiff_opts.output_format |= DIFF_FORMAT_CALLBACK;\n+\t\t\tdiff_opts.format_callback = submodule_collect_changed_cb;\n+\t\t\tif (diff_setup_done(&diff_opts) < 0)\n+\t\t\t\tdie(\"diff_setup_done failed\");\n+\t\t\tdiff_tree_sha1(parent->item->object.sha1, commit->object.sha1, \"\", &diff_opts);\n+\t\t\tdiffcore_std(&diff_opts);\n+\t\t\tdiff_flush(&diff_opts);\n+\t\t\tparent = parent->next;\n+\t\t}\n+\t}\n+\tfree((char *)argv[1]);\n+}\n+\n int fetch_populated_submodules(int num_options, const char **options,\n \t\t\t       const char *prefix, int ignore_config,\n \t\t\t       int quiet)\n {\n-\tint i, result = 0, argc = 0;\n+\tint i, result = 0, argc = 0, default_argc;\n \tstruct child_process cp;\n \tconst char **argv;\n \tstruct string_list_item *name_for_path;\n@@ -264,11 +341,13 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\tif (read_cache() < 0)\n \t\t\tdie(\"index file corrupt\");\n\n-\t/* 4: \"fetch\" (options) \"--submodule-prefix\" prefix NULL */\n-\targv = xcalloc(num_options + 4, sizeof(const char *));\n+\t/* 6: \"fetch\" (options) --submodule-default default \"--submodule-prefix\" prefix NULL */\n+\targv = xcalloc(num_options + 6, sizeof(const char *));\n \targv[argc++] = \"fetch\";\n \tfor (i = 0; i < num_options; i++)\n \t\targv[argc++] = options[i];\n+\targv[argc++] = \"--submodule-default\";\n+\tdefault_argc = argc++;\n \targv[argc++] = \"--submodule-prefix\";\n\n \tmemset(&cp, 0, sizeof(cp));\n@@ -282,7 +361,7 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\tstruct strbuf submodule_git_dir = STRBUF_INIT;\n \t\tstruct strbuf submodule_prefix = STRBUF_INIT;\n \t\tstruct cache_entry *ce = active_cache[i];\n-\t\tconst char *git_dir, *name;\n+\t\tconst char *git_dir, *name, *default_argv;\n\n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n@@ -292,6 +371,7 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\tif (name_for_path)\n \t\t\tname = name_for_path->util;\n\n+\t\tdefault_argv = \"yes\";\n \t\tif (!ignore_config) {\n \t\t\tstruct string_list_item *fetch_recurse_submodules_option;\n \t\t\tfetch_recurse_submodules_option = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);\n@@ -299,8 +379,13 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\t\t\tif (!fetch_recurse_submodules_option->util)\n \t\t\t\t\tcontinue;\n \t\t\t} else {\n-\t\t\t\tif (!config_fetch_recurse_submodules)\n+\t\t\t\tif (config_fetch_recurse_submodules == RECURSE_SUBMODULES_OFF)\n \t\t\t\t\tcontinue;\n+\t\t\t\tif (config_fetch_recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) {\n+\t\t\t\t\tif (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\tdefault_argv = \"on-demand\";\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n\n@@ -314,6 +399,7 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\t\tif (!quiet)\n \t\t\t\tprintf(\"Fetching submodule %s%s\\n\", prefix, ce->name);\n \t\t\tcp.dir = submodule_path.buf;\n+\t\t\targv[default_argc] = default_argv;\n \t\t\targv[argc] = submodule_prefix.buf;\n \t\t\tif (run_command(&cp))\n \t\t\t\tresult = 1;\ndiff --git a/submodule.h b/submodule.h\nindex 4729023..aca023a 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -3,17 +3,26 @@\n\n struct diff_options;\n\n+enum {\n+\tRECURSE_SUBMODULES_ON_DEMAND = -1,\n+\tRECURSE_SUBMODULES_OFF = 0,\n+\tRECURSE_SUBMODULES_DEFAULT = 1,\n+\tRECURSE_SUBMODULES_ON = 2\n+};\n+\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\tconst char *path);\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config();\n int parse_submodule_config_option(const char *var, const char *value);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n+int parse_fetch_recurse_submodules_arg(const char *arg);\n void show_submodule_summary(FILE *f, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule,\n \t\tconst char *del, const char *add, const char *reset);\n void set_config_fetch_recurse_submodules(int value);\n+void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n int fetch_populated_submodules(int num_options, const char **options,\n \t\t\t       const char *prefix, int ignore_config,\n \t\t\t       int quiet);\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex a5f4585..6d92f7a 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -192,4 +192,113 @@ test_expect_success \"--no-recurse-submodules overrides config setting\" '\n \t! test -s actual.err\n '\n\n+test_expect_success \"Recursion doesn't happen when no new commits are fetched in the superproject\" '\n+\t(\n+\t\tcd downstream &&\n+\t\t(\n+\t\t\tcd submodule &&\n+\t\t\tgit config --unset fetch.recurseSubmodules\n+\t\t) &&\n+\t\tgit config --unset fetch.recurseSubmodules\n+\t\tgit fetch >../actual.out 2>../actual.err\n+\t) &&\n+\t! test -s actual.out &&\n+\t! test -s actual.err\n+'\n+\n+test_expect_success \"Recursion stops when no new submodule commits are fetched\" '\n+\thead1=$(git rev-parse --short HEAD) &&\n+\tgit add submodule &&\n+\tgit commit -m \"new submodule\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"Fetching submodule submodule\" > expect.out.sub &&\n+\techo \"From $pwd/.\" > expect.err.sub &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.sub\n+\thead -2 expect.err >> expect.err.sub &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err\n+\t) &&\n+\ttest_cmp expect.err.sub actual.err &&\n+\ttest_cmp expect.out.sub actual.out\n+'\n+\n+test_expect_success \"Recursion doesn't happen when new superproject commits don't change any submodules\" '\n+\tadd_upstream_commit &&\n+\thead1=$(git rev-parse --short HEAD) &&\n+\techo a > file &&\n+\tgit add file &&\n+\tgit commit -m \"new file\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"From $pwd/.\" > expect.err.file &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.file &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err\n+\t) &&\n+\t! test -s actual.out &&\n+\ttest_cmp expect.err.file actual.err\n+'\n+\n+test_expect_success \"Recursion picks up config in submodule\" '\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch --recurse-submodules &&\n+\t\t(\n+\t\t\tcd submodule &&\n+\t\t\tgit config fetch.recurseSubmodules true\n+\t\t)\n+\t) &&\n+\tadd_upstream_commit &&\n+\thead1=$(git rev-parse --short HEAD) &&\n+\tgit add submodule &&\n+\tgit commit -m \"new submodule\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"From $pwd/.\" > expect.err.sub &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.sub &&\n+\tcat expect.err >> expect.err.sub &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err &&\n+\t\t(\n+\t\t\tcd submodule &&\n+\t\t\tgit config --unset fetch.recurseSubmodules\n+\t\t)\n+\t) &&\n+\ttest_cmp expect.err.sub actual.err &&\n+\ttest_cmp expect.out actual.out\n+'\n+\n+test_expect_success \"Recursion picks up all submodules when necessary\" '\n+\tadd_upstream_commit &&\n+\t(\n+\t\tcd submodule &&\n+\t\t(\n+\t\t\tcd deepsubmodule &&\n+\t\t\tgit fetch &&\n+\t\t\tgit checkout -q FETCH_HEAD\n+\t\t) &&\n+\t\thead1=$(git rev-parse --short HEAD^) &&\n+\t\tgit add deepsubmodule &&\n+\t\tgit commit -m \"new deepsubmodule\"\n+\t\thead2=$(git rev-parse --short HEAD) &&\n+\t\techo \"From $pwd/submodule\" > ../expect.err.sub &&\n+\t\techo \"   $head1..$head2  master     -> origin/master\" >> ../expect.err.sub\n+\t) &&\n+\thead1=$(git rev-parse --short HEAD) &&\n+\tgit add submodule &&\n+\tgit commit -m \"new submodule\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"From $pwd/.\" > expect.err.2 &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.2 &&\n+\tcat expect.err.sub >> expect.err.2 &&\n+\ttail -2 expect.err >> expect.err.2 &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err\n+\t) &&\n+\ttest_cmp expect.err.2 actual.err &&\n+\ttest_cmp expect.out actual.out\n+'\n+\n test_done\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex e5b1953..d600583 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -52,7 +52,7 @@ test_expect_success 'change submodule url' '\n\n test_expect_success '\"git submodule sync\" should update submodule URLs' '\n \t(cd super-clone &&\n-\t git pull &&\n+\t git pull --no-recurse-submodules &&\n \t git submodule sync\n \t) &&\n \ttest -d \"$(git config -f super-clone/submodule/.git/config \\\n-- \n1.7.4.1.190.g13e20\n"},{"id":"162054","messageId":"4D656F77.7060306@web.de","threadId":"26584","inReplyTo":"4D656F25.5090007@web.de","subject":"[PATCH 2/6] fetch/pull: Add the 'on-demand' value to the --recurse-submodules option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T20:35:03Z","receivedAt":"2011-02-23T20:35:03Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Until now the --recurse-submodules option could only be used to either\nfetch all populated submodules recursively or to disable recursion\ncompletely. As fetch and pull now by default just fetch those submodules\nfor which new commits have been fetched in the superproject, a command\nline option to enforce that behavior is needed to be able to override\nconfiguration settings.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n Documentation/fetch-options.txt |   16 +++++++--\n Documentation/git-pull.txt      |    2 +-\n builtin/fetch.c                 |   24 +++++++++++--\n git-pull.sh                     |    3 ++\n submodule.c                     |    8 +++-\n submodule.h                     |    2 +-\n t/t5526-fetch-submodules.sh     |   71 +++++++++++++++++++++++++++++++++++++++\n 7 files changed, 115 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex 1d770d9..3fbdb53 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -65,9 +65,19 @@ ifndef::git-pull[]\n \tspecified with the remote.<name>.tagopt setting. See\n \tlinkgit:git-config[1].\n\n---[no-]recurse-submodules::\n-\tThis option controls if new commits of all populated submodules should\n-\tbe fetched too (see linkgit:git-config[1] and linkgit:gitmodules[5]).\n+--recurse-submodules[=yes|on-demand|no]::\n+\tThis option controls if and under what conditions new commits of all\n+\tpopulated submodules should be fetched too. It can be used as a\n+\tboolean option to completely disable recursion when set to 'no' or to\n+\tunconditionally recurse into all populated submodules when set to\n+\t'yes', which is the default when this option is used without any\n+\tvalue. If 'on-demand' is used, it will only recurse into those\n+\tsubmodules where new commits have been fetched in the superproject\n+\t(also see linkgit:git-config[1] and linkgit:gitmodules[5]).\n+\n+--no-recurse-submodules::\n+\tDisable recursive fetching of submodules (this has the same effect as\n+\tusing the '--recurse-submodules=no' option).\n\n --submodule-prefix=<path>::\n \tPrepend <path> to paths printed in informative messages\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex b33e6be..c45efb3 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -84,7 +84,7 @@ must be given before the options meant for 'git fetch'.\n --verbose::\n \tPass --verbose to git-fetch and git-merge.\n\n---[no-]recurse-submodules::\n+--[no-]recurse-submodules[=yes|on-demand|no]::\n \tThis option controls if new commits of all populated submodules should\n \tbe fetched too (see linkgit:git-config[1] and linkgit:gitmodules[5]).\n \tThat might be necessary to get the data needed for merging submodule\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex feb9392..3743b01 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -38,6 +38,20 @@ static struct transport *transport;\n static const char *submodule_prefix = \"\";\n static const char *submodule_default;\n\n+static int option_parse_recurse_submodules(const struct option *opt,\n+\t\t\t\t   const char *arg, int unset)\n+{\n+\tif (unset) {\n+\t\trecurse_submodules = RECURSE_SUBMODULES_OFF;\n+\t} else {\n+\t\tif (arg)\n+\t\t\trecurse_submodules = parse_fetch_recurse_submodules_arg(arg);\n+\t\telse\n+\t\t\trecurse_submodules = RECURSE_SUBMODULES_ON;\n+\t}\n+\treturn 0;\n+}\n+\n static struct option builtin_fetch_options[] = {\n \tOPT__VERBOSITY(&verbosity),\n \tOPT_BOOLEAN(0, \"all\", &all,\n@@ -55,9 +69,9 @@ static struct option builtin_fetch_options[] = {\n \t\t    \"do not fetch all tags (--no-tags)\", TAGS_UNSET),\n \tOPT_BOOLEAN('p', \"prune\", &prune,\n \t\t    \"prune remote-tracking branches no longer on remote\"),\n-\tOPT_SET_INT(0, \"recurse-submodules\", &recurse_submodules,\n-\t\t    \"control recursive fetching of submodules\",\n-\t\t    RECURSE_SUBMODULES_ON),\n+\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", NULL, \"on-demand\",\n+\t\t\"control recursive fetching of submodules\",\n+\t\tPARSE_OPT_OPTARG, option_parse_recurse_submodules },\n \tOPT_BOOLEAN(0, \"dry-run\", &dry_run,\n \t\t    \"dry run\"),\n \tOPT_BOOLEAN('k', \"keep\", &keep, \"keep downloaded pack\"),\n@@ -816,6 +830,8 @@ static void add_options_to_argv(int *argc, const char **argv)\n \t\targv[(*argc)++] = \"--keep\";\n \tif (recurse_submodules == RECURSE_SUBMODULES_ON)\n \t\targv[(*argc)++] = \"--recurse-submodules\";\n+\telse if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n+\t\targv[(*argc)++] = \"--recurse-submodules=on-demand\";\n \tif (verbosity >= 2)\n \t\targv[(*argc)++] = \"-v\";\n \tif (verbosity >= 1)\n@@ -964,7 +980,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tadd_options_to_argv(&num_options, options);\n \t\tresult = fetch_populated_submodules(num_options, options,\n \t\t\t\t\t\t    submodule_prefix,\n-\t\t\t\t\t\t    recurse_submodules == RECURSE_SUBMODULES_ON,\n+\t\t\t\t\t\t    recurse_submodules,\n \t\t\t\t\t\t    verbosity < 0);\n \t}\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 86517e9..7a97d8e 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -108,6 +108,9 @@ do\n \t--recurse-submodules)\n \t\trecurse_submodules=--recurse-submodules\n \t\t;;\n+\t--recurse-submodules=*)\n+\t\trecurse_submodules=\"$1\"\n+\t\t;;\n \t--no-recurse-submodules)\n \t\trecurse_submodules=--no-recurse-submodules\n \t\t;;\ndiff --git a/submodule.c b/submodule.c\nindex c8c3a99..398616a 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -326,7 +326,7 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20])\n }\n\n int fetch_populated_submodules(int num_options, const char **options,\n-\t\t\t       const char *prefix, int ignore_config,\n+\t\t\t       const char *prefix, int command_line_option,\n \t\t\t       int quiet)\n {\n \tint i, result = 0, argc = 0, default_argc;\n@@ -372,7 +372,7 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\t\tname = name_for_path->util;\n\n \t\tdefault_argv = \"yes\";\n-\t\tif (!ignore_config) {\n+\t\tif (command_line_option == RECURSE_SUBMODULES_DEFAULT) {\n \t\t\tstruct string_list_item *fetch_recurse_submodules_option;\n \t\t\tfetch_recurse_submodules_option = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);\n \t\t\tif (fetch_recurse_submodules_option) {\n@@ -387,6 +387,10 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\t\t\t\tdefault_argv = \"on-demand\";\n \t\t\t\t}\n \t\t\t}\n+\t\t} else if (command_line_option == RECURSE_SUBMODULES_ON_DEMAND) {\n+\t\t\tif (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))\n+\t\t\t\tcontinue;\n+\t\t\tdefault_argv = \"on-demand\";\n \t\t}\n\n \t\tstrbuf_addf(&submodule_path, \"%s/%s\", work_tree, ce->name);\ndiff --git a/submodule.h b/submodule.h\nindex aca023a..974cc86 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -24,7 +24,7 @@ void show_submodule_summary(FILE *f, const char *path,\n void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n int fetch_populated_submodules(int num_options, const char **options,\n-\t\t\t       const char *prefix, int ignore_config,\n+\t\t\t       const char *prefix, int command_line_option,\n \t\t\t       int quiet);\n unsigned is_submodule_modified(const char *path, int ignore_untracked);\n int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex 6d92f7a..4cd723c 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -301,4 +301,75 @@ test_expect_success \"Recursion picks up all submodules when necessary\" '\n \ttest_cmp expect.out actual.out\n '\n\n+test_expect_success \"'--recurse-submodules=on-demand' doesn't recurse when no new commits are fetched in the superproject (and ignores config)\" '\n+\tadd_upstream_commit &&\n+\t(\n+\t\tcd submodule &&\n+\t\t(\n+\t\t\tcd deepsubmodule &&\n+\t\t\tgit fetch &&\n+\t\t\tgit checkout -q FETCH_HEAD\n+\t\t) &&\n+\t\thead1=$(git rev-parse --short HEAD^) &&\n+\t\tgit add deepsubmodule &&\n+\t\tgit commit -m \"new deepsubmodule\"\n+\t\thead2=$(git rev-parse --short HEAD) &&\n+\t\techo \"From $pwd/submodule\" > ../expect.err.sub &&\n+\t\techo \"   $head1..$head2  master     -> origin/master\" >> ../expect.err.sub\n+\t) &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit config fetch.recurseSubmodules true &&\n+\t\tgit fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err &&\n+\t\tgit config --unset fetch.recurseSubmodules\n+\t) &&\n+\t! test -s actual.out &&\n+\t! test -s actual.err\n+'\n+\n+test_expect_success \"'--recurse-submodules=on-demand' recurses as deep as necessary (and ignores config)\" '\n+\thead1=$(git rev-parse --short HEAD) &&\n+\tgit add submodule &&\n+\tgit commit -m \"new submodule\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\ttail -2 expect.err > expect.err.deepsub &&\n+\techo \"From $pwd/.\" > expect.err &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err\n+\tcat expect.err.sub >> expect.err &&\n+\tcat expect.err.deepsub >> expect.err &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit config fetch.recurseSubmodules false &&\n+\t\t(\n+\t\t\tcd submodule &&\n+\t\t\tgit config -f .gitmodules submodule.deepsubmodule.fetchRecursive false\n+\t\t) &&\n+\t\tgit fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err &&\n+\t\tgit config --unset fetch.recurseSubmodules\n+\t\t(\n+\t\t\tcd submodule &&\n+\t\t\tgit config --unset -f .gitmodules submodule.deepsubmodule.fetchRecursive\n+\t\t)\n+\t) &&\n+\ttest_cmp expect.out actual.out &&\n+\ttest_cmp expect.err actual.err\n+'\n+\n+test_expect_success \"'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)\" '\n+\tadd_upstream_commit &&\n+\thead1=$(git rev-parse --short HEAD) &&\n+\techo a >> file &&\n+\tgit add file &&\n+\tgit commit -m \"new file\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"From $pwd/.\" > expect.err.file &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.file &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err\n+\t) &&\n+\t! test -s actual.out &&\n+\ttest_cmp expect.err.file actual.err\n+'\n+\n test_done\n-- \n1.7.4.1.190.g13e20\n"},{"id":"162055","messageId":"4D656F91.3000706@web.de","threadId":"26584","inReplyTo":"4D656F25.5090007@web.de","subject":"[PATCH 3/6] config: teach the fetch.recurseSubmodules option the 'on-demand' value","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T20:35:29Z","receivedAt":"2011-02-23T20:35:29Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"To enable the user to change the default behavior of \"git fetch\" and \"git\npull\" regarding submodule recursion add the new \"on-demand\" value which\nhas just been added to the \"--recurse-submodules\" command line option.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n Documentation/config.txt    |   10 +++++++---\n submodule.c                 |    2 +-\n t/t5526-fetch-submodules.sh |   28 ++++++++++++++++++++++++++++\n 3 files changed, 36 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex b4e65b8..688d4fd 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -900,9 +900,13 @@ diff.wordRegex::\n \tcharacters are *ignorable* whitespace.\n\n fetch.recurseSubmodules::\n-\tA boolean value which changes the behavior for fetch and pull, the\n-\tdefault is to not recursively fetch populated submodules unless\n-\tconfigured otherwise.\n+\tThis option can be either set to a boolean value or to 'on-demand'.\n+\tSetting it to a boolean changes the behavior of fetch and pull to\n+\tunconditionally recurse into submodules when set to true or to not\n+\trecurse at all when set to false. When set to 'on-demand' (the default\n+\tvalue), it tells fetch and pull to recurse only into those submodules\n+\twhere new commits are recorded in the commmits fetched for the\n+\tsuperproject.\n\n fetch.unpackLimit::\n \tIf the number of objects fetched over the git native\ndiff --git a/submodule.c b/submodule.c\nindex 398616a..cccd728 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -71,7 +71,7 @@ int submodule_config(const char *var, const char *value, void *cb)\n \tif (!prefixcmp(var, \"submodule.\"))\n \t\treturn parse_submodule_config_option(var, value);\n \telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n-\t\tconfig_fetch_recurse_submodules = git_config_bool(var, value);\n+\t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(value);\n \t\treturn 0;\n \t}\n \treturn 0;\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex 4cd723c..e6d873a 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -372,4 +372,32 @@ test_expect_success \"'--recurse-submodules=on-demand' stops when no new submodul\n \ttest_cmp expect.err.file actual.err\n '\n\n+test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\" '\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch --recurse-submodules\n+\t) &&\n+\tadd_upstream_commit &&\n+\tgit config --global fetch.recurseSubmodules false &&\n+\thead1=$(git rev-parse --short HEAD) &&\n+\tgit add submodule &&\n+\tgit commit -m \"new submodule\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"From $pwd/.\" > expect.err.2 &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.2\n+\thead -2 expect.err >> expect.err.2 &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit config fetch.recurseSubmodules on-demand &&\n+\t\tgit fetch >../actual.out 2>../actual.err\n+\t) &&\n+\tgit config --global --unset fetch.recurseSubmodules &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit config --unset fetch.recurseSubmodules\n+\t) &&\n+\ttest_cmp expect.out.sub actual.out &&\n+\ttest_cmp expect.err.2 actual.err\n+'\n+\n test_done\n-- \n1.7.4.1.190.g13e20\n"},{"id":"162056","messageId":"4D656FB0.7040904@web.de","threadId":"26584","inReplyTo":"4D656F25.5090007@web.de","subject":"[PATCH 4/6] Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T20:36:00Z","receivedAt":"2011-02-23T20:36:00Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Now the behavior of fetch and pull can be configured to the recently added\n'on-demand' mode separately for each submodule too.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n Documentation/config.txt     |    2 +-\n Documentation/gitmodules.txt |    4 ++--\n submodule.c                  |    9 +++++++--\n t/t5526-fetch-submodules.sh  |   28 ++++++++++++++++++++++++++++\n 4 files changed, 38 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 688d4fd..a34b886 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1823,7 +1823,7 @@ submodule.<name>.update::\n \tlinkgit:git-submodule[1] and linkgit:gitmodules[5] for details.\n\n submodule.<name>.fetchRecurseSubmodules::\n-\tThis option can be used to enable/disable recursive fetching of this\n+\tThis option can be used to control recursive fetching of this\n \tsubmodule. It can be overridden by using the --[no-]recurse-submodules\n \tcommand line option to \"git fetch\" and \"git pull\".\n \tThis setting will override that from in the linkgit:gitmodules[5]\ndiff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt\nindex 6897794..25daee2 100644\n--- a/Documentation/gitmodules.txt\n+++ b/Documentation/gitmodules.txt\n@@ -45,12 +45,12 @@ submodule.<name>.update::\n \tthe '--merge' or '--rebase' options.\n\n submodule.<name>.fetchRecurseSubmodules::\n-\tThis option can be used to enable/disable recursive fetching of this\n+\tThis option can be used to control recursive fetching of this\n \tsubmodule. If this option is also present in the submodules entry in\n \t.git/config of the superproject, the setting there will override the\n \tone found in .gitmodules.\n \tBoth settings can be overridden on the command line by using the\n-\t\"--[no-]recurse-submodules\" option to \"git fetch\" and \"git pull\"..\n+\t\"--[no-]recurse-submodules\" option to \"git fetch\" and \"git pull\".\n\n submodule.<name>.ignore::\n \tDefines under what circumstances \"git status\" and the diff family show\ndiff --git a/submodule.c b/submodule.c\nindex cccd728..b477c3c 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -113,7 +113,7 @@ int parse_submodule_config_option(const char *var, const char *value)\n \t\tif (!config)\n \t\t\tconfig = string_list_append(&config_fetch_recurse_submodules_for_name,\n \t\t\t\t\t\t    strbuf_detach(&submodname, NULL));\n-\t\tconfig->util = git_config_bool(var, value) ? (void *)1 : NULL;\n+\t\tconfig->util = (void *)(size_t)parse_fetch_recurse_submodules_arg(value);\n \t\tstrbuf_release(&submodname);\n \t} else if ((len > 7) && !strcmp(var + len - 7, \".ignore\")) {\n \t\tif (strcmp(value, \"untracked\") && strcmp(value, \"dirty\") &&\n@@ -376,8 +376,13 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\t\tstruct string_list_item *fetch_recurse_submodules_option;\n \t\t\tfetch_recurse_submodules_option = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);\n \t\t\tif (fetch_recurse_submodules_option) {\n-\t\t\t\tif (!fetch_recurse_submodules_option->util)\n+\t\t\t\tif ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_OFF)\n \t\t\t\t\tcontinue;\n+\t\t\t\tif ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_ON_DEMAND) {\n+\t\t\t\t\tif (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\tdefault_argv = \"on-demand\";\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\tif (config_fetch_recurse_submodules == RECURSE_SUBMODULES_OFF)\n \t\t\t\t\tcontinue;\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex e6d873a..09701aa 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -400,4 +400,32 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \ttest_cmp expect.err.2 actual.err\n '\n\n+test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' overrides fetch.recurseSubmodules\" '\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch --recurse-submodules\n+\t) &&\n+\tadd_upstream_commit &&\n+\tgit config fetch.recurseSubmodules false &&\n+\thead1=$(git rev-parse --short HEAD) &&\n+\tgit add submodule &&\n+\tgit commit -m \"new submodule\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"From $pwd/.\" > expect.err.2 &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.2\n+\thead -2 expect.err >> expect.err.2 &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit config submodule.submodule.fetchRecurseSubmodules on-demand &&\n+\t\tgit fetch >../actual.out 2>../actual.err\n+\t) &&\n+\tgit config --unset fetch.recurseSubmodules &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit config --unset submodule.submodule.fetchRecurseSubmodules\n+\t) &&\n+\ttest_cmp expect.out.sub actual.out &&\n+\ttest_cmp expect.err.2 actual.err\n+'\n+\n test_done\n-- \n1.7.4.1.190.g13e20\n"},{"id":"162057","messageId":"4D656FC7.2060201@web.de","threadId":"26584","inReplyTo":"4D656F25.5090007@web.de","subject":"[PATCH 5/6] fetch/pull: Don't recurse into a submodule when commits are already present","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T20:36:23Z","receivedAt":"2011-02-23T20:36:23Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When looking for submodules where new commits have been recorded in the\nsuperproject ignore those cases where the submodules commits are already\npresent locally. This can happen e.g. when the submodule has been rewound\nto an earlier state. Then there is no need to fetch the submodule again\nas the commit recorded in the newly fetched superproject commit has\nalready been fetched earlier into the submodule.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n submodule.c                 |    9 ++++++++-\n t/t5526-fetch-submodules.sh |   19 +++++++++++++++++++\n 2 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex b477c3c..ddb0a29 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -263,6 +263,13 @@ void set_config_fetch_recurse_submodules(int value)\n \tconfig_fetch_recurse_submodules = value;\n }\n\n+static int is_submodule_commit_present(const char *path, unsigned char sha1[20])\n+{\n+\tif (!add_submodule_odb(path))\n+\t\treturn lookup_commit_reference(sha1) != 0;\n+\treturn 0;\n+}\n+\n static void submodule_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t\t\t struct diff_options *options,\n \t\t\t\t\t void *data)\n@@ -280,7 +287,7 @@ static void submodule_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t * being moved around. */\n \t\t\tstruct string_list_item *path;\n \t\t\tpath = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);\n-\t\t\tif (!path)\n+\t\t\tif (!path && !is_submodule_commit_present(p->two->path, p->two->sha1))\n \t\t\t\tstring_list_append(&changed_submodule_paths, xstrdup(p->two->path));\n \t\t} else {\n \t\t\t/* Submodule is new or was moved here */\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex 09701aa..3decfae 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -428,4 +428,23 @@ test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' override\n \ttest_cmp expect.err.2 actual.err\n '\n\n+test_expect_success \"don't fetch submodule when newly recorded commits are already present\" '\n+\t(\n+\t\tcd submodule &&\n+\t\tgit checkout -q HEAD^^\n+\t) &&\n+\thead1=$(git rev-parse --short HEAD) &&\n+\tgit add submodule &&\n+\tgit commit -m \"submodule rewound\" &&\n+\thead2=$(git rev-parse --short HEAD) &&\n+\techo \"From $pwd/.\" > expect.err &&\n+\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err\n+\t) &&\n+\t! test -s actual.out &&\n+\ttest_cmp expect.err actual.err\n+'\n+\n test_done\n-- \n1.7.4.1.190.g13e20\n"},{"id":"162058","messageId":"4D656FDF.5050400@web.de","threadId":"26584","inReplyTo":"4D656F25.5090007@web.de","subject":"[PATCH 6/6] submodule update: Don't fetch when the submodule commit is already present","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T20:36:47Z","receivedAt":"2011-02-23T20:36:47Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"If the commit to be checked out on \"git submodule update\" has already been\nfetched in the submodule there is no need to run \"git fetch\" again. Since\n\"git fetch\" recently learned recursion (and the new on-demand mode to\nfetch commits recorded in the superproject is enabled by default) this\nwill happen pretty often, thereby making the unconditional fetch during\n\"git submodule update\" unnecessary.\n\nIf the commit is not present in the submodule (e.g. the user disabled the\nfetch on-demand mode) the fetch will be run as before.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n git-submodule.sh            |    2 +-\n t/t7406-submodule-update.sh |   20 ++++++++++++++++++++\n 2 files changed, 21 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 8b90589..ea49760 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -465,7 +465,7 @@ cmd_update()\n \t\t\tif test -z \"$nofetch\"\n \t\t\tthen\n \t\t\t\t(clear_local_git_env; cd \"$path\" &&\n-\t\t\t\t\tgit-fetch) ||\n+\t\t\t\t\t(git rev-parse --verify $sha1 >/dev/null || git-fetch)) ||\n \t\t\t\tdie \"Unable to fetch in submodule path '$path'\"\n \t\t\tfi\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex bfb4975..ee3eec5 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -74,6 +74,26 @@ test_expect_success 'submodule update detaching the HEAD ' '\n \t)\n '\n\n+apos=\"'\";\n+test_expect_success 'submodule update does not fetch already present commits' '\n+\t(cd submodule &&\n+\t  echo line3 >> file &&\n+\t  git add file &&\n+\t  test_tick &&\n+\t  git commit -m \"upstream line3\"\n+\t) &&\n+\t(cd super/submodule &&\n+\t  head=$(git rev-parse --verify HEAD) &&\n+\t  echo \"Submodule path ${apos}submodule$apos: checked out $apos$head$apos\" > ../../expected &&\n+\t  git reset --hard HEAD~1\n+\t) &&\n+\t(cd super &&\n+\t  git submodule update > ../actual 2> ../actual.err\n+\t) &&\n+\ttest_cmp expected actual &&\n+\t! test -s actual.err\n+'\n+\n test_expect_success 'submodule update --rebase staying on master' '\n \t(cd super/submodule &&\n \t  git checkout master\n-- \n1.7.4.1.190.g13e20\n"},{"id":"162063","messageId":"7vipwa5phh.fsf@alter.siamese.dyndns.org","threadId":"26584","inReplyTo":"4D656F4D.6080401@web.de","subject":"Re: [PATCH 1/6] fetch/pull: recurse into submodules when necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T22:56:58Z","receivedAt":"2011-02-23T22:56:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> diff --git a/submodule.c b/submodule.c\n> index 6f1c107..c8c3a99 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -152,6 +153,20 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n> ...\n> +int parse_fetch_recurse_submodules_arg(const char *arg)\n> +{\n> +\tswitch (git_config_maybe_bool(\"\", arg)) {\n\nIt's a bit unusual to see \"\" as the variable name; doesn't config-maybe-bool\nbarf when arg is not what it likes, with the name as part of its message?\n\n> @@ -248,11 +263,73 @@ void set_config_fetch_recurse_submodules(int value)\n> ...\n> +static void submodule_collect_changed_cb(struct diff_queue_struct *q,\n> +\t\t\t\t\t struct diff_options *options,\n> +\t\t\t\t\t void *data)\n> +{\n> +\tint i;\n> +\tfor (i = 0; i < q->nr; i++) {\n> +\t\tstruct diff_filepair *p = q->queue[i];\n> +\t\tif (!S_ISGITLINK(p->two->mode))\n> +\t\t\tcontinue;\n> +\n> +\t\tif (S_ISGITLINK(p->one->mode)) {\n> +\t\t\t/* NEEDSWORK: We should honor the name configured in\n> +\t\t\t * the .gitmodules file of the commit we are examining\n> +\t\t\t * here to be able to correctly follow submodules\n> +\t\t\t * being moved around. */\n> +\t\t\tstruct string_list_item *path;\n> +\t\t\tpath = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);\n> +\t\t\tif (!path)\n> +\t\t\t\tstring_list_append(&changed_submodule_paths, xstrdup(p->two->path));\n\nI wondered why there is no mention of \"data\" in the implementation of this\nfunction; changed_submodule_paths global is used instead here.\n\nI do not see anywhere the global string_list is initialized, freed, nor\nre-initialized for reuse.  Are you guaranteeing that the caller only calls\nthe check_for_new_submodule_commits() function once, and if so how?  The\nfunction is called from update_local_ref() in builtin/fetch.c, which in\nturn gets called number of times during a fetch.  IOW, does the code do\nthe right thing when you are fetching more than one refs?\n"},{"id":"162064","messageId":"20110223230713.GB6819@elie","threadId":"26584","inReplyTo":"4D656F4D.6080401@web.de","subject":"Re: [PATCH 1/6] fetch/pull: recurse into submodules when necessary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-23T23:07:13Z","receivedAt":"2011-02-23T23:07:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n\n> To be able to access all commits of populated submodules referenced by the\n> superproject it is sufficient to only then let \"git fetch\" recurse into a\n> submodule when the new commits fetched in the superproject record new\n> commits for it.\n\nExciting stuff.  This would use the default refspec rather than fetching\nthe bare minimum of commits in submodules, right?\n\n> There is currently no infrastructure for storing deleted and new\n> submodules in the .git directory of the superproject. Thats why fetch and\n> pull for now only fetch submodules that are already checked out and are\n> not renamed.\n\nAh.\n\n> --- a/Documentation/fetch-options.txt\n> +++ b/Documentation/fetch-options.txt\n> @@ -73,6 +73,14 @@ ifndef::git-pull[]\n>  \tPrepend <path> to paths printed in informative messages\n>  \tsuch as \"Fetching submodule foo\".  This option is used\n>  \tinternally when recursing over submodules.\n> +\n> +--submodule-default=[yes|on-demand]::\n> +\tThis option is used internally to set the submodule recursion default\n> +\tto either a boolean configuration value representing \"true\" (for\n> +\tunconditonal recursion) or to \"on-demand\" (when only those submodules\n> +\tshould be fetched of which new commits have been fetched in its\n> +\tsuperproject).\n> +\tUsing the \"--[no-]recurse-submodules\" option ignores this setting.\n\nCould this be combined with the --recurse-submodules, with a \"last instance\nof the option wins\" rule?  Something like:\n\n --recurse-submodules[=(yes | no | changed)]::\n --no-recurse-submodules::\n\n> +++ b/submodule.c\n[...]\n> @@ -248,11 +263,73 @@ void set_config_fetch_recurse_submodules(int value)\n[...]\n> +static void submodule_collect_changed_cb(struct diff_queue_struct *q,\n[...]\n> +\t\tif (S_ISGITLINK(p->one->mode)) {\n> +\t\t\t/* NEEDSWORK: We should honor the name configured in\n> +\t\t\t * the .gitmodules file of the commit we are examining\n> +\t\t\t * here to be able to correctly follow submodules\n> +\t\t\t * being moved around. */\n\nHmm, a sort of variant on rename detection.  Does \"git submodule update\" /\n\"git checkout --recurse-submodules\" use .gitmodules to move pre-populated\nsubrepositories?\n\n> +\t\t\t/* Submodule is new or was moved here */\n> +\t\t\t/* NEEDSWORK: When the .git directories of submodules\n> +\t\t\t * live inside the superprojects .git directory some\n> +\t\t\t * day we should fetch new submodules directly into\n> +\t\t\t * that location too when config or options request\n> +\t\t\t * that so they can be checked out from there. */\n> +\t\t\tcontinue;\n\nMaybe this can be mentioned in a BUGS section on the git-fetch(1)\nmanpage to give readers a warning and clue about the intended\nmeaning of --recurse-submodules?\n\nI'm afraid a certain kind of person might get used to the \"don't fetch\nnew submodules\" behavior (e.g., if upstream is including unneeded\nconvenience copies of libraries right and left in addition to some\nuseful submodules).\n\n> +void check_for_new_submodule_commits(unsigned char new_sha1[20])\n> +{\n> +\tstruct rev_info rev;\n> +\tstruct commit *commit;\n> +\tint argc = 5;\n> +\tconst char *argv[] = {NULL, NULL, \"--not\", \"--branches\", \"--remotes\", NULL};\n\nNit: maybe\n\n\tconst char *argv[] = ...;\n\tint argc = ARRAY_SIZE(argv) - 1;\n\n?\n\n> +\n> +\tinit_revisions(&rev, NULL);\n> +\targv[1] = xstrdup(sha1_to_hex(new_sha1));\n\nMaybe:\n\n\tchar *new_rev;\n\t...\n\targv[1] = new_rev = xstrdup(...);\n\t...\n\tfree(new_rev);\n\n> +\tsetup_revisions(argc, argv, &rev, NULL);\n> +\tif (prepare_revision_walk(&rev))\n> +\t\tdie(\"revision walk setup failed\");\n> +\n\nMaybe a comment so the reader doesn't have to delve deeper?\n\n\t/*\n\t * Collect checked out submodules that have changed upstream\n\t * in \"changed_submodule_paths\".\n\t */\n\n> +\twhile ((commit = get_revision(&rev))) {\n[...]\n> +++ b/t/t5526-fetch-submodules.sh\n> @@ -192,4 +192,113 @@ test_expect_success \"--no-recurse-submodules overrides config setting\" '\n[...]\n> +\techo \"Fetching submodule submodule\" > expect.out.sub &&\n> +\techo \"From $pwd/.\" > expect.err.sub &&\n> +\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.sub\n\nI wonder if we should be testing the output in this much detail.\n\nWouldn't it be nicer to check the remote-tracking refs to make sure\nthe lasting effects were correct, rather than the detailed output\nformat?\n\nSo: aside from the option name, nothing but nits.  Thanks; that was\nfun.\n"},{"id":"162066","messageId":"20110223231210.GC6819@elie","threadId":"26584","inReplyTo":"4D656F77.7060306@web.de","subject":"Re: [PATCH 2/6] fetch/pull: Add the 'on-demand' value to the --recurse-submodules option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-23T23:12:10Z","receivedAt":"2011-02-23T23:12:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n\n>             As fetch and pull now by default just fetch those submodules\n> for which new commits have been fetched in the superproject, a command\n> line option to enforce that behavior is needed to be able to override\n> configuration settings.\n\nProbably this should go first in the series (the usual procedure:\nfirst command line for easy testing, then configuration for routine\nuse, then defaults).\n\nAside from that, it looks sane from skimming this over.  I think I\nprefer the name \"changed\" over \"on-demand\" since it is a little more\nobvious what it will do.  Neither name captures that this only affects\nchecked-out submodules, unfortunately.\n"},{"id":"162067","messageId":"20110223231430.GD6819@elie","threadId":"26584","inReplyTo":"20110223231210.GC6819@elie","subject":"Re: [PATCH 2/6] fetch/pull: Add the 'on-demand' value to the --recurse-submodules option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-23T23:14:30Z","receivedAt":"2011-02-23T23:14:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Jens Lehmann wrote:\n\n>>             As fetch and pull now by default just fetch those submodules\n>> for which new commits have been fetched in the superproject, a command\n>> line option to enforce that behavior is needed to be able to override\n>> configuration settings.\n>\n> Probably this should go first in the series (the usual procedure:\n> first command line for easy testing, then configuration for routine\n> use, then defaults).\n\nAh, sorry for the nonsense.  The series already follows that order.\n"},{"id":"162068","messageId":"7vei6y5ol6.fsf@alter.siamese.dyndns.org","threadId":"26584","inReplyTo":"4D656FB0.7040904@web.de","subject":"Re: [PATCH 4/6] Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T23:16:21Z","receivedAt":"2011-02-23T23:16:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> diff --git a/submodule.c b/submodule.c\n> index cccd728..b477c3c 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -113,7 +113,7 @@ int parse_submodule_config_option(const char *var, const char *value)\n>  \t\tif (!config)\n>  \t\t\tconfig = string_list_append(&config_fetch_recurse_submodules_for_name,\n>  \t\t\t\t\t\t    strbuf_detach(&submodname, NULL));\n> -\t\tconfig->util = git_config_bool(var, value) ? (void *)1 : NULL;\n> +\t\tconfig->util = (void *)(size_t)parse_fetch_recurse_submodules_arg(value);\n\nWhat is this double-cast about?\n\n> @@ -376,8 +376,13 @@ int fetch_populated_submodules(int num_options, const char **options,\n> ...\n> +\t\t\t\tif ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_OFF)\n>  \t\t\t\t\tcontinue;\n> +\t\t\t\tif ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_ON_DEMAND) {\n\nLikewise here; size_t feels a strange type to cast to in this comparison\nbetween (void *) and an enum, no?\n"},{"id":"162071","messageId":"7vaahm5oby.fsf@alter.siamese.dyndns.org","threadId":"26584","inReplyTo":"4D656FC7.2060201@web.de","subject":"Re: [PATCH 5/6] fetch/pull: Don't recurse into a submodule when commits are already present","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T23:21:53Z","receivedAt":"2011-02-23T23:21:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> diff --git a/submodule.c b/submodule.c\n> index b477c3c..ddb0a29 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -263,6 +263,13 @@ void set_config_fetch_recurse_submodules(int value)\n>  \tconfig_fetch_recurse_submodules = value;\n>  }\n>\n> +static int is_submodule_commit_present(const char *path, unsigned char sha1[20])\n> +{\n> +\tif (!add_submodule_odb(path))\n> +\t\treturn lookup_commit_reference(sha1) != 0;\n> +\treturn 0;\n> +}\n\nDon't you need to prove the usual \"reachabile from the refs\" here, instead\nof just the presense of a commit object?\n\nIf not, why not?\n"},{"id":"162070","messageId":"20110223232156.GE6819@elie","threadId":"26584","inReplyTo":"4D656F25.5090007@web.de","subject":"Re: [PATCH 0/6] Teach fetch/pull the on-demand mode and make it the default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-23T23:21:56Z","receivedAt":"2011-02-23T23:21:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n\n> *) The fetch is only done when the recorded submodule commit isn't\n>    already present.\n\nI like this part a lot.\n\n> I tend to think that this is suited for 1.7.5 but don't have any\n> objections against holding it back until 1.8.0 either. What do\n> others think?\n\nI see no backward-compatibility to wait for this, but I would be more\nincluded to trust people using \"git submodule update\" heavily than I\ndo.  The \"submodule update\" change could cause the following to break.\nWould that be disruptive?\n\n\tcd submodule\n\tgit fetch --no-recurse-submodules\n\t...\n\n\tcd ..\n\tbin/script-to-update-submodules-that-calls-submodule-update\n\nThanks a lot for working on this.\nJonathan\n"},{"id":"162072","messageId":"7v62sa5o9o.fsf@alter.siamese.dyndns.org","threadId":"26584","inReplyTo":"4D656FDF.5050400@web.de","subject":"Re: [PATCH 6/6] submodule update: Don't fetch when the submodule commit is already present","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T23:23:15Z","receivedAt":"2011-02-23T23:23:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 8b90589..ea49760 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -465,7 +465,7 @@ cmd_update()\n>  \t\t\tif test -z \"$nofetch\"\n>  \t\t\tthen\n>  \t\t\t\t(clear_local_git_env; cd \"$path\" &&\n> -\t\t\t\t\tgit-fetch) ||\n> +\t\t\t\t\t(git rev-parse --verify $sha1 >/dev/null || git-fetch)) ||\n\nExactly the same comment as the one for [5/6] applies here.\n"},{"id":"162074","messageId":"4D659818.4070107@web.de","threadId":"26584","inReplyTo":"7vipwa5phh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/6] fetch/pull: recurse into submodules when necessary","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T23:28:24Z","receivedAt":"2011-02-23T23:28:24Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 23.02.2011 23:56, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n> \n>> diff --git a/submodule.c b/submodule.c\n>> index 6f1c107..c8c3a99 100644\n>> --- a/submodule.c\n>> +++ b/submodule.c\n>> @@ -152,6 +153,20 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n>> ...\n>> +int parse_fetch_recurse_submodules_arg(const char *arg)\n>> +{\n>> +\tswitch (git_config_maybe_bool(\"\", arg)) {\n> \n> It's a bit unusual to see \"\" as the variable name; doesn't config-maybe-bool\n> barf when arg is not what it likes, with the name as part of its message?\n\ngit_config_*maybe*_bool() itself calls git_config_maybe_bool_text() and\ngit_parse_long() which all don't die() on anything (while git_config_bool()\ncan do that in git_config_int() via git_config_bool_or_int()). But you are\nright, using something more descriptive than \"\" is much better here.\n\n>> @@ -248,11 +263,73 @@ void set_config_fetch_recurse_submodules(int value)\n>> ...\n>> +static void submodule_collect_changed_cb(struct diff_queue_struct *q,\n>> +\t\t\t\t\t struct diff_options *options,\n>> +\t\t\t\t\t void *data)\n>> +{\n>> +\tint i;\n>> +\tfor (i = 0; i < q->nr; i++) {\n>> +\t\tstruct diff_filepair *p = q->queue[i];\n>> +\t\tif (!S_ISGITLINK(p->two->mode))\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tif (S_ISGITLINK(p->one->mode)) {\n>> +\t\t\t/* NEEDSWORK: We should honor the name configured in\n>> +\t\t\t * the .gitmodules file of the commit we are examining\n>> +\t\t\t * here to be able to correctly follow submodules\n>> +\t\t\t * being moved around. */\n>> +\t\t\tstruct string_list_item *path;\n>> +\t\t\tpath = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);\n>> +\t\t\tif (!path)\n>> +\t\t\t\tstring_list_append(&changed_submodule_paths, xstrdup(p->two->path));\n> \n> I wondered why there is no mention of \"data\" in the implementation of this\n> function; changed_submodule_paths global is used instead here.\n> \n> I do not see anywhere the global string_list is initialized, freed, nor\n> re-initialized for reuse.  Are you guaranteeing that the caller only calls\n> the check_for_new_submodule_commits() function once, and if so how?  The\n> function is called from update_local_ref() in builtin/fetch.c, which in\n> turn gets called number of times during a fetch.  IOW, does the code do\n> the right thing when you are fetching more than one refs?\n\nI assume that a string_list initialized with 0 is initialized properly.\nThe idea is to let check_for_new_submodule_commits() be called as often\nas needed and for each ref to collect all submodule changes recorded in\nany ref and afterwards only once call fetch_populated_submodules() to\ncollect all submodules touched by any commits on any ref. But maybe\nfetch_populated_submodules() should empty the string_list it just\nworked through?\n"},{"id":"162075","messageId":"4D659BB2.3020805@web.de","threadId":"26584","inReplyTo":"20110223230713.GB6819@elie","subject":"Re: [PATCH 1/6] fetch/pull: recurse into submodules when necessary","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T23:43:46Z","receivedAt":"2011-02-23T23:43:46Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.02.2011 00:07, schrieb Jonathan Nieder:\n> Jens Lehmann wrote:\n> \n>> To be able to access all commits of populated submodules referenced by the\n>> superproject it is sufficient to only then let \"git fetch\" recurse into a\n>> submodule when the new commits fetched in the superproject record new\n>> commits for it.\n> \n> Exciting stuff.  This would use the default refspec rather than fetching\n> the bare minimum of commits in submodules, right?\n\nYup, AFAIK I have no means right now to tell \"git fetch\" to only fetch\ncertain commits. But we were talking at the GitTogether that this might\nbe useful and with a bit of tweaking the code would support that too.\n\n>> --- a/Documentation/fetch-options.txt\n>> +++ b/Documentation/fetch-options.txt\n>> @@ -73,6 +73,14 @@ ifndef::git-pull[]\n>>  \tPrepend <path> to paths printed in informative messages\n>>  \tsuch as \"Fetching submodule foo\".  This option is used\n>>  \tinternally when recursing over submodules.\n>> +\n>> +--submodule-default=[yes|on-demand]::\n>> +\tThis option is used internally to set the submodule recursion default\n>> +\tto either a boolean configuration value representing \"true\" (for\n>> +\tunconditonal recursion) or to \"on-demand\" (when only those submodules\n>> +\tshould be fetched of which new commits have been fetched in its\n>> +\tsuperproject).\n>> +\tUsing the \"--[no-]recurse-submodules\" option ignores this setting.\n> \n> Could this be combined with the --recurse-submodules, with a \"last instance\n> of the option wins\" rule?  Something like:\n> \n>  --recurse-submodules[=(yes | no | changed)]::\n>  --no-recurse-submodules::\n\nNope, as this only sets the default. \"--recurse-submodules\" overrides\nanything configured, which is not what we want here. This option should\nonly set the default.\n\n>> +++ b/submodule.c\n> [...]\n>> @@ -248,11 +263,73 @@ void set_config_fetch_recurse_submodules(int value)\n> [...]\n>> +static void submodule_collect_changed_cb(struct diff_queue_struct *q,\n> [...]\n>> +\t\tif (S_ISGITLINK(p->one->mode)) {\n>> +\t\t\t/* NEEDSWORK: We should honor the name configured in\n>> +\t\t\t * the .gitmodules file of the commit we are examining\n>> +\t\t\t * here to be able to correctly follow submodules\n>> +\t\t\t * being moved around. */\n> \n> Hmm, a sort of variant on rename detection.  Does \"git submodule update\" /\n> \"git checkout --recurse-submodules\" use .gitmodules to move pre-populated\n> subrepositories?\n\nNot yet ;-)\n\n>> +\t\t\t/* Submodule is new or was moved here */\n>> +\t\t\t/* NEEDSWORK: When the .git directories of submodules\n>> +\t\t\t * live inside the superprojects .git directory some\n>> +\t\t\t * day we should fetch new submodules directly into\n>> +\t\t\t * that location too when config or options request\n>> +\t\t\t * that so they can be checked out from there. */\n>> +\t\t\tcontinue;\n> \n> Maybe this can be mentioned in a BUGS section on the git-fetch(1)\n> manpage to give readers a warning and clue about the intended\n> meaning of --recurse-submodules?\n> \n> I'm afraid a certain kind of person might get used to the \"don't fetch\n> new submodules\" behavior (e.g., if upstream is including unneeded\n> convenience copies of libraries right and left in addition to some\n> useful submodules).\n\nI'm not sure I understand what you mean by this, right now this can\nonly work for populated submodules. I hope this will change soon, but\nI'm not quite there yet ;-)\n\n>> +void check_for_new_submodule_commits(unsigned char new_sha1[20])\n>> +{\n>> +\tstruct rev_info rev;\n>> +\tstruct commit *commit;\n>> +\tint argc = 5;\n>> +\tconst char *argv[] = {NULL, NULL, \"--not\", \"--branches\", \"--remotes\", NULL};\n> \n> Nit: maybe\n> \n> \tconst char *argv[] = ...;\n> \tint argc = ARRAY_SIZE(argv) - 1;\n> \n> ?\n\nFine by me!\n\n>> +\n>> +\tinit_revisions(&rev, NULL);\n>> +\targv[1] = xstrdup(sha1_to_hex(new_sha1));\n> \n> Maybe:\n> \n> \tchar *new_rev;\n> \t...\n> \targv[1] = new_rev = xstrdup(...);\n> \t...\n> \tfree(new_rev);\n\nI'm not sure I get what the extra variable gains us ...\n\n>> +\tsetup_revisions(argc, argv, &rev, NULL);\n>> +\tif (prepare_revision_walk(&rev))\n>> +\t\tdie(\"revision walk setup failed\");\n>> +\n> \n> Maybe a comment so the reader doesn't have to delve deeper?\n\n?\n\n> \t/*\n> \t * Collect checked out submodules that have changed upstream\n> \t * in \"changed_submodule_paths\".\n> \t */\n> \n>> +\twhile ((commit = get_revision(&rev))) {\n> [...]\n>> +++ b/t/t5526-fetch-submodules.sh\n>> @@ -192,4 +192,113 @@ test_expect_success \"--no-recurse-submodules overrides config setting\" '\n> [...]\n>> +\techo \"Fetching submodule submodule\" > expect.out.sub &&\n>> +\techo \"From $pwd/.\" > expect.err.sub &&\n>> +\techo \"   $head1..$head2  master     -> origin/master\" >> expect.err.sub\n> \n> I wonder if we should be testing the output in this much detail.\n> \n> Wouldn't it be nicer to check the remote-tracking refs to make sure\n> the lasting effects were correct, rather than the detailed output\n> format?\n\nYes, that makes sense!\n\n> So: aside from the option name, nothing but nits.  Thanks; that was\n> fun.\n\nThanks!\n"},{"id":"162077","messageId":"4D659CE8.3030604@web.de","threadId":"26584","inReplyTo":"20110223232156.GE6819@elie","subject":"Re: [PATCH 0/6] Teach fetch/pull the on-demand mode and make it the default","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T23:48:56Z","receivedAt":"2011-02-23T23:48:56Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.02.2011 00:21, schrieb Jonathan Nieder:\n> Jens Lehmann wrote:\n> \n>> *) The fetch is only done when the recorded submodule commit isn't\n>>    already present.\n> \n> I like this part a lot.\n\nGlad to hear ;-)\n\n>> I tend to think that this is suited for 1.7.5 but don't have any\n>> objections against holding it back until 1.8.0 either. What do\n>> others think?\n> \n> I see no backward-compatibility to wait for this, but I would be more\n> included to trust people using \"git submodule update\" heavily than I\n> do.\n\nYeah, I would appreciate some feedback here too.\n\n> The \"submodule update\" change could cause the following to break.\n> Would that be disruptive?\n> \n> \tcd submodule\n> \tgit fetch --no-recurse-submodules\n> \t...\n> \n> \tcd ..\n> \tbin/script-to-update-submodules-that-calls-submodule-update\n\nBut then \"git submodule update\" would notice that the commit isn't\npresent and do a \"git fetch\" itself, no?\n"},{"id":"162078","messageId":"4D659D3A.2020704@web.de","threadId":"26584","inReplyTo":"7vaahm5oby.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/6] fetch/pull: Don't recurse into a submodule when commits are already present","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T23:50:18Z","receivedAt":"2011-02-23T23:50:18Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.02.2011 00:21, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n> \n>> diff --git a/submodule.c b/submodule.c\n>> index b477c3c..ddb0a29 100644\n>> --- a/submodule.c\n>> +++ b/submodule.c\n>> @@ -263,6 +263,13 @@ void set_config_fetch_recurse_submodules(int value)\n>>  \tconfig_fetch_recurse_submodules = value;\n>>  }\n>>\n>> +static int is_submodule_commit_present(const char *path, unsigned char sha1[20])\n>> +{\n>> +\tif (!add_submodule_odb(path))\n>> +\t\treturn lookup_commit_reference(sha1) != 0;\n>> +\treturn 0;\n>> +}\n> \n> Don't you need to prove the usual \"reachabile from the refs\" here, instead\n> of just the presense of a commit object?\n\nLooks like I would. Could you please enlighten me?\n"},{"id":"162080","messageId":"20110223235808.GB7286@elie","threadId":"26584","inReplyTo":"4D659BB2.3020805@web.de","subject":"Re: [PATCH 1/6] fetch/pull: recurse into submodules when necessary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-23T23:58:08Z","receivedAt":"2011-02-23T23:58:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n> Am 24.02.2011 00:07, schrieb Jonathan Nieder:\n\n>> Could this be combined with the --recurse-submodules, with a \"last instance\n>> of the option wins\" rule?  Something like:\n>> \n>>  --recurse-submodules[=(yes | no | changed)]::\n>>  --no-recurse-submodules::\n>\n> Nope, as this only sets the default. \"--recurse-submodules\" overrides\n> anything configured, which is not what we want here. This option should\n> only set the default.\n\nAh.  So --recurse-submodules-default means \"like --recurse-submodules,\nbut with lower precedence than the configuration\".  Sensible.  (Maybe\nit could be documented in --help-all that way?)\n\n>>> +\t\t\t/* Submodule is new or was moved here */\n>>> +\t\t\t/* NEEDSWORK: When the .git directories of submodules\n>>> +\t\t\t * live inside the superprojects .git directory some\n>>> +\t\t\t * day we should fetch new submodules directly into\n>>> +\t\t\t * that location too when config or options request\n>>> +\t\t\t * that so they can be checked out from there. */\n>>> +\t\t\tcontinue;\n>> \n>> Maybe this can be mentioned in a BUGS section on the git-fetch(1)\n>> manpage to give readers a warning and clue about the intended\n>> meaning of --recurse-submodules?\n[...]\n> I'm not sure I understand what you mean by this, right now this can\n> only work for populated submodules. I hope this will change soon, but\n> I'm not quite there yet ;-)\n\nWhat I mean is the following: to make life easier for people and\nscripts using --recurse-submodules today, it might be nice to\ndocument how stable or unstable its meaning is.\n\nIn this case, there is a plan to make --recurse-submodules=on-demand\ndo more in the future than it does now;\n\n - a note in BUGS could explain that --recurse-submodules's current\n   behavior is considered an infelicity and likely to change;\n\n - unfortunately not all users will necessarily see it that way (c.f.\n   aforementioned use case), so it might be better to plan on yet\n   another choice in the list of options provided by\n   --recurse-submodules.\n\n>> Maybe:\n>>\n>> \tchar *new_rev;\n>> \t...\n>> \targv[1] = new_rev = xstrdup(...);\n>> \t...\n>> \tfree(new_rev);\n>\n> I'm not sure I get what the extra variable gains us ...\n\nA reminder to free that memory in all code paths exiting the function.\nBut it might not be worth the noise.\n\n>> Maybe a comment so the reader doesn't have to delve deeper?\n>\n> ?\n\nSorry for the confusion.  That suggestion refers to the loop after it\nrather than what comes before it.\n\n>> \t/*\n>> \t * Collect checked out submodules that have changed upstream\n>> \t * in \"changed_submodule_paths\".\n>> \t */\n>> \n>>> +\twhile ((commit = get_revision(&rev))) {\n[...]\n> Thanks!\n\nThanks for deciphering the gibberish I sent. :)\nJonathan\n"},{"id":"162085","messageId":"20110224002236.GB7433@elie","threadId":"26584","inReplyTo":"4D659818.4070107@web.de","subject":"Re: [PATCH 1/6] fetch/pull: recurse into submodules when necessary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-24T00:22:49Z","receivedAt":"2011-02-24T00:22:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n\n> But maybe\n> fetch_populated_submodules() should empty the string_list it just\n> worked through?\n\nNo, I don't think so.  There might be cases where you want to fetch\nand then check out the changed submodules some day.\n\nWhat might be nice is for cmd_fetch to clear the list before it\nreturns.  That way, if this functionality ever gets lib-ified then\nthere will be that reminder about how to reset the state.  And\nvalgrind can be a little quieter. :)\n"},{"id":"162113","messageId":"4D6614CF.2020406@web.de","threadId":"26584","inReplyTo":"4D659D3A.2020704@web.de","subject":"Re: [PATCH 5/6] fetch/pull: Don't recurse into a submodule when commits are already present","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-24T08:20:31Z","receivedAt":"2011-02-24T08:20:31Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.02.2011 00:50, schrieb Jens Lehmann:\n> Am 24.02.2011 00:21, schrieb Junio C Hamano:\n>> Jens Lehmann <Jens.Lehmann@web.de> writes:\n>>\n>>> diff --git a/submodule.c b/submodule.c\n>>> index b477c3c..ddb0a29 100644\n>>> --- a/submodule.c\n>>> +++ b/submodule.c\n>>> @@ -263,6 +263,13 @@ void set_config_fetch_recurse_submodules(int value)\n>>>  \tconfig_fetch_recurse_submodules = value;\n>>>  }\n>>>\n>>> +static int is_submodule_commit_present(const char *path, unsigned char sha1[20])\n>>> +{\n>>> +\tif (!add_submodule_odb(path))\n>>> +\t\treturn lookup_commit_reference(sha1) != 0;\n>>> +\treturn 0;\n>>> +}\n>>\n>> Don't you need to prove the usual \"reachabile from the refs\" here, instead\n>> of just the presense of a commit object?\n> \n> Looks like I would. Could you please enlighten me?\n\nOf course you are right. Will change that.\n"},{"id":"162190","messageId":"4D66C32E.5090103@web.de","threadId":"26584","inReplyTo":"7vei6y5ol6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-24T20:44:30Z","receivedAt":"2011-02-24T20:44:30Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.02.2011 00:16, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n> \n>> diff --git a/submodule.c b/submodule.c\n>> index cccd728..b477c3c 100644\n>> --- a/submodule.c\n>> +++ b/submodule.c\n>> @@ -113,7 +113,7 @@ int parse_submodule_config_option(const char *var, const char *value)\n>>  \t\tif (!config)\n>>  \t\t\tconfig = string_list_append(&config_fetch_recurse_submodules_for_name,\n>>  \t\t\t\t\t\t    strbuf_detach(&submodname, NULL));\n>> -\t\tconfig->util = git_config_bool(var, value) ? (void *)1 : NULL;\n>> +\t\tconfig->util = (void *)(size_t)parse_fetch_recurse_submodules_arg(value);\n> \n> What is this double-cast about?\n\n64bit gcc warns when I drop either of them because a 32bit integer\nis assigned to a 64bit wide pointer here.\n\n>> @@ -376,8 +376,13 @@ int fetch_populated_submodules(int num_options, const char **options,\n>> ...\n>> +\t\t\t\tif ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_OFF)\n>>  \t\t\t\t\tcontinue;\n>> +\t\t\t\tif ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_ON_DEMAND) {\n> \n> Likewise here; size_t feels a strange type to cast to in this comparison\n> between (void *) and an enum, no?\n\nI get a warning here if I drop the second cast. gcc doesn't warn if\nI drop the first one, but that is just because the enum value happens\nto be 0 there. So I added that cast there too to be on the safe side\nin case the value changes in the future and to be consistent to other\nreaders of this code.\n"}]}