{"thread":{"id":"40914","subject":"[PATCH v3] push: add recurseSubmodules config option","startedAt":"2015-12-01T11:49:43Z","lastAt":"2015-12-17T16:41:52Z","messageCount":16,"participants":["Mike Crowe","Jeff King","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"273858","messageId":"1448970583-14513-1-git-send-email-mac@mcrowe.com","threadId":"40914","inReplyTo":null,"subject":"[PATCH v3] push: add recurseSubmodules config option","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-12-01T11:49:43Z","receivedAt":"2015-12-01T11:49:43Z","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,\ninvent push.recurseSubmodules to provide a default for this parameter.\nThis also requires the addition of --recurse-submodules=no to allow\nthe configuration to be overridden on the command line when required.\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 \n---\nChanges in v3:\n\n * Incorporate feedback from Junio C Hamano:\n\n ** Declare recurse_submodules variable on a separate line.\n\n ** Accept multiple --recurse-submodules options on the command line\n and the last one wins.\n\n * Add extra tests for multiple --recurse-submodules options on\n command line and improve existing tests slightly.\n\nChanges in v2:\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\n Documentation/config.txt | 14 ++++\n Documentation/git-push.txt | 24 +++---\n builtin/push.c | 35 ++++----\n submodule-config.c | 29 +++++++\n submodule-config.h | 1 +\n submodule.h | 1 +\n t/t5531-deep-submodule-push.sh | 182 ++++++++++++++++++++++++++++++++++++++++-\n 7 files changed, 259 insertions(+), 27 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex b4b0194..8c02e43 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..cc29277 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@@ -21,6 +22,7 @@ static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n static int progress = -1;\n+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n \n static struct push_cas_option cas;\n \n@@ -452,22 +454,14 @@ 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-\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 +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@@ -549,7 +547,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>: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 recurse-submodules cmdline overrides 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\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\t\t# Now try the reverse which should succeed\n+\t\tgit -c push.recurseSubmodules=check push --recurse-submodules=on-demand ../pub.git master &&\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master &&\n+\t\t(cd gar/bage && git diff --quiet origin/master master)\n+\t)\n+'\n+\n+test_expect_success 'push recurse-submodules on cmdline overrides earlier cmdline' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-check-on-command-line-overriding-earlier-command-line &&\n+\t\tgit add recurse-check-on-command-line-overriding-earlier-command-line &&\n+\t\tgit commit -m \"Recurse on command-line overridiing earlier command-line 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 earlier command-line for gar/bage\" &&\n+\t\ttest_must_fail git push --recurse-submodules=on-demand --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 FETCH_HEAD master^ &&\n+\t\tgit diff --quiet FETCH_HEAD master^ &&\n+\t\t# Check that the submodule commit did not get there\n+\t\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\t\t# But the options in the other order should push the submodule\n+\t\tgit push --recurse-submodules=check --recurse-submodules=on-demand ../pub.git master &&\n+\t\t# Check that the submodule commit did get there\n+\t\tgit fetch ../pub.git &&\n+\t\t(cd gar/bage && git 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"},{"id":"273888","messageId":"20151202004031.GA28197@sigill.intra.peff.net","threadId":"40914","inReplyTo":"1448970583-14513-1-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH v3] push: add recurseSubmodules config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-02T00:40:32Z","receivedAt":"2015-12-02T00:40:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 01, 2015 at 11:49:43AM +0000, Mike Crowe wrote:\n\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,\n> invent push.recurseSubmodules to provide a default for this parameter.\n> This also requires the addition of --recurse-submodules=no to allow\n> the configuration to be overridden on the command line when 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 \n> ---\n> Changes in v3:\n\nHrm, I merged v2 of this to 'next' last week.\n\nThe options at this point are either to revert that and re-start the\ntopic, or just make the further changes a patch on top. Thoughts?\n\n-Peff\n"},{"id":"273904","messageId":"20151202095451.GA22568@mcrowe.com","threadId":"40914","inReplyTo":"20151202004031.GA28197@sigill.intra.peff.net","subject":"Re: [PATCH v3] push: add recurseSubmodules config option","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-12-02T09:54:51Z","receivedAt":"2015-12-02T09:54:51Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"On Tuesday 01 December 2015 at 19:40:32 -0500, Jeff King wrote:\n> On Tue, Dec 01, 2015 at 11:49:43AM +0000, Mike Crowe wrote:\n> \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,\n> > invent push.recurseSubmodules to provide a default for this parameter.\n> > This also requires the addition of --recurse-submodules=no to allow\n> > the configuration to be overridden on the command line when 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 \n> > ---\n> > Changes in v3:\n> \n> Hrm, I merged v2 of this to 'next' last week.\n\nThanks! Sorry I didn't spot that.\n\n> The options at this point are either to revert that and re-start the\n> topic, or just make the further changes a patch on top. Thoughts?\n\nI don't mind which you choose to do. I'll reply to this message with the\nincremental patch in case you decide you need it. Please let me know if\nyou'd like me to split it further since the patch modifies a test that is\notherwise unrelated to the rest of the change.\n\nMike.\n"},{"id":"273905","messageId":"1449050172-1119-1-git-send-email-mac@mcrowe.com","threadId":"40914","inReplyTo":"20151202095451.GA22568@mcrowe.com","subject":"[PATCH] push: Improve --recurse-submodules support","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-12-02T09:56:12Z","receivedAt":"2015-12-02T09:56:12Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"b33a15b08131514b593015cb3e719faf9db20208 added support for the\npush.recurseSubmodules config option. After it was merged Junio C Hamano\nsuggested some improvements:\n\n - Declare recurse_submodules on a separate line.\n\n - Accept multiple --recurse-submodules options on command line with the\n last one winning. (This simplified the implementation too.)\n\nAlso slightly improve one of the tests added in\nb33a15b08131514b593015cb3e719faf9db20208.\n\nSigned-off-by: Mike Crowe \n---\n\n builtin/push.c | 12 +++---------\n t/t5531-deep-submodule-push.sh | 36 +++++++++++++++++++++++++++++++++---\n 2 files changed, 36 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f9b59b4..cc29277 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -21,7 +21,8 @@ static int thin = 1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n-static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n+static int progress = -1;\n+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n \n static struct push_cas_option cas;\n \n@@ -455,9 +456,6 @@ static int option_parse_recurse_submodules(const struct option *opt,\n {\n \tint *recurse_submodules = opt->value;\n \n-\tif (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n-\t\tdie(\"%s can only be used once.\", opt->long_name);\n-\n \tif (unset)\n \t\t*recurse_submodules = RECURSE_SUBMODULES_OFF;\n \telse if (arg)\n@@ -532,7 +530,6 @@ 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@@ -550,7 +547,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>:recurse-check-on-command-line-overriding-config &&\n@@ -142,8 +142,38 @@ test_expect_success 'push fails if submodule commit not on remote using check fr\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\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\t\t# Now try the reverse which should succeed\n+\t\tgit -c push.recurseSubmodules=check push --recurse-submodules=on-demand ../pub.git master &&\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master &&\n+\t\t(cd gar/bage && git diff --quiet origin/master master)\n+\t)\n+'\n+\n+test_expect_success 'push recurse-submodules on cmdline overrides earlier cmdline' '\n+\t(\n+\t\tcd work/gar/bage &&\n+\t\t>recurse-check-on-command-line-overriding-earlier-command-line &&\n+\t\tgit add recurse-check-on-command-line-overriding-earlier-command-line &&\n+\t\tgit commit -m \"Recurse on command-line overridiing earlier command-line 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 earlier command-line for gar/bage\" &&\n+\t\ttest_must_fail git push --recurse-submodules=on-demand --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 FETCH_HEAD master^ &&\n+\t\tgit diff --quiet FETCH_HEAD master^ &&\n+\t\t# Check that the submodule commit did not get there\n+\t\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\t\t# But the options in the other order should push the submodule\n+\t\tgit push --recurse-submodules=check --recurse-submodules=on-demand ../pub.git master &&\n+\t\t# Check that the submodule commit did get there\n+\t\tgit fetch ../pub.git &&\n+\t\t(cd gar/bage && git diff --quiet origin/master master)\n \t)\n '\n \n-- \n2.1.4\n"},{"id":"273941","messageId":"xmqqsi3k4ety.fsf@gitster.mtv.corp.google.com","threadId":"40914","inReplyTo":"1449050172-1119-1-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH] push: Improve --recurse-submodules support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-02T23:21:13Z","receivedAt":"2015-12-02T23:21:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Crowe writes:\n\n> b33a15b08131514b593015cb3e719faf9db20208 added support for the\n> push.recurseSubmodules config option. After it was merged Junio C Hamano\n> suggested some improvements:\n>\n> - Declare recurse_submodules on a separate line.\n>\n> - Accept multiple --recurse-submodules options on command line with the\n> last one winning. (This simplified the implementation too.)\n>\n> Also slightly improve one of the tests added in\n> b33a15b08131514b593015cb3e719faf9db20208.\n\nThe above is overly verbose about how the commit materialized,\ncompared to the description of the merit of this update.\n\n push: fix --recurse-submodules breakage\n\n When b33a15b0 (push: add recurseSubmodules config option,\n 2015-11-17) added push.recurseSubmodules configuration option,\n it also changed the command line parsing to allow\n --no-recurse-submodules to override configured default.\n However, the parsing of configuration variables and command line\n options did not follow the usual \"last one wins\" convention.\n Fix this.\n\n Also fix the declaration of the new file-scope global variable\n to put it on a separate line on its own.\n\nor something?\n\nAlso describe what \"slightly improve\" really means. What did the\nold one not test that should have been tested?\n\nThanks.\n\n> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\n> index 9fda7b0..9a637f5 100755\n> --- a/t/t5531-deep-submodule-push.sh\n> +++ b/t/t5531-deep-submodule-push.sh\n> @@ -126,7 +126,7 @@ test_expect_success 'push succeeds if submodule commit not on remote but using o\n> \t)\n> '\n> \n> -test_expect_success 'push fails if submodule commit not on remote using check from cmdline overriding config' '\n> +test_expect_success 'push recurse-submodules cmdline overrides config' '\n> \t(\n> \t\tcd work/gar/bage &&\n> \t\t>recurse-check-on-command-line-overriding-config &&\n> @@ -142,8 +142,38 @@ test_expect_success 'push fails if submodule commit not on remote using check fr\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\t(cd gar/bage && git diff --quiet origin/master master^) &&\n\nThese days, you can do:\n\n\t\tgit -C gar/bage --quiet origin/master master^\n\ninstead.\n"},{"id":"273972","messageId":"20151203131006.GA5119@mcrowe.com","threadId":"40914","inReplyTo":"xmqqsi3k4ety.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] push: Improve --recurse-submodules support","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-12-03T13:10:06Z","receivedAt":"2015-12-03T13:10:06Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"On Wednesday 02 December 2015 at 15:21:13 -0800, Junio C Hamano wrote:\n> Mike Crowe writes:\n> \n> > b33a15b08131514b593015cb3e719faf9db20208 added support for the\n> > push.recurseSubmodules config option. After it was merged Junio C Hamano\n> > suggested some improvements:\n> >\n> > - Declare recurse_submodules on a separate line.\n> >\n> > - Accept multiple --recurse-submodules options on command line with the\n> > last one winning. (This simplified the implementation too.)\n> >\n> > Also slightly improve one of the tests added in\n> > b33a15b08131514b593015cb3e719faf9db20208.\n> \n> The above is overly verbose about how the commit materialized,\n> compared to the description of the merit of this update.\n> \n> push: fix --recurse-submodules breakage\n>\n> When b33a15b0 (push: add recurseSubmodules config option,\n> 2015-11-17) added push.recurseSubmodules configuration option,\n> it also changed the command line parsing to allow\n> --no-recurse-submodules to override configured default.\n>\n> However, the parsing of configuration variables and command line\n> options did not follow the usual \"last one wins\" convention.\n> Fix this.\n\nThat's not quite true.\n\nThe check for conflicting options was added back in 2012 by eb21c732 when\n--recurse-submodules=on-demand support was originally implemented. b33a15b0\ntreated that as correct and maintained the behaviour.\n\n> \n> Also fix the declaration of the new file-scope global variable\n> to put it on a separate line on its own.\n> \n> or something?\n\nThanks for the better wording. Hopefully I've included enough of the right\nbits in the updated patches that follow.\n\n> Also describe what \"slightly improve\" really means. What did the\n> old one not test that should have been tested?\n\nIn attempting to describe this change I've found that both of the tests had\nfailings so I have improved them too.\n\nThanks.\n\nMike.\n"},{"id":"273973","messageId":"1449148235-29569-1-git-send-email-mac@mcrowe.com","threadId":"40914","inReplyTo":"20151203131006.GA5119@mcrowe.com","subject":"[PATCH 1/2] push: Fully test --recurse-submodules on command line overrides config","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-12-03T13:10:34Z","receivedAt":"2015-12-03T13:10:34Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"t5531 only checked that the push.recurseSubmodules config option was\noverridden by passing --recurse-submodules=check on the command line.\nAdd new tests for overriding with --recurse-submodules=no,\n--no-recurse-submodules and --recurse-submodules=push too.\n\nAlso correct minor typo in test commit message.\n\nSigned-off-by: Mike Crowe \n---\n t/t5531-deep-submodule-push.sh | 32 ++++++++++++++++++++++++++++----\n 1 file changed, 28 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 9fda7b0..721be32 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -126,24 +126,48 @@ test_expect_success 'push succeeds if submodule commit not on remote but using o\n \t)\n '\n \n-test_expect_success 'push fails if submodule commit not on remote using check from cmdline overriding config' '\n+test_expect_success 'push recurse-submodules on command line overrides 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\tgit commit -m \"Recurse 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 command-line overriding config for gar/bage\" &&\n+\n+\t\t# Ensure that we can override on-demand in the config\n+\t\t# to just check submodules\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\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\n+\t\t# Ensure that we can override check in the config to\n+\t\t# disable submodule recursion entirely\n+\t\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\t\tgit -c push.recurseSubmodules=on-demand push --recurse-submodules=no ../pub.git master &&\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master &&\n+\t\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\n+\t\t# Ensure that we can override check in the config to\n+\t\t# disable submodule recursion entirely (alternative form)\n+\t\tgit -c push.recurseSubmodules=on-demand push --no-recurse-submodules ../pub.git master &&\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master &&\n+\t\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\n+\t\t# Ensure that we can override check in the config to\n+\t\t# push the submodule too\n+\t\tgit -c push.recurseSubmodules=check push --recurse-submodules=on-demand ../pub.git master &&\n+\t\tgit fetch ../pub.git &&\n+\t\tgit diff --quiet FETCH_HEAD master &&\n+\t\t(cd gar/bage && git diff --quiet origin/master master)\n \t)\n '\n \n-- \n2.1.4\n"},{"id":"273974","messageId":"1449148235-29569-2-git-send-email-mac@mcrowe.com","threadId":"40914","inReplyTo":"1449148235-29569-1-git-send-email-mac@mcrowe.com","subject":"[PATCH 2/2] push: Use \"last one wins\" convention for --recurse-submodules","fromName":"Mike Crowe","fromEmail":"mac@mcrowe.com","sentAt":"2015-12-03T13:10:35Z","receivedAt":"2015-12-03T13:10:35Z","isPatch":true,"sender":{"key":"mac@mcrowe.com","avatar":"https://avatars.githubusercontent.com/u/93615?v=4"},"body":"Use the \"last one wins\" convention for --recurse-submodules rather than\ntreating conflicting options as an error.\n\nAlso, fix the declaration of the file-scope recurse_submodules global\nvariable to put it on a separate line.\n\nSigned-off-by: Mike Crowe \n---\n builtin/push.c | 12 +++---------\n t/t5531-deep-submodule-push.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f9b59b4..cc29277 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -21,7 +21,8 @@ static int thin = 1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n-static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n+static int progress = -1;\n+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n \n static struct push_cas_option cas;\n \n@@ -455,9 +456,6 @@ static int option_parse_recurse_submodules(const struct option *opt,\n {\n \tint *recurse_submodules = opt->value;\n \n-\tif (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n-\t\tdie(\"%s can only be used once.\", opt->long_name);\n-\n \tif (unset)\n \t\t*recurse_submodules = RECURSE_SUBMODULES_OFF;\n \telse if (arg)\n@@ -532,7 +530,6 @@ 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@@ -550,7 +547,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>:recurse-check-on-command-line-overriding-earlier-command-line &&\n+\t\tgit add recurse-check-on-command-line-overriding-earlier-command-line &&\n+\t\tgit commit -m \"Recurse on command-line overridiing earlier command-line 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 earlier command-line for gar/bage\" &&\n+\n+\t\t# should result in \"check\"\n+\t\ttest_must_fail git push --recurse-submodules=on-demand --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\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\n+\t\t# should result in \"no\"\n+\t\tgit push --recurse-submodules=on-demand --recurse-submodules=no ../pub.git master &&\n+\t\t# Check that the supermodule commit did 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\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\n+\t\t# should result in \"no\"\n+\t\tgit push --recurse-submodules=on-demand --no-recurse-submodules ../pub.git master &&\n+\t\t# Check that the submodule commit did not get there\n+\t\t(cd gar/bage && git diff --quiet origin/master master^) &&\n+\n+\t\t# But the options in the other order should push the submodule\n+\t\tgit push --recurse-submodules=check --recurse-submodules=on-demand ../pub.git master &&\n+\t\t# Check that the submodule commit did get there\n+\t\tgit fetch ../pub.git &&\n+\t\t(cd gar/bage && git 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-- \n2.1.4\n"},{"id":"274022","messageId":"xmqq610erkm2.fsf@gitster.mtv.corp.google.com","threadId":"40914","inReplyTo":"1449148235-29569-2-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH 2/2] push: Use \"last one wins\" convention for --recurse-submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-04T21:04:37Z","receivedAt":"2015-12-04T21:04:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue.\n"},{"id":"274255","messageId":"CAGZ79kbxvrMHnJx9iACus44+rmFf6ZNFPArrpVhNr6ZTDj+XOg@mail.gmail.com","threadId":"40914","inReplyTo":"1449148235-29569-2-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH 2/2] push: Use \"last one wins\" convention for --recurse-submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-10T23:31:42Z","receivedAt":"2015-12-10T23:31:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe wrote:\n> Use the \"last one wins\" convention for --recurse-submodules rather than\n> treating conflicting options as an error.\n>\n> Also, fix the declaration of the file-scope recurse_submodules global\n> variable to put it on a separate line.\n>\n> Signed-off-by: Mike Crowe \n> ---\n> builtin/push.c | 12 +++---------\n> t/t5531-deep-submodule-push.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 44 insertions(+), 9 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index f9b59b4..cc29277 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -21,7 +21,8 @@ static int thin = 1;\n> static int deleterefs;\n> static const char *receivepack;\n> static int verbosity;\n> -static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n> +static int progress = -1;\n> +static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n>\n> static struct push_cas_option cas;\n>\n> @@ -455,9 +456,6 @@ static int option_parse_recurse_submodules(const struct option *opt,\n> {\n> int *recurse_submodules = opt->value;\n>\n> - if (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n> - die(\"%s can only be used once.\", opt->long_name);\n> -\n> if (unset)\n> *recurse_submodules = RECURSE_SUBMODULES_OFF;\n> else if (arg)\n> @@ -532,7 +530,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> int flags = 0;\n> int tags = 0;\n> int push_cert = -1;\n> - int recurse_submodules_from_cmdline = RECURSE_SUBMODULES_DEFAULT;\n> int rc;\n> const char *repo = NULL; /* default repository */\n> struct option options[] = {\n> @@ -550,7 +547,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> 0, CAS_OPT_NAME, &cas, N_(\"refname>: N_(\"require old value of ref to be at this value\"),\n> PARSE_OPT_OPTARG, parseopt_push_cas_option },\n> - { OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules_from_cmdline, N_(\"check|on-demand|no\"),\n> + { OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, N_(\"check|on-demand|no\"),\n> N_(\"control recursive pushing of submodules\"),\n> PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n> OPT_BOOL( 0 , \"thin\", &thin, N_(\"use thin pack\")),\n> @@ -581,9 +578,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> if (deleterefs && argc < 2)\n> die(_(\"--delete doesn't make sense without any refs\"));\n>\n> - if (recurse_submodules_from_cmdline != RECURSE_SUBMODULES_DEFAULT)\n> - recurse_submodules = recurse_submodules_from_cmdline;\n> -\n> if (recurse_submodules == RECURSE_SUBMODULES_CHECK)\n> flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;\n> else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\n> index 721be32..198ce84 100755\n> --- a/t/t5531-deep-submodule-push.sh\n> +++ b/t/t5531-deep-submodule-push.sh\n> @@ -171,6 +171,47 @@ test_expect_success 'push recurse-submodules on command line overrides config' '\n> )\n> '\n>\n> +test_expect_success 'push recurse-submodules last one wins on command line' '\n> + (\n> + cd work/gar/bage &&\n> + >recurse-check-on-command-line-overriding-earlier-command-line &&\n> + git add recurse-check-on-command-line-overriding-earlier-command-line &&\n> + git commit -m \"Recurse on command-line overridiing earlier command-line junk\"\n> + ) &&\n> + (\n> + cd work &&\n> + git add gar/bage &&\n> + git commit -m \"Recurse on command-line overriding earlier command-line for gar/bage\" &&\n> +\n> + # should result in \"check\"\n> + test_must_fail git push --recurse-submodules=on-demand --recurse-submodules=check ../pub.git master &&\n> + # Check that the supermodule commit did not get there\n> + git fetch ../pub.git &&\n> + git diff --quiet FETCH_HEAD master^ &&\n> + # Check that the submodule commit did not get there\n> + (cd gar/bage && git diff --quiet origin/master master^) &&\n> +\n> + # should result in \"no\"\n> + git push --recurse-submodules=on-demand --recurse-submodules=no ../pub.git master &&\n> + # Check that the supermodule commit did get there\n> + git fetch ../pub.git &&\n> + git diff --quiet FETCH_HEAD master &&\n> + # Check that the submodule commit did not get there\n> + (cd gar/bage && git diff --quiet origin/master master^) &&\n> +\n> + # should result in \"no\"\n> + git push --recurse-submodules=on-demand --no-recurse-submodules ../pub.git master &&\n> + # Check that the submodule commit did not get there\n\nDo we want to check here that the supermodule commit did get there,\ninstead of only checking the submodule?\nI just wonder why we stop checking the superproject starting here, so\neither it makes sense to drop\nthat check before or continue to check the superproject check here, no?\n\n> + (cd gar/bage && git diff --quiet origin/master master^) &&\n> +\n> + # But the options in the other order should push the submodule\n> + git push --recurse-submodules=check --recurse-submodules=on-demand ../pub.git master &&\n> + # Check that the submodule commit did get there\n> + git fetch ../pub.git &&\n> + (cd gar/bage && git diff --quiet origin/master master)\n> + )\n> +'\n> +\n> test_expect_success 'push succeeds if submodule commit not on remote using on-demand from cmdline overriding config' '\n> (\n> cd work/gar/bage &&\n> --\n> 2.1.4\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"274257","messageId":"xmqq8u51ho27.fsf@gitster.mtv.corp.google.com","threadId":"40914","inReplyTo":"CAGZ79kbxvrMHnJx9iACus44+rmFf6ZNFPArrpVhNr6ZTDj+XOg@mail.gmail.com","subject":"Re: [PATCH 2/2] push: Use \"last one wins\" convention for --recurse-submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-10T23:38:24Z","receivedAt":"2015-12-10T23:38:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> + git push --recurse-submodules=on-demand --no-recurse-submodules ../pub.git master &&\n>> + # Check that the submodule commit did not get there\n>\n> Do we want to check here that the supermodule commit did get there,\n> instead of only checking the submodule?\n\nHmm, your point is that when the push succeeds, (1) the command\nshould return with 0 status, (2) the branch in the superproject\nshould update to the right commit, and (3) none of the submodule\nshould be affected, and the current test does not check the second\none?\n\nI think that makes sense, in somewhat a paranoid way ;-).\n"},{"id":"274259","messageId":"CAGZ79kaehf+o9qwznTZyG743OrO2EerEO0PAWois4L2G6=933w@mail.gmail.com","threadId":"40914","inReplyTo":"xmqq8u51ho27.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] push: Use \"last one wins\" convention for --recurse-submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-10T23:44:14Z","receivedAt":"2015-12-10T23:44:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 10, 2015 at 3:38 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>>> + git push --recurse-submodules=on-demand --no-recurse-submodules ../pub.git master &&\n>>> + # Check that the submodule commit did not get there\n>>\n>> Do we want to check here that the supermodule commit did get there,\n>> instead of only checking the submodule?\n>\n> Hmm, your point is that when the push succeeds, (1) the command\n> should return with 0 status, (2) the branch in the superproject\n> should update to the right commit, and (3) none of the submodule\n> should be affected, and the current test does not check the second\n> one?\n>\n> I think that makes sense, in somewhat a paranoid way ;-).\n\nI was just comparing to the case before,\nwhere we had\n\n> + # Check that the supermodule commit did get there\n> + git fetch ../pub.git &&\n> + git diff --quiet FETCH_HEAD master &&\n\nwhich I just skimmed over and mistakenly thought it would be the same check as\nbefore (checking the superproject did *not* get there).\n\nSo looking at it, the superprojects history would need no update,\nthe commit stays the same, so no need check for it to stay the same.\n\nSo, sorry for the noise.\n"},{"id":"274603","messageId":"CAGZ79kb3XCkabxUq6Sh-aLa=a6kzRZtR6WG+wTk1SQY9_Mehog@mail.gmail.com","threadId":"40914","inReplyTo":"1449148235-29569-1-git-send-email-mac@mcrowe.com","subject":"Re: [PATCH 1/2] push: Fully test --recurse-submodules on command line overrides config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-16T20:48:56Z","receivedAt":"2015-12-16T20:48:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe wrote:\n> t5531 only checked that the push.recurseSubmodules config option was\n> overridden by passing --recurse-submodules=check on the command line.\n> Add new tests for overriding with --recurse-submodules=no,\n> --no-recurse-submodules and --recurse-submodules=push too.\n>\n> Also correct minor typo in test commit message.\n>\n> Signed-off-by: Mike Crowe \n\nThis looks good to me.\n\nThanks,\nStefan\n\n\n> ---\n> t/t5531-deep-submodule-push.sh | 32 ++++++++++++++++++++++++++++----\n> 1 file changed, 28 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\n> index 9fda7b0..721be32 100755\n> --- a/t/t5531-deep-submodule-push.sh\n> +++ b/t/t5531-deep-submodule-push.sh\n> @@ -126,24 +126,48 @@ test_expect_success 'push succeeds if submodule commit not on remote but using o\n> )\n> '\n>\n> -test_expect_success 'push fails if submodule commit not on remote using check from cmdline overriding config' '\n> +test_expect_success 'push recurse-submodules on command line overrides config' '\n> (\n> cd work/gar/bage &&\n> >recurse-check-on-command-line-overriding-config &&\n> git add recurse-check-on-command-line-overriding-config &&\n> - git commit -m \"Recurse on command-line overridiing config junk\"\n> + git commit -m \"Recurse on command-line overriding config junk\"\n> ) &&\n> (\n> cd work &&\n> git add gar/bage &&\n> git commit -m \"Recurse on command-line overriding config for gar/bage\" &&\n> +\n> + # Ensure that we can override on-demand in the config\n> + # to just check submodules\n> test_must_fail git -c push.recurseSubmodules=on-demand push --recurse-submodules=check ../pub.git master &&\n> # Check that the supermodule commit did not get there\n> git fetch ../pub.git &&\n> git diff --quiet FETCH_HEAD master^ &&\n> # Check that the submodule commit did not get there\n> - cd gar/bage &&\n> - git diff --quiet origin/master master^\n> + (cd gar/bage && git diff --quiet origin/master master^) &&\n> +\n> + # Ensure that we can override check in the config to\n> + # disable submodule recursion entirely\n> + (cd gar/bage && git diff --quiet origin/master master^) &&\n> + git -c push.recurseSubmodules=on-demand push --recurse-submodules=no ../pub.git master &&\n> + git fetch ../pub.git &&\n> + git diff --quiet FETCH_HEAD master &&\n> + (cd gar/bage && git diff --quiet origin/master master^) &&\n> +\n> + # Ensure that we can override check in the config to\n> + # disable submodule recursion entirely (alternative form)\n> + git -c push.recurseSubmodules=on-demand push --no-recurse-submodules ../pub.git master &&\n> + git fetch ../pub.git &&\n> + git diff --quiet FETCH_HEAD master &&\n> + (cd gar/bage && git diff --quiet origin/master master^) &&\n> +\n> + # Ensure that we can override check in the config to\n> + # push the submodule too\n> + git -c push.recurseSubmodules=check push --recurse-submodules=on-demand ../pub.git master &&\n> + git fetch ../pub.git &&\n> + git diff --quiet FETCH_HEAD master &&\n> + (cd gar/bage && git diff --quiet origin/master master)\n> )\n> '\n>\n> --\n> 2.1.4\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"274632","messageId":"xmqqio3yc8yd.fsf@gitster.mtv.corp.google.com","threadId":"40914","inReplyTo":"CAGZ79kb3XCkabxUq6Sh-aLa=a6kzRZtR6WG+wTk1SQY9_Mehog@mail.gmail.com","subject":"Re: [PATCH 1/2] push: Fully test --recurse-submodules on command line overrides config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-16T22:41:46Z","receivedAt":"2015-12-16T22:41:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe wrote:\n>> t5531 only checked that the push.recurseSubmodules config option was\n>> overridden by passing --recurse-submodules=check on the command line.\n>> Add new tests for overriding with --recurse-submodules=no,\n>> --no-recurse-submodules and --recurse-submodules=push too.\n>>\n>> Also correct minor typo in test commit message.\n>>\n>> Signed-off-by: Mike Crowe \n>\n> This looks good to me.\n>\n> Thanks,\n> Stefan\n\nThanks. Does \"This\" refer to 1/2 alone or the whole series?\n"},{"id":"274633","messageId":"CAGZ79kZpCPp6CrfknQRDObKvuNnCe2+bZwCAF8XKrkkVNS+e3w@mail.gmail.com","threadId":"40914","inReplyTo":"xmqqio3yc8yd.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] push: Fully test --recurse-submodules on command line overrides config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-16T22:46:23Z","receivedAt":"2015-12-16T22:46:23Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Dec 16, 2015 at 2:41 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe wrote:\n>>> t5531 only checked that the push.recurseSubmodules config option was\n>>> overridden by passing --recurse-submodules=check on the command line.\n>>> Add new tests for overriding with --recurse-submodules=no,\n>>> --no-recurse-submodules and --recurse-submodules=push too.\n>>>\n>>> Also correct minor typo in test commit message.\n>>>\n>>> Signed-off-by: Mike Crowe \n>>\n>> This looks good to me.\n>>\n>> Thanks,\n>> Stefan\n>\n> Thanks. Does \"This\" refer to 1/2 alone or the whole series?\n\nYes. :)\n\n\"This\" is applicable to both patches. We had the discussion on 2/2 about me\nmisreading a line a few days earlier, but apart from that it looked good, too.\n"},{"id":"274672","messageId":"xmqqa8p9c9in.fsf@gitster.mtv.corp.google.com","threadId":"40914","inReplyTo":"CAGZ79kZpCPp6CrfknQRDObKvuNnCe2+bZwCAF8XKrkkVNS+e3w@mail.gmail.com","subject":"Re: [PATCH 1/2] push: Fully test --recurse-submodules on command line overrides config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-17T16:41:52Z","receivedAt":"2015-12-17T16:41:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>>> This looks good to me.\n>>>\n>> Thanks. Does \"This\" refer to 1/2 alone or the whole series?\n>\n> Yes. :)\n>\n> \"This\" is applicable to both patches. We had the discussion on 2/2 about me\n> misreading a line a few days earlier, but apart from that it looked good, too.\n\nThanks.\n"}]}