{"thread":{"id":"45574","subject":"[PATCH] push: propagate push-options with --recurse-submodules","startedAt":"2017-03-31T23:11:54Z","lastAt":"2017-04-11T16:33:15Z","messageCount":19,"participants":["Brandon Williams","Jonathan Nieder","Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"315958","messageId":"20170331231135.195195-1-bmwill@google.com","threadId":"45574","inReplyTo":null,"subject":"[PATCH] push: propagate push-options with --recurse-submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-03-31T23:11:35Z","receivedAt":"2017-03-31T23:11:54Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Teach push --recurse-submodules to propagate push-options recursively to\nthe pushes performed in the submodules.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n submodule.c             | 14 +++++++++++---\n submodule.h             |  3 ++-\n t/t5545-push-options.sh | 39 +++++++++++++++++++++++++++++++++++++++\n transport.c             |  3 ++-\n 4 files changed, 54 insertions(+), 5 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex c52d6634c..3d6d5e62d 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -782,7 +782,8 @@ int find_unpushed_submodules(struct sha1_array *commits,\n \treturn needs_pushing->nr;\n }\n \n-static int push_submodule(const char *path, int dry_run)\n+static int push_submodule(const char *path, int dry_run,\n+\t\t\t  const struct string_list *push_options)\n {\n \tif (add_submodule_odb(path))\n \t\treturn 1;\n@@ -793,6 +794,12 @@ static int push_submodule(const char *path, int dry_run)\n \t\tif (dry_run)\n \t\t\targv_array_push(&cp.args, \"--dry-run\");\n \n+\t\tif (push_options && push_options->nr) {\n+\t\t\tstatic struct string_list_item *item;\n+\t\t\tfor_each_string_list_item(item, push_options)\n+\t\t\t\targv_array_pushf(&cp.args, \"--push-option=%s\",\n+\t\t\t\t\t\t item->string);\n+\t\t}\n \t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n@@ -807,7 +814,8 @@ static int push_submodule(const char *path, int dry_run)\n \n int push_unpushed_submodules(struct sha1_array *commits,\n \t\t\t     const char *remotes_name,\n-\t\t\t     int dry_run)\n+\t\t\t     int dry_run,\n+\t\t\t     const struct string_list *push_options)\n {\n \tint i, ret = 1;\n \tstruct string_list needs_pushing = STRING_LIST_INIT_DUP;\n@@ -818,7 +826,7 @@ int push_unpushed_submodules(struct sha1_array *commits,\n \tfor (i = 0; i < needs_pushing.nr; i++) {\n \t\tconst char *path = needs_pushing.items[i].string;\n \t\tfprintf(stderr, \"Pushing submodule '%s'\\n\", path);\n-\t\tif (!push_submodule(path, dry_run)) {\n+\t\tif (!push_submodule(path, dry_run, push_options)) {\n \t\t\tfprintf(stderr, \"Unable to push submodule '%s'\\n\", path);\n \t\t\tret = 0;\n \t\t}\ndiff --git a/submodule.h b/submodule.h\nindex 8a8bc49dc..036ce4c5c 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -92,7 +92,8 @@ extern int find_unpushed_submodules(struct sha1_array *commits,\n \t\t\t\t    struct string_list *needs_pushing);\n extern int push_unpushed_submodules(struct sha1_array *commits,\n \t\t\t\t    const char *remotes_name,\n-\t\t\t\t    int dry_run);\n+\t\t\t\t    int dry_run,\n+\t\t\t\t    const struct string_list *push_options);\n extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n extern int parallel_submodules(void);\n \ndiff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\nindex 97065e62b..32c513c78 100755\n--- a/t/t5545-push-options.sh\n+++ b/t/t5545-push-options.sh\n@@ -142,6 +142,45 @@ test_expect_success 'push options work properly across http' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'push options and submodules' '\n+\ttest_when_finished \"rm -rf parent\" &&\n+\ttest_when_finished \"rm -rf parent_upstream\" &&\n+\tmk_repo_pair &&\n+\tgit -C upstream config receive.advertisePushOptions true &&\n+\tcp -r upstream parent_upstream &&\n+\ttest_commit -C upstream one &&\n+\n+\ttest_create_repo parent &&\n+\tgit -C parent remote add up ../parent_upstream &&\n+\ttest_commit -C parent one &&\n+\tgit -C parent push --mirror up &&\n+\n+\tgit -C parent submodule add ../upstream workbench &&\n+\tgit -C parent commit -m \"add submoule\" &&\n+\n+\ttest_commit -C parent/workbench two &&\n+\tgit -C parent add workbench &&\n+\tgit -C parent commit -m \"update workbench\" &&\n+\n+\tgit -C parent push \\\n+\t\t--push-option=asdf --push-option=\"more structured text\" \\\n+\t\t--recurse-submodules=on-demand up master &&\n+\n+\tgit -C upstream rev-parse --verify master >expect &&\n+\tgit -C parent/workbench rev-parse --verify master >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tgit -C parent_upstream rev-parse --verify master >expect &&\n+\tgit -C parent rev-parse --verify master >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tprintf \"asdf\\nmore structured text\\n\" >expect &&\n+\ttest_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+\ttest_cmp expect upstream/.git/hooks/post-receive.push_options &&\n+\ttest_cmp expect parent_upstream/.git/hooks/pre-receive.push_options &&\n+\ttest_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n+'\n+\n stop_httpd\n \n test_done\ndiff --git a/transport.c b/transport.c\nindex 417ed7f19..b1e680881 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1031,7 +1031,8 @@ int transport_push(struct transport *transport,\n \n \t\t\tif (!push_unpushed_submodules(&commits,\n \t\t\t\t\t\t      transport->remote->name,\n-\t\t\t\t\t\t      pretend)) {\n+\t\t\t\t\t\t      pretend,\n+\t\t\t\t\t\t      transport->push_options)) {\n \t\t\t\tsha1_array_clear(&commits);\n \t\t\t\tdie(\"Failed to push all needed submodules!\");\n \t\t\t}\n-- \n2.12.2.564.g063fe858b8-goog\n\n"},{"id":"315963","messageId":"20170331232030.GA8741@aiede.mtv.corp.google.com","threadId":"45574","inReplyTo":"20170331231135.195195-1-bmwill@google.com","subject":"Re: [PATCH] push: propagate push-options with --recurse-submodules","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-03-31T23:20:30Z","receivedAt":"2017-03-31T23:21:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nBrandon Williams wrote:\n\n> Teach push --recurse-submodules to propagate push-options recursively to\n> the pushes performed in the submodules.\n\nSounds like a good change.\n\n[...]\n> +++ b/submodule.c\n[...]\n> @@ -793,6 +794,12 @@ static int push_submodule(const char *path, int dry_run)\n>  \t\tif (dry_run)\n>  \t\t\targv_array_push(&cp.args, \"--dry-run\");\n>  \n> +\t\tif (push_options && push_options->nr) {\n> +\t\t\tstatic struct string_list_item *item;\n\nWhy static?  It would be simpler (and would use less bss) to let this\npointer be an automatic variable instead.\n\n> +\t\t\tfor_each_string_list_item(item, push_options)\n> +\t\t\t\targv_array_pushf(&cp.args, \"--push-option=%s\",\n> +\t\t\t\t\t\t item->string);\n> +\t\t}\n>  \t\tprepare_submodule_repo_env(&cp.env_array);\n>  \t\tcp.git_cmd = 1;\n>  \t\tcp.no_stdin = 1;\n> @@ -807,7 +814,8 @@ static int push_submodule(const char *path, int dry_run)\n>  \n>  int push_unpushed_submodules(struct sha1_array *commits,\n>  \t\t\t     const char *remotes_name,\n> -\t\t\t     int dry_run)\n> +\t\t\t     int dry_run,\n> +\t\t\t     const struct string_list *push_options)\n\nnit: I wonder if dry_run should stay as the last argument.  That would\nmake it closer to the idiom of have a flag word as last argument.\n\n[...]\n> +++ b/t/t5545-push-options.sh\n> @@ -142,6 +142,45 @@ test_expect_success 'push options work properly across http' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'push options and submodules' '\n\nYay!\n\nWhat happens if the upstream of the parent repo supports push options\nbut the upstream of the child repo doesn't?  What should happen?\n\nThanks and hope that helps,\nJonathan\n"},{"id":"315964","messageId":"20170331232631.GA40772@google.com","threadId":"45574","inReplyTo":"20170331232030.GA8741@aiede.mtv.corp.google.com","subject":"Re: [PATCH] push: propagate push-options with --recurse-submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-03-31T23:26:31Z","receivedAt":"2017-03-31T23:26:39Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 03/31, Jonathan Nieder wrote:\n> Hi,\n> \n> Brandon Williams wrote:\n> \n> > Teach push --recurse-submodules to propagate push-options recursively to\n> > the pushes performed in the submodules.\n> \n> Sounds like a good change.\n> \n> [...]\n> > +++ b/submodule.c\n> [...]\n> > @@ -793,6 +794,12 @@ static int push_submodule(const char *path, int dry_run)\n> >  \t\tif (dry_run)\n> >  \t\t\targv_array_push(&cp.args, \"--dry-run\");\n> >  \n> > +\t\tif (push_options && push_options->nr) {\n> > +\t\t\tstatic struct string_list_item *item;\n> \n> Why static?  It would be simpler (and would use less bss) to let this\n> pointer be an automatic variable instead.\n\nOops, definitely shouldn't be static! Thanks for catching that.\n\n> \n> > +\t\t\tfor_each_string_list_item(item, push_options)\n> > +\t\t\t\targv_array_pushf(&cp.args, \"--push-option=%s\",\n> > +\t\t\t\t\t\t item->string);\n> > +\t\t}\n> >  \t\tprepare_submodule_repo_env(&cp.env_array);\n> >  \t\tcp.git_cmd = 1;\n> >  \t\tcp.no_stdin = 1;\n> > @@ -807,7 +814,8 @@ static int push_submodule(const char *path, int dry_run)\n> >  \n> >  int push_unpushed_submodules(struct sha1_array *commits,\n> >  \t\t\t     const char *remotes_name,\n> > -\t\t\t     int dry_run)\n> > +\t\t\t     int dry_run,\n> > +\t\t\t     const struct string_list *push_options)\n> \n> nit: I wonder if dry_run should stay as the last argument.  That would\n> make it closer to the idiom of have a flag word as last argument.\n\nYeah I can flip the order.\n\n> \n> [...]\n> > +++ b/t/t5545-push-options.sh\n> > @@ -142,6 +142,45 @@ test_expect_success 'push options work properly across http' '\n> >  \ttest_cmp expect actual\n> >  '\n> >  \n> > +test_expect_success 'push options and submodules' '\n> \n> Yay!\n> \n> What happens if the upstream of the parent repo supports push options\n> but the upstream of the child repo doesn't?  What should happen?\n\npush would fail since the children are pushed first.\n\n> \n> Thanks and hope that helps,\n> Jonathan\n\n-- \nBrandon Williams\n"},{"id":"315965","messageId":"20170331235623.166408-1-bmwill@google.com","threadId":"45574","inReplyTo":"20170331231135.195195-1-bmwill@google.com","subject":"[PATCH v2 0/2] propagate push-options","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-03-31T23:56:21Z","receivedAt":"2017-03-31T23:56:35Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"v2 addresses Jonathan's comments as well as adds an additional patch to unmark\na local variable as static.\n\nBrandon Williams (2):\n  push: unmark a local variable as static\n  push: propagate push-options with --recurse-submodules\n\n builtin/push.c          |  5 +++--\n submodule.c             | 13 +++++++++++--\n submodule.h             |  1 +\n t/t5545-push-options.sh | 39 +++++++++++++++++++++++++++++++++++++++\n transport.c             |  1 +\n 5 files changed, 55 insertions(+), 4 deletions(-)\n\n-- \n2.12.2.564.g063fe858b8-goog\n\n"},{"id":"315966","messageId":"20170331235623.166408-2-bmwill@google.com","threadId":"45574","inReplyTo":"20170331235623.166408-1-bmwill@google.com","subject":"[PATCH v2 1/2] push: unmark a local variable as static","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-03-31T23:56:22Z","receivedAt":"2017-03-31T23:56:36Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"There isn't any obvious reason for the 'struct string_list push_options'\nand 'struct string_list_item *item' to be marked as static, so unmark\nthem as being static.  Also, clear the push_options string_list to\nprevent memory leaking.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n builtin/push.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 5c22e9f2e..a597759d8 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -510,8 +510,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tint push_cert = -1;\n \tint rc;\n \tconst char *repo = NULL;\t/* default repository */\n-\tstatic struct string_list push_options = STRING_LIST_INIT_DUP;\n-\tstatic struct string_list_item *item;\n+\tstruct string_list push_options = STRING_LIST_INIT_DUP;\n+\tconst struct string_list_item *item;\n \n \tstruct option options[] = {\n \t\tOPT__VERBOSITY(&verbosity),\n@@ -584,6 +584,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"push options must not have new line characters\"));\n \n \trc = do_push(repo, flags, &push_options);\n+\tstring_list_clear(&push_options, 0);\n \tif (rc == -1)\n \t\tusage_with_options(push_usage, options);\n \telse\n-- \n2.12.2.564.g063fe858b8-goog\n\n"},{"id":"315967","messageId":"20170331235623.166408-3-bmwill@google.com","threadId":"45574","inReplyTo":"20170331235623.166408-1-bmwill@google.com","subject":"[PATCH v2 2/2] push: propagate push-options with --recurse-submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-03-31T23:56:23Z","receivedAt":"2017-03-31T23:56:37Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Teach push --recurse-submodules to propagate push-options recursively to\nthe pushes performed in the submodules.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n submodule.c             | 13 +++++++++++--\n submodule.h             |  1 +\n t/t5545-push-options.sh | 39 +++++++++++++++++++++++++++++++++++++++\n transport.c             |  1 +\n 4 files changed, 52 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex c52d6634c..de444be61 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -782,7 +782,9 @@ int find_unpushed_submodules(struct sha1_array *commits,\n \treturn needs_pushing->nr;\n }\n \n-static int push_submodule(const char *path, int dry_run)\n+static int push_submodule(const char *path,\n+\t\t\t  const struct string_list *push_options,\n+\t\t\t  int dry_run)\n {\n \tif (add_submodule_odb(path))\n \t\treturn 1;\n@@ -793,6 +795,12 @@ static int push_submodule(const char *path, int dry_run)\n \t\tif (dry_run)\n \t\t\targv_array_push(&cp.args, \"--dry-run\");\n \n+\t\tif (push_options && push_options->nr) {\n+\t\t\tconst struct string_list_item *item;\n+\t\t\tfor_each_string_list_item(item, push_options)\n+\t\t\t\targv_array_pushf(&cp.args, \"--push-option=%s\",\n+\t\t\t\t\t\t item->string);\n+\t\t}\n \t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n@@ -807,6 +815,7 @@ static int push_submodule(const char *path, int dry_run)\n \n int push_unpushed_submodules(struct sha1_array *commits,\n \t\t\t     const char *remotes_name,\n+\t\t\t     const struct string_list *push_options,\n \t\t\t     int dry_run)\n {\n \tint i, ret = 1;\n@@ -818,7 +827,7 @@ int push_unpushed_submodules(struct sha1_array *commits,\n \tfor (i = 0; i < needs_pushing.nr; i++) {\n \t\tconst char *path = needs_pushing.items[i].string;\n \t\tfprintf(stderr, \"Pushing submodule '%s'\\n\", path);\n-\t\tif (!push_submodule(path, dry_run)) {\n+\t\tif (!push_submodule(path, push_options, dry_run)) {\n \t\t\tfprintf(stderr, \"Unable to push submodule '%s'\\n\", path);\n \t\t\tret = 0;\n \t\t}\ndiff --git a/submodule.h b/submodule.h\nindex 8a8bc49dc..0e26430fd 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -92,6 +92,7 @@ extern int find_unpushed_submodules(struct sha1_array *commits,\n \t\t\t\t    struct string_list *needs_pushing);\n extern int push_unpushed_submodules(struct sha1_array *commits,\n \t\t\t\t    const char *remotes_name,\n+\t\t\t\t    const struct string_list *push_options,\n \t\t\t\t    int dry_run);\n extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n extern int parallel_submodules(void);\ndiff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\nindex 97065e62b..32c513c78 100755\n--- a/t/t5545-push-options.sh\n+++ b/t/t5545-push-options.sh\n@@ -142,6 +142,45 @@ test_expect_success 'push options work properly across http' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'push options and submodules' '\n+\ttest_when_finished \"rm -rf parent\" &&\n+\ttest_when_finished \"rm -rf parent_upstream\" &&\n+\tmk_repo_pair &&\n+\tgit -C upstream config receive.advertisePushOptions true &&\n+\tcp -r upstream parent_upstream &&\n+\ttest_commit -C upstream one &&\n+\n+\ttest_create_repo parent &&\n+\tgit -C parent remote add up ../parent_upstream &&\n+\ttest_commit -C parent one &&\n+\tgit -C parent push --mirror up &&\n+\n+\tgit -C parent submodule add ../upstream workbench &&\n+\tgit -C parent commit -m \"add submoule\" &&\n+\n+\ttest_commit -C parent/workbench two &&\n+\tgit -C parent add workbench &&\n+\tgit -C parent commit -m \"update workbench\" &&\n+\n+\tgit -C parent push \\\n+\t\t--push-option=asdf --push-option=\"more structured text\" \\\n+\t\t--recurse-submodules=on-demand up master &&\n+\n+\tgit -C upstream rev-parse --verify master >expect &&\n+\tgit -C parent/workbench rev-parse --verify master >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tgit -C parent_upstream rev-parse --verify master >expect &&\n+\tgit -C parent rev-parse --verify master >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tprintf \"asdf\\nmore structured text\\n\" >expect &&\n+\ttest_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+\ttest_cmp expect upstream/.git/hooks/post-receive.push_options &&\n+\ttest_cmp expect parent_upstream/.git/hooks/pre-receive.push_options &&\n+\ttest_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n+'\n+\n stop_httpd\n \n test_done\ndiff --git a/transport.c b/transport.c\nindex 417ed7f19..64e60b635 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1031,6 +1031,7 @@ int transport_push(struct transport *transport,\n \n \t\t\tif (!push_unpushed_submodules(&commits,\n \t\t\t\t\t\t      transport->remote->name,\n+\t\t\t\t\t\t      transport->push_options,\n \t\t\t\t\t\t      pretend)) {\n \t\t\t\tsha1_array_clear(&commits);\n \t\t\t\tdie(\"Failed to push all needed submodules!\");\n-- \n2.12.2.564.g063fe858b8-goog\n\n"},{"id":"315969","messageId":"20170401001152.GB8741@aiede.mtv.corp.google.com","threadId":"45574","inReplyTo":"20170331235623.166408-2-bmwill@google.com","subject":"Re: [PATCH v2 1/2] push: unmark a local variable as static","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-04-01T00:11:52Z","receivedAt":"2017-04-01T00:12:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Brandon Williams wrote:\n\n>                         Also, clear the push_options string_list to\n> prevent memory leaking.\n\nThat's not a real leak, right?  Is the motivation to make it not show up\nin valgrind output?\n\nAt first it didn't look related to the other change but then the\nconnection dawned on me.\n\n> Signed-off-by: Brandon Williams <bmwill@google.com>\n> ---\n>  builtin/push.c | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n\nFor what it's worth,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"315970","messageId":"20170401001655.GB40772@google.com","threadId":"45574","inReplyTo":"20170401001152.GB8741@aiede.mtv.corp.google.com","subject":"Re: [PATCH v2 1/2] push: unmark a local variable as static","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-01T00:16:55Z","receivedAt":"2017-04-01T00:17:03Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 03/31, Jonathan Nieder wrote:\n> Brandon Williams wrote:\n> \n> >                         Also, clear the push_options string_list to\n> > prevent memory leaking.\n> \n> That's not a real leak, right?  Is the motivation to make it not show up\n> in valgrind output?\n\nwell depends on what you classify as a leak.  I guess since the program\nwill exit soon its not an issue, but there is still memory that was\nalloc'd that wasn't freed before the variable pointing to it went out of\nscope. :D\n> \n> At first it didn't look related to the other change but then the\n> connection dawned on me.\n> \n> > Signed-off-by: Brandon Williams <bmwill@google.com>\n> > ---\n> >  builtin/push.c | 5 +++--\n> >  1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> For what it's worth,\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\n-- \nBrandon Williams\n"},{"id":"315971","messageId":"20170401001924.GC8741@aiede.mtv.corp.google.com","threadId":"45574","inReplyTo":"20170331235623.166408-3-bmwill@google.com","subject":"Re: [PATCH v2 2/2] push: propagate push-options with --recurse-submodules","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-04-01T00:19:24Z","receivedAt":"2017-04-01T00:19:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Brandon Williams wrote:\n\n> Teach push --recurse-submodules to propagate push-options recursively to\n> the pushes performed in the submodules.\n\nSome time in the future we may want \"push --recurse-submodules\" to do a\ndry run pass before doing the final push, so that if it is known that\nsome of the pushes wouldn't succeed (e.g. due to not being\nfast-forward, or the server not being reachable, or the server not\nsupporting push options) then git could stop early instead of some\nsucceeding and some failing.\n\nBut that's a larger and separate change from this one.  Users of push\n--recurse-submodules today know they are effectively asking for\nmultiple pushes that are not guaranteed to succeed or fail together.\n\n> Signed-off-by: Brandon Williams <bmwill@google.com>\n> ---\n>  submodule.c             | 13 +++++++++++--\n>  submodule.h             |  1 +\n>  t/t5545-push-options.sh | 39 +++++++++++++++++++++++++++++++++++++++\n>  transport.c             |  1 +\n>  4 files changed, 52 insertions(+), 2 deletions(-)\n\nFor what it's worth,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"316218","messageId":"20170405174719.1297-1-bmwill@google.com","threadId":"45574","inReplyTo":"20170331235623.166408-1-bmwill@google.com","subject":"[PATCH v3 0/5] propagating push-options, remote and refspec","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-05T17:47:14Z","receivedAt":"2017-04-05T17:49:11Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"v3 builds upon v2 by adding 3 additional patches to add functionality to also\npropagate the remote and refspec down to children process's working to push\nsubmodules.  The remote and refspec will only be propagated if the provided\nremote is configured. A remote provided in the form of a URL will cause the\nremote and refspec to not be propagated down to the submodules and instead the\ndefault remote and refspec will be used, preserving existing functionality.\nCurrently the only supported LHS of a refspec must exactly match a single ref\nin the submodule (e.g. a branch name).\n\nPatches [1/5] and [2/5] remain unchanged from v2.\n\nBrandon Williams (5):\n  push: unmark a local variable as static\n  push: propagate push-options with --recurse-submodules\n  remote: expose parse_push_refspec function\n  submodule--helper: add push-check subcommand\n  push: propagate remote and refspec with --recurse-submodules\n\n builtin/push.c                 |  5 +--\n builtin/submodule--helper.c    | 45 +++++++++++++++++++++++++\n remote.c                       |  2 +-\n remote.h                       |  1 +\n submodule.c                    | 74 +++++++++++++++++++++++++++++++++++++++---\n submodule.h                    |  5 ++-\n t/t5531-deep-submodule-push.sh | 52 +++++++++++++++++++++++++++++\n t/t5545-push-options.sh        | 40 +++++++++++++++++++++++\n transport.c                    |  4 ++-\n 9 files changed, 219 insertions(+), 9 deletions(-)\n\n-- \n2.12.2.715.g7642488e1d-goog\n\n"},{"id":"316219","messageId":"20170405174719.1297-3-bmwill@google.com","threadId":"45574","inReplyTo":"20170405174719.1297-1-bmwill@google.com","subject":"[PATCH v3 2/5] push: propagate push-options with --recurse-submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-05T17:47:16Z","receivedAt":"2017-04-05T17:49:13Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Teach push --recurse-submodules to propagate push-options recursively to\nthe pushes performed in the submodules.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n submodule.c             | 13 +++++++++++--\n submodule.h             |  1 +\n t/t5545-push-options.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n transport.c             |  1 +\n 4 files changed, 53 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex c52d6634c..de444be61 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -782,7 +782,9 @@ int find_unpushed_submodules(struct sha1_array *commits,\n \treturn needs_pushing->nr;\n }\n \n-static int push_submodule(const char *path, int dry_run)\n+static int push_submodule(const char *path,\n+\t\t\t  const struct string_list *push_options,\n+\t\t\t  int dry_run)\n {\n \tif (add_submodule_odb(path))\n \t\treturn 1;\n@@ -793,6 +795,12 @@ static int push_submodule(const char *path, int dry_run)\n \t\tif (dry_run)\n \t\t\targv_array_push(&cp.args, \"--dry-run\");\n \n+\t\tif (push_options && push_options->nr) {\n+\t\t\tconst struct string_list_item *item;\n+\t\t\tfor_each_string_list_item(item, push_options)\n+\t\t\t\targv_array_pushf(&cp.args, \"--push-option=%s\",\n+\t\t\t\t\t\t item->string);\n+\t\t}\n \t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n@@ -807,6 +815,7 @@ static int push_submodule(const char *path, int dry_run)\n \n int push_unpushed_submodules(struct sha1_array *commits,\n \t\t\t     const char *remotes_name,\n+\t\t\t     const struct string_list *push_options,\n \t\t\t     int dry_run)\n {\n \tint i, ret = 1;\n@@ -818,7 +827,7 @@ int push_unpushed_submodules(struct sha1_array *commits,\n \tfor (i = 0; i < needs_pushing.nr; i++) {\n \t\tconst char *path = needs_pushing.items[i].string;\n \t\tfprintf(stderr, \"Pushing submodule '%s'\\n\", path);\n-\t\tif (!push_submodule(path, dry_run)) {\n+\t\tif (!push_submodule(path, push_options, dry_run)) {\n \t\t\tfprintf(stderr, \"Unable to push submodule '%s'\\n\", path);\n \t\t\tret = 0;\n \t\t}\ndiff --git a/submodule.h b/submodule.h\nindex 8a8bc49dc..0e26430fd 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -92,6 +92,7 @@ extern int find_unpushed_submodules(struct sha1_array *commits,\n \t\t\t\t    struct string_list *needs_pushing);\n extern int push_unpushed_submodules(struct sha1_array *commits,\n \t\t\t\t    const char *remotes_name,\n+\t\t\t\t    const struct string_list *push_options,\n \t\t\t\t    int dry_run);\n extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n extern int parallel_submodules(void);\ndiff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\nindex 97065e62b..f9232f5d0 100755\n--- a/t/t5545-push-options.sh\n+++ b/t/t5545-push-options.sh\n@@ -142,6 +142,46 @@ test_expect_success 'push options work properly across http' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'push options and submodules' '\n+\ttest_when_finished \"rm -rf parent\" &&\n+\ttest_when_finished \"rm -rf parent_upstream\" &&\n+\tmk_repo_pair &&\n+\tgit -C upstream config receive.advertisePushOptions true &&\n+\tcp -r upstream parent_upstream &&\n+\ttest_commit -C upstream one &&\n+\n+\ttest_create_repo parent &&\n+\tgit -C parent remote add up ../parent_upstream &&\n+\ttest_commit -C parent one &&\n+\tgit -C parent push --mirror up &&\n+\n+\tgit -C parent submodule add ../upstream workbench &&\n+\tgit -C parent/workbench remote add up ../../upstream &&\n+\tgit -C parent commit -m \"add submoule\" &&\n+\n+\ttest_commit -C parent/workbench two &&\n+\tgit -C parent add workbench &&\n+\tgit -C parent commit -m \"update workbench\" &&\n+\n+\tgit -C parent push \\\n+\t\t--push-option=asdf --push-option=\"more structured text\" \\\n+\t\t--recurse-submodules=on-demand up master &&\n+\n+\tgit -C upstream rev-parse --verify master >expect &&\n+\tgit -C parent/workbench rev-parse --verify master >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tgit -C parent_upstream rev-parse --verify master >expect &&\n+\tgit -C parent rev-parse --verify master >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tprintf \"asdf\\nmore structured text\\n\" >expect &&\n+\ttest_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+\ttest_cmp expect upstream/.git/hooks/post-receive.push_options &&\n+\ttest_cmp expect parent_upstream/.git/hooks/pre-receive.push_options &&\n+\ttest_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n+'\n+\n stop_httpd\n \n test_done\ndiff --git a/transport.c b/transport.c\nindex 417ed7f19..64e60b635 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1031,6 +1031,7 @@ int transport_push(struct transport *transport,\n \n \t\t\tif (!push_unpushed_submodules(&commits,\n \t\t\t\t\t\t      transport->remote->name,\n+\t\t\t\t\t\t      transport->push_options,\n \t\t\t\t\t\t      pretend)) {\n \t\t\t\tsha1_array_clear(&commits);\n \t\t\t\tdie(\"Failed to push all needed submodules!\");\n-- \n2.12.2.715.g7642488e1d-goog\n\n"},{"id":"316220","messageId":"20170405174719.1297-6-bmwill@google.com","threadId":"45574","inReplyTo":"20170405174719.1297-1-bmwill@google.com","subject":"[PATCH v3 5/5] push: propagate remote and refspec with --recurse-submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-05T17:47:19Z","receivedAt":"2017-04-05T17:49:15Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Teach \"push --recurse-submodules\" to propagate, if given a name as remote, the\nprovided remote and refspec recursively to the pushes performed in the\nsubmodules. The push will therefore only succeed if all submodules have a\nremote with such a name configured.\n\nNote that \"push --recurse-submodules\" with a path or URL as remote will not\npropagate the remote or refspec and instead use the default remote and refspec\nconfigured in the submodule, preserving the current behavior.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n submodule.c                    | 63 ++++++++++++++++++++++++++++++++++++++++--\n submodule.h                    |  4 ++-\n t/t5531-deep-submodule-push.sh | 52 ++++++++++++++++++++++++++++++++++\n transport.c                    |  3 +-\n 4 files changed, 117 insertions(+), 5 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex de444be61..49ab132d0 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -14,6 +14,7 @@\n #include \"blob.h\"\n #include \"thread-utils.h\"\n #include \"quote.h\"\n+#include \"remote.h\"\n #include \"worktree.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n@@ -783,6 +784,8 @@ int find_unpushed_submodules(struct sha1_array *commits,\n }\n \n static int push_submodule(const char *path,\n+\t\t\t  const struct remote *remote,\n+\t\t\t  const char **refspec, int refspec_nr,\n \t\t\t  const struct string_list *push_options,\n \t\t\t  int dry_run)\n {\n@@ -801,6 +804,14 @@ static int push_submodule(const char *path,\n \t\t\t\targv_array_pushf(&cp.args, \"--push-option=%s\",\n \t\t\t\t\t\t item->string);\n \t\t}\n+\n+\t\tif (remote->origin != REMOTE_UNCONFIGURED) {\n+\t\t\tint i;\n+\t\t\targv_array_push(&cp.args, remote->name);\n+\t\t\tfor (i = 0; i < refspec_nr; i++)\n+\t\t\t\targv_array_push(&cp.args, refspec[i]);\n+\t\t}\n+\n \t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n@@ -813,21 +824,67 @@ static int push_submodule(const char *path,\n \treturn 1;\n }\n \n+/*\n+ * Perform a check in the submodule to see if the remote and refspec work.\n+ * Die if the submodule can't be pushed.\n+ */\n+static void submodule_push_check(const char *path, const struct remote *remote,\n+\t\t\t\t const char **refspec, int refspec_nr)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint i;\n+\n+\targv_array_push(&cp.args, \"submodule--helper\");\n+\targv_array_push(&cp.args, \"push-check\");\n+\targv_array_push(&cp.args, remote->name);\n+\n+\tfor (i = 0; i < refspec_nr; i++)\n+\t\targv_array_push(&cp.args, refspec[i]);\n+\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\tcp.git_cmd = 1;\n+\tcp.no_stdin = 1;\n+\tcp.no_stdout = 1;\n+\tcp.dir = path;\n+\n+\t/*\n+\t * Simply indicate if 'submodule--helper push-check' failed.\n+\t * More detailed error information will be provided by the\n+\t * child process.\n+\t */\n+\tif (run_command(&cp))\n+\t\tdie(\"process for submodule '%s' failed\", path);\n+}\n+\n int push_unpushed_submodules(struct sha1_array *commits,\n-\t\t\t     const char *remotes_name,\n+\t\t\t     const struct remote *remote,\n+\t\t\t     const char **refspec, int refspec_nr,\n \t\t\t     const struct string_list *push_options,\n \t\t\t     int dry_run)\n {\n \tint i, ret = 1;\n \tstruct string_list needs_pushing = STRING_LIST_INIT_DUP;\n \n-\tif (!find_unpushed_submodules(commits, remotes_name, &needs_pushing))\n+\tif (!find_unpushed_submodules(commits, remote->name, &needs_pushing))\n \t\treturn 1;\n \n+\t/*\n+\t * Verify that the remote and refspec can be propagated to all\n+\t * submodules.  This check can be skipped if the remote and refspec\n+\t * won't be propagated due to the remote being unconfigured (e.g. a URL\n+\t * instead of a remote name).\n+\t */\n+\tif (remote->origin != REMOTE_UNCONFIGURED)\n+\t\tfor (i = 0; i < needs_pushing.nr; i++)\n+\t\t\tsubmodule_push_check(needs_pushing.items[i].string,\n+\t\t\t\t\t     remote, refspec, refspec_nr);\n+\n+\t/* Actually push the submodules */\n \tfor (i = 0; i < needs_pushing.nr; i++) {\n \t\tconst char *path = needs_pushing.items[i].string;\n \t\tfprintf(stderr, \"Pushing submodule '%s'\\n\", path);\n-\t\tif (!push_submodule(path, push_options, dry_run)) {\n+\t\tif (!push_submodule(path, remote, refspec, refspec_nr,\n+\t\t\t\t    push_options, dry_run)) {\n \t\t\tfprintf(stderr, \"Unable to push submodule '%s'\\n\", path);\n \t\t\tret = 0;\n \t\t}\ndiff --git a/submodule.h b/submodule.h\nindex 0e26430fd..127ff9be8 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -4,6 +4,7 @@\n struct diff_options;\n struct argv_array;\n struct sha1_array;\n+struct remote;\n \n enum {\n \tRECURSE_SUBMODULES_ONLY = -5,\n@@ -91,7 +92,8 @@ extern int find_unpushed_submodules(struct sha1_array *commits,\n \t\t\t\t    const char *remotes_name,\n \t\t\t\t    struct string_list *needs_pushing);\n extern int push_unpushed_submodules(struct sha1_array *commits,\n-\t\t\t\t    const char *remotes_name,\n+\t\t\t\t    const struct remote *remote,\n+\t\t\t\t    const char **refspec, int refspec_nr,\n \t\t\t\t    const struct string_list *push_options,\n \t\t\t\t    int dry_run);\n extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex f55137f76..57ba32262 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -475,4 +475,56 @@ test_expect_success 'push only unpushed submodules recursively' '\n \ttest_cmp expected_pub actual_pub\n '\n \n+test_expect_success 'push propagating the remotes name to a submodule' '\n+\tgit -C work remote add origin ../pub.git &&\n+\tgit -C work remote add pub ../pub.git &&\n+\n+\t> work/gar/bage/junk10 &&\n+\tgit -C work/gar/bage add junk10 &&\n+\tgit -C work/gar/bage commit -m \"Tenth junk\" &&\n+\tgit -C work add gar/bage &&\n+\tgit -C work commit -m \"Tenth junk added to gar/bage\" &&\n+\n+\t# Fails when submodule does not have a matching remote\n+\ttest_must_fail git -C work push --recurse-submodules=on-demand pub master &&\n+\t# Succeeds when submodules has matching remote and refspec\n+\tgit -C work push --recurse-submodules=on-demand origin master &&\n+\n+\tgit -C submodule.git rev-parse master >actual_submodule &&\n+\tgit -C pub.git rev-parse master >actual_pub &&\n+\tgit -C work/gar/bage rev-parse master >expected_submodule &&\n+\tgit -C work rev-parse master >expected_pub &&\n+\ttest_cmp expected_submodule actual_submodule &&\n+\ttest_cmp expected_pub actual_pub\n+'\n+\n+test_expect_success 'push propagating refspec to a submodule' '\n+\t> work/gar/bage/junk11 &&\n+\tgit -C work/gar/bage add junk11 &&\n+\tgit -C work/gar/bage commit -m \"Eleventh junk\" &&\n+\n+\tgit -C work checkout branch2 &&\n+\tgit -C work add gar/bage &&\n+\tgit -C work commit -m \"updating gar/bage in branch2\" &&\n+\n+\t# Fails when submodule does not have a matching branch\n+\ttest_must_fail git -C work push --recurse-submodules=on-demand origin branch2 &&\n+\t# Fails when refspec includes an object id\n+\ttest_must_fail git -C work push --recurse-submodules=on-demand origin \\\n+\t\t\"$(git -C work rev-parse branch2):refs/heads/branch2\" &&\n+\t# Fails when refspec includes 'HEAD' as it is unsupported at this time\n+\ttest_must_fail git -C work push --recurse-submodules=on-demand origin \\\n+\t\tHEAD:refs/heads/branch2 &&\n+\n+\tgit -C work/gar/bage branch branch2 master &&\n+\tgit -C work push --recurse-submodules=on-demand origin branch2 &&\n+\n+\tgit -C submodule.git rev-parse branch2 >actual_submodule &&\n+\tgit -C pub.git rev-parse branch2 >actual_pub &&\n+\tgit -C work/gar/bage rev-parse branch2 >expected_submodule &&\n+\tgit -C work rev-parse branch2 >expected_pub &&\n+\ttest_cmp expected_submodule actual_submodule &&\n+\ttest_cmp expected_pub actual_pub\n+'\n+\n test_done\ndiff --git a/transport.c b/transport.c\nindex 64e60b635..a62e5118c 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1030,7 +1030,8 @@ int transport_push(struct transport *transport,\n \t\t\t\t\tsha1_array_append(&commits, ref->new_oid.hash);\n \n \t\t\tif (!push_unpushed_submodules(&commits,\n-\t\t\t\t\t\t      transport->remote->name,\n+\t\t\t\t\t\t      transport->remote,\n+\t\t\t\t\t\t      refspec, refspec_nr,\n \t\t\t\t\t\t      transport->push_options,\n \t\t\t\t\t\t      pretend)) {\n \t\t\t\tsha1_array_clear(&commits);\n-- \n2.12.2.715.g7642488e1d-goog\n\n"},{"id":"316221","messageId":"20170405174719.1297-5-bmwill@google.com","threadId":"45574","inReplyTo":"20170405174719.1297-1-bmwill@google.com","subject":"[PATCH v3 4/5] submodule--helper: add push-check subcommand","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-05T17:47:18Z","receivedAt":"2017-04-05T17:49:20Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Add the 'push-check' subcommand to submodule--helper which is used to\ncheck if the provided remote and refspec can be used as part of a push\noperation in the submodule.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n builtin/submodule--helper.c | 45 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 45 insertions(+)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 85aafe46a..6ee962207 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1105,6 +1105,50 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,\n \treturn 0;\n }\n \n+static int push_check(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct remote *remote;\n+\n+\tif (argc < 2)\n+\t\tdie(\"submodule--helper push-check requires at least 1 argument\");\n+\n+\t/*\n+\t * The remote must be configured.\n+\t * This is to avoid pushing to the exact same URL as the parent.\n+\t */\n+\tremote = pushremote_get(argv[1]);\n+\tif (!remote || remote->origin == REMOTE_UNCONFIGURED)\n+\t\tdie(\"remote '%s' not configured\", argv[1]);\n+\n+\t/* Check the refspec */\n+\tif (argc > 2) {\n+\t\tint i, refspec_nr = argc - 2;\n+\t\tstruct ref *local_refs = get_local_heads();\n+\t\tstruct refspec *refspec = parse_push_refspec(refspec_nr,\n+\t\t\t\t\t\t\t     argv + 2);\n+\n+\t\tfor (i = 0; i < refspec_nr; i++) {\n+\t\t\tstruct refspec *rs = refspec + i;\n+\n+\t\t\tif (rs->pattern || rs->matching)\n+\t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * LHS must match a single ref\n+\t\t\t * NEEDSWORK: add logic to special case 'HEAD' once\n+\t\t\t * working with submodules in a detached head state\n+\t\t\t * ceases to be the norm.\n+\t\t\t */\n+\t\t\tif (count_refspec_match(rs->src, local_refs, NULL) != 1)\n+\t\t\t\tdie(\"src refspec '%s' must name a ref\",\n+\t\t\t\t    rs->src);\n+\t\t}\n+\t\tfree_refspec(refspec_nr, refspec);\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -1170,6 +1214,7 @@ static struct cmd_struct commands[] = {\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n+\t{\"push-check\", push_check, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\n \t{\"is-active\", is_active, 0},\n };\n-- \n2.12.2.715.g7642488e1d-goog\n\n"},{"id":"316222","messageId":"20170405174719.1297-2-bmwill@google.com","threadId":"45574","inReplyTo":"20170405174719.1297-1-bmwill@google.com","subject":"[PATCH v3 1/5] push: unmark a local variable as static","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-05T17:47:15Z","receivedAt":"2017-04-05T17:49:21Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"There isn't any obvious reason for the 'struct string_list push_options'\nand 'struct string_list_item *item' to be marked as static, so unmark\nthem as being static.  Also, clear the push_options string_list to\nprevent memory leaking.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n builtin/push.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 5c22e9f2e..a597759d8 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -510,8 +510,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tint push_cert = -1;\n \tint rc;\n \tconst char *repo = NULL;\t/* default repository */\n-\tstatic struct string_list push_options = STRING_LIST_INIT_DUP;\n-\tstatic struct string_list_item *item;\n+\tstruct string_list push_options = STRING_LIST_INIT_DUP;\n+\tconst struct string_list_item *item;\n \n \tstruct option options[] = {\n \t\tOPT__VERBOSITY(&verbosity),\n@@ -584,6 +584,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"push options must not have new line characters\"));\n \n \trc = do_push(repo, flags, &push_options);\n+\tstring_list_clear(&push_options, 0);\n \tif (rc == -1)\n \t\tusage_with_options(push_usage, options);\n \telse\n-- \n2.12.2.715.g7642488e1d-goog\n\n"},{"id":"316223","messageId":"20170405174719.1297-4-bmwill@google.com","threadId":"45574","inReplyTo":"20170405174719.1297-1-bmwill@google.com","subject":"[PATCH v3 3/5] remote: expose parse_push_refspec function","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-05T17:47:17Z","receivedAt":"2017-04-05T17:49:25Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"A future patch needs access to the 'parse_push_refspec()' function so\nlet's export the function so other modules can use it.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n remote.c | 2 +-\n remote.h | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/remote.c b/remote.c\nindex 9f83fe2c4..d335a6417 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -630,7 +630,7 @@ struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec)\n \treturn parse_refspec_internal(nr_refspec, refspec, 1, 0);\n }\n \n-static struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)\n+struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)\n {\n \treturn parse_refspec_internal(nr_refspec, refspec, 0, 0);\n }\ndiff --git a/remote.h b/remote.h\nindex dd8c51757..42c8f017b 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -169,6 +169,7 @@ struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n int valid_fetch_refspec(const char *refspec);\n struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);\n+extern struct refspec *parse_push_refspec(int nr_refspec, const char **refspec);\n \n void free_refspec(int nr_refspec, struct refspec *refspec);\n \n-- \n2.12.2.715.g7642488e1d-goog\n\n"},{"id":"316245","messageId":"CA+P7+xq58Bc2KVb2ewBPWPj8Zyf82tJWgQeq=oFvbs0ACysvLQ@mail.gmail.com","threadId":"45574","inReplyTo":"20170401001924.GC8741@aiede.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] push: propagate push-options with --recurse-submodules","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-04-06T00:17:53Z","receivedAt":"2017-04-06T00:18:21Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Mar 31, 2017 at 5:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Brandon Williams wrote:\n>\n>> Teach push --recurse-submodules to propagate push-options recursively to\n>> the pushes performed in the submodules.\n>\n> Some time in the future we may want \"push --recurse-submodules\" to do a\n> dry run pass before doing the final push, so that if it is known that\n> some of the pushes wouldn't succeed (e.g. due to not being\n> fast-forward, or the server not being reachable, or the server not\n> supporting push options) then git could stop early instead of some\n> succeeding and some failing.\n>\n> But that's a larger and separate change from this one.  Users of push\n> --recurse-submodules today know they are effectively asking for\n> multiple pushes that are not guaranteed to succeed or fail together.\n>\n\nIf you want it to be truly atomic it will require more effort than the\nabove. Suppose that you do a dry-run first, and then find out\neverything will succeed. After this, you do the real pushes. But in\nbetween these two commands something could have changed, and you could\nstill end up with a non-atomic set of pushes.\n\nI think that's ok and better than before, but it should be noted that\nyou stll don't guarantee that all the pushes succeed or fail together.\n\nI'm really not sure if you even can make these pushes work atomically\nconsidering they are going to different hosts.\n\nThanks,\nJake\n"},{"id":"316294","messageId":"20170406173936.GA142670@google.com","threadId":"45574","inReplyTo":"CA+P7+xq58Bc2KVb2ewBPWPj8Zyf82tJWgQeq=oFvbs0ACysvLQ@mail.gmail.com","subject":"Re: [PATCH v2 2/2] push: propagate push-options with --recurse-submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-06T17:39:36Z","receivedAt":"2017-04-06T17:39:54Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 04/05, Jacob Keller wrote:\n> On Fri, Mar 31, 2017 at 5:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> > Brandon Williams wrote:\n> >\n> >> Teach push --recurse-submodules to propagate push-options recursively to\n> >> the pushes performed in the submodules.\n> >\n> > Some time in the future we may want \"push --recurse-submodules\" to do a\n> > dry run pass before doing the final push, so that if it is known that\n> > some of the pushes wouldn't succeed (e.g. due to not being\n> > fast-forward, or the server not being reachable, or the server not\n> > supporting push options) then git could stop early instead of some\n> > succeeding and some failing.\n> >\n> > But that's a larger and separate change from this one.  Users of push\n> > --recurse-submodules today know they are effectively asking for\n> > multiple pushes that are not guaranteed to succeed or fail together.\n> >\n> \n> If you want it to be truly atomic it will require more effort than the\n> above. Suppose that you do a dry-run first, and then find out\n> everything will succeed. After this, you do the real pushes. But in\n> between these two commands something could have changed, and you could\n> still end up with a non-atomic set of pushes.\n> \n> I think that's ok and better than before, but it should be noted that\n> you stll don't guarantee that all the pushes succeed or fail together.\n> \n> I'm really not sure if you even can make these pushes work atomically\n> considering they are going to different hosts.\n\nYeah, really it becomes impossible to have submodules pushed to multiple\nhosts in an atomic way.  I think what Jonathan is mentioning is a way to\nensure that the options and everything are supported by the server you\nare communicating with (for each submodule), though you could just run\nan explicit dry-run yourself.\n\nIf you truly wanted the superproject and all submodules updated in an\natomic way you would need some other server side service (e.g. gerrit)\nto ensure that it was done properly.\n\n-- \nBrandon Williams\n"},{"id":"316551","messageId":"xmqqr30zqtgb.fsf@gitster.mtv.corp.google.com","threadId":"45574","inReplyTo":"20170405174719.1297-1-bmwill@google.com","subject":"Re: [PATCH v3 0/5] propagating push-options, remote and refspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-11T07:44:36Z","receivedAt":"2017-04-11T07:44:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams <bmwill@google.com> writes:\n\n> v3 builds upon v2 by adding 3 additional patches to add functionality to also\n> propagate the remote and refspec down to children process's working to push\n> submodules.  The remote and refspec will only be propagated if the provided\n> remote is configured. A remote provided in the form of a URL will cause the\n> remote and refspec to not be propagated down to the submodules and instead the\n> default remote and refspec will be used, preserving existing functionality.\n> Currently the only supported LHS of a refspec must exactly match a single ref\n> in the submodule (e.g. a branch name).\n>\n> Patches [1/5] and [2/5] remain unchanged from v2.\n\n2/5 seems to be a bit different in 5545 ll.156,+6, though.\n\n>\n> Brandon Williams (5):\n>   push: unmark a local variable as static\n>   push: propagate push-options with --recurse-submodules\n>   remote: expose parse_push_refspec function\n>   submodule--helper: add push-check subcommand\n>   push: propagate remote and refspec with --recurse-submodules\n>\n>  builtin/push.c                 |  5 +--\n>  builtin/submodule--helper.c    | 45 +++++++++++++++++++++++++\n>  remote.c                       |  2 +-\n>  remote.h                       |  1 +\n>  submodule.c                    | 74 +++++++++++++++++++++++++++++++++++++++---\n>  submodule.h                    |  5 ++-\n>  t/t5531-deep-submodule-push.sh | 52 +++++++++++++++++++++++++++++\n>  t/t5545-push-options.sh        | 40 +++++++++++++++++++++++\n>  transport.c                    |  4 ++-\n>  9 files changed, 219 insertions(+), 9 deletions(-)\n"},{"id":"316602","messageId":"20170411163307.GB36152@google.com","threadId":"45574","inReplyTo":"xmqqr30zqtgb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 0/5] propagating push-options, remote and refspec","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-11T16:33:07Z","receivedAt":"2017-04-11T16:33:15Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 04/11, Junio C Hamano wrote:\n> Brandon Williams <bmwill@google.com> writes:\n> \n> > v3 builds upon v2 by adding 3 additional patches to add functionality to also\n> > propagate the remote and refspec down to children process's working to push\n> > submodules.  The remote and refspec will only be propagated if the provided\n> > remote is configured. A remote provided in the form of a URL will cause the\n> > remote and refspec to not be propagated down to the submodules and instead the\n> > default remote and refspec will be used, preserving existing functionality.\n> > Currently the only supported LHS of a refspec must exactly match a single ref\n> > in the submodule (e.g. a branch name).\n> >\n> > Patches [1/5] and [2/5] remain unchanged from v2.\n> \n> 2/5 seems to be a bit different in 5545 ll.156,+6, though.\n\nOh, oops you're right.  Looks like I forgot that I added a single line\nto a test in 2/5:\n\n  git -C parent/workbench remote add up ../../upstream &&\n\nWhich just configures a remote in one of the submodules.  I must have\nadded that to prep for the propagating of the remote/refspec in later\npatches.\n\n> >\n> > Brandon Williams (5):\n> >   push: unmark a local variable as static\n> >   push: propagate push-options with --recurse-submodules\n> >   remote: expose parse_push_refspec function\n> >   submodule--helper: add push-check subcommand\n> >   push: propagate remote and refspec with --recurse-submodules\n> >\n> >  builtin/push.c                 |  5 +--\n> >  builtin/submodule--helper.c    | 45 +++++++++++++++++++++++++\n> >  remote.c                       |  2 +-\n> >  remote.h                       |  1 +\n> >  submodule.c                    | 74 +++++++++++++++++++++++++++++++++++++++---\n> >  submodule.h                    |  5 ++-\n> >  t/t5531-deep-submodule-push.sh | 52 +++++++++++++++++++++++++++++\n> >  t/t5545-push-options.sh        | 40 +++++++++++++++++++++++\n> >  transport.c                    |  4 ++-\n> >  9 files changed, 219 insertions(+), 9 deletions(-)\n\n-- \nBrandon Williams\n"}]}