mirrors/git
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

push: disallow --all and refspecs when remote.<name>.mirror is set

Browse Source 
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:
Thomas Gummerer
2019-09-02 19:08:28 +01:00
committed by Junio C Hamano
parent 0d0ac3826a
commit 8e4c8af058
2 changed files with 46 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
builtin/push.c
View File

@ -382,30 +382,14 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
}
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;
struct remote *remote = pushremote_get(repo);
const char **url;
int url_nr;
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)
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;
const struct string_list_item *item;
struct remote *remote;
struct option options[] = {
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"));
if (deleterefs && argc < 2)
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)
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);
}
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)
if (strchr(item->string, '\n'))
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_config, 0);
if (rc == -1)

10
t/t5517-push-mirror.sh
View File

@ -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