kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

refs: stop using mkpath() in refname_match() 

refname_match() formats every ref_rev_parse_rules entry through mkpath()
to then strcmp() with its given full_name.  It turns out this formatting
function dominates the client-side cost of matching explicit refspecs
against a remote that advertises lots of refs.

Reuse match_parse_rule() instead to efficiently compare each rule
against its given full_name.  With this change we avoid mkpath()'s
strbuf_vaddf(), and each match is now a direct prefix + suffix
comparison, length check, and memcmp of the refname at hand.

Timings from the test added in the previous commit:

  Test                           HEAD~1            HEAD
  -----------------------------------------------------------------------
  5516.3: empty:refspecs:1       0.15(0.09+0.11)   0.14(0.07+0.11) -6.7%
  5516.5: empty:refspecs:10      0.38(0.32+0.11)   0.17(0.11+0.11) -55.3%
  5516.7: empty:refspecs:100     2.50(2.43+0.11)   0.49(0.42+0.11) -80.4%
  5516.9: mirror:refspecs:1      0.21(0.15+0.11)   0.16(0.09+0.11) -23.8%
  5516.11: mirror:refspecs:10    0.80(0.73+0.11)   0.26(0.19+0.11) -67.5%
  5516.13: mirror:refspecs:100   6.68(6.59+0.13)   1.18(1.11+0.11) -82.3%

Notes on a change in behavior:

 - mkpath() runs cleanup_path(), which strips leading "./".  So,
   "./refs/heads/foo" previously matched "refs/heads/foo" through the
   "%.*s" rule.  As of this commit, this is no longer true.

 - It's considered a bug that "./"-prefixed strings previously could
   have been matched with refnames in this way:

   - cleanup_path() dates back to 26c8a533af (Add "mkpath()" helper
     function, 2005-07-08), and is intended to be used for filepaths,
     not refnames.  The leading-"./" strip itself comes from f17a1b1bec
     (Fix up path-cleanup in git_path() properly, 2005-07-05).

   - refname_match() has used mkpath() in its matching loop since its
     inception with 79803322c1 (add refname_match(), 2007-11-11).

   - 6cd4a8982d (avoid using mksnpath for refs, 2017-03-28) previously
     removed other cleanup_path() spots reachable from mksnpath() and
     notes they were "questionable when dealing with refnames, as we
     could silently canonicalize a syntactically bogus refname into a
     valid one."

Tests in t5510 and t5516 that cover the aliasing behavior are toggled
to test_expect_success.

Signed-off-by: Jon Simons <jon@jonsimons.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
seen
Jon Simons 2026-10-09 15:29:43 -04:00 committed by Junio C Hamano
parent a4c803513e
commit ac7a88513c
3 changed files with 50 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
refs.c
View File

@ -652,6 +652,44 @@ static const char *ref_rev_parse_rules[] = {

#define NUM_REV_PARSE_RULES (ARRAY_SIZE(ref_rev_parse_rules) - 1)

/*
* Check that the string refname matches a rule of the form
* "{prefix}%.*s{suffix}". So "foo/bar/baz" would match the rule
* "foo/%.*s/baz", and return the string "bar".
*/
static const char *match_parse_rule(const char *refname, const char *rule,
size_t *len)
{
/*
* Check that rule matches refname up to the first percent in the rule.
* We can bail immediately if not, but otherwise we leave "rule" at the
* %-placeholder, and "refname" at the start of the potential matched
* name.
*/
while (*rule != '%') {
if (!*rule)
BUG("rev-parse rule did not have percent");
if (*refname++ != *rule++)
return NULL;
}

/*
* Check that our "%" is the expected placeholder. This assumes there
* are no other percents (placeholder or quoted) in the string, but
* that is sufficient for our rev-parse rules.
*/
if (!skip_prefix(rule, "%.*s", &rule))
return NULL;

/*
* And now check that our suffix (if any) matches.
*/
if (!strip_suffix(refname, rule, len))
return NULL;

return refname; /* len set by strip_suffix() */
}

/*
* Is it possible that the caller meant full_name with abbrev_name?
* If so return a non-zero value to signal "yes"; the magnitude of
@ -662,12 +700,18 @@ static const char *ref_rev_parse_rules[] = {
int refname_match(const char *abbrev_name, const char *full_name)
{
const char **p;
const int abbrev_name_len = strlen(abbrev_name);
const size_t abbrev_name_len = strlen(abbrev_name);
const int num_rules = NUM_REV_PARSE_RULES;

for (p = ref_rev_parse_rules; *p; p++)
if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name)))
for (p = ref_rev_parse_rules; *p; p++) {
size_t short_name_len;
const char *short_name = match_parse_rule(full_name, *p,
&short_name_len);

if (short_name && short_name_len == abbrev_name_len &&
!memcmp(short_name, abbrev_name, abbrev_name_len))
return &ref_rev_parse_rules[num_rules] - p;
}

return 0;
}
@ -1615,44 +1659,6 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
return 0;
}

/*
* Check that the string refname matches a rule of the form
* "{prefix}%.*s{suffix}". So "foo/bar/baz" would match the rule
* "foo/%.*s/baz", and return the string "bar".
*/
static const char *match_parse_rule(const char *refname, const char *rule,
size_t *len)
{
/*
* Check that rule matches refname up to the first percent in the rule.
* We can bail immediately if not, but otherwise we leave "rule" at the
* %-placeholder, and "refname" at the start of the potential matched
* name.
*/
while (*rule != '%') {
if (!*rule)
BUG("rev-parse rule did not have percent");
if (*refname++ != *rule++)
return NULL;
}

/*
* Check that our "%" is the expected placeholder. This assumes there
* are no other percents (placeholder or quoted) in the string, but
* that is sufficient for our rev-parse rules.
*/
if (!skip_prefix(rule, "%.*s", &rule))
return NULL;

/*
* And now check that our suffix (if any) matches.
*/
if (!strip_suffix(refname, rule, len))
return NULL;

return refname; /* len set by strip_suffix() */
}

char *refs_shorten_unambiguous_ref(struct ref_store *refs,
const char *refname, int strict)
{

4
t/t5510-fetch.sh
View File

@ -1074,7 +1074,7 @@ test_expect_success 'LHS of refspec follows ref disambiguation rules' '
)
'

test_expect_failure 'fetch with "./"-prefixed branch.<name>.merge does not mark any ref for merge' '
test_expect_success 'fetch with "./"-prefixed branch.<name>.merge does not mark any ref for merge' '
mkdir dotslash-merge-default-refspec &&
(
cd dotslash-merge-default-refspec &&
@ -1096,7 +1096,7 @@ test_expect_failure 'fetch with "./"-prefixed branch.<name>.merge does not mark
)
'

test_expect_failure 'fetch protocol v0 with "./"-prefixed branch.<name>.merge does not match any remote ref' '
test_expect_success 'fetch protocol v0 with "./"-prefixed branch.<name>.merge does not match any remote ref' '
mkdir dotslash-merge-fetch-protocol-v0 &&
(
cd dotslash-merge-fetch-protocol-v0 &&

2
t/t5516-fetch-push.sh
View File

@ -433,7 +433,7 @@ test_expect_success 'push with onelevel ref' '
test_must_fail git push testrepo HEAD:refs/onelevel
'

test_expect_failure 'push with "./"-prefixed src does not match any ref' '
test_expect_success 'push with "./"-prefixed src does not match any ref' '
mk_test testrepo heads/main &&
test_must_fail git push testrepo ./refs/heads/main:refs/heads/frotz 2>err &&
test_grep "src refspec ./refs/heads/main does not match any" err