Skip to content

Commit 15d6de1

Browse files
avargitster
authored andcommitted
fetch: stop clobbering existing tags without --force
Change "fetch" to treat "+" in refspecs (aka --force) to mean we should clobber a local tag of the same name. This changes the long-standing behavior of "fetch" added in 853a369 ("[PATCH] Multi-head fetch.", 2005-08-20), before this change all tag fetches effectively had --force enabled. See the git-fetch-script code in fast_forward_local() with the comment: > Tags need not be pointing at commits so there is no way to > guarantee "fast-forward" anyway. That commit and the rest of the history of "fetch" shows that the "+" (--force) part of refpecs was only conceived for branch updates, while tags have accepted any changes from upstream unconditionally and clobbered the local tag object. Changing this behavior has been discussed as early as 2011[1]. The current behavior doesn't make sense to me, it easily results in local tags accidentally being clobbered. We could namespace our tags per-remote and not locally populate refs/tags/*, but as with my 97716d2 ("fetch: add a --prune-tags option and fetch.pruneTags config", 2018-02-09) it's easier to work around the current implementation than to fix the root cause. So this change implements suggestion #1 from Jeff's 2011 E-Mail[1], "fetch" now only clobbers the tag if either "+" is provided as part of the refspec, or if "--force" is provided on the command-line. This also makes it nicely symmetrical with how "tag" itself works when creating tags. I.e. we refuse to clobber any existing tags unless "--force" is supplied. Now we can refuse all such clobbering, whether it would happen by clobbering a local tag with "tag", or by fetching it from the remote with "fetch". It's still not at all nicely symmetrical with how "git push" works, as discussed in the updated pull-fetch-param.txt documentation, but this change brings them more into line with one another. I don't think there's any reason "fetch" couldn't fully converge with the behavior used by "push", but that's a topic for another change. One of the tests added in 31b808a ("clone --single: limit the fetch refspec to fetched branch", 2012-09-20) is being changed to use --force where a clone would clobber a tag. This changes nothing about the existing behavior of the test. 1. https://public-inbox.org/git/[email protected]/ Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 784cf1b commit 15d6de1

File tree

5 files changed

+43
-21
lines changed

5 files changed

+43
-21
lines changed

Documentation/fetch-options.txt

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,16 @@ endif::git-pull[]
4949

5050
-f::
5151
--force::
52-
When 'git fetch' is used with `<rbranch>:<lbranch>`
53-
refspec, it refuses to update the local branch
54-
`<lbranch>` unless the remote branch `<rbranch>` it
55-
fetches is a descendant of `<lbranch>`. This option
56-
overrides that check.
52+
When 'git fetch' is used with `<src>:<dst>` refspec it may
53+
refuse to update the local branch as discussed
54+
ifdef::git-pull[]
55+
in the `<refspec>` part of the linkgit:git-fetch[1]
56+
documentation.
57+
endif::git-pull[]
58+
ifndef::git-pull[]
59+
in the `<refspec>` part below.
60+
endif::git-pull[]
61+
This option overrides that check.
5762

5863
-k::
5964
--keep::

Documentation/pull-fetch-param.txt

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,21 @@ name.
3333
it requests fetching everything up to the given tag.
3434
+
3535
The remote ref that matches <src>
36-
is fetched, and if <dst> is not an empty string, the local
37-
ref that matches it is fast-forwarded using <src>.
38-
If the optional plus `+` is used, the local ref
39-
is updated even if it does not result in a fast-forward
40-
update.
36+
is fetched, and if <dst> is not an empty string, an attempt
37+
is made to update the local ref that matches it.
38+
+
39+
Whether that update is allowed without `--force` depends on the ref
40+
namespace it's being fetched to, and the type of object being
41+
fetched. If it's a commit under `refs/heads/*` only fast-forwards are
42+
allowed.
43+
+
44+
By having the optional leading `+` to a refspec (or using `--force`
45+
command line option) you can tell Git to update the local ref even if
46+
it is not allowed by its respective namespace clobbering rules.
47+
+
48+
Before Git version 2.19 tag objects under `refs/tags/*` would not be
49+
protected from updates, but since then the `+` (or `--force`) syntax
50+
is required to clobber them.
4151
+
4252
[NOTE]
4353
When the remote branch you want to fetch is known to

