push: disallow --all and refspecs when remote.<name>.mirror is set
Pushes with --all, or refspecs are disallowed when --mirror is given to 'git push', or when 'remote.<name>.mirror' is set in the config of the repository, because they can have surprising effects. 800a4ab399 ("push: check for errors earlier", 2018-05-16) refactored this code to do that check earlier, so we can explicitly check for the presence of flags, instead of their sideeffects. However when 'remote.<name>.mirror' is set in the config, the TRANSPORT_PUSH_MIRROR flag would only be set after we calling 'do_push()', so the checks would miss it entirely. This leads to surprises for users [*1*]. Fix this by making sure we set the flag (if appropriate) before checking for compatibility of the various options. *1*: https://twitter.com/FiloSottile/status/1163918701462249472 Reported-by: Filippo Valsorda <filippo@ml.filippo.io> Helped-by: Saleem Rashid Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thomas Gummerer committed
Sep 2, 2019 at 19:08 UTC
8e4c8af058d7eb1b887184deb3bf79f3818b3a65
2 files changed
+46
-33
builtin/push.c
+36
-33
@@ -382,30 +382,14 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
382
}
383
384
static int do_push(const char *repo, int flags,
385
- const struct string_list *push_options)
385
+ const struct string_list *push_options,
386
+ struct remote *remote)
387
{
388
int i, errs;
388
- struct remote *remote = pushremote_get(repo);
389
const char **url;
390
int url_nr;
391
struct refspec *push_refspec = &rs;
392
393
- if (!remote) {
394
- if (repo)
395
- die(_("bad repository '%s'"), repo);
396
- die(_("No configured push destination.\n"
397
- "Either specify the URL from the command-line or configure a remote repository using\n"
398
- "\n"
399
- " git remote add <name> <url>\n"
400
- "\n"
401
- "and then push using the remote name\n"
402
- "\n"
403
- " git push <name>\n"));
404
- }
405
-
406
- if (remote->mirror)
407
- flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
408
-
393
if (push_options->nr)
394
flags |= TRANSPORT_PUSH_OPTIONS;
395
@@ -545,6 +529,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
529
struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
530
struct string_list *push_options;
531
const struct string_list_item *item;
532
+ struct remote *remote;
533
534
struct option options[] = {
535
OPT__VERBOSITY(&verbosity),
@@ -599,20 +584,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)
584
die(_("--delete is incompatible with --all, --mirror and --tags"));
585
if (deleterefs && argc < 2)
586
die(_("--delete doesn't make sense without any refs"));
602
- if (flags & TRANSPORT_PUSH_ALL) {
603
- if (tags)
604
- die(_("--all and --tags are incompatible"));
605
- if (argc >= 2)
606
- die(_("--all can't be combined with refspecs"));
607
- }
608
- if (flags & TRANSPORT_PUSH_MIRROR) {
609
- if (tags)
610
- die(_("--mirror and --tags are incompatible"));
611
- if (argc >= 2)
612
- die(_("--mirror can't be combined with refspecs"));
613
- }
614
- if ((flags & TRANSPORT_PUSH_ALL) && (flags & TRANSPORT_PUSH_MIRROR))
615
- die(_("--all and --mirror are incompatible"));
587
588
if (recurse_submodules == RECURSE_SUBMODULES_CHECK)
589
flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;
@@ -629,11 +600,43 @@ int cmd_push(int argc, const char **argv, const char *prefix)
600
set_refspecs(argv + 1, argc - 1, repo);
601
}
602
603
+ remote = pushremote_get(repo);
604
+ if (!remote) {
605
+ if (repo)
606
+ die(_("bad repository '%s'"), repo);
607
+ die(_("No configured push destination.\n"
608
+ "Either specify the URL from the command-line or configure a remote repository using\n"
609
+ "\n"
610
+ " git remote add <name> <url>\n"
611
+ "\n"
612
+ "and then push using the remote name\n"
613
+ "\n"
614
+ " git push <name>\n"));
615
+ }
616
+
617
+ if (remote->mirror)
618
+ flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
619
+
620
+ if (flags & TRANSPORT_PUSH_ALL) {
621
+ if (tags)
622
+ die(_("--all and --tags are incompatible"));
623
+ if (argc >= 2)
624
+ die(_("--all can't be combined with refspecs"));
625
+ }
626
+ if (flags & TRANSPORT_PUSH_MIRROR) {
627
+ if (tags)
628
+ die(_("--mirror and --tags are incompatible"));
629
+ if (argc >= 2)
630
+ die(_("--mirror can't be combined with refspecs"));
631
+ }
632
+ if ((flags & TRANSPORT_PUSH_ALL) && (flags & TRANSPORT_PUSH_MIRROR))
633
+ die(_("--all and --mirror are incompatible"));
634
+
635
for_each_string_list_item(item, push_options)
636
if (strchr(item->string, '\n'))
637
die(_("push options must not have new line characters"));
638
636
- rc = do_push(repo, flags, push_options);
639
+ rc = do_push(repo, flags, push_options, remote);
640
string_list_clear(&push_options_cmdline, 0);
641
string_list_clear(&push_options_config, 0);
642
if (rc == -1)
t/t5517-push-mirror.sh
+10
@@ -265,4 +265,14 @@ test_expect_success 'remote.foo.mirror=no has no effect' '
265
266
'
267
268
+test_expect_success 'push to mirrored repository with refspec fails' '
269
+ mk_repo_pair &&
270
+ (
271
+ cd master &&
272
+ echo one >foo && git add foo && git commit -m one &&
273
+ git config --add remote.up.mirror true &&
274
+ test_must_fail git push up master
275
+ )
276
+'
277
+
278
test_done