{"thread":{"id":"44494","subject":"[PATCH v4 0/4] Speedup finding of unpushed submodules","startedAt":"2016-11-16T15:12:07Z","lastAt":"2016-11-17T17:42:09Z","messageCount":8,"participants":["Heiko Voigt","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":4,"patchTotal":4},"messages":[{"id":"306049","messageId":"cover.1479308877.git.hvoigt@hvoigt.net","threadId":"44494","inReplyTo":null,"subject":"[PATCH v4 0/4] Speedup finding of unpushed submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-11-16T15:11:03Z","receivedAt":"2016-11-16T15:12:07Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"You can find the third iteration of this series here:\n\nhttp://public-inbox.org/git/cover.1479221071.git.hvoigt@hvoigt.net/\n\nAll comments from the last iteration should be addressed.\n\nCheers Heiko\n\nHeiko Voigt (4):\n  serialize collection of changed submodules\n  serialize collection of refs that contain submodule changes\n  batch check whether submodule needs pushing into one call\n  submodule_needs_pushing() NEEDSWORK when we can not answer this\n    question\n\n submodule.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++-------------\n submodule.h |   5 ++-\n transport.c |  29 ++++++++++----\n 3 files changed, 121 insertions(+), 36 deletions(-)\n\n-- \n2.10.1.386.gc503e45\n\n"},{"id":"306050","messageId":"a71bae460cddfa7a532d06abba5c229446c5ed29.1479308877.git.hvoigt@hvoigt.net","threadId":"44494","inReplyTo":"cover.1479308877.git.hvoigt@hvoigt.net","subject":"[PATCH v4 1/4] serialize collection of changed submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-11-16T15:11:04Z","receivedAt":"2016-11-16T15:12:12Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"To check whether a submodule needs to be pushed we need to collect all\nchanged submodules. Lets collect them first and then execute the\npossibly expensive test whether certain revisions are already pushed\nonly once per submodule.\n\nThere is further potential for optimization since we can assemble one\ncommand and only issued that instead of one call for each remote ref in\nthe submodule.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n submodule.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 55 insertions(+), 4 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 6f7d883..b2908fe 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -532,19 +532,34 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20\n \treturn 0;\n }\n \n+static struct sha1_array *submodule_commits(struct string_list *submodules,\n+\t\t\t\t\t    const char *path)\n+{\n+\tstruct string_list_item *item;\n+\n+\titem = string_list_insert(submodules, path);\n+\tif (item->util)\n+\t\treturn (struct sha1_array *) item->util;\n+\n+\t/* NEEDSWORK: should we have sha1_array_init()? */\n+\titem->util = xcalloc(1, sizeof(struct sha1_array));\n+\treturn (struct sha1_array *) item->util;\n+}\n+\n static void collect_submodules_from_diff(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-\tstruct string_list *needs_pushing = data;\n+\tstruct string_list *submodules = data;\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n+\t\tstruct sha1_array *commits;\n \t\tif (!S_ISGITLINK(p->two->mode))\n \t\t\tcontinue;\n-\t\tif (submodule_needs_pushing(p->two->path, p->two->oid.hash))\n-\t\t\tstring_list_insert(needs_pushing, p->two->path);\n+\t\tcommits = submodule_commits(submodules, p->two->path);\n+\t\tsha1_array_append(commits, p->two->oid.hash);\n \t}\n }\n \n@@ -560,6 +575,30 @@ static void find_unpushed_submodule_commits(struct commit *commit,\n \tdiff_tree_combined_merge(commit, 1, &rev);\n }\n \n+struct collect_submodule_from_sha1s_data {\n+\tchar *submodule_path;\n+\tstruct string_list *needs_pushing;\n+};\n+\n+static int collect_submodules_from_sha1s(const unsigned char sha1[20],\n+\t\tvoid *data)\n+{\n+\tstruct collect_submodule_from_sha1s_data *me = data;\n+\n+\tif (submodule_needs_pushing(me->submodule_path, sha1))\n+\t\tstring_list_insert(me->needs_pushing, me->submodule_path);\n+\n+\treturn 0;\n+}\n+\n+static void free_submodules_sha1s(struct string_list *submodules)\n+{\n+\tstruct string_list_item *item;\n+\tfor_each_string_list_item(item, submodules)\n+\t\tsha1_array_clear((struct sha1_array *) item->util);\n+\tstring_list_clear(submodules, 1);\n+}\n+\n int find_unpushed_submodules(unsigned char new_sha1[20],\n \t\tconst char *remotes_name, struct string_list *needs_pushing)\n {\n@@ -568,6 +607,8 @@ int find_unpushed_submodules(unsigned char new_sha1[20],\n \tconst char *argv[] = {NULL, NULL, \"--not\", \"NULL\", NULL};\n \tint argc = ARRAY_SIZE(argv) - 1;\n \tchar *sha1_copy;\n+\tstruct string_list submodules = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *submodule;\n \n \tstruct strbuf remotes_arg = STRBUF_INIT;\n \n@@ -581,12 +622,22 @@ int find_unpushed_submodules(unsigned char new_sha1[20],\n \t\tdie(\"revision walk setup failed\");\n \n \twhile ((commit = get_revision(&rev)) != NULL)\n-\t\tfind_unpushed_submodule_commits(commit, needs_pushing);\n+\t\tfind_unpushed_submodule_commits(commit, &submodules);\n \n \treset_revision_walk();\n \tfree(sha1_copy);\n \tstrbuf_release(&remotes_arg);\n \n+\tfor_each_string_list_item(submodule, &submodules) {\n+\t\tstruct collect_submodule_from_sha1s_data data;\n+\t\tdata.submodule_path = submodule->string;\n+\t\tdata.needs_pushing = needs_pushing;\n+\t\tsha1_array_for_each_unique((struct sha1_array *) submodule->util,\n+\t\t\t\tcollect_submodules_from_sha1s,\n+\t\t\t\t&data);\n+\t}\n+\tfree_submodules_sha1s(&submodules);\n+\n \treturn needs_pushing->nr;\n }\n \n-- \n2.10.1.386.gc503e45\n\n"},{"id":"306051","messageId":"9c95594f73625e06374f323fa5dc7d6487aa0356.1479308877.git.hvoigt@hvoigt.net","threadId":"44494","inReplyTo":"cover.1479308877.git.hvoigt@hvoigt.net","subject":"[PATCH v4 4/4] submodule_needs_pushing() NEEDSWORK when we can not answer this question","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-11-16T15:11:07Z","receivedAt":"2016-11-16T15:12:17Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n submodule.c | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex 11391fa..00dd655 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -531,6 +531,17 @@ static int submodule_has_commits(const char *path, struct sha1_array *commits)\n static int submodule_needs_pushing(const char *path, struct sha1_array *commits)\n {\n \tif (!submodule_has_commits(path, commits))\n+\t\t/*\n+\t\t * NOTE: We do consider it safe to return \"no\" here. The\n+\t\t * correct answer would be \"We do not know\" instead of\n+\t\t * \"No push needed\", but it is quite hard to change\n+\t\t * the submodule pointer without having the submodule\n+\t\t * around. If a user did however change the submodules\n+\t\t * without having the submodule around, this indicates\n+\t\t * an expert who knows what they are doing or a\n+\t\t * maintainer integrating work from other people. In\n+\t\t * both cases it should be safe to skip this check.\n+\t\t */\n \t\treturn 0;\n \n \tif (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {\n-- \n2.10.1.386.gc503e45\n\n"},{"id":"306052","messageId":"3bb9abb760ac8bed0e980aa98a1767740104b2d9.1479308877.git.hvoigt@hvoigt.net","threadId":"44494","inReplyTo":"cover.1479308877.git.hvoigt@hvoigt.net","subject":"[PATCH v4 2/4] serialize collection of refs that contain submodule changes","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-11-16T15:11:05Z","receivedAt":"2016-11-16T15:12:21Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"We are iterating over each pushed ref and want to check whether it\ncontains changes to submodules. Instead of immediately checking each ref\nlets first collect them and then do the check for all of them in one\nrevision walk.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n submodule.c | 35 ++++++++++++++++++++---------------\n submodule.h |  5 +++--\n transport.c | 29 +++++++++++++++++++++--------\n 3 files changed, 44 insertions(+), 25 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex b2908fe..12ac1ea 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -500,6 +500,13 @@ static int has_remote(const char *refname, const struct object_id *oid,\n \treturn 1;\n }\n \n+static int append_sha1_to_argv(const unsigned char sha1[20], void *data)\n+{\n+\tstruct argv_array *argv = data;\n+\targv_array_push(argv, sha1_to_hex(sha1));\n+\treturn 0;\n+}\n+\n static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])\n {\n \tif (add_submodule_odb(path) || !lookup_commit_reference(sha1))\n@@ -599,25 +606,24 @@ static void free_submodules_sha1s(struct string_list *submodules)\n \tstring_list_clear(submodules, 1);\n }\n \n-int find_unpushed_submodules(unsigned char new_sha1[20],\n+int find_unpushed_submodules(struct sha1_array *commits,\n \t\tconst char *remotes_name, struct string_list *needs_pushing)\n {\n \tstruct rev_info rev;\n \tstruct commit *commit;\n-\tconst char *argv[] = {NULL, NULL, \"--not\", \"NULL\", NULL};\n-\tint argc = ARRAY_SIZE(argv) - 1;\n-\tchar *sha1_copy;\n \tstruct string_list submodules = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *submodule;\n+\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n-\tstruct strbuf remotes_arg = STRBUF_INIT;\n-\n-\tstrbuf_addf(&remotes_arg, \"--remotes=%s\", remotes_name);\n \tinit_revisions(&rev, NULL);\n-\tsha1_copy = xstrdup(sha1_to_hex(new_sha1));\n-\targv[1] = sha1_copy;\n-\targv[3] = remotes_arg.buf;\n-\tsetup_revisions(argc, argv, &rev, NULL);\n+\n+\t/* argv.argv[0] will be ignored by setup_revisions */\n+\targv_array_push(&argv, \"find_unpushed_submodules\");\n+\tsha1_array_for_each_unique(commits, append_sha1_to_argv, &argv);\n+\targv_array_push(&argv, \"--not\");\n+\targv_array_pushf(&argv, \"--remotes=%s\", remotes_name);\n+\n+\tsetup_revisions(argv.argc, argv.argv, &rev, NULL);\n \tif (prepare_revision_walk(&rev))\n \t\tdie(\"revision walk setup failed\");\n \n@@ -625,8 +631,7 @@ int find_unpushed_submodules(unsigned char new_sha1[20],\n \t\tfind_unpushed_submodule_commits(commit, &submodules);\n \n \treset_revision_walk();\n-\tfree(sha1_copy);\n-\tstrbuf_release(&remotes_arg);\n+\targv_array_clear(&argv);\n \n \tfor_each_string_list_item(submodule, &submodules) {\n \t\tstruct collect_submodule_from_sha1s_data data;\n@@ -663,12 +668,12 @@ static int push_submodule(const char *path)\n \treturn 1;\n }\n \n-int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name)\n+int push_unpushed_submodules(struct sha1_array *commits, const char *remotes_name)\n {\n \tint i, ret = 1;\n \tstruct string_list needs_pushing = STRING_LIST_INIT_DUP;\n \n-\tif (!find_unpushed_submodules(new_sha1, remotes_name, &needs_pushing))\n+\tif (!find_unpushed_submodules(commits, remotes_name, &needs_pushing))\n \t\treturn 1;\n \n \tfor (i = 0; i < needs_pushing.nr; i++) {\ndiff --git a/submodule.h b/submodule.h\nindex d9e197a..9454806 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -3,6 +3,7 @@\n \n struct diff_options;\n struct argv_array;\n+struct sha1_array;\n \n enum {\n \tRECURSE_SUBMODULES_CHECK = -4,\n@@ -62,9 +63,9 @@ int submodule_uses_gitfile(const char *path);\n int ok_to_remove_submodule(const char *path);\n int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],\n \t\t    const unsigned char a[20], const unsigned char b[20], int search);\n-int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,\n+int find_unpushed_submodules(struct sha1_array *commits, const char *remotes_name,\n \t\tstruct string_list *needs_pushing);\n-int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n+int push_unpushed_submodules(struct sha1_array *commits, const char *remotes_name);\n void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n int parallel_submodules(void);\n \ndiff --git a/transport.c b/transport.c\nindex d57e8de..f482869 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -949,23 +949,36 @@ int transport_push(struct transport *transport,\n \n \t\tif ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n+\t\t\tstruct sha1_array commits = SHA1_ARRAY_INIT;\n+\n \t\t\tfor (; ref; ref = ref->next)\n-\t\t\t\tif (!is_null_oid(&ref->new_oid) &&\n-\t\t\t\t    !push_unpushed_submodules(ref->new_oid.hash,\n-\t\t\t\t\t    transport->remote->name))\n-\t\t\t\t    die (\"Failed to push all needed submodules!\");\n+\t\t\t\tif (!is_null_oid(&ref->new_oid))\n+\t\t\t\t\tsha1_array_append(&commits, ref->new_oid.hash);\n+\n+\t\t\tif (!push_unpushed_submodules(&commits, transport->remote->name)) {\n+\t\t\t\tsha1_array_clear(&commits);\n+\t\t\t\tdie(\"Failed to push all needed submodules!\");\n+\t\t\t}\n+\t\t\tsha1_array_clear(&commits);\n \t\t}\n \n \t\tif ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |\n \t\t\t      TRANSPORT_RECURSE_SUBMODULES_CHECK)) && !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n \t\t\tstruct string_list needs_pushing = STRING_LIST_INIT_DUP;\n+\t\t\tstruct sha1_array commits = SHA1_ARRAY_INIT;\n \n \t\t\tfor (; ref; ref = ref->next)\n-\t\t\t\tif (!is_null_oid(&ref->new_oid) &&\n-\t\t\t\t    find_unpushed_submodules(ref->new_oid.hash,\n-\t\t\t\t\t    transport->remote->name, &needs_pushing))\n-\t\t\t\t\tdie_with_unpushed_submodules(&needs_pushing);\n+\t\t\t\tif (!is_null_oid(&ref->new_oid))\n+\t\t\t\t\tsha1_array_append(&commits, ref->new_oid.hash);\n+\n+\t\t\tif (find_unpushed_submodules(&commits, transport->remote->name,\n+\t\t\t\t\t\t&needs_pushing)) {\n+\t\t\t\tsha1_array_clear(&commits);\n+\t\t\t\tdie_with_unpushed_submodules(&needs_pushing);\n+\t\t\t}\n+\t\t\tstring_list_clear(&needs_pushing, 0);\n+\t\t\tsha1_array_clear(&commits);\n \t\t}\n \n \t\tpush_ret = transport->push_refs(transport, remote_refs, flags);\n-- \n2.10.1.386.gc503e45\n\n"},{"id":"306053","messageId":"3998e7aa1e1071d068e8d3eb650bce18cc1a740d.1479308877.git.hvoigt@hvoigt.net","threadId":"44494","inReplyTo":"cover.1479308877.git.hvoigt@hvoigt.net","subject":"[PATCH v4 3/4] batch check whether submodule needs pushing into one call","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-11-16T15:11:06Z","receivedAt":"2016-11-16T15:12:45Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"We run a command for each sha1 change in a submodule. This is\nunnecessary since we can simply batch all sha1's we want to check into\none command. Lets do it so we can speedup the check when many submodule\nchanges are in need of checking.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n submodule.c | 62 ++++++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 33 insertions(+), 29 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 12ac1ea..11391fa 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -507,27 +507,49 @@ static int append_sha1_to_argv(const unsigned char sha1[20], void *data)\n \treturn 0;\n }\n \n-static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])\n+static int check_has_commit(const unsigned char sha1[20], void *data)\n {\n-\tif (add_submodule_odb(path) || !lookup_commit_reference(sha1))\n+\tint *has_commit = data;\n+\n+\tif (!lookup_commit_reference(sha1))\n+\t\t*has_commit = 0;\n+\n+\treturn 0;\n+}\n+\n+static int submodule_has_commits(const char *path, struct sha1_array *commits)\n+{\n+\tint has_commit = 1;\n+\n+\tif (add_submodule_odb(path))\n+\t\treturn 0;\n+\n+\tsha1_array_for_each_unique(commits, check_has_commit, &has_commit);\n+\treturn has_commit;\n+}\n+\n+static int submodule_needs_pushing(const char *path, struct sha1_array *commits)\n+{\n+\tif (!submodule_has_commits(path, commits))\n \t\treturn 0;\n \n \tif (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {\n \t\tstruct child_process cp = CHILD_PROCESS_INIT;\n-\t\tconst char *argv[] = {\"rev-list\", NULL, \"--not\", \"--remotes\", \"-n\", \"1\" , NULL};\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\tint needs_pushing = 0;\n \n-\t\targv[1] = sha1_to_hex(sha1);\n-\t\tcp.argv = argv;\n+\t\targv_array_push(&cp.args, \"rev-list\");\n+\t\tsha1_array_for_each_unique(commits, append_sha1_to_argv, &cp.args);\n+\t\targv_array_pushl(&cp.args, \"--not\", \"--remotes\", \"-n\", \"1\" , NULL);\n+\n \t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n \t\tcp.out = -1;\n \t\tcp.dir = path;\n \t\tif (start_command(&cp))\n-\t\t\tdie(\"Could not run 'git rev-list %s --not --remotes -n 1' command in submodule %s\",\n-\t\t\t\tsha1_to_hex(sha1), path);\n+\t\t\tdie(\"Could not run 'git rev-list <commits> --not --remotes -n 1' command in submodule %s\",\n+\t\t\t\t\tpath);\n \t\tif (strbuf_read(&buf, cp.out, 41))\n \t\t\tneeds_pushing = 1;\n \t\tfinish_command(&cp);\n@@ -582,22 +604,6 @@ static void find_unpushed_submodule_commits(struct commit *commit,\n \tdiff_tree_combined_merge(commit, 1, &rev);\n }\n \n-struct collect_submodule_from_sha1s_data {\n-\tchar *submodule_path;\n-\tstruct string_list *needs_pushing;\n-};\n-\n-static int collect_submodules_from_sha1s(const unsigned char sha1[20],\n-\t\tvoid *data)\n-{\n-\tstruct collect_submodule_from_sha1s_data *me = data;\n-\n-\tif (submodule_needs_pushing(me->submodule_path, sha1))\n-\t\tstring_list_insert(me->needs_pushing, me->submodule_path);\n-\n-\treturn 0;\n-}\n-\n static void free_submodules_sha1s(struct string_list *submodules)\n {\n \tstruct string_list_item *item;\n@@ -634,12 +640,10 @@ int find_unpushed_submodules(struct sha1_array *commits,\n \targv_array_clear(&argv);\n \n \tfor_each_string_list_item(submodule, &submodules) {\n-\t\tstruct collect_submodule_from_sha1s_data data;\n-\t\tdata.submodule_path = submodule->string;\n-\t\tdata.needs_pushing = needs_pushing;\n-\t\tsha1_array_for_each_unique((struct sha1_array *) submodule->util,\n-\t\t\t\tcollect_submodules_from_sha1s,\n-\t\t\t\t&data);\n+\t\tstruct sha1_array *commits = (struct sha1_array *) submodule->util;\n+\n+\t\tif (submodule_needs_pushing(submodule->string, commits))\n+\t\t\tstring_list_insert(needs_pushing, submodule->string);\n \t}\n \tfree_submodules_sha1s(&submodules);\n \n-- \n2.10.1.386.gc503e45\n\n"},{"id":"306076","messageId":"xmqqoa1fp72o.fsf@gitster.mtv.corp.google.com","threadId":"44494","inReplyTo":"9c95594f73625e06374f323fa5dc7d6487aa0356.1479308877.git.hvoigt@hvoigt.net","subject":"Re: [PATCH v4 4/4] submodule_needs_pushing() NEEDSWORK when we can not answer this question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T19:18:07Z","receivedAt":"2016-11-16T19:18:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt <hvoigt@hvoigt.net> writes:\n\n> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n> ---\n\nNeeds retitle ;-)  Here is what I tentatively queued.\n\n    submodule_needs_pushing(): explain the behaviour when we cannot answer\n    \n    When we do not have commits that are involved in the update of the\n    superproject in our copy of submodule, we cannot tell if the remote\n    end needs to acquire these commits to be able to check out the\n    superproject tree.  Explain why we answer \"no there is no need/point\n    in pushing from our submodule repository\" in this case.\n    \n    Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\n>  submodule.c | 11 +++++++++++\n>  1 file changed, 11 insertions(+)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 11391fa..00dd655 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -531,6 +531,17 @@ static int submodule_has_commits(const char *path, struct sha1_array *commits)\n>  static int submodule_needs_pushing(const char *path, struct sha1_array *commits)\n>  {\n>  \tif (!submodule_has_commits(path, commits))\n> +\t\t/*\n> +\t\t * NOTE: We do consider it safe to return \"no\" here. The\n> +\t\t * correct answer would be \"We do not know\" instead of\n> +\t\t * \"No push needed\", but it is quite hard to change\n> +\t\t * the submodule pointer without having the submodule\n> +\t\t * around. If a user did however change the submodules\n> +\t\t * without having the submodule around, this indicates\n> +\t\t * an expert who knows what they are doing or a\n> +\t\t * maintainer integrating work from other people. In\n> +\t\t * both cases it should be safe to skip this check.\n> +\t\t */\n>  \t\treturn 0;\n>  \n>  \tif (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {\n"},{"id":"306081","messageId":"20161116213100.GA38510@book.hvoigt.net","threadId":"44494","inReplyTo":"xmqqoa1fp72o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/4] submodule_needs_pushing() NEEDSWORK when we can not answer this question","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-11-16T21:31:00Z","receivedAt":"2016-11-16T21:31:20Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Wed, Nov 16, 2016 at 11:18:07AM -0800, Junio C Hamano wrote:\n> Heiko Voigt <hvoigt@hvoigt.net> writes:\n> \n> > Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n> > ---\n> \n> Needs retitle ;-)  Here is what I tentatively queued.\n\nThanks ;-) Missed that one.\n\n>     submodule_needs_pushing(): explain the behaviour when we cannot answer\n>     \n>     When we do not have commits that are involved in the update of the\n>     superproject in our copy of submodule, we cannot tell if the remote\n>     end needs to acquire these commits to be able to check out the\n>     superproject tree.  Explain why we answer \"no there is no need/point\n>     in pushing from our submodule repository\" in this case.\n>     \n>     Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n>     Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nSound fine to me.\n\nCheers Heiko\n"},{"id":"306106","messageId":"CAGZ79kaSfPicBj9Re9+kLuF0Y+K4T8M-k+O8y1r9QNS-bX36vA@mail.gmail.com","threadId":"44494","inReplyTo":"cover.1479308877.git.hvoigt@hvoigt.net","subject":"Re: [PATCH v4 0/4] Speedup finding of unpushed submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-17T17:41:57Z","receivedAt":"2016-11-17T17:42:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 16, 2016 at 7:11 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> You can find the third iteration of this series here:\n>\n> http://public-inbox.org/git/cover.1479221071.git.hvoigt@hvoigt.net/\n>\n> All comments from the last iteration should be addressed.\n>\n> Cheers Heiko\n\nThanks for this series!\n\nI looked at the updated diff of hv/submodule-not-yet-pushed-fix\n(git diff a1a385d..250ab24) and the series looks good to me.\n\nThanks,\nStefan\n"}]}