{"thread":{"id":"40806","subject":"[PATCH] push: add recurseSubmodules config option","startedAt":"2015-11-16T13:24:54Z","lastAt":"2015-11-30T19:00:21Z","messageCount":7,"participants":["Mike Crowe","Stefan Beller","Jens Lehmann","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"273364","messageId":"1447680294-13395-1-git-send-email-mac@mcrowe.com","threadId":"40806","inReplyTo":null,"subject":"[PATCH] push: add recurseSubmodules config option","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-11-16T13:24:54Z","receivedAt":"2015-11-16T13:24:54Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"The --recurse-submodules command line parameter has existed for some\ntime but it has no config file equivalent.\n\nFollowing the style of the corresponding parameter for git fetch, let's\ninvent push.recurseSubmodules to provide a default for this\nparameter. This also requires the addition of --recurse-submodules=no to\nallow the configuration to be overridden on the command line when\nrequired.\n\nThe most straightforward way to implement this appears to be to make\npush use code in submodule-config in a similar way to fetch.\n\nSigned-off-by: Mike Crowe <mac@mcrowe.com>\n---\n Documentation/config.txt       |  13 +++++\n Documentation/git-push.txt     |   4 +-\n builtin/push.c                 |  37 ++++++++-----\n submodule-config.c             |  20 +++++++\n submodule-config.h             |   1 +\n submodule.h                    |   1 +\n t/t5531-deep-submodule-push.sh | 123 ++++++++++++++++++++++++++++++++++++++++-\n 7 files changed, 182 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 391a0c3..0546da5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2226,6 +2226,19 @@ push.gpgSign::\n \toverride a value from a lower-priority config file. An explicit\n \tcommand-line flag always overrides this config option.\n \n+push.recurseSubmodules::\n+\tMake sure all submodule commits used by the revisions to be pushed\n+\tare available on a remote-tracking branch. If the value is 'check'\n+\tthen Git will verify that all submodule commits that changed in the\n+\trevisions to be pushed are available on at least one remote of the\n+\tsubmodule. If any commits are missing the push will be aborted and\n+\texit with non-zero status. If the value is 'on-demand' then all\n+\tsubmodules that changed in the revisions to be pushed will be\n+\tpushed. If on-demand was not able to push all necessary revisions\n+\tit will also be aborted and exit with non-zero status. You may\n+\toverride this configuration at time of push by specifying\n+\t'--recurse-submodules=check|on-demand|no'.\n+\n rebase.stat::\n \tWhether to show a diffstat of what changed upstream since the last\n \trebase. False by default.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 85a4d7d..fb0e9b7 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -257,7 +257,7 @@ origin +master` to force a push to the `master` branch). See the\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|on-demand::\n+--recurse-submodules=check|on-demand|no::\n \tMake sure all submodule commits used by the revisions to be\n \tpushed are available on a remote-tracking branch. If 'check' is\n \tused Git will verify that all submodule commits that changed in\n@@ -267,6 +267,8 @@ origin +master` to force a push to the `master` branch). See the\n \tall submodules that changed in the revisions to be pushed will\n \tbe pushed. If on-demand was not able to push all necessary\n \trevisions it will also be aborted and exit with non-zero status.\n+\tA value of 'no' is used to override the push.recurseSubmodules\n+\tvariable when no submodule recursion is required.\n \n --[no-]verify::\n \tToggle the pre-push hook (see linkgit:githooks[5]).  The\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 3bda430..dfced74 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -9,6 +9,7 @@\n #include \"transport.h\"\n #include \"parse-options.h\"\n #include \"submodule.h\"\n+#include \"submodule-config.h\"\n #include \"send-pack.h\"\n \n static const char * const push_usage[] = {\n@@ -20,7 +21,7 @@ static int thin = 1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n-static int progress = -1;\n+static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n \n static struct push_cas_option cas;\n \n@@ -452,22 +453,15 @@ static int do_push(const char *repo, int flags)\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+\tint *recurse_submodules = opt->value;\n \n-\tif (*flags & (TRANSPORT_RECURSE_SUBMODULES_CHECK |\n-\t\t      TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND))\n+\tif (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n \t\tdie(\"%s can only be used once.\", opt->long_name);\n \n-\tif (arg) {\n-\t\tif (!strcmp(arg, \"check\"))\n-\t\t\t*flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;\n-\t\telse if (!strcmp(arg, \"on-demand\"))\n-\t\t\t*flags |= TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND;\n-\t\telse\n-\t\t\tdie(\"bad %s argument: %s\", opt->long_name, arg);\n-\t} else\n-\t\tdie(\"option %s needs an argument (check|on-demand)\",\n-\t\t\t\topt->long_name);\n+\tif (arg)\n+\t\t*recurse_submodules = parse_push_recurse_submodules_arg(opt->long_name, arg);\n+\telse\n+\t\tdie(\"%s missing parameter\", opt->long_name);\n \n \treturn 0;\n }\n@@ -522,6 +516,10 @@ static int git_push_config(const char *k, const char *v, void *cb)\n \t\t\t\t\treturn error(\"Invalid value for '%s'\", k);\n \t\t\t}\n \t\t}\n+\t} else if (!strcmp(k, \"push.recursesubmodules\")) {\n+\t\tconst char *value;\n+\t\tif (!git_config_get_value(\"push.recursesubmodules\", &value))\n+\t\t\trecurse_submodules = parse_push_recurse_submodules_arg(k, value);\n \t}\n \n \treturn git_default_config(k, v, NULL);\n@@ -532,6 +530,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tint flags = 0;\n \tint tags = 0;\n \tint push_cert = -1;\n+\tint recurse_submodules_from_cmdline = RECURSE_SUBMODULES_DEFAULT;\n \tint rc;\n \tconst char *repo = NULL;\t/* default repository */\n \tstruct option options[] = {\n@@ -549,7 +548,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\t  0, CAS_OPT_NAME, &cas, N_(\"refname>:<expect\"),\n \t\t  N_(\"require old value of ref to be at this value\"),\n \t\t  PARSE_OPT_OPTARG, parseopt_push_cas_option },\n-\t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &flags, \"check|on-demand\",\n+\t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules_from_cmdline, N_(\"check|on-demand|no\"),\n \t\t\tN_(\"control recursive pushing of submodules\"),\n \t\t\tPARSE_OPT_OPTARG, option_parse_recurse_submodules },\n \t\tOPT_BOOL( 0 , \"thin\", &thin, N_(\"use thin pack\")),\n@@ -580,6 +579,14 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tif (deleterefs && argc < 2)\n \t\tdie(_(\"--delete doesn't make sense without any refs\"));\n \n+\tif (recurse_submodules_from_cmdline != RECURSE_SUBMODULES_DEFAULT)\n+\t\trecurse_submodules = recurse_submodules_from_cmdline;\n+\n+\tif (recurse_submodules == RECURSE_SUBMODULES_CHECK)\n+\t\tflags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;\n+\telse if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n+\t\tflags |= TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND;\n+\n \tif (tags)\n \t\tadd_refspec(\"refs/tags/*\");\n \ndiff --git a/submodule-config.c b/submodule-config.c\nindex afe0ea8..33d8790 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -228,6 +228,26 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)\n \treturn parse_fetch_recurse(opt, arg, 1);\n }\n \n+static int parse_push_recurse(const char *opt, const char *arg,\n+\t\t\t       int die_on_error)\n+{\n+    if (!strcmp(arg, \"on-demand\"))\n+\treturn RECURSE_SUBMODULES_ON_DEMAND;\n+    else if (!strcmp(arg, \"check\"))\n+\treturn RECURSE_SUBMODULES_CHECK;\n+    else if (!strcmp(arg, \"no\"))\n+\treturn RECURSE_SUBMODULES_OFF;\n+    else if (die_on_error)\n+\tdie(\"bad %s argument: %s\", opt, arg);\n+    else\n+\treturn RECURSE_SUBMODULES_ERROR;\n+}\n+\n+int parse_push_recurse_submodules_arg(const char *opt, const char *arg)\n+{\n+\treturn parse_push_recurse(opt, arg, 1);\n+}\n+\n static void warn_multiple_config(const unsigned char *commit_sha1,\n \t\t\t\t const char *name, const char *option)\n {\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 9061e4e..9bfa65a 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -19,6 +19,7 @@ struct submodule {\n };\n \n int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n+int parse_push_recurse_submodules_arg(const char *opt, const char *arg);\n int parse_submodule_config_option(const char *var, const char *value);\n const struct submodule *submodule_from_name(const unsigned char *commit_sha1,\n \t\tconst char *name);\ndiff --git a/submodule.h b/submodule.h\nindex 5507c3d..ddff512 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -5,6 +5,7 @@ struct diff_options;\n struct argv_array;\n \n enum {\n+\tRECURSE_SUBMODULES_CHECK = -4,\n \tRECURSE_SUBMODULES_ERROR = -3,\n \tRECURSE_SUBMODULES_NONE = -2,\n \tRECURSE_SUBMODULES_ON_DEMAND = -1,\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 6507487..d2fb072 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -64,7 +64,15 @@ test_expect_success 'push fails if submodule commit not on remote' '\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\t# the push should fail with --recurse-submodules=check\n+\t\t# on the command line...\n+\t\ttest_must_fail git push --recurse-submodules=check ../pub.git master &&\n+\n+\t\t# ...or if specified in the configuration..\n+\t\tgit config push.recurseSubmodules check &&\n+\t\ttest_must_fail git push ../pub.git master &&\n+\n+\t\tgit config --unset push.recurseSubmodules\n \t)\n '\n \n@@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was pushed to remote' '\n \t)\n '\n \n+test_expect_success 'push succeeds if submodule commit not on remote but using on-demand on command line' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-on-demand-on-command-line &&\n+\t\tgit add recurse-on-demand-on-command-line &&\n+\t\tgit commit -m \"Recurse on-demand on command line junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Recurse on-demand on command line for gar/bage\" &&\n+\t\tgit push --recurse-submodules=on-demand ../pub.git master &&\n+\t\t# Check that the supermodule commit got there\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master\n+\t\t# Check that the submodule commit got there too\n+\t\tcd gar/bage &&\n+\t\tgit diff --quiet origin/master master\n+\t)\n+'\n+\n+test_expect_success 'push succeeds if submodule commit not on remote but using on-demand from config' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-on-demand-from-config &&\n+\t\tgit add recurse-on-demand-from-config &&\n+\t\tgit commit -m \"Recurse on-demand from config junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Recurse on-demand on command line for gar/bage\" &&\n+\t\tgit config push.recurseSubmodules on-demand &&\n+\t\tgit push ../pub.git master &&\n+\t\tgit config --unset push.recurseSubmodules &&\n+\t\t# Check that the supermodule commit got there\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master\n+\t\t# Check that the submodule commit got there too\n+\t\tcd gar/bage &&\n+\t\tgit diff --quiet origin/master master\n+\t)\n+'\n+\n+test_expect_success 'push fails if submodule commit not on remote using check from cmdline overriding config' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-check-on-command-line-overriding-config &&\n+\t\tgit add recurse-check-on-command-line-overriding-config &&\n+\t\tgit commit -m \"Recurse on command-line overridiing config junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Recurse on command-line overriding config for gar/bage\" &&\n+\t\tgit config push.recurseSubmodules on-demand &&\n+\t\ttest_must_fail git push --recurse-submodules=check ../pub.git master &&\n+\t\tgit config --unset push.recurseSubmodules &&\n+\t\t# Check that the supermodule commit did not get there\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master^\n+\t\t# Check that the submodule commit did not get there\n+\t\tcd gar/bage &&\n+\t\tgit diff --quiet origin/master master^\n+\t)\n+'\n+\n+test_expect_success 'push succeeds if submodule commit not on remote using on-demand from cmdline overriding config' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-on-demand-on-command-line-overriding-config &&\n+\t\tgit add recurse-on-demand-on-command-line-overriding-config &&\n+\t\tgit commit -m \"Recurse on-demand on command-line overriding config junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Recurse on-demand on command-line overriding config for gar/bage\" &&\n+\t\tgit config push.recurseSubmodules check &&\n+\t\tgit push --recurse-submodules=on-demand ../pub.git master &&\n+\t\tgit config --unset push.recurseSubmodules &&\n+\t\t# Check that the supermodule commit got there\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master\n+\t\t# Check that the submodule commit got there\n+\t\tcd gar/bage &&\n+\t\tgit diff --quiet origin/master master\n+\t)\n+'\n+\n+test_expect_success 'push succeeds if submodule commit disabling recursion from cmdline overriding config' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-disable-on-command-line-overriding-config &&\n+\t\tgit add recurse-disable-on-command-line-overriding-config &&\n+\t\tgit commit -m \"Recurse disable on command-line overriding config junk\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Recurse disable on command-line overriding config for gar/bage\" &&\n+\t\tgit config push.recurseSubmodules check &&\n+\t\tgit push --recurse-submodules=no ../pub.git master &&\n+\t\tgit config --unset push.recurseSubmodules &&\n+\t\t# Check that the supermodule commit got there\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master &&\n+\t\t# But that the submodule commit did not\n+\t\tcd gar/bage &&\n+\t\tgit diff --quiet origin/master 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-- \n2.1.4\n"},{"id":"273374","messageId":"CAGZ79kacpWFFWiE-KjwEQZC+3PZw2MrpsgQWLJyS82X5LF+Lqw@mail.gmail.com","threadId":"40806","inReplyTo":"1447680294-13395-1-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH] push: add recurseSubmodules config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-16T18:15:24Z","receivedAt":"2015-11-16T18:15:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Nov 16, 2015 at 5:24 AM, Mike Crowe <mac@mcrowe.com> wrote:\n> The --recurse-submodules command line parameter has existed for some\n> time but it has no config file equivalent.\n>\n> Following the style of the corresponding parameter for git fetch, let's\n> invent push.recurseSubmodules to provide a default for this\n> parameter. This also requires the addition of --recurse-submodules=no to\n> allow the configuration to be overridden on the command line when\n> required.\n>\n> The most straightforward way to implement this appears to be to make\n> push use code in submodule-config in a similar way to fetch.\n>\n> Signed-off-by: Mike Crowe <mac@mcrowe.com>\n> ---\n\nThe code itself looks good to me, one nit in the tests though.\n\n> @@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was pushed to remote' '\n>         )\n>  '\n>\n> +test_expect_success 'push succeeds if submodule commit not on remote but using on-demand on command line' '\n> +       (\n> +               cd work/gar/bage &&\n> +               >recurse-on-demand-on-command-line &&\n> +               git add recurse-on-demand-on-command-line &&\n> +               git commit -m \"Recurse on-demand on command line junk\"\n> +       ) &&\n> +       (\n> +               cd work &&\n> +               git add gar/bage &&\n> +               git commit -m \"Recurse on-demand on command line for gar/bage\" &&\n> +               git push --recurse-submodules=on-demand ../pub.git master &&\n> +               # Check that the supermodule commit got there\n> +               git fetch ../pub.git &&\n> +               git diff --quiet FETCH_HEAD master\n\nMissing && chain here.\n\n> +               # Check that the submodule commit got there too\n> +               cd gar/bage &&\n> +               git diff --quiet origin/master master\n> +       )\n> +'\n> +\n"},{"id":"273375","messageId":"20151116183106.GA31731@mcrowe.com","threadId":"40806","inReplyTo":"CAGZ79kacpWFFWiE-KjwEQZC+3PZw2MrpsgQWLJyS82X5LF+Lqw@mail.gmail.com","subject":"Re: [PATCH] push: add recurseSubmodules config option","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-11-16T18:31:06Z","receivedAt":"2015-11-16T18:31:06Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"On Monday 16 November 2015 at 10:15:24 -0800, Stefan Beller wrote:\n> The code itself looks good to me, one nit in the tests though.\n> \n> > @@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was pushed to remote' '\n> >         )\n> >  '\n> >\n> > +test_expect_success 'push succeeds if submodule commit not on remote but using on-demand on command line' '\n> > +       (\n> > +               cd work/gar/bage &&\n> > +               >recurse-on-demand-on-command-line &&\n> > +               git add recurse-on-demand-on-command-line &&\n> > +               git commit -m \"Recurse on-demand on command line junk\"\n> > +       ) &&\n> > +       (\n> > +               cd work &&\n> > +               git add gar/bage &&\n> > +               git commit -m \"Recurse on-demand on command line for gar/bage\" &&\n> > +               git push --recurse-submodules=on-demand ../pub.git master &&\n> > +               # Check that the supermodule commit got there\n> > +               git fetch ../pub.git &&\n> > +               git diff --quiet FETCH_HEAD master\n> \n> Missing && chain here.\n\nOh, well spotted! I'll provide an updated version.\n\nThanks.\n\nMike.\n"},{"id":"273378","messageId":"564A2906.7010902@web.de","threadId":"40806","inReplyTo":"20151116183106.GA31731@mcrowe.com","subject":"Re: [PATCH] push: add recurseSubmodules config option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2015-11-16T19:05:42Z","receivedAt":"2015-11-16T19:05:42Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 16.11.2015 um 19:31 schrieb Mike Crowe:\n> On Monday 16 November 2015 at 10:15:24 -0800, Stefan Beller wrote:\n>> The code itself looks good to me, one nit in the tests though.\n>>\n>>> @@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was pushed to remote' '\n>>>          )\n>>>   '\n>>>\n>>> +test_expect_success 'push succeeds if submodule commit not on remote but using on-demand on command line' '\n>>> +       (\n>>> +               cd work/gar/bage &&\n>>> +               >recurse-on-demand-on-command-line &&\n>>> +               git add recurse-on-demand-on-command-line &&\n>>> +               git commit -m \"Recurse on-demand on command line junk\"\n>>> +       ) &&\n>>> +       (\n>>> +               cd work &&\n>>> +               git add gar/bage &&\n>>> +               git commit -m \"Recurse on-demand on command line for gar/bage\" &&\n>>> +               git push --recurse-submodules=on-demand ../pub.git master &&\n>>> +               # Check that the supermodule commit got there\n>>> +               git fetch ../pub.git &&\n>>> +               git diff --quiet FETCH_HEAD master\n>>\n>> Missing && chain here.\n>\n> Oh, well spotted! I'll provide an updated version.\n\nLooking good for me too!\n\nCool, another issue from my Wiki that's being worked on!\n"},{"id":"273388","messageId":"CAPig+cRRBhRsjMTLK3YVPRLMJd0kTDPycTPXFQ1S-XxsTzaiBQ@mail.gmail.com","threadId":"40806","inReplyTo":"1447680294-13395-1-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH] push: add recurseSubmodules config option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-16T23:13:14Z","receivedAt":"2015-11-16T23:13:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Nov 16, 2015 at 8:24 AM, Mike Crowe <mac@mcrowe.com> wrote:\n> The --recurse-submodules command line parameter has existed for some\n> time but it has no config file equivalent.\n>\n> Following the style of the corresponding parameter for git fetch, let's\n> invent push.recurseSubmodules to provide a default for this\n> parameter. This also requires the addition of --recurse-submodules=no to\n> allow the configuration to be overridden on the command line when\n> required.\n>\n> The most straightforward way to implement this appears to be to make\n> push use code in submodule-config in a similar way to fetch.\n>\n> Signed-off-by: Mike Crowe <mac@mcrowe.com>\n> ---\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> @@ -2226,6 +2226,19 @@ push.gpgSign::\n> +push.recurseSubmodules::\n> +       Make sure all submodule commits used by the revisions to be pushed\n> +       are available on a remote-tracking branch. If the value is 'check'\n> +       then Git will verify that all submodule commits that changed in the\n> +       revisions to be pushed are available on at least one remote of the\n> +       submodule. If any commits are missing the push will be aborted and\n> +       exit with non-zero status. If the value is 'on-demand' then all\n> +       submodules that changed in the revisions to be pushed will be\n> +       pushed. If on-demand was not able to push all necessary revisions\n> +       it will also be aborted and exit with non-zero status. You may\n> +       override this configuration at time of push by specifying\n> +       '--recurse-submodules=check|on-demand|no'.\n\nDoes this configuration variable also support 'no' as a value? If so,\nthen it probably ought to be documented. If not, shouldn't it do so to\nallow a configuration file to override a 'check' or 'on-demand' value\nspecified in a more global git configuration file?\n\n>  rebase.stat::\n>         Whether to show a diffstat of what changed upstream since the last\n>         rebase. False by default.\n> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\n> @@ -257,7 +257,7 @@ origin +master` to force a push to the `master` branch). See the\n> ---recurse-submodules=check|on-demand::\n> +--recurse-submodules=check|on-demand|no::\n>         Make sure all submodule commits used by the revisions to be\n>         pushed are available on a remote-tracking branch. If 'check' is\n>         used Git will verify that all submodule commits that changed in\n> @@ -267,6 +267,8 @@ origin +master` to force a push to the `master` branch). See the\n>         all submodules that changed in the revisions to be pushed will\n>         be pushed. If on-demand was not able to push all necessary\n>         revisions it will also be aborted and exit with non-zero status.\n> +       A value of 'no' is used to override the push.recurseSubmodules\n> +       variable when no submodule recursion is required.\n\nDoes this deserve a --no-recurse-submodules alias for consistency with\nhow other options are turned off?\n\n>  --[no-]verify::\n>         Toggle the pre-push hook (see linkgit:githooks[5]).  The\n> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\n> @@ -64,7 +64,15 @@ test_expect_success 'push fails if submodule commit not on remote' '\n>                 cd work &&\n>                 git add gar/bage &&\n>                 git commit -m \"Third commit for gar/bage\" &&\n> -               test_must_fail git push --recurse-submodules=check ../pub.git master\n> +               # the push should fail with --recurse-submodules=check\n> +               # on the command line...\n> +               test_must_fail git push --recurse-submodules=check ../pub.git master &&\n> +\n> +               # ...or if specified in the configuration..\n> +               git config push.recurseSubmodules check &&\n> +               test_must_fail git push ../pub.git master &&\n> +\n> +               git config --unset push.recurseSubmodules\n\nIf something above this line fails, then 'git config --unset' will not\nbe invoked, so the expected cleanup won't happen. Typically, to ensure\ncleanup, you'd use test_config(), however that function doesn't work\nin subshells. Probably the easiest fix, in this case, is to set the\nconfig variable as a one-shot and drop 'git config' and 'git config\n--unset' altogether:\n\n    test_must_fail git -c push.recurseSubmodules check \\\n        push ../pub.git master\n\n>         )\n>  '\n>\n> @@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was pushed to remote' '\n> +test_expect_success 'push succeeds if submodule commit not on remote but using on-demand from config' '\n> +       (\n> +               cd work/gar/bage &&\n> +               >recurse-on-demand-from-config &&\n> +               git add recurse-on-demand-from-config &&\n> +               git commit -m \"Recurse on-demand from config junk\"\n> +       ) &&\n> +       (\n> +               cd work &&\n> +               git add gar/bage &&\n> +               git commit -m \"Recurse on-demand on command line for gar/bage\" &&\n> +               git config push.recurseSubmodules on-demand &&\n> +               git push ../pub.git master &&\n> +               git config --unset push.recurseSubmodules &&\n\nDitto regarding 'git config --unset' cleanup not being run there is a\nfailure above this. Same for following tests.\n\n> +               # Check that the supermodule commit got there\n> +               git fetch ../pub.git &&\n> +               git diff --quiet FETCH_HEAD master\n> +               # Check that the submodule commit got there too\n> +               cd gar/bage &&\n> +               git diff --quiet origin/master master\n> +       )\n> +'\n"},{"id":"273830","messageId":"xmqqa8pv8hkx.fsf@gitster.mtv.corp.google.com","threadId":"40806","inReplyTo":"1447680294-13395-1-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH] push: add recurseSubmodules config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-30T18:31:26Z","receivedAt":"2015-11-30T18:31:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Crowe <mac@mcrowe.com> writes:\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 3bda430..dfced74 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -9,6 +9,7 @@\n>  #include \"transport.h\"\n>  #include \"parse-options.h\"\n>  #include \"submodule.h\"\n> +#include \"submodule-config.h\"\n>  #include \"send-pack.h\"\n>  \n>  static const char * const push_usage[] = {\n> @@ -20,7 +21,7 @@ static int thin = 1;\n>  static int deleterefs;\n>  static const char *receivepack;\n>  static int verbosity;\n> -static int progress = -1;\n> +static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n\nOne variable per line, please.  Especially when the two variables do\nnot have anything to do with each other, and do not have any logical\nsimilarity between them.\n\n> @@ -452,22 +453,15 @@ static int do_push(const char *repo, int flags)\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> +\tint *recurse_submodules = opt->value;\n>  \n> -\tif (*flags & (TRANSPORT_RECURSE_SUBMODULES_CHECK |\n> -\t\t      TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND))\n> +\tif (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n>  \t\tdie(\"%s can only be used once.\", opt->long_name);\n\nThe usual convention thoughout Git user experience is \"the last one\nwins\" (both in the configuration and in the command line options).\nIs there a good reason to deviate from that here?\n"},{"id":"273831","messageId":"20151130190021.GA29232@mcrowe.com","threadId":"40806","inReplyTo":"xmqqa8pv8hkx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] push: add recurseSubmodules config option","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-11-30T19:00:21Z","receivedAt":"2015-11-30T19:00:21Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"On Monday 30 November 2015 at 10:31:26 -0800, Junio C Hamano wrote:\n> Mike Crowe <mac@mcrowe.com> writes:\n> \n> > diff --git a/builtin/push.c b/builtin/push.c\n> > index 3bda430..dfced74 100644\n> > --- a/builtin/push.c\n> > +++ b/builtin/push.c\n> > @@ -9,6 +9,7 @@\n> >  #include \"transport.h\"\n> >  #include \"parse-options.h\"\n> >  #include \"submodule.h\"\n> > +#include \"submodule-config.h\"\n> >  #include \"send-pack.h\"\n> >  \n> >  static const char * const push_usage[] = {\n> > @@ -20,7 +21,7 @@ static int thin = 1;\n> >  static int deleterefs;\n> >  static const char *receivepack;\n> >  static int verbosity;\n> > -static int progress = -1;\n> > +static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n> \n> One variable per line, please.  Especially when the two variables do\n> not have anything to do with each other, and do not have any logical\n> similarity between them.\n\nI wouldn't normally have done that either, but I was mirroring the\nequivalent code in fetch.c. I will change it.\n\n> > @@ -452,22 +453,15 @@ static int do_push(const char *repo, int flags)\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> > +\tint *recurse_submodules = opt->value;\n> >  \n> > -\tif (*flags & (TRANSPORT_RECURSE_SUBMODULES_CHECK |\n> > -\t\t      TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND))\n> > +\tif (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n> >  \t\tdie(\"%s can only be used once.\", opt->long_name);\n> \n> The usual convention thoughout Git user experience is \"the last one\n> wins\" (both in the configuration and in the command line options).\n> Is there a good reason to deviate from that here?\n\nI was aiming to retain the existing behaviour, which was to complain if\nconflicting options were supplied on the command line. Making the last one\nwin would have been rather simpler. I can change this too, unless someone\nknows why complaining about conflicting options would be useful.\n\nNote that I previously sent an updated patch as\n<1447758356-7727-1-git-send-email-mac@mcrowe.com> but I believe that your\ncriticisms still apply.\n\nThanks for the review.\n\nMike.\n"}]}