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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe <mac@xxxxxxxxxx> 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 <mac@xxxxxxxxxx>
> ---
>  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

Do we want to check here that the supermodule commit did get there,
instead of only checking the submodule?
I just wonder why we stop checking the superproject starting here, so
either it makes sense to drop
that check before or continue to check the superproject check here, no?

> +               (cd gar/bage && git diff --quiet origin/master master^) &&
> +
> +               # But the options in the other order should push the submodule
> +               git push --recurse-submodules=check --recurse-submodules=on-demand ../pub.git master &&
> +               # Check that the submodule commit did get there
> +               git fetch ../pub.git &&
> +               (cd gar/bage && git diff --quiet origin/master master)
> +       )
> +'
> +
>  test_expect_success 'push succeeds if submodule commit not on remote using on-demand from cmdline overriding config' '
>         (
>                 cd work/gar/bage &&
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]