@@ -37,6 +37,13 @@ advice.*::
we can still suggest that the user push to either
refs/heads/* or refs/tags/* based on the type of the
source object.
+ pushPartialAmbigiousName::
+ Shown when linkgit:git-push[1] is given a refspec
+ where the <src> in earlier versions of Git would have
+ matched a <dst> on the remote based on its existence
+ over appending a prefix based on the type of the
+ <src>. See the "<refspec>..." documentation in
+ linkgit:git-push[1] for details.
statusHints::
Show directions on how to proceed from the current
state in the output of linkgit:git-status[1], in
@@ -84,6 +84,19 @@ is ambiguous.
* If <src> resolves to a ref starting with refs/heads/ or refs/tags/,
then prepend that to <dst>.
++
+Versions of Git before 2.23.0 would override this rule and match
+e.g. `HEAD:tags/mark` to either `refs/tags/mark` or `refs/tags/mark`
+depending on, respectively, if `refs/tags/mark` existed or not on the
+remote.
++
+We'll now consistently pick `refs/heads/tags/mark` based on this rule
+and so that we're not so eager in guessing the <dst> on the remote
+that we'll pick a different <dst> based on what refs exist there
+already than we otherwise would have. This exception guards for cases
+where the match would be different due to a subsequent
+"refs/{tags,heads,remotes}/" matching rule". than a plain "refs/"
+prefix match.
* Other ambiguity resolutions might be added in the future, but for
now any other cases will error out with an error indicating what we
@@ -10,6 +10,7 @@ int advice_push_already_exists = 1;
int advice_push_fetch_first = 1;
int advice_push_needs_force = 1;
int advice_push_unqualified_ref_name = 1;
+int advice_partial_ambiguous_ref_name = 1;
int advice_status_hints = 1;
int advice_status_u_option = 1;
int advice_commit_before_merge = 1;
@@ -66,6 +67,7 @@ static struct {
{ "pushFetchFirst", &advice_push_fetch_first },
{ "pushNeedsForce", &advice_push_needs_force },
{ "pushUnqualifiedRefName", &advice_push_unqualified_ref_name },
+ { "pushPartialAmbigiousName", &advice_partial_ambiguous_ref_name },
{ "statusHints", &advice_status_hints },
{ "statusUoption", &advice_status_u_option },
{ "commitBeforeMerge", &advice_commit_before_merge },
@@ -10,6 +10,7 @@ extern int advice_push_already_exists;
extern int advice_push_fetch_first;
extern int advice_push_needs_force;
extern int advice_push_unqualified_ref_name;
+extern int advice_partial_ambiguous_ref_name;
extern int advice_status_hints;
extern int advice_status_u_option;
extern int advice_commit_before_merge;
@@ -1066,7 +1066,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
struct ref ***dst_tail,
struct refspec_item *rs)
{
- struct ref *matched_src, *matched_dst;
+ struct ref *matched_src, *matched_dst, *tmp;
int allocated_src;
const char *dst_value = rs->dst;
@@ -1094,6 +1094,27 @@ static int match_explicit(struct ref *src, struct ref *dst,
switch (count_refspec_match(dst_value, dst, &matched_dst)) {
case 1:
+ if ((starts_with(dst_value, "tags/") ||
+ starts_with(dst_value, "heads/") ||
+ starts_with(dst_value, "remotes/")) &&
+ (dst_guess = guess_ref(dst_value, matched_src))) {
+ tmp = make_linked_ref(dst_guess, dst_tail);
+ if (advice_partial_ambiguous_ref_name)
+ advise(_("The <dst> part of the refspec matched both of:\n"
+ "\n"
+ " 1. %1$s -> %2$s\n"
+ " 2. %1$s -> %3$s\n"
+ "\n"
+ "Earlier versions of git would have picked (1) as the RHS starts\n"
+ "with a second-level ref prefix which could be fully-qualified by\n"
+ "adding 'refs/' in front of it. We now pick (2) which uses the prefix\n"
+ "inferred from the <src> part of the refspec.\n"
+ "\n"
+ "See the \"<refspec>...\" rules discussed in 'git help push'.\n"),
+ dst_value, matched_dst->name, tmp->name);
+ matched_dst = tmp;
+ }
+
break;
case 0:
if (starts_with(dst_value, "refs/")) {
@@ -1277,4 +1277,22 @@ test_expect_success 'refs/remotes/* <src> refspec and unqualified <dst> DWIM and
)
'
+test_expect_success 'HEAD:tags/A and HEAD:tags/B should not be different depending on if refs/tags/[AB] exists or not' '
+ git clone "file://$PWD/two" tags-match &&
+ (
+ cd tags-match &&
+ test_commit A &&
+ git rev-parse HEAD >expected &&
+
+ git push origin HEAD:tags/my-not-a-tag &&
+ git -C ../two rev-parse refs/heads/tags/my-not-a-tag >actual &&
+ test_cmp expected actual &&
+
+ git push origin HEAD:tags/my-tag 2>advice &&
+ test_i18ngrep "hint: The <dst> part of the refspec matched both of" advice &&
+ git -C ../two rev-parse refs/heads/tags/my-tag >actual &&
+ test_cmp expected actual
+ )
+'
+
test_done
When a refspec like "HEAD:tags/my-tag" is pushed where "HEAD" is a branch, we'll push a *branch* that'll be located at "refs/heads/tags/my-tag". This is part of the rather straightforward rules I documented in 2219c09e23 ("push doc: document the DWYM behavior pushing to unqualified <dst>", 2018-11-13). However, if there exists a "refs/tags/my-tag" on the remote the count_refspec_match() logic will, as a result of calling refname_match(), match partially-qualified RHS of the refspec "refs/tags/my-tag", because it's in a loop where it tries to match "tags/my-tag" to "refs/tags/my-tag', then "refs/tags/tags/my-tag" etc. This resulted in a case[1] where someone on LKML did: git push kvm +HEAD:tags/for-linus Which would have created a new "refs/heads/tags/for-linus" branch in their "kvm" repository. But since they happened to have an existing "refs/tags/for-linus" reference we pushed there instead, and replaced an annotated tag with a lightweight tag. We do want a RHS ref like "master" to match "refs/heads/master", but it's confusing and dangerous that the DWYM behavior for matching partial RHS refspecs acts differently when the start of the RHS happens to be a second-level namespace under "refs/" namespace like "tags". Now we'll print out the following advice when this happens, and act differently as described therein: hint: The <dst> part of the refspec matched both of: hint: hint: 1. tags/my-tag -> refs/tags/my-tag hint: 2. tags/my-tag -> refs/heads/tags/my-tag hint: hint: Earlier versions of git would have picked (1) as the RHS starts hint: with a second-level ref prefix which could be fully-qualified by hint: adding 'refs/' in front of it. We now pick (2) which uses the prefix hint: inferred from the <src> part of the refspec. hint: hint: See the "<refspec>..." rules discussed in 'git help push'. An earlier version of this patch[2] used the much more heavy-handed approach of changing this logic in refname_match(). As shown from the tests that patch needed to modify that results in changes that are overzealous for fixing this push-specific issue. The right place to fix this is in match_explicit(). There we can see if we have both a DWYM match and a match based on the prefix of the LHS of the refspec, in those cases the match based on the LHS's ref prefix should win. 1. https://lore.kernel.org/lkml/2d55fd2a-afbf-1b7c-ca82-8bffaa18e0d0@redhat.com/ 2. https://public-inbox.org/git/20190526225445.21618-1-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Now that the 2.22.0 release is out I cleaned this up into a more sensible patch. Documentation/config/advice.txt | 7 +++++++ Documentation/git-push.txt | 13 +++++++++++++ advice.c | 2 ++ advice.h | 1 + remote.c | 23 ++++++++++++++++++++++- t/t5505-remote.sh | 18 ++++++++++++++++++ 6 files changed, 63 insertions(+), 1 deletion(-)