Re: [PATCH] push: Improve --recurse-submodules support

diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 9fda7b0..9a637f5 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh

@@ -126,7 +126,7 @@ test_expect_success 'push succeeds if submodule commit not on remote but using o
 	)
 '
 
-test_expect_success 'push fails if submodule commit not on remote using check from cmdline overriding config' '
+test_expect_success 'push recurse-submodules cmdline overrides config' '
 	(
 		cd work/gar/bage &&
 		>recurse-check-on-command-line-overriding-config &&

@@ -142,8 +142,38 @@ test_expect_success 'push fails if submodule commit not on remote using check fr
 		git fetch ../pub.git &&
 		git diff --quiet FETCH_HEAD master^ &&
 		# Check that the submodule commit did not get there
-		cd gar/bage &&
-		git diff --quiet origin/master master^
+		(cd gar/bage && git diff --quiet origin/master master^) &&

Use the "last one wins" convention for --recurse-submodules rather than
treating conflicting options as an error.

Also, fix the declaration of the file-scope recurse_submodules global
variable to put it on a separate line.

Signed-off-by: Mike Crowe <redacted>
---
 builtin/push.c                 | 12 +++---------
 t/t5531-deep-submodule-push.sh | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index f9b59b4..cc29277 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -21,7 +21,8 @@ static int thin = 1;
 static int deleterefs;
 static const char *receivepack;
 static int verbosity;
-static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int progress = -1;
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;

 static struct push_cas_option cas;

@@ -455,9 +456,6 @@ static int option_parse_recurse_submodules(const struct option *opt,
 {
        int *recurse_submodules = opt->value;

-       if (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
-               die("%s can only be used once.", opt->long_name);
-
        if (unset)
                *recurse_submodules = RECURSE_SUBMODULES_OFF;
        else if (arg)

@@ -532,7 +530,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)
        int flags = 0;
        int tags = 0;
        int push_cert = -1;
-       int recurse_submodules_from_cmdline = RECURSE_SUBMODULES_DEFAULT;
        int rc;
        const char *repo = NULL;        /* default repository */
        struct option options[] = {

@@ -550,7 +547,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
                  0, CAS_OPT_NAME, &cas, N_("refname>:<expect"),
                  N_("require old value of ref to be at this value"),
                  PARSE_OPT_OPTARG, parseopt_push_cas_option },
-               { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules_from_cmdline, N_("check|on-demand|no"),
+               { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules, N_("check|on-demand|no"),
                        N_("control recursive pushing of submodules"),
                        PARSE_OPT_OPTARG, option_parse_recurse_submodules },
                OPT_BOOL( 0 , "thin", &thin, N_("use thin pack")),

@@ -581,9 +578,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)
        if (deleterefs && argc < 2)
                die(_("--delete doesn't make sense without any refs"));

-       if (recurse_submodules_from_cmdline != RECURSE_SUBMODULES_DEFAULT)
-               recurse_submodules = recurse_submodules_from_cmdline;
-
        if (recurse_submodules == RECURSE_SUBMODULES_CHECK)
                flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;
        else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)

diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 721be32..198ce84 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh

@@ -171,6 +171,47 @@ test_expect_success 'push recurse-submodules on command line overrides config' '
        )
 '

+test_expect_success 'push recurse-submodules last one wins on command line' '
+       (
+               cd work/gar/bage &&
+               >recurse-check-on-command-line-overriding-earlier-command-line &&
+               git add recurse-check-on-command-line-overriding-earlier-command-line &&
+               git commit -m "Recurse on command-line overridiing earlier command-line junk"
+       ) &&
+       (
+               cd work &&
+               git add gar/bage &&
+               git commit -m "Recurse on command-line overriding earlier command-line for gar/bage" &&
+
+               # should result in "check"
+               test_must_fail git push --recurse-submodules=on-demand --recurse-submodules=check ../pub.git master &&
+               # Check that the supermodule commit did not get there
+               git fetch ../pub.git &&
+               git diff --quiet FETCH_HEAD master^ &&
+               # Check that the submodule commit did not get there
+               (cd gar/bage && git diff --quiet origin/master master^) &&
+
+               # should result in "no"
+               git push --recurse-submodules=on-demand --recurse-submodules=no ../pub.git master &&
+               # Check that the supermodule commit did get there
+               git fetch ../pub.git &&
+               git diff --quiet FETCH_HEAD master &&
+               # Check that the submodule commit did not get there
+               (cd gar/bage && git diff --quiet origin/master master^) &&
+
+               # should result in "no"
+               git push --recurse-submodules=on-demand --no-recurse-submodules ../pub.git master &&
+               # Check that the submodule commit did not get there

---
 t/t5531-deep-submodule-push.sh | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 9fda7b0..721be32 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh

@@ -126,24 +126,48 @@ test_expect_success 'push succeeds if submodule commit not on remote but using o
        )
 '

-test_expect_success 'push fails if submodule commit not on remote using check from cmdline overriding config' '
+test_expect_success 'push recurse-submodules on command line overrides config' '
        (
                cd work/gar/bage &&
                >recurse-check-on-command-line-overriding-config &&
                git add recurse-check-on-command-line-overriding-config &&
-               git commit -m "Recurse on command-line overridiing config junk"
+               git commit -m "Recurse on command-line overriding config junk"
        ) &&
        (
                cd work &&
                git add gar/bage &&
                git commit -m "Recurse on command-line overriding config for gar/bage" &&
+
+               # Ensure that we can override on-demand in the config
+               # to just check submodules
                test_must_fail git -c push.recurseSubmodules=on-demand push --recurse-submodules=check ../pub.git master &&
                # Check that the supermodule commit did not get there
                git fetch ../pub.git &&
                git diff --quiet FETCH_HEAD master^ &&
                # Check that the submodule commit did not get there
-               cd gar/bage &&
-               git diff --quiet origin/master master^
+               (cd gar/bage && git diff --quiet origin/master master^) &&
+
+               # Ensure that we can override check in the config to
+               # disable submodule recursion entirely
+               (cd gar/bage && git diff --quiet origin/master master^) &&
+               git -c push.recurseSubmodules=on-demand push --recurse-submodules=no ../pub.git master &&
+               git fetch ../pub.git &&
+               git diff --quiet FETCH_HEAD master &&
+               (cd gar/bage && git diff --quiet origin/master master^) &&
+
+               # Ensure that we can override check in the config to
+               # disable submodule recursion entirely (alternative form)
+               git -c push.recurseSubmodules=on-demand push --no-recurse-submodules ../pub.git master &&
+               git fetch ../pub.git &&
+               git diff --quiet FETCH_HEAD master &&
+               (cd gar/bage && git diff --quiet origin/master master^) &&
+
+               # Ensure that we can override check in the config to
+               # push the submodule too
+               git -c push.recurseSubmodules=check push --recurse-submodules=on-demand ../pub.git master &&
+               git fetch ../pub.git &&
+               git diff --quiet FETCH_HEAD master &&
+               (cd gar/bage && git diff --quiet origin/master master)
        )
 '

--

2.1.4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html