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>
This commit is contained in:
parent
0d0ac3826a
commit
8e4c8af058
@ -382,30 +382,14 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
|
|||||||
}
|
}
|
||||||
|
|
||||||
static int do_push(const char *repo, int flags,
|
static int do_push(const char *repo, int flags,
|
||||||
const struct string_list *push_options)
|
const struct string_list *push_options,
|
||||||
|
struct remote *remote)
|
||||||
{
|
{
|
||||||
int i, errs;
|
int i, errs;
|
||||||
struct remote *remote = pushremote_get(repo);
|
|
||||||
const char **url;
|
const char **url;
|
||||||
int url_nr;
|
int url_nr;
|
||||||
struct refspec *push_refspec = &rs;
|
struct refspec *push_refspec = &rs;
|
||||||
|
|
||||||
if (!remote) {
|
|
||||||
if (repo)
|
|
||||||
die(_("bad repository '%s'"), repo);
|
|
||||||
die(_("No configured push destination.\n"
|
|
||||||
"Either specify the URL from the command-line or configure a remote repository using\n"
|
|
||||||
"\n"
|
|
||||||
" git remote add <name> <url>\n"
|
|
||||||
"\n"
|
|
||||||
"and then push using the remote name\n"
|
|
||||||
"\n"
|
|
||||||
" git push <name>\n"));
|
|
||||||
}
|
|
||||||
|
|
||||||
if (remote->mirror)
|
|
||||||
flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
|
|
||||||
|
|
||||||
if (push_options->nr)
|
if (push_options->nr)
|
||||||
flags |= TRANSPORT_PUSH_OPTIONS;
|
flags |= TRANSPORT_PUSH_OPTIONS;
|
||||||
|
|
||||||
@ -545,6 +529,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
|
|||||||
struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
|
struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
|
||||||
struct string_list *push_options;
|
struct string_list *push_options;
|
||||||
const struct string_list_item *item;
|
const struct string_list_item *item;
|
||||||
|
struct remote *remote;
|
||||||
|
|
||||||
struct option options[] = {
|
struct option options[] = {
|
||||||
OPT__VERBOSITY(&verbosity),
|
OPT__VERBOSITY(&verbosity),
|
||||||
@ -599,20 +584,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)
|
|||||||
die(_("--delete is incompatible with --all, --mirror and --tags"));
|
die(_("--delete is incompatible with --all, --mirror and --tags"));
|
||||||
if (deleterefs && argc < 2)
|
if (deleterefs && argc < 2)
|
||||||
die(_("--delete doesn't make sense without any refs"));
|
die(_("--delete doesn't make sense without any refs"));
|
||||||
if (flags & TRANSPORT_PUSH_ALL) {
|
|
||||||
if (tags)
|
|
||||||
die(_("--all and --tags are incompatible"));
|
|
||||||
if (argc >= 2)
|
|
||||||
die(_("--all can't be combined with refspecs"));
|
|
||||||
}
|
|
||||||
if (flags & TRANSPORT_PUSH_MIRROR) {
|
|
||||||
if (tags)
|
|
||||||
die(_("--mirror and --tags are incompatible"));
|
|
||||||
if (argc >= 2)
|
|
||||||
die(_("--mirror can't be combined with refspecs"));
|
|
||||||
}
|
|
||||||
if ((flags & TRANSPORT_PUSH_ALL) && (flags & TRANSPORT_PUSH_MIRROR))
|
|
||||||
die(_("--all and --mirror are incompatible"));
|
|
||||||
|
|
||||||
if (recurse_submodules == RECURSE_SUBMODULES_CHECK)
|
if (recurse_submodules == RECURSE_SUBMODULES_CHECK)
|
||||||
flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;
|
flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;
|
||||||
@ -629,11 +600,43 @@ int cmd_push(int argc, const char **argv, const char *prefix)
|
|||||||
set_refspecs(argv + 1, argc - 1, repo);
|
set_refspecs(argv + 1, argc - 1, repo);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
remote = pushremote_get(repo);
|
||||||
|
if (!remote) {
|
||||||
|
if (repo)
|
||||||
|
die(_("bad repository '%s'"), repo);
|
||||||
|
die(_("No configured push destination.\n"
|
||||||
|
"Either specify the URL from the command-line or configure a remote repository using\n"
|
||||||
|
"\n"
|
||||||
|
" git remote add <name> <url>\n"
|
||||||
|
"\n"
|
||||||
|
"and then push using the remote name\n"
|
||||||
|
"\n"
|
||||||
|
" git push <name>\n"));
|
||||||
|
}
|
||||||
|
|
||||||
|
if (remote->mirror)
|
||||||
|
flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
|
||||||
|
|
||||||
|
if (flags & TRANSPORT_PUSH_ALL) {
|
||||||
|
if (tags)
|
||||||
|
die(_("--all and --tags are incompatible"));
|
||||||
|
if (argc >= 2)
|
||||||
|
die(_("--all can't be combined with refspecs"));
|
||||||
|
}
|
||||||
|
if (flags & TRANSPORT_PUSH_MIRROR) {
|
||||||
|
if (tags)
|
||||||
|
die(_("--mirror and --tags are incompatible"));
|
||||||
|
if (argc >= 2)
|
||||||
|
die(_("--mirror can't be combined with refspecs"));
|
||||||
|
}
|
||||||
|
if ((flags & TRANSPORT_PUSH_ALL) && (flags & TRANSPORT_PUSH_MIRROR))
|
||||||
|
die(_("--all and --mirror are incompatible"));
|
||||||
|
|
||||||
for_each_string_list_item(item, push_options)
|
for_each_string_list_item(item, push_options)
|
||||||
if (strchr(item->string, '\n'))
|
if (strchr(item->string, '\n'))
|
||||||
die(_("push options must not have new line characters"));
|
die(_("push options must not have new line characters"));
|
||||||
|
|
||||||
rc = do_push(repo, flags, push_options);
|
rc = do_push(repo, flags, push_options, remote);
|
||||||
string_list_clear(&push_options_cmdline, 0);
|
string_list_clear(&push_options_cmdline, 0);
|
||||||
string_list_clear(&push_options_config, 0);
|
string_list_clear(&push_options_config, 0);
|
||||||
if (rc == -1)
|
if (rc == -1)
|
||||||
|
@ -265,4 +265,14 @@ test_expect_success 'remote.foo.mirror=no has no effect' '
|
|||||||
|
|
||||||
'
|
'
|
||||||
|
|
||||||
|
test_expect_success 'push to mirrored repository with refspec fails' '
|
||||||
|
mk_repo_pair &&
|
||||||
|
(
|
||||||
|
cd master &&
|
||||||
|
echo one >foo && git add foo && git commit -m one &&
|
||||||
|
git config --add remote.up.mirror true &&
|
||||||
|
test_must_fail git push up master
|
||||||
|
)
|
||||||
|
'
|
||||||
|
|
||||||
test_done
|
test_done
|
||||||
|
Loading…
x
Reference in New Issue
Block a user