Re: [PATCH 2/2] push: Use "last one wins" convention for --recurse-submodules

[Junio C Hamano]

Stefan Beller  writes:

>> +   git push --recurse-submodules=on-demand 
>> --no-recurse-submodules ../pub.git master &&
>> +   # Check that the submodule commit did not get there
>
> Do we want to check here that the supermodule commit did get there,
> instead of only checking the submodule?

Hmm, your point is that when the push succeeds, (1) the command
should return with 0 status, (2) the branch in the superproject
should update to the right commit, and (3) none of the submodule
should be affected, and the current test does not check the second
one?

I think that makes sense, in somewhat a paranoid way ;-).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] push: Use "last one wins" convention for --recurse-submodules

[Stefan Beller]

On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe  wrote:
> 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 
> ---
>  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, , N_("refname>:   N_("require old value of ref to be at this value"),
>   PARSE_OPT_OPTARG, parseopt_push_cas_option },
> -   { OPTION_CALLBACK, 0, "recurse-submodules", 
> _submodules_from_cmdline, N_("check|on-demand|no"),
> +   { OPTION_CALLBACK, 0, "recurse-submodules", 
> _submodules, N_("check|on-demand|no"),
> N_("control recursive pushing of submodules"),
> PARSE_OPT_OPTARG, option_parse_recurse_submodules },
> OPT_BOOL( 0 , "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 

Re: [PATCH 2/2] push: Use "last one wins" convention for --recurse-submodules

[Stefan Beller]

On Thu, Dec 10, 2015 at 3:38 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>> +   git push --recurse-submodules=on-demand 
>>> --no-recurse-submodules ../pub.git master &&
>>> +   # Check that the submodule commit did not get there
>>
>> Do we want to check here that the supermodule commit did get there,
>> instead of only checking the submodule?
>
> Hmm, your point is that when the push succeeds, (1) the command
> should return with 0 status, (2) the branch in the superproject
> should update to the right commit, and (3) none of the submodule
> should be affected, and the current test does not check the second
> one?
>
> I think that makes sense, in somewhat a paranoid way ;-).

I was just comparing to the case before,
where we had

> +   # Check that the supermodule commit did get there
> +   git fetch ../pub.git &&
> +   git diff --quiet FETCH_HEAD master &&

which I just skimmed over and mistakenly thought it would be the same check as
before (checking the superproject did *not* get there).

So looking at it, the superprojects history would need no update,
the commit stays the same, so no need check for it to stay the same.

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

Re: [PATCH 2/2] push: Use "last one wins" convention for --recurse-submodules

[Junio C Hamano]

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

[PATCH 2/2] push: Use "last one wins" convention for --recurse-submodules

[Mike Crowe]

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 
---
 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, , N_("refname>: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
+   (cd gar/bage && git diff --quiet origin/master master^) &&
+
+   # But the options in the other order should