@matheusbsbs / matheus-bagliano / commits / 60650a48c0

remote: make refspec follow the same disambiguation rule as local refs

When matching a non-wildcard LHS of a refspec against a list of refs, find_ref_by_name_abbrev() returns the first ref that matches using any DWIM rules used by refname_match() in refs.c, even if a better match occurs later in the list of refs. This causes unexpected behavior when (for example) fetching using the refspec "refs/heads/s:<something>" from a remote with both "refs/heads/refs/heads/s" and "refs/heads/s"; even if the former was inadvertently created, one would still expect the latter to be fetched. Similarly, when both a tag T and a branch T exist, fetching T should favor the tag, just like how local refname disambiguation rule works. But because the code walks over ls-remote output from the remote, which happens to be sorted in alphabetical order and has refs/heads/T before refs/tags/T, a request to fetch T is (mis)interpreted as fetching refs/heads/T. Update refname_match(), all of whose current callers care only if it returns non-zero (i.e. matches) to see if an abbreviated name can mean the full name being tested, so that it returns a positive integer whose magnitude can be used to tell the precedence, and fix the find_ref_by_name_abbrev() function not to stop at the first match but find the match with the highest precedence. This is based on an earlier work, which special cased only the exact matches, by Jonathan Tan. Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Aug 1, 2018 at 09:22 UTC 60650a48c0b24ecf64468426ec13b88c07069b28

3 files changed +58 -8

refs.c

+13 -5

@@ -487,16 +487,24 @@ static const char *ref_rev_parse_rules[] = {

487 NULL

488 };

489

490 +#define NUM_REV_PARSE_RULES (ARRAY_SIZE(ref_rev_parse_rules) - 1)

491 +

492 +/*

493 + * Is it possible that the caller meant full_name with abbrev_name?

494 + * If so return a non-zero value to signal "yes"; the magnitude of

495 + * the returned value gives the precedence used for disambiguation.

496 + *

497 + * If abbrev_name cannot mean full_name, return 0.

498 + */

499 int refname_match(const char *abbrev_name, const char *full_name)

500 {

501 const char **p;

502 const int abbrev_name_len = strlen(abbrev_name);

503 + const int num_rules = NUM_REV_PARSE_RULES;

504

495 - for (p = ref_rev_parse_rules; *p; p++) {

496 - if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) {

497 - return 1;

498 - }

499 - }

505 + for (p = ref_rev_parse_rules; *p; p++)

506 + if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name)))

507 + return &ref_rev_parse_rules[num_rules] - p;

508

509 return 0;

510 }

remote.c

+10 -3

@@ -1880,11 +1880,18 @@ static struct ref *get_expanded_map(const struct ref *remote_refs,

1880 static const struct ref *find_ref_by_name_abbrev(const struct ref *refs, const char *name)

1881 {

1882 const struct ref *ref;

1883 + const struct ref *best_match = NULL;

1884 + int best_score = 0;

1885 +

1886 for (ref = refs; ref; ref = ref->next) {

1884 - if (refname_match(name, ref->name))

1885 - return ref;

1887 + int score = refname_match(name, ref->name);

1888 +

1889 + if (best_score < score) {

1890 + best_match = ref;

1891 + best_score = score;

1892 + }

1893 }

1887 - return NULL;

1894 + return best_match;

1895 }

1896

1897 struct ref *get_remote_ref(const struct ref *remote_refs, const char *name)

t/t5510-fetch.sh

+35

@@ -535,6 +535,41 @@ test_expect_success "should be able to fetch with duplicate refspecs" '

535 )

536 '

537

538 +test_expect_success 'LHS of refspec follows ref disambiguation rules' '

539 + mkdir lhs-ambiguous &&

540 + (

541 + cd lhs-ambiguous &&

542 + git init server &&

543 + test_commit -C server unwanted &&

544 + test_commit -C server wanted &&

545 +

546 + git init client &&

547 +

548 + # Check a name coming after "refs" alphabetically ...

549 + git -C server update-ref refs/heads/s wanted &&

550 + git -C server update-ref refs/heads/refs/heads/s unwanted &&

551 + git -C client fetch ../server +refs/heads/s:refs/heads/checkthis &&

552 + git -C server rev-parse wanted >expect &&

553 + git -C client rev-parse checkthis >actual &&

554 + test_cmp expect actual &&

555 +

556 + # ... and one before.

557 + git -C server update-ref refs/heads/q wanted &&

558 + git -C server update-ref refs/heads/refs/heads/q unwanted &&

559 + git -C client fetch ../server +refs/heads/q:refs/heads/checkthis &&

560 + git -C server rev-parse wanted >expect &&

561 + git -C client rev-parse checkthis >actual &&

562 + test_cmp expect actual &&

563 +

564 + # Tags are preferred over branches like refs/{heads,tags}/*

565 + git -C server update-ref refs/tags/t wanted &&

566 + git -C server update-ref refs/heads/t unwanted &&

567 + git -C client fetch ../server +t:refs/heads/checkthis &&

568 + git -C server rev-parse wanted >expect &&

569 + git -C client rev-parse checkthis >actual

570 + )

571 +'

572 +

573 # configured prune tests

574

575 set_config_tristate () {