{"thread":{"id":"28094","subject":"[PATCH v4 2/2] push: Don't push a repository with unpushed submodules","startedAt":"2011-08-14T08:57:40Z","lastAt":"2011-08-14T17:29:49Z","messageCount":3,"participants":["Fredrik Gustafsson","Jens Lehmann"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"173487","messageId":"1313312262-2460-1-git-send-email-iveqy@iveqy.com","threadId":"28094","inReplyTo":null,"subject":"[PATCH v4 0/2] push limitations","fromName":"Fredrik Gustafsson","fromEmail":"iveqy@iveqy.com","sentAt":"2011-08-14T08:57:40Z","receivedAt":"2011-08-14T08:57:40Z","isPatch":true,"sender":{"key":"iveqy@iveqy.com","avatar":"https://avatars.githubusercontent.com/u/761743?v=4"},"body":"The first iteration of this patch series can be found here:\nhttp://thread.gmane.org/gmane.comp.version-control.git/176328/focus=176327\n\nThe second iteration of this patch series can be found here:\nhttp://thread.gmane.org/gmane.comp.version-control.git/177992\n\nThe third iteration of this patch series can be found here:\nhttp://thread.gmane.org/gmane.comp.version-control.git/179037/focus=179048\n\n\nFredrik Gustafsson (2):\n  rev-parse: add option --is-well-formed-git-dir [path]\n  push: Don't push a repository with unpushed submodules\n\n Documentation/git-push.txt      |    6 ++\n Documentation/git-rev-parse.txt |    4 ++\n builtin/push.c                  |   17 ++++++\n builtin/rev-parse.c             |    8 +++\n cache.h                         |    1 +\n combine-diff.c                  |    2 +-\n setup.c                         |    7 +++\n submodule.c                     |  108 +++++++++++++++++++++++++++++++++++++++\n submodule.h                     |    1 +\n t/t5531-deep-submodule-push.sh  |   94 ++++++++++++++++++++++++++++++++++\n t/t7400-submodule-basic.sh      |    4 +-\n t/t7403-submodule-sync.sh       |    5 +-\n t/t7407-submodule-foreach.sh    |   97 ++++++++++++++++++-----------------\n transport.c                     |    9 +++\n transport.h                     |    1 +\n 15 files changed, 312 insertions(+), 52 deletions(-)\n\n-- \n1.7.6.398.g64865.dirty\n"},{"id":"173486","messageId":"1313312262-2460-3-git-send-email-iveqy@iveqy.com","threadId":"28094","inReplyTo":"1313312262-2460-1-git-send-email-iveqy@iveqy.com","subject":"[PATCH v4 2/2] push: Don't push a repository with unpushed submodules","fromName":"Fredrik Gustafsson","fromEmail":"iveqy@iveqy.com","sentAt":"2011-08-14T08:57:42Z","receivedAt":"2011-08-14T08:57:42Z","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\nsubmodule to the server but pushing a super-project that\ncontains a commit for that submodule. The result is that the\nsuperproject points at a submodule commit that is not available\non the server.\n\nThis adds the option --recurse-submodules=check to push. When\nusing this option git will check that all submodule commits that\nare about to be pushed are present on a remote of the submodule.\n\nTo be able to use a combined diff, disabling a diff callback has\nbeen removed from combined-diff.c.\n\nSigned-off-by: Fredrik Gustafsson <iveqy@iveqy.com>\nMentored-by: Jens Lehmann <Jens.Lehmann@web.de>\nMentored-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\nI am not sure whether the change required for the diff callback\nhas other side effects. Another pair of eyes appreciated.\n\n Documentation/git-push.txt     |    6 ++\n builtin/push.c                 |   17 ++++++\n combine-diff.c                 |    2 +-\n submodule.c                    |  108 ++++++++++++++++++++++++++++++++++++++++\n submodule.h                    |    1 +\n t/t5531-deep-submodule-push.sh |   94 ++++++++++++++++++++++++++++++++++\n transport.c                    |    9 +++\n transport.h                    |    1 +\n 8 files changed, 237 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 88acfcd..6ae6ba3 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -162,6 +162,12 @@ 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+--recurse-submodules=check::\n+\tCheck whether all submodule commits used by the revisions to be\n+\tpushed are available on a remote tracking branch. Otherwise the\n+\tpush will be aborted and the command will exit with non-zero status.\n+\n+\n include::urls-remotes.txt[]\n \n OUTPUT\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 9cebf9e..23ba365 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -8,6 +8,7 @@\n #include \"remote.h\"\n #include \"transport.h\"\n #include \"parse-options.h\"\n+#include \"submodule.h\"\n \n static const char * const push_usage[] = {\n \t\"git push [<options>] [<repository> [<refspec>...]]\",\n@@ -219,6 +220,19 @@ static int do_push(const char *repo, int flags)\n \treturn !!errs;\n }\n \n+static int option_parse_recurse_submodules(const struct option *opt,\n+\t\t\t\t   const char *arg, int unset)\n+{\n+\tint *flags = opt->value;\n+\tif (arg) {\n+\t\tif (!strcmp(arg, \"check\"))\n+\t\t\t*flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;\n+\t\telse\n+\t\t\tdie(\"bad %s argument: %s\", opt->long_name, arg);\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_push(int argc, const char **argv, const char *prefix)\n {\n \tint flags = 0;\n@@ -236,6 +250,9 @@ 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\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &flags, \"check\",\n+\t\t\t\"controls recursive pushing of submodules\",\n+\t\t\tPARSE_OPT_OPTARG, option_parse_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/combine-diff.c b/combine-diff.c\nindex be67cfc..e5a0d84 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1068,7 +1068,7 @@ void diff_tree_combined(const unsigned char *sha1,\n \t\t * when doing combined diff.\n \t\t */\n \t\tint stat_opt = (opt->output_format &\n-\t\t\t\t(DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIFFSTAT));\n+\t\t\t\t(DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_CALLBACK));\n \t\tif (i == 0 && stat_opt)\n \t\t\tdiffopts.output_format = stat_opt;\n \t\telse\ndiff --git a/submodule.c b/submodule.c\nindex 1ba9646..45f508c 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -308,6 +308,114 @@ void set_config_fetch_recurse_submodules(int value)\n \tconfig_fetch_recurse_submodules = value;\n }\n \n+static int has_remote(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n+{\n+\treturn 1;\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+\t\treturn 0;\n+\n+\tif (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {\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+\t\tint needs_pushing = 0;\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 (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\tif (strbuf_read(&buf, cp.out, 41))\n+\t\t\tneeds_pushing = 1;\n+\t\tfinish_command(&cp);\n+\t\tclose(cp.out);\n+\t\tstrbuf_release(&buf);\n+\t\treturn needs_pushing;\n+\t}\n+\n+\treturn 0;\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+\tint *needs_pushing = 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 (submodule_needs_pushing(p->two->path, p->two->sha1)) {\n+\t\t\t*needs_pushing = 1;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+\n+static void commit_need_pushing(struct commit *commit, struct commit_list *parent, int *needs_pushing)\n+{\n+\tconst unsigned char (*parents)[20];\n+\tunsigned int i, n;\n+\tstruct rev_info rev;\n+\n+\tn = commit_list_count(parent);\n+\tparents = xmalloc(n * sizeof(*parents));\n+\n+\tfor (i = 0; i < n; i++) {\n+\t\thashcpy((unsigned char *)(parents + i), parent->item->object.sha1);\n+\t\tparent = parent->next;\n+\t}\n+\n+\tinit_revisions(&rev, NULL);\n+\trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n+\trev.diffopt.format_callback = collect_submodules_from_diff;\n+\trev.diffopt.format_callback_data = needs_pushing;\n+\tdiff_tree_combined(commit->object.sha1, parents, n, 1, &rev);\n+\n+\tfree(parents);\n+}\n+\n+int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name)\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+\tint needs_pushing = 0;\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+\tif (prepare_revision_walk(&rev))\n+\t\tdie(\"revision walk setup failed\");\n+\n+\twhile ((commit = get_revision(&rev)) && !needs_pushing)\n+\t\tcommit_need_pushing(commit, commit->parents, &needs_pushing);\n+\n+\tfree(sha1_copy);\n+\tstrbuf_release(&remotes_arg);\n+\n+\treturn needs_pushing;\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..799c22d 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_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name);\n \n #endif\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex faa2e96..4dda8b8 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -32,4 +32,98 @@ test_expect_success push '\n \t)\n '\n \n+test_expect_success 'push 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\tgit push --recurse-submodules=check ../pub.git master\n+\t)\n+'\n+\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+\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 --recurse-submodules=check ../pub.git master\n+\t)\n+'\n+\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+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit push --recurse-submodules=check ../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 --recurse-submodules=check ../pub.git\n+\t)\n+'\n+\n+test_expect_success 'push succeeds if submodule has no remote and is on the first superproject commit' '\n+\tgit init --bare a\n+\tgit clone a a1 &&\n+\t(\n+\t\tcd a1 &&\n+\t\tgit init b\n+\t\t(\n+\t\t\tcd b &&\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\tgit push --recurse-submodule=check origin master\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 c9c8056..fa279d5 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,14 @@ 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_RECURSE_SUBMODULES_CHECK) && !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) &&\n+\t\t\t\t    check_submodule_needs_pushing(ref->new_sha1,transport->remote->name))\n+\t\t\t\t\tdie(\"There are unpushed submodules, aborting.\");\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;\ndiff --git a/transport.h b/transport.h\nindex 161d724..059b330 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_RECURSE_SUBMODULES_CHECK 64\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n \n-- \n1.7.6.398.g64865.dirty\n"},{"id":"173507","messageId":"4E48060D.9010106@web.de","threadId":"28094","inReplyTo":"1313312262-2460-1-git-send-email-iveqy@iveqy.com","subject":"Re: [PATCH v4 0/2] push limitations","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-08-14T17:29:49Z","receivedAt":"2011-08-14T17:29:49Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Please ignore this series, it slipped through by accident.\n\nAm 14.08.2011 10:57, schrieb Fredrik Gustafsson:\n> The first iteration of this patch series can be found here:\n> http://thread.gmane.org/gmane.comp.version-control.git/176328/focus=176327\n> \n> The second iteration of this patch series can be found here:\n> http://thread.gmane.org/gmane.comp.version-control.git/177992\n> \n> The third iteration of this patch series can be found here:\n> http://thread.gmane.org/gmane.comp.version-control.git/179037/focus=179048\n> \n> \n> Fredrik Gustafsson (2):\n>   rev-parse: add option --is-well-formed-git-dir [path]\n>   push: Don't push a repository with unpushed submodules\n> \n>  Documentation/git-push.txt      |    6 ++\n>  Documentation/git-rev-parse.txt |    4 ++\n>  builtin/push.c                  |   17 ++++++\n>  builtin/rev-parse.c             |    8 +++\n>  cache.h                         |    1 +\n>  combine-diff.c                  |    2 +-\n>  setup.c                         |    7 +++\n>  submodule.c                     |  108 +++++++++++++++++++++++++++++++++++++++\n>  submodule.h                     |    1 +\n>  t/t5531-deep-submodule-push.sh  |   94 ++++++++++++++++++++++++++++++++++\n>  t/t7400-submodule-basic.sh      |    4 +-\n>  t/t7403-submodule-sync.sh       |    5 +-\n>  t/t7407-submodule-foreach.sh    |   97 ++++++++++++++++++-----------------\n>  transport.c                     |    9 +++\n>  transport.h                     |    1 +\n>  15 files changed, 312 insertions(+), 52 deletions(-)\n> \n"}]}