{"thread":{"id":"27936","subject":"[PATCH v2 1/3] test whether push checks for unpushed remotes in submodules","startedAt":"2011-07-27T18:10:47Z","lastAt":"2011-08-01T01:16:52Z","messageCount":10,"participants":["Fredrik Gustafsson","Junio C Hamano","Jens Lehmann"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"172185","messageId":"1311790250-32454-1-git-send-email-iveqy@iveqy.com","threadId":"27936","inReplyTo":null,"subject":"[PATCH v2 0/3] check for unpushed remotes in submodules","fromName":"Fredrik Gustafsson","fromEmail":"iveqy@iveqy.com","sentAt":"2011-07-27T18:10:47Z","receivedAt":"2011-07-27T18:10:47Z","isPatch":true,"sender":{"key":"iveqy@iveqy.com","avatar":"https://avatars.githubusercontent.com/u/761743?v=4"},"body":"When working with submodules it is easy to end up in a state when submodule\ncommits required by the superproject only is present locally. This is most\noften a human error (although technical errors such as connection failure\ncan be a reason).\n\nThis patch-series tries to prevent pushing a superproject if not all (by\nthe superproject used) submodules are pushed first. This will prevent the\nhuman error of forgetting to push submodules before pushing the\nsuperproject.\n\nThe first iteration of this patch series can be found here:\nhttp://thread.gmane.org/gmane.comp.version-control.git/176328/focus=176327\n\nThe changes from the previous iteration are in patch 0003.\n\nRegarding the discussion of superprojects with submodules that have no\nremote tracking branches: A push will still be denied. After some\ndiscussion we did not consider this a drawback since all submodules that\nare added/handled using the submodule script will have remote tracking\nbranches. Even if the submodules are not pushable when the superproject\nrecords a commit that is reachable from a remote branch the push will not\nbe denied.\n\nA new test is added in this iteration to show a bug that now is fixed.\n\nFredrik Gustafsson (2):\n push: Don't push a repository with unpushed submodules\n push: Add the --no-recurse-submodules option\n\nHeiko Voigt (1):\n test whether push checks for unpushed remotes in submodules\n\n Documentation/git-push.txt | 6 ++\n builtin/push.c | 1 +\n submodule.c | 115 ++++++++++++++++++++++++++++++++++++++++\n submodule.h | 1 +\n t/t5531-deep-submodule-push.sh | 99 ++++++++++++++++++++++++++++++++++\n transport.c | 10 ++++\n transport.h | 1 +\n 7 files changed, 233 insertions(+), 0 deletions(-)\n\n-- \n1.7.6.236.g7ad21\n"},{"id":"172182","messageId":"1311790250-32454-2-git-send-email-iveqy@iveqy.com","threadId":"27936","inReplyTo":"1311790250-32454-1-git-send-email-iveqy@iveqy.com","subject":"[PATCH v2 1/3] test whether push checks for unpushed remotes in submodules","fromName":"Fredrik Gustafsson","fromEmail":"iveqy@iveqy.com","sentAt":"2011-07-27T18:10:48Z","receivedAt":"2011-07-27T18:10:48Z","isPatch":true,"sender":{"key":"iveqy@iveqy.com","avatar":"https://avatars.githubusercontent.com/u/761743?v=4"},"body":"From: Heiko Voigt \n\nThese tests are used to extend push to check whether all recorded\nsubmodule commits have also been pushed.\n\nSigned-off-by: Heiko Voigt \n---\n t/t5531-deep-submodule-push.sh | 46 +++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 45 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex faa2e96..0b55466 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -28,8 +28,52 @@ test_expect_success setup '\n test_expect_success push '\n \t(\n \t\tcd work &&\n-\t\tgit push ../pub.git master\n+\t\tgit push -f ../pub.git master\n \t)\n '\n \n+test_expect_failure 'push fails if submodule has no remote' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>junk2 &&\n+\t\tgit add junk2 &&\n+\t\tgit commit -m \"Second junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Second commit for gar/bage\" &&\n+\t\ttest_must_fail git push ../pub.git master\n+\t)\n+'\n+\n+test_expect_failure 'push fails if submodule commit not on remote' '\n+\t(\n+\t\tcd work/gar &&\n+\t\tgit clone --bare bage ../../submodule.git &&\n+\t\tcd bage &&\n+\t\tgit remote add origin ../../../submodule.git &&\n+\t\tgit fetch &&\n+\t\t>junk3 &&\n+\t\tgit add junk3 &&\n+\t\tgit commit -m \"Third junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Third commit for gar/bage\" &&\n+\t\ttest_must_fail git push ../pub.git master\n+\t)\n+'\n+\n+test_expect_failure 'push succeeds after commit was pushed to remote' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\tgit push origin master\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit push ../pub.git master\n+\t)\n+'\n test_done\n-- \n1.7.6.236.g7ad21\n"},{"id":"172183","messageId":"1311790250-32454-3-git-send-email-iveqy@iveqy.com","threadId":"27936","inReplyTo":"1311790250-32454-1-git-send-email-iveqy@iveqy.com","subject":"[PATCH v2 2/3] push: Don't push a repository with unpushed submodules","fromName":"Fredrik Gustafsson","fromEmail":"iveqy@iveqy.com","sentAt":"2011-07-27T18:10:49Z","receivedAt":"2011-07-27T18:10:49Z","isPatch":true,"sender":{"key":"iveqy@iveqy.com","avatar":"https://avatars.githubusercontent.com/u/761743?v=4"},"body":"When working with submodules it is easy to forget to push a submodule\nto the server but pushing a super-project that contains a commit for\nthat submodule. The result is that the superproject points at a\nsubmodule commit that is not avaliable on the server.\n\nCheck that all submodule commits that are about to be pushed are present\non a remote of the submodule and require forcing if that is not the\ncase.\n\nThis does not guarantee that all submodules a super-project needs will be\navailable on the server. In that case both the super-project and the\nsubmodules would need an atomic push. This does however prevent the\nhuman error of forgetting to push a submodule.\n\nSigned-off-by: Fredrik Gustafsson \nMentored-by: Jens Lehmann \nMentored-by: Heiko Voigt \n---\n submodule.c | 115 ++++++++++++++++++++++++++++++++++++++++\n submodule.h | 1 +\n t/t5531-deep-submodule-push.sh | 54 ++++++++++++++++++-\n transport.c | 8 +++\n 4 files changed, 175 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 1ba9646..3820f1b 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -308,6 +308,121 @@ void set_config_fetch_recurse_submodules(int value)\n \tconfig_fetch_recurse_submodules = value;\n }\n \n+static int is_submodule_commit_pushed(const char *path, const unsigned char sha1[20])\n+{\n+\tint is_pushed = 0;\n+\tif (!add_submodule_odb(path) && lookup_commit_reference(sha1)) {\n+\t\tstruct child_process cp;\n+\t\tconst char *argv[] = {\"rev-list\", NULL, \"--not\", \"--remotes\", \"-n\", \"1\" , NULL};\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\targv[1] = sha1_to_hex(sha1);\n+\t\tmemset(&cp, 0, sizeof(cp));\n+\t\tcp.argv = argv;\n+\t\tcp.env = local_repo_env;\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 (!run_command(&cp) && !strbuf_read(&buf, cp.out, 41))\n+\t\t\tis_pushed = 1;\n+\t\tclose(cp.out);\n+\t\tstrbuf_release(&buf);\n+\t}\n+\treturn is_pushed;\n+}\n+\n+static void collect_unpushed_submodules_from_revs(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+\tint *found_unpushed_submodule = data;\n+\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+\t\tif (!is_submodule_commit_pushed(p->two->path, p->two->sha1)) {\n+\t\t\t*found_unpushed_submodule = 1;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+static int collect_unpushed_submodules_in_tree(const unsigned char *sha1, const char *base, int baselen,\n+\t\t\t\t\t\tconst char *pathname, unsigned mode, int stage, void *context)\n+{\n+\tint *found_unpushed_submodules = context;\n+\tstruct strbuf path = STRBUF_INIT;\n+\n+\tstrbuf_add(&path,base,strlen(base));\n+\tstrbuf_add(&path,pathname,strlen(pathname));\n+\n+\tif (S_ISGITLINK(mode)) {\n+\t\tif(!is_submodule_commit_pushed(path.buf,sha1))\n+\t\t\t*found_unpushed_submodules = 1;\n+\t\treturn 0;\n+\t}\n+\treturn READ_TREE_RECURSIVE;\n+}\n+\n+static void parent_commits_pushed(struct commit *commit, struct commit_list *parent, int *found_unpushed_submodule)\n+{\n+\twhile (parent) {\n+\t\tstruct diff_options diff_opts;\n+\t\tdiff_setup(&diff_opts);\n+\t\tDIFF_OPT_SET(&diff_opts, RECURSIVE);\n+\t\tdiff_opts.output_format |= DIFF_FORMAT_CALLBACK;\n+\t\tdiff_opts.format_callback = collect_unpushed_submodules_from_revs;\n+\t\tdiff_opts.format_callback_data = found_unpushed_submodule;\n+\t\tif (diff_setup_done(&diff_opts) < 0)\n+\t\t\tdie(\"diff_setup_done failed\");\n+\t\tdiff_tree_sha1(parent->item->object.sha1, commit->object.sha1, \"\", &diff_opts);\n+\t\tdiffcore_std(&diff_opts);\n+\t\tdiff_flush(&diff_opts);\n+\t\tparent = parent->next;\n+\t}\n+}\n+\n+static void tree_commits_pushed(struct commit *commit, int *found_unpushed_submodule)\n+{\n+\tstruct tree * tree;\n+\tstruct pathspec pathspec;\n+\ttree = parse_tree_indirect(commit->object.sha1);\n+\tinit_pathspec(&pathspec,NULL);\n+\tread_tree_recursive(tree, \"\", 0, 0, &pathspec, collect_unpushed_submodules_in_tree,\n+\t\t\t found_unpushed_submodule);\n+}\n+\n+int check_for_unpushed_submodule_commits(unsigned char new_sha1[20])\n+{\n+\tstruct rev_info rev;\n+\tstruct commit *commit;\n+\tconst char *argv[] = {NULL, NULL, \"--not\", \"--remotes\", NULL};\n+\tint argc = ARRAY_SIZE(argv) - 1;\n+\tchar *sha1_copy;\n+\tint found_unpushed_submodule = 0;\n+\n+\tinit_revisions(&rev, NULL);\n+\tsha1_copy = xstrdup(sha1_to_hex(new_sha1));\n+\targv[1] = sha1_copy;\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)) && !found_unpushed_submodule) {\n+\t\tstruct commit_list *parent = commit->parents;\n+\t\tif(parent)\n+\t\t\tparent_commits_pushed(commit,parent,&found_unpushed_submodule);\n+\t\telse\n+\t\t\ttree_commits_pushed(commit,&found_unpushed_submodule);\n+\t}\n+\n+\tfree(sha1_copy);\n+\treturn found_unpushed_submodule;\n+}\n+\n static int is_submodule_commit_present(const char *path, unsigned char sha1[20])\n {\n \tint is_present = 0;\ndiff --git a/submodule.h b/submodule.h\nindex 5350b0d..0a4d395 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -29,5 +29,6 @@ int fetch_populated_submodules(int num_options, const char **options,\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],\n \t\t const unsigned char a[20], const unsigned char b[20]);\n+int check_for_unpushed_submodule_commits(unsigned char sha1[20]);\n \n #endif\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 0b55466..15474c1 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -32,7 +32,7 @@ test_expect_success push '\n \t)\n '\n \n-test_expect_failure 'push fails if submodule has no remote' '\n+test_expect_success 'push fails if submodule has no remote' '\n \t(\n \t\tcd work/gar/bage &&\n \t\t>junk2 &&\n@@ -47,7 +47,7 @@ test_expect_failure 'push fails if submodule has no remote' '\n \t)\n '\n \n-test_expect_failure 'push fails if submodule commit not on remote' '\n+test_expect_success 'push fails if submodule commit not on remote' '\n \t(\n \t\tcd work/gar &&\n \t\tgit clone --bare bage ../../submodule.git &&\n@@ -66,7 +66,7 @@ test_expect_failure 'push fails if submodule commit not on remote' '\n \t)\n '\n \n-test_expect_failure 'push succeeds after commit was pushed to remote' '\n+test_expect_success 'push succeeds after commit was pushed to remote' '\n \t(\n \t\tcd work/gar/bage &&\n \t\tgit push origin master\n@@ -76,4 +76,52 @@ test_expect_failure 'push succeeds after commit was pushed to remote' '\n \t\tgit push ../pub.git master\n \t)\n '\n+\n+test_expect_success 'push fails when commit on multiple branches if one branch has no remote' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>junk4 &&\n+\t\tgit add junk4 &&\n+\t\tgit commit -m \"Fourth junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit branch branch2 &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Fourth commit for gar/bage\" &&\n+\t\tgit checkout branch2 &&\n+\t\t(\n+\t\t\tcd gar/bage &&\n+\t\t\tgit checkout HEAD~1\n+\t\t) &&\n+\t\t>junk1 &&\n+\t\tgit add junk1 &&\n+\t\tgit commit -m \"First junk\" &&\n+\t\ttest_must_fail git push ../pub.git\n+\t)\n+'\n+\n+test_expect_success 'push fails if submodule has no remote and is on the first superproject commit' '\n+\tmkdir a &&\n+\t(\n+\t\tcd a &&\n+\t\tgit init --bare\n+\t) &&\n+\tgit clone a a1 &&\n+\t(\n+\t\tcd a1 &&\n+\t\tmkdir b &&\n+\t\t(\n+\t\t\tcd b &&\n+\t\t\tgit init &&\n+\t\t\t>junk &&\n+\t\t\tgit add junk &&\n+\t\t\tgit commit -m \"initial\"\n+\t\t) &&\n+\t\tgit add b &&\n+\t\tgit commit -m \"added submodule\" &&\n+\t\ttest_must_fail git push origin master\n+\t)\n+'\n+\n test_done\ndiff --git a/transport.c b/transport.c\nindex c9c8056..e0fd435 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -10,6 +10,7 @@\n #include \"refs.h\"\n #include \"branch.h\"\n #include \"url.h\"\n+#include \"submodule.h\"\n \n /* rsync support */\n \n@@ -1041,6 +1042,13 @@ int transport_push(struct transport *transport,\n \t\t\tflags & TRANSPORT_PUSH_MIRROR,\n \t\t\tflags & TRANSPORT_PUSH_FORCE);\n \n+\t\tif(!(flags & TRANSPORT_PUSH_FORCE) && !is_bare_repository()) {\n+\t\t\tstruct ref *ref = remote_refs;\n+\t\t\tfor (; ref; ref = ref->next)\n+\t\t\t\tif(!is_null_sha1(ref->new_sha1) && check_for_unpushed_submodule_commits(ref->new_sha1))\n+\t\t\t\t\tdie(\"There are unpushed submodules, aborting. Use -f to force a push\");\n+\t\t}\n+\n \t\tpush_ret = transport->push_refs(transport, remote_refs, flags);\n \t\terr = push_had_errors(remote_refs);\n \t\tret = push_ret | err;\n-- \n1.7.6.236.g7ad21\n"},{"id":"172184","messageId":"1311790250-32454-4-git-send-email-iveqy@iveqy.com","threadId":"27936","inReplyTo":"1311790250-32454-1-git-send-email-iveqy@iveqy.com","subject":"[PATCH v2 3/3] push: Add the --no-recurse-submodules option","fromName":"Fredrik Gustafsson","fromEmail":"iveqy@iveqy.com","sentAt":"2011-07-27T18:10:50Z","receivedAt":"2011-07-27T18:10:50Z","isPatch":true,"sender":{"key":"iveqy@iveqy.com","avatar":"https://avatars.githubusercontent.com/u/761743?v=4"},"body":"This adds the option --no-recurse-submodules to push. That is, git\nwill not check if the submodules are pushed. -f or --force still also\ndisables this check. Documentation is also updated.\n\nSigned-off-by: Fredrik Gustafsson \nMentored-by: Jens Lehmann \nMentored-by: Heiko Voigt \n---\n Documentation/git-push.txt | 6 ++++++\n builtin/push.c | 1 +\n t/t5531-deep-submodule-push.sh | 7 +++++++\n transport.c | 4 +++-\n transport.h | 1 +\n 5 files changed, 18 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 88acfcd..d63a57c 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -113,6 +113,8 @@ nor in any Push line of the corresponding remotes file---see below).\n \tnot an ancestor of the local ref used to overwrite it.\n \tThis flag disables the check. This can cause the\n \tremote repository to lose commits; use it with care.\n++\n+\tThis also enforces --no-recurse-submodules\n \n --repo=::\n \tThis option is only relevant if no argument is\n@@ -162,6 +164,10 @@ useful if you write an alias or script around 'git push'.\n \tis specified. This flag forces progress status even if the\n \tstandard error stream is not directed to a terminal.\n \n+--no-recurse-submodules::\n+\tDon't check if all submodule commits this repository refers to are\n+\tpushed to their remotes.\n+\n include::urls-remotes.txt[]\n \n OUTPUT\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 9cebf9e..07a8b11 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -236,6 +236,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('n' , \"dry-run\", &flags, \"dry run\", TRANSPORT_PUSH_DRY_RUN),\n \t\tOPT_BIT( 0, \"porcelain\", &flags, \"machine-readable output\", TRANSPORT_PUSH_PORCELAIN),\n \t\tOPT_BIT('f', \"force\", &flags, \"force updates\", TRANSPORT_PUSH_FORCE),\n+\t\tOPT_BIT(0, \"no-recurse-submodules\", &flags, \"do not recurse submodules\", TRANSPORT_PUSH_NO_RECURSE_SUBMODULES),\n \t\tOPT_BOOLEAN( 0 , \"thin\", &thin, \"use thin pack\"),\n \t\tOPT_STRING( 0 , \"receive-pack\", &receivepack, \"receive-pack\", \"receive pack program\"),\n \t\tOPT_STRING( 0 , \"exec\", &receivepack, \"receive-pack\", \"receive pack program\"),\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 15474c1..74c615c 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -124,4 +124,11 @@ test_expect_success 'push fails if submodule has no remote and is on the first s\n \t)\n '\n \n+test_expect_success 'push succeeds when --no-recurse-submodules is used' '\n+\t(\n+\t\tcd work &&\n+\t\tgit push ../pub.git --no-recurse-submodules\n+\t)\n+'\n+\n test_done\ndiff --git a/transport.c b/transport.c\nindex e0fd435..9681560 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1042,7 +1042,9 @@ int transport_push(struct transport *transport,\n \t\t\tflags & TRANSPORT_PUSH_MIRROR,\n \t\t\tflags & TRANSPORT_PUSH_FORCE);\n \n-\t\tif(!(flags & TRANSPORT_PUSH_FORCE) && !is_bare_repository()) {\n+\t\tif(!(flags & TRANSPORT_PUSH_NO_RECURSE_SUBMODULES) &&\n+\t\t !(flags & TRANSPORT_PUSH_FORCE) &&\n+\t\t !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n \t\t\tfor (; ref; ref = ref->next)\n \t\t\t\tif(!is_null_sha1(ref->new_sha1) && check_for_unpushed_submodule_commits(ref->new_sha1))\ndiff --git a/transport.h b/transport.h\nindex 161d724..c6ccf8c 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -101,6 +101,7 @@ struct transport {\n #define TRANSPORT_PUSH_MIRROR 8\n #define TRANSPORT_PUSH_PORCELAIN 16\n #define TRANSPORT_PUSH_SET_UPSTREAM 32\n+#define TRANSPORT_PUSH_NO_RECURSE_SUBMODULES 64\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n \n-- \n1.7.6.236.g7ad21\n"},{"id":"172303","messageId":"7v39hqmb30.fsf@alter.siamese.dyndns.org","threadId":"27936","inReplyTo":"1311790250-32454-1-git-send-email-iveqy@iveqy.com","subject":"Re: [PATCH v2 0/3] check for unpushed remotes in submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-07-28T19:58:27Z","receivedAt":"2011-07-28T19:58:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Gustafsson writes:\n\n> Regarding the discussion of superprojects with submodules that have no\n> remote tracking branches: A push will still be denied.\n\nI have marked one part of the data synchronized across machines as\n\"private\" submodule (which contains my gpg keychains, encrypted password\nfiles, personaly memos, etc.) and push only the outer \"shell\" superproject\n(which has tools that I use everywhere to go to $HOME/bin among other\nthings) to certain machines without the private parts, and the\nsuperproject is designed to work without a checkout (nor clone) of a\nsubmodule.\n\nWith this patch series, it sounds like I cannot use this repository\nstructure anymore, which is sad.\n"},{"id":"172304","messageId":"7vy5zikw6e.fsf@alter.siamese.dyndns.org","threadId":"27936","inReplyTo":"1311790250-32454-4-git-send-email-iveqy@iveqy.com","subject":"Re: [PATCH v2 3/3] push: Add the --no-recurse-submodules option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-07-28T20:05:45Z","receivedAt":"2011-07-28T20:05:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Gustafsson writes:\n\n> This adds the option --no-recurse-submodules to push. That is, git\n\nI think this needs to be renamed at least for two reasons.\n\nThe name makes it sound as if \"push --recurse-submodules\" would\nrecursively visit the submodules and runs \"push\" there, but I do not think\nthat is what this flag does.\n\n> diff --git a/transport.h b/transport.h\n> index 161d724..c6ccf8c 100644\n> --- a/transport.h\n> +++ b/transport.h\n> @@ -101,6 +101,7 @@ struct transport {\n> #define TRANSPORT_PUSH_MIRROR 8\n> #define TRANSPORT_PUSH_PORCELAIN 16\n> #define TRANSPORT_PUSH_SET_UPSTREAM 32\n> +#define TRANSPORT_PUSH_NO_RECURSE_SUBMODULES 64\n\nAlso naming the constant with NO will invite an unnecessary double\nnegation, like this:\n\n\tif (!(flags & FROTZ_NO_NITFOL))\n \tdo_nitfol_to_frotz();\n\nBesides, I would be moderately annoyed if this check were the default.\n"},{"id":"172311","messageId":"4E31DF5A.4050307@web.de","threadId":"27936","inReplyTo":"7v39hqmb30.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/3] check for unpushed remotes in submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-07-28T22:14:50Z","receivedAt":"2011-07-28T22:14:50Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 28.07.2011 21:58, schrieb Junio C Hamano:\n> Fredrik Gustafsson writes:\n> \n>> Regarding the discussion of superprojects with submodules that have no\n>> remote tracking branches: A push will still be denied.\n> \n> I have marked one part of the data synchronized across machines as\n> \"private\" submodule (which contains my gpg keychains, encrypted password\n> files, personaly memos, etc.) and push only the outer \"shell\" superproject\n> (which has tools that I use everywhere to go to $HOME/bin among other\n> things) to certain machines without the private parts, and the\n> superproject is designed to work without a checkout (nor clone) of a\n> submodule.\n> \n> With this patch series, it sounds like I cannot use this repository\n> structure anymore, which is sad.\n\nThanks for bringing this use case up. Now I understand why you asked if\nsubmodules without remote tracking branches should be checked too. We\ndiscussed that and couldn't think of a scenario where the user doesn't\nwant to have remote tracking branches in a submodule, but we missed the\nuse case you described here. Back to the drawing board ...\n"},{"id":"172312","messageId":"4E31E140.8020105@web.de","threadId":"27936","inReplyTo":"7vy5zikw6e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/3] push: Add the --no-recurse-submodules option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-07-28T22:22:56Z","receivedAt":"2011-07-28T22:22:56Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 28.07.2011 22:05, schrieb Junio C Hamano:\n> Fredrik Gustafsson writes:\n> \n>> This adds the option --no-recurse-submodules to push. That is, git\n> \n> I think this needs to be renamed at least for two reasons.\n> \n> The name makes it sound as if \"push --recurse-submodules\" would\n> recursively visit the submodules and runs \"push\" there, but I do not think\n> that is what this flag does.\n\nThat is because the patch that does this is still in the making ;-)\n\nThe cover letter should have mentioned it, but we talked about making push\npretty symmetric to fetch:\n\n- Use \"--no-recurse-submodules\" if you don't want submodules to be pushed\n (we added that right now so users can disable the behavior the second\n commit introduces)\n- Use \"--recurse-submodules=on-demand\" to push only those submodules where\n new commits have been recorded in the superproject's refs to be pushed\n- Use \"--recurse-submodules\" to unconditionally push everything in the\n submodules too\n- Make the default configurable by a \"push.recurseSubmodules\" option\n\nWe'll need another round to discuss how to handle private submodules which\nwere never intended to be pushed, but I think the general idea of having\nfetch and push use similar options makes sense, no?\n"},{"id":"172358","messageId":"4E3315E6.8060501@web.de","threadId":"27936","inReplyTo":"7vy5zikw6e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/3] push: Add the --no-recurse-submodules option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-07-29T20:19:50Z","receivedAt":"2011-07-29T20:19:50Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 28.07.2011 22:05, schrieb Junio C Hamano:\n> Fredrik Gustafsson writes:\n>> diff --git a/transport.h b/transport.h\n>> index 161d724..c6ccf8c 100644\n>> --- a/transport.h\n>> +++ b/transport.h\n>> @@ -101,6 +101,7 @@ struct transport {\n>> #define TRANSPORT_PUSH_MIRROR 8\n>> #define TRANSPORT_PUSH_PORCELAIN 16\n>> #define TRANSPORT_PUSH_SET_UPSTREAM 32\n>> +#define TRANSPORT_PUSH_NO_RECURSE_SUBMODULES 64\n> \n> Also naming the constant with NO will invite an unnecessary double\n> negation, like this:\n> \n> \tif (!(flags & FROTZ_NO_NITFOL))\n> \tdo_nitfol_to_frotz();\n\nRight, we'll drop the \"NO_\" from that constant.\n\n> Besides, I would be moderately annoyed if this check were the default.\n\nWe will skip the check for submodules without remotes, does that lessen\nyour annoyance? We still think it is a good idea to have that test enabled\nby default, but it might be a good idea to wait with that until we provide\na central config option to enable users to turn that off with a simple\n\"git config push.recurseSubmodules off\". What do you think?\n"},{"id":"172483","messageId":"7vpqkqj5h7.fsf@alter.siamese.dyndns.org","threadId":"27936","inReplyTo":"4E3315E6.8060501@web.de","subject":"Re: [PATCH v2 3/3] push: Add the --no-recurse-submodules option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-01T01:16:52Z","receivedAt":"2011-08-01T01:16:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> We will skip the check for submodules without remotes, does that lessen\n> your annoyance? We still think it is a good idea to have that test enabled\n> by default, but it might be a good idea to wait with that until we provide\n> a central config option to enable users to turn that off with a simple\n> \"git config push.recurseSubmodules off\". What do you think?\n\nI would rather see a questionable (in the sense that it can trigger false\npositive and annoy users of workflows you didn't anticipate) new feature\nturned off by default, so that interested people who _do_ want to can use\nit knowing what they are getting into, to find out as much unexpected\nfallout as possible. So if we want to give this topic early graduation, I\nwould suggest making it a command line option that defaults to off first,\nthen in a later commit add a configuration option to enable people to turn\nit on (and allow those who do not see the point of the new feature to keep\nit turned off permanently). Enabling the feature by default can be done\nin a later version after the feature proves to be truly useful for people\nwho opted into the experiment.\n"}]}