{"thread":{"id":"40813","subject":"[PATCHv2] push: add recurseSubmodules config option","startedAt":"2015-11-17T11:05:56Z","lastAt":"2015-11-17T11:05:56Z","messageCount":1,"participants":["Mike Crowe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"273425","messageId":"1447758356-7727-1-git-send-email-mac@mcrowe.com","threadId":"40813","inReplyTo":null,"subject":"[PATCHv2] push: add recurseSubmodules config option","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-11-17T11:05:56Z","receivedAt":"2015-11-17T11:05:56Z","isPatch":false,"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---\n Documentation/config.txt       |  14 ++++\n Documentation/git-push.txt     |  24 ++++---\n builtin/push.c                 |  39 +++++++----\n submodule-config.c             |  29 ++++++++\n submodule-config.h             |   1 +\n submodule.h                    |   1 +\n t/t5531-deep-submodule-push.sh | 152 ++++++++++++++++++++++++++++++++++++++++-\n 7 files changed, 234 insertions(+), 26 deletions(-)\n\nChanges from v1:\n\n * Incorporate feedback from Eric Sunshine:\n\n ** push.recurseSubmodules config option now supports 'no' value.\n\n ** --no-recurse-submodules is now a synonym for\n    --recurse-submodules=no.\n\n ** use \"git -c\" rather than \"git config\" in tests to avoid leaving\n    config options set if a test fails.\n\n * Fix several && chain failures in tests noticed by Stefan Beller.\n\n * Minor tweaks to documentation\n\n * Fix minor naming issues in tests\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 391a0c3..5a9f2ee 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2226,6 +2226,20 @@ 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. If the value\n+\tis 'no' then default behavior of ignoring submodules when pushing\n+\tis retained. You may override this configuration at time of push by\n+\tspecifying '--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..4c775bc 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -257,16 +257,20 @@ 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-\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-\tthe revisions to be pushed are available on at least one remote\n-\tof the submodule. If any commits are missing the push will be\n-\taborted and exit with non-zero status. If 'on-demand' is used\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+--no-recurse-submodules::\n+--recurse-submodules=check|on-demand|no::\n+\tMay be used to make sure all submodule commits used by the\n+\trevisions to be pushed are available on a remote-tracking branch.\n+\tIf 'check' is used Git will verify that all submodule commits that\n+\tchanged in the revisions to be pushed are available on at least one\n+\tremote of the submodule. If any commits are missing the push will\n+\tbe aborted and exit with non-zero status. If 'on-demand' is used\n+\tall submodules 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. A value of\n+\t'no' or using '--no-recurse-submodules' can be used to override the\n+\tpush.recurseSubmodules configuration variable when no submodule\n+\trecursion 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..f9b59b4 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,17 @@ 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 (unset)\n+\t\t*recurse_submodules = RECURSE_SUBMODULES_OFF;\n+\telse if (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 +518,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 +532,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 +550,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 +581,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..fe8ceab 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -228,6 +228,35 @@ 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+\tswitch (git_config_maybe_bool(opt, arg)) {\n+\tcase 1:\n+\t\t/* There's no simple \"on\" value when pushing */\n+\t\tif (die_on_error)\n+\t\t\tdie(\"bad %s argument: %s\", opt, arg);\n+\t\telse\n+\t\t\treturn RECURSE_SUBMODULES_ERROR;\n+\tcase 0:\n+\t\treturn RECURSE_SUBMODULES_OFF;\n+\tdefault:\n+\t\tif (!strcmp(arg, \"on-demand\"))\n+\t\t\treturn RECURSE_SUBMODULES_ON_DEMAND;\n+\t\telse if (!strcmp(arg, \"check\"))\n+\t\t\treturn RECURSE_SUBMODULES_CHECK;\n+\t\telse if (die_on_error)\n+\t\t\tdie(\"bad %s argument: %s\", opt, arg);\n+\t\telse\n+\t\t\treturn RECURSE_SUBMODULES_ERROR;\n+\t}\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..9fda7b0 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -64,7 +64,12 @@ 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\ttest_must_fail git -c push.recurseSubmodules=check push ../pub.git master\n \t)\n '\n \n@@ -79,6 +84,151 @@ 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 from config for gar/bage\" &&\n+\t\tgit -c push.recurseSubmodules=on-demand push ../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 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\ttest_must_fail git -c push.recurseSubmodules=on-demand push --recurse-submodules=check ../pub.git master &&\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 -c push.recurseSubmodules=check 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\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 -c push.recurseSubmodules=check push --recurse-submodules=no ../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# But that the submodule commit did not\n+\t\t( cd gar/bage && git diff --quiet origin/master master^ ) &&\n+\t\t# Now push it to avoid confusing future tests\n+\t\tgit push --recurse-submodules=on-demand ../pub.git master\n+\t)\n+'\n+\n+test_expect_success 'push succeeds if submodule commit disabling recursion from cmdline (alternative form) overriding config' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-disable-on-command-line-alt-overriding-config &&\n+\t\tgit add recurse-disable-on-command-line-alt-overriding-config &&\n+\t\tgit commit -m \"Recurse disable on command-line alternative 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 alternative overriding config for gar/bage\" &&\n+\t\tgit -c push.recurseSubmodules=check push --no-recurse-submodules ../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# But that the submodule commit did not\n+\t\t( cd gar/bage && git diff --quiet origin/master master^ ) &&\n+\t\t# Now push it to avoid confusing future tests\n+\t\tgit push --recurse-submodules=on-demand ../pub.git master\n+\t)\n+'\n+\n+test_expect_success 'push fails if recurse submodules option passed as yes' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-push-fails-if-recurse-submodules-passed-as-yes &&\n+\t\tgit add recurse-push-fails-if-recurse-submodules-passed-as-yes &&\n+\t\tgit commit -m \"Recurse push fails if recurse submodules option passed as yes\"\n+\t) &&\n+\t(\n+\t\tcd work &&\n+\t\tgit add gar/bage &&\n+\t\tgit commit -m \"Recurse push fails if recurse submodules option passed as yes for gar/bage\" &&\n+\t\ttest_must_fail git push --recurse-submodules=yes ../pub.git master &&\n+\t\ttest_must_fail git -c push.recurseSubmodules=yes push ../pub.git master &&\n+\t\tgit push --recurse-submodules=on-demand ../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-- \n2.1.4\n"}]}