builtin/fetch.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ static struct option builtin_fetch_options[] = {
113113
N_("append to .git/FETCH_HEAD instead of overwriting")),
114114
OPT_STRING(0, "upload-pack", &upload_pack, N_("path"),
115115
N_("path to upload pack on remote end")),
116-
OPT__FORCE(&force, N_("force overwrite of local branch"), 0),
116+
OPT__FORCE(&force, N_("force overwrite of local reference"), 0),
117117
OPT_BOOL('m', "multiple", &multiple,
118118
N_("fetch from multiple remotes")),
119119
OPT_SET_INT('t', "tags", &tags,
@@ -664,12 +664,18 @@ static int update_local_ref(struct ref *ref,
664664

665665
if (!is_null_oid(&ref->old_oid) &&
666666
starts_with(ref->name, "refs/tags/")) {
667-
int r;
668-
r = s_update_ref("updating tag", ref, 0);
669-
format_display(display, r ? '!' : 't', _("[tag update]"),
670-
r ? _("unable to update local ref") : NULL,
671-
remote, pretty_ref, summary_width);
672-
return r;
667+
if (force || ref->force) {
668+
int r;
669+
r = s_update_ref("updating tag", ref, 0);
670+
format_display(display, r ? '!' : 't', _("[tag update]"),
671+
r ? _("unable to update local ref") : NULL,
672+
remote, pretty_ref, summary_width);
673+
return r;
674+
} else {
675+
format_display(display, '!', _("[rejected]"), _("would clobber existing tag"),
676+
remote, pretty_ref, summary_width);
677+
return 1;
678+
}
673679
}
674680

675681
current = lookup_commit_reference_gently(&ref->old_oid, 1);

t/t5516-fetch-push.sh

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ test_force_fetch_tag () {
10151015
tag_type_description=$1
10161016
tag_args=$2
10171017

1018-
test_expect_success "fetch will clobber an existing $tag_type_description" "
1018+
test_expect_success "fetch will not clobber an existing $tag_type_description without --force" "
10191019
mk_test testrepo heads/master &&
10201020
mk_child testrepo child1 &&
10211021
mk_child testrepo child2 &&
@@ -1027,7 +1027,8 @@ test_force_fetch_tag () {
10271027
git add file1 &&
10281028
git commit -m 'file1' &&
10291029
git tag $tag_args Tag &&
1030-
git -C ../child1 fetch origin tag Tag
1030+
test_must_fail git -C ../child1 fetch origin tag Tag &&
1031+
git -C ../child1 fetch origin '+refs/tags/*:refs/tags/*'
10311032
)
10321033
"
10331034
}

t/t5612-clone-refspec.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ test_expect_success 'clone with --no-tags' '
104104
test_expect_success '--single-branch while HEAD pointing at master' '
105105
(
106106
cd dir_master &&
107-
git fetch &&
107+
git fetch --force &&
108108
git for-each-ref refs/remotes/origin |
109109
sed -e "/HEAD$/d" \
110110
-e "s|/remotes/origin/|/heads/|" >../actual
@@ -115,7 +115,7 @@ test_expect_success '--single-branch while HEAD pointing at master' '
115115
test_cmp expect actual &&
116116
(
117117
cd dir_master &&
118-
git fetch --tags &&
118+
git fetch --tags --force &&
119119
git for-each-ref refs/tags >../actual
120120
) &&
121121
git for-each-ref refs/tags >expect &&

0 commit comments

Comments
 (0)