* [PATCH 01/15] remote: validate --force-with-lease <refname> argument
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 02/15] t5516: demonstrate push with "./"-prefixed source Jon Simons
` (14 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Use check_refname_format() to validate the <refname> component of
'git push --force-with-lease=<refname>[:<expect>]'.
apply_push_cas() will silently ignore a lease entry that does not match
any remote ref. And today "./refs/heads/main" will happen to be matched
with "refs/heads/main" due to refname_match() usage of mkpath(), whose
cleanup_path() strips leading "./".
An upcoming commit removes mkpath() from refname_match(), after which
there is no unintentional match in this situation, so that the lease
is ignored instead of applied. Reject an invalid refname now to make
this situation fail fast both before and after that change.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
remote.c | 2 ++
t/t5533-push-cas.sh | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/remote.c b/remote.c
index 71170f36a9..114d4d983c 100644
--- a/remote.c
+++ b/remote.c
@@ -2740,6 +2740,8 @@ static int parse_push_cas_option(struct push_cas_option *cas, const char *arg, i
/* "--<option>=refname" or "--<option>=refname:value" */
colon = strchrnul(arg, ':');
entry = add_cas_entry(cas, arg, colon - arg);
+ if (check_refname_format(entry->refname, REFNAME_ALLOW_ONELEVEL))
+ return error(_("'%s' is not a valid refname"), entry->refname);
if (!*colon)
entry->use_tracking = 1;
else if (!colon[1])
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index e6e1360a42..f6eafee7ad 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -63,6 +63,15 @@ test_expect_success setup '
test_commit C
'
+test_expect_success 'push --force-with-lease rejects invalid refname' '
+ setup_srcdst_basic &&
+ (
+ cd dst &&
+ test_must_fail git push --force-with-lease=./refs/heads/main origin main 2>err &&
+ test_grep "is not a valid refname" err
+ )
+'
+
test_expect_success 'push to update (protected)' '
setup_srcdst_basic &&
(
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 02/15] t5516: demonstrate push with "./"-prefixed source
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
2026-10-09 19:29 ` [PATCH 01/15] remote: validate --force-with-lease <refname> argument Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 03/15] t5510: document fetch with "./"-prefixed branch.<name>.merge Jon Simons
` (13 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
'git push <repo> ./refs/heads/main:refs/heads/frotz' succeeds today
and pushes refs/heads/main, although "./refs/heads/main" is not a
valid refname. count_refspec_match() compares the source to each
local ref using refname_match(), which formats its given name with
mkpath(), whose cleanup_path() strips leading "./".
Add a test_expect_failure asserting that such a source is rejected
with "src refspec ./refs/heads/main does not match any". An upcoming
commit stops using mkpath() in refname_match(), at which point the
test is toggled to test_expect_success.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/t5516-fetch-push.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b982b209bf..aaeb251e2f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -433,6 +433,12 @@ 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' '
+ 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
+'
+
test_expect_success 'push with colon-less refspec (1)' '
mk_test testrepo heads/frotz tags/frotz &&
git branch -f frotz main &&
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 03/15] t5510: document fetch with "./"-prefixed branch.<name>.merge
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
2026-10-09 19:29 ` [PATCH 01/15] remote: validate --force-with-lease <refname> argument Jon Simons
2026-10-09 19:29 ` [PATCH 02/15] t5516: demonstrate push with "./"-prefixed source Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 04/15] t/perf: add explicit delete refspec matching test Jon Simons
` (12 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
refname_match() formats each candidate through mkpath(), which strips
a leading "./", so a branch.<name>.merge value of "./refs/heads/main"
matches "refs/heads/main" in both places that fetch compares them:
- branch_merge_matches() when marking fetched refs for merge
in FETCH_HEAD
- find_ref_by_name_abbrev() when the merge source is not covered by the
fetch refspec and is looked up amongst the remote advertised refs
Only a hand-edited config reaches these paths: clone, branch
--set-upstream-to, fetch --set-upstream, and push -u write only names
that have passed check_refname_format(), which rejects a leading "./".
Add two fetch tests asserting that such a "./"-prefixed value does not
match for these cases. Both are test_expect_failure because the name
matches today:
- The default fetch refspec should not match "./refs/heads/main"
with fetched "refs/heads/main" and mark the ref for merge.
- A protocol v0 fetch with refspec that omits "refs/heads/main"
should not match "./refs/heads/main" against the remote
advertised "refs/heads/main".
An upcoming commit stops using mkpath() in refname_match(), at which
point the tests are toggled to test_expect_success.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/t5510-fetch.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 300bd5396d..0303784b1f 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1074,6 +1074,50 @@ 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' '
+ mkdir dotslash-merge-default-refspec &&
+ (
+ cd dotslash-merge-default-refspec &&
+ git init -b main server &&
+ test_commit -C server one &&
+ git -C server branch other &&
+ test_commit -C server two &&
+
+ # The bogus ./refs/heads/main should not match the remote refs/heads/main.
+ git clone server client &&
+ git -C client config branch.main.merge ./refs/heads/main &&
+ git -C client fetch &&
+ {
+ echo "$(git -C server rev-parse main) not-for-merge" &&
+ echo "$(git -C server rev-parse other) not-for-merge"
+ } >expect &&
+ cut -f -2 client/.git/FETCH_HEAD >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_failure '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 &&
+ git init -b main server &&
+ test_commit -C server one &&
+ git -C server branch other &&
+ test_commit -C server two &&
+
+ # Omit refs/heads/main from the fetch refspec so that the merge
+ # source is instead looked up among the refs the remote advertised.
+ git clone server client-v0 &&
+ git -C client-v0 config remote.origin.fetch \
+ +refs/heads/other:refs/remotes/origin/other &&
+ git -C client-v0 config branch.main.merge ./refs/heads/main &&
+ git -C client-v0 -c protocol.version=0 fetch &&
+ echo "$(git -C server rev-parse other) not-for-merge" >expect &&
+ cut -f -2 client-v0/.git/FETCH_HEAD >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'fetch.writeCommitGraph' '
git clone three write &&
(
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 04/15] t/perf: add explicit delete refspec matching test
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (2 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 03/15] t5510: document fetch with "./"-prefixed branch.<name>.merge Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 05/15] refs: stop using mkpath() in refname_match() Jon Simons
` (11 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Add p5516 to measure the client-side cost of matching explicit
refspecs on 'git push' against a server that advertises many refs.
The server gets 100k branches, and two clients push 1, 10, 100 delete
refspecs each with --dry-run: an empty client (no local refs), which
isolates matching against the advertised refs, and a mirror client
(full local copy of server refs), to exercise matching against the
same number of local refs. Delete refspecs and --dry-run are used
to avoid any object transfer or ref update activity in the measurement
and leave the scratch repos unmodified, such that each repetition does
the same amount of matching work.
The test is used to demonstrate speedups in subsequent commits.
Baseline numbers on my machine:
Test this tree
----------------------------------------------
5516.3: empty:refspecs:1 0.14(0.08+0.10)
5516.5: empty:refspecs:10 0.38(0.32+0.10)
5516.7: empty:refspecs:100 2.50(2.44+0.10)
5516.9: mirror:refspecs:1 0.21(0.14+0.11)
5516.11: mirror:refspecs:10 0.80(0.73+0.11)
5516.13: mirror:refspecs:100 6.85(6.78+0.12)
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/perf/p5516-push-delete-refspec.sh | 42 +++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
create mode 100755 t/perf/p5516-push-delete-refspec.sh
diff --git a/t/perf/p5516-push-delete-refspec.sh b/t/perf/p5516-push-delete-refspec.sh
new file mode 100755
index 0000000000..0e425193e5
--- /dev/null
+++ b/t/perf/p5516-push-delete-refspec.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='explicit delete refspec matching on push
+
+Measure client-side matching of explicit delete refspecs with "git push
+--dry-run" against a server that advertises lots of refs. An empty client
+(no local refs) and a mirror client (full local copy of the server refs)
+are tested pushing 1, 10, and 100 refspecs each.
+'
+. ./perf-lib.sh
+
+test_perf_fresh_repo
+
+ref_count=100000
+
+test_expect_success 'create server with many refs and two clients' '
+ test_commit base &&
+ git clone --bare --ref-format=reftable . server &&
+ test_seq -f "create refs/heads/b%d HEAD" $ref_count |
+ git -C server update-ref --stdin &&
+ git init --bare client_empty &&
+ git -C client_empty remote add origin "$PWD/server" &&
+ git clone --mirror --ref-format=reftable "$PWD/server" client_mirror &&
+ git -C client_mirror config --unset remote.origin.mirror
+'
+
+for mode in empty mirror
+do
+ client=client_$mode
+ for nr_refspecs in 1 10 100
+ do
+ test_expect_success "create $mode refspecs: $nr_refspecs" '
+ test_seq -f ":refs/heads/b%d" $nr_refspecs >refspecs
+ '
+
+ test_perf "$mode:refspecs:$nr_refspecs" '
+ git -C '"$client"' push --dry-run origin $(cat refspecs)
+ '
+ done
+done
+
+test_done
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 05/15] refs: stop using mkpath() in refname_match()
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (3 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 04/15] t/perf: add explicit delete refspec matching test Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 06/15] remote: use strmap for check_push_refs() Jon Simons
` (10 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
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>
---
refs.c | 88 +++++++++++++++++++++++--------------------
t/t5510-fetch.sh | 4 +-
t/t5516-fetch-push.sh | 2 +-
3 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/refs.c b/refs.c
index 951db56113..9bf3bc8153 100644
--- a/refs.c
+++ b/refs.c
@@ -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)
{
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 0303784b1f..941213f36a 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -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 &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index aaeb251e2f..81ad6cd52f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -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
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 06/15] remote: use strmap for check_push_refs()
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (4 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 05/15] refs: stop using mkpath() in refname_match() Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 07/15] t5516: test pushing two refspecs creating the same new branch Jon Simons
` (9 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Optimize check_push_refs() by replacing a linear traversal of all
local refs with strmap lookups.
Before this change, matching R explicit refspecs against N local refs
in check_push_refs() entails O(R * N) calls to refname_match().
After this change, we build a strmap of local refs O(N) and use it
for O(R * rules) lookups of the refspecs.
The new count_refspec_match_in_map() is equivalent to the previous
count_refspec_match():
- count_refspec_match() for 'pattern' iterates every local ref,
adding a match for each 'refname_match(pattern, refname)',
which searches against the six ref_rev_parse_rules.
- count_refspec_match_in_map() for 'pattern' generates the six
possible matches with expand_ref_prefix(), and then issues
one strmap lookup for each one.
match_explicit() still works on the ref list, so temporarily introduce
match_explicit_lhs_map() alongside match_explicit_lhs(). These two
functions are recombined in a subsequent commit that converts
match_explicit().
Timings show benefit for the case where the client has many local refs
and specifies multiple refspecs:
Test HEAD~1 HEAD
-----------------------------------------------------------------------
5516.3: empty:refspecs:1 0.14(0.07+0.11) 0.13(0.07+0.10) -7.1%
5516.5: empty:refspecs:10 0.16(0.10+0.10) 0.16(0.10+0.10) +0.0%
5516.7: empty:refspecs:100 0.47(0.41+0.10) 0.47(0.41+0.10) +0.0%
5516.9: mirror:refspecs:1 0.16(0.09+0.11) 0.16(0.10+0.11) +0.0%
5516.11: mirror:refspecs:10 0.26(0.19+0.11) 0.22(0.16+0.11) -15.4%
5516.13: mirror:refspecs:100 1.19(1.13+0.10) 0.82(0.75+0.11) -31.1%
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
remote.c | 186 ++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 129 insertions(+), 57 deletions(-)
diff --git a/remote.c b/remote.c
index 114d4d983c..91d35b37fe 100644
--- a/remote.c
+++ b/remote.c
@@ -21,6 +21,7 @@
#include "dir.h"
#include "setup.h"
#include "string-list.h"
+#include "strmap.h"
#include "strvec.h"
#include "commit-reach.h"
#include "advice.h"
@@ -1056,60 +1057,95 @@ void free_refs(struct ref *ref)
}
}
+struct refspec_match {
+ struct ref *matched_weak;
+ struct ref *matched;
+ int weak_match;
+ int match;
+};
+
+static void add_refspec_match(struct refspec_match *m, const char *pattern,
+ struct ref *ref)
+{
+ size_t patlen = strlen(pattern);
+ size_t namelen = strlen(ref->name);
+
+ /* A match is "weak" if it is with refs outside
+ * heads or tags, and did not specify the pattern
+ * in full (e.g. "refs/remotes/origin/master") or at
+ * least from the toplevel (e.g. "remotes/origin/master");
+ * otherwise "git push $URL master" would result in
+ * ambiguity between remotes/origin/master and heads/master
+ * at the remote site.
+ */
+ if (namelen != patlen &&
+ patlen != namelen - 5 &&
+ !starts_with(ref->name, "refs/heads/") &&
+ !starts_with(ref->name, "refs/tags/")) {
+ /* We want to catch the case where only weak
+ * matches are found and there are multiple
+ * matches, and where more than one strong
+ * matches are found, as ambiguous. One
+ * strong match with zero or more weak matches
+ * are acceptable as a unique match.
+ */
+ m->matched_weak = ref;
+ m->weak_match++;
+ } else {
+ m->matched = ref;
+ m->match++;
+ }
+}
+
+static int finish_refspec_match(const struct refspec_match *m,
+ struct ref **matched_ref)
+{
+ if (!m->matched) {
+ if (matched_ref)
+ *matched_ref = m->matched_weak;
+ return m->weak_match;
+ }
+ if (matched_ref)
+ *matched_ref = m->matched;
+ return m->match;
+}
+
int count_refspec_match(const char *pattern,
struct ref *refs,
struct ref **matched_ref)
{
- int patlen = strlen(pattern);
- struct ref *matched_weak = NULL;
- struct ref *matched = NULL;
- int weak_match = 0;
- int match = 0;
+ struct refspec_match m = { 0 };
- for (weak_match = match = 0; refs; refs = refs->next) {
- char *name = refs->name;
- int namelen = strlen(name);
+ for (; refs; refs = refs->next) {
+ if (refname_match(pattern, refs->name))
+ add_refspec_match(&m, pattern, refs);
+ }
+ return finish_refspec_match(&m, matched_ref);
+}
- if (!refname_match(pattern, name))
- continue;
+static void ref_map_init(struct strmap *map, struct ref *refs)
+{
+ strmap_init_with_options(map, NULL, 0);
+ for (; refs; refs = refs->next)
+ strmap_put(map, refs->name, refs);
+}
- /* A match is "weak" if it is with refs outside
- * heads or tags, and did not specify the pattern
- * in full (e.g. "refs/remotes/origin/master") or at
- * least from the toplevel (e.g. "remotes/origin/master");
- * otherwise "git push $URL master" would result in
- * ambiguity between remotes/origin/master and heads/master
- * at the remote site.
- */
- if (namelen != patlen &&
- patlen != namelen - 5 &&
- !starts_with(name, "refs/heads/") &&
- !starts_with(name, "refs/tags/")) {
- /* We want to catch the case where only weak
- * matches are found and there are multiple
- * matches, and where more than one strong
- * matches are found, as ambiguous. One
- * strong match with zero or more weak matches
- * are acceptable as a unique match.
- */
- matched_weak = refs;
- weak_match++;
- }
- else {
- matched = refs;
- match++;
- }
- }
- if (!matched) {
- if (matched_ref)
- *matched_ref = matched_weak;
- return weak_match;
- }
- else {
- if (matched_ref)
- *matched_ref = matched;
- return match;
+static int count_refspec_match_in_map(const char *pattern,
+ struct strmap *refs,
+ struct ref **matched_ref)
+{
+ struct refspec_match m = { 0 };
+ struct strvec names = STRVEC_INIT;
+ size_t i;
+
+ expand_ref_prefix(&names, pattern);
+ for (i = 0; i < names.nr; i++) {
+ struct ref *ref = strmap_get(refs, names.v[i]);
+ if (ref)
+ add_refspec_match(&m, pattern, ref);
}
+ strvec_clear(&names);
+ return finish_refspec_match(&m, matched_ref);
}
void tail_link_ref(struct ref *ref, struct ref ***tail)
@@ -1178,12 +1214,12 @@ static char *guess_ref(const char *name, struct ref *peer)
return strbuf_detach(&buf, NULL);
}
-static int match_explicit_lhs(struct ref *src,
- struct refspec_item *rs,
- struct ref **match,
- int *allocated_match)
+static int match_explicit_lhs_count(const int count,
+ struct refspec_item *rs,
+ struct ref **match,
+ int *allocated_match)
{
- switch (count_refspec_match(rs->src, src, match)) {
+ switch (count) {
case 1:
if (allocated_match)
*allocated_match = 0;
@@ -1203,6 +1239,24 @@ static int match_explicit_lhs(struct ref *src,
}
}
+static int match_explicit_lhs(struct ref *src,
+ struct refspec_item *rs,
+ struct ref **match,
+ int *allocated_match)
+{
+ return match_explicit_lhs_count(count_refspec_match(rs->src, src, match),
+ rs, match, allocated_match);
+}
+
+static int match_explicit_lhs_map(struct strmap *src,
+ struct refspec_item *rs,
+ struct ref **match,
+ int *allocated_match)
+{
+ return match_explicit_lhs_count(count_refspec_match_in_map(rs->src, src, match),
+ rs, match, allocated_match);
+}
+
static void show_push_unqualified_ref_name_error(const char *dst_value,
const char *matched_src_name)
{
@@ -1265,6 +1319,20 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,
}
}
+static bool refspec_item_is_explicit(const struct refspec_item *item)
+{
+ return !item->pattern && !item->matching && !item->negative;
+}
+
+static bool any_refspec_item_is_explicit(const struct refspec *rs)
+{
+ for (int i = 0; i < rs->nr; i++) {
+ if (refspec_item_is_explicit(&rs->items[i]))
+ return true;
+ }
+ return false;
+}
+
static int match_explicit(struct ref *src, struct ref *dst,
struct ref ***dst_tail,
struct refspec_item *rs)
@@ -1275,7 +1343,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
const char *dst_value = rs->dst;
char *dst_guess;
- if (rs->pattern || rs->matching || rs->negative) {
+ if (!refspec_item_is_explicit(rs)) {
ret = 0;
goto out;
}
@@ -1564,17 +1632,21 @@ static void prepare_ref_index(struct string_list *ref_index, struct ref *ref)
*/
int check_push_refs(struct ref *src, struct refspec *rs)
{
+ struct strmap src_map;
int ret = 0;
- int i;
- for (i = 0; i < rs->nr; i++) {
- struct refspec_item *item = &rs->items[i];
+ if (!any_refspec_item_is_explicit(rs))
+ return 0;
- if (item->pattern || item->matching || item->negative)
+ ref_map_init(&src_map, src);
+ for (int i = 0; i < rs->nr; i++) {
+ struct refspec_item *item = &rs->items[i];
+ if (!refspec_item_is_explicit(item))
continue;
- ret |= match_explicit_lhs(src, item, NULL, NULL);
+ ret |= match_explicit_lhs_map(&src_map, item, NULL, NULL);
}
+ strmap_clear(&src_map, 0);
return ret;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 07/15] t5516: test pushing two refspecs creating the same new branch
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (5 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 06/15] remote: use strmap for check_push_refs() Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 08/15] t5408, t5410: test duplicate updates without relying on the client Jon Simons
` (8 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Pushing two refspecs that create the same new branch fails client-side
as desired with "dst ref <dst> receives from more than one src", as long
as the remote advertises at least one ref.
But pushing the same to an empty remote does not fail client-side as
expected: a --dry-run push prints "[new branch]" twice, and an actual
push fails on the server side with "multiple updates for ref '<dst>'
not allowed".
Add tests for both remotes, with and without --dry-run. The empty repo
passes are marked as test_expect_failure, in preparation for a commit
that includes a fix and toggles them to test_expect_success.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/t5516-fetch-push.sh | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 81ad6cd52f..e150246577 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -400,6 +400,42 @@ test_expect_success 'push with no ambiguity (2)' '
check_push_result testrepo $the_commit remotes/origin/main
'
+test_expect_success 'push --dry-run two refspecs targeting the same ref fails' '
+ mk_test testrepo heads/main &&
+ test_must_fail git push --dry-run testrepo main:frotz main:frotz 2>err &&
+ test_grep "dst ref refs/heads/frotz receives from more than one src" err
+'
+
+test_expect_success 'push two refspecs targeting the same ref fails' '
+ mk_test testrepo heads/main &&
+ test_must_fail git push testrepo main:frotz main:frotz 2>err &&
+ test_grep "dst ref refs/heads/frotz receives from more than one src" err
+'
+
+test_expect_failure 'push --dry-run two refspecs creating the same ref fails on empty repo' '
+ mk_empty testrepo &&
+ test_must_fail git push --dry-run testrepo main:frotz main:frotz 2>err &&
+ test_grep "dst ref refs/heads/frotz receives from more than one src" err
+'
+
+test_expect_failure 'push two refspecs creating the same ref fails on empty repo' '
+ mk_empty testrepo &&
+ test_must_fail git push testrepo main:frotz main:frotz 2>err &&
+ test_grep "dst ref refs/heads/frotz receives from more than one src" err
+'
+
+test_expect_failure 'push --dry-run abbreviated then full refspec creating the same ref fails on empty repo' '
+ mk_empty testrepo &&
+ test_must_fail git push --dry-run testrepo main:frotz main:refs/heads/frotz 2>err &&
+ test_grep "dst ref refs/heads/frotz receives from more than one src" err
+'
+
+test_expect_failure 'push abbreviated then full refspec creating the same ref fails on empty repo' '
+ mk_empty testrepo &&
+ test_must_fail git push testrepo main:frotz main:refs/heads/frotz 2>err &&
+ test_grep "dst ref refs/heads/frotz receives from more than one src" err
+'
+
test_expect_success 'push with colon-less refspec, no ambiguity' '
mk_test testrepo heads/main heads/t/main &&
git branch -f t/main main &&
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 08/15] t5408, t5410: test duplicate updates without relying on the client
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (6 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 07/15] t5516: test pushing two refspecs creating the same new branch Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 09/15] t5408: check refspec order with distinct destinations Jon Simons
` (7 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
The existing t5408 tests that use send-pack to transmit duplicate
updates for the same destination happen to rely on the client not
recognizing the duplicates when the remote is empty.
As a result, they are the only push tests that exercise each of:
- receive-pack rejection of multiple updates for the same ref, from
9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19)
- send-pack's receive_status() handling of a ref reported twice by the
remote, from 77188b5bba (send-pack: fix memory leak around duplicate
refs, 2025-05-19)
Add explicit tests for each of those paths, in preparation of an
upcoming commit that fixes the client to reject such pushes before
sending any update.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/t5408-send-pack-stdin.sh | 12 ++++++++++++
t/t5410-receive-pack.sh | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/t/t5408-send-pack-stdin.sh b/t/t5408-send-pack-stdin.sh
index ec339761c2..3c47be1af8 100755
--- a/t/t5408-send-pack-stdin.sh
+++ b/t/t5408-send-pack-stdin.sh
@@ -89,6 +89,18 @@ test_expect_success '--stdin refs come after cmdline' '
test_must_fail git --git-dir=remote.git rev-parse foo
'
+test_expect_success 'send-pack handles repeated status for the same ref' '
+ clear_remote &&
+ test_hook -C remote.git receive-report <<-\EOF &&
+ cat >/dev/null &&
+ printf "%s\n" "unpack ok" "ng refs/heads/foo first" \
+ "ng refs/heads/foo second" 0000 |
+ test-tool pkt-line pack
+ EOF
+ test_must_fail git send-pack remote.git A:foo 2>err &&
+ test_grep "remote rejected.*A -> foo (second)" err
+'
+
test_expect_success 'refspecs and --mirror do not mix (cmdline)' '
clear_remote &&
test_must_fail git send-pack remote.git --mirror $(cat refs)
diff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh
index 09d6bfd2a1..8fbc0c6bc9 100755
--- a/t/t5410-receive-pack.sh
+++ b/t/t5410-receive-pack.sh
@@ -97,4 +97,39 @@ test_expect_success TEE_DOES_NOT_HANG \
test_must_fail git -C remote.git rev-list $(git -C repo rev-parse HEAD)
'
+test_expect_success 'receive-pack rejects multiple updates for the same ref' '
+ test_when_finished "rm -rf repo remote.git" &&
+
+ git init repo &&
+ git -C repo commit --allow-empty -m A &&
+ git -C repo branch A &&
+ git -C repo commit --allow-empty -m B &&
+ git -C repo branch B &&
+ git init --bare remote.git &&
+ git -C repo send-pack ../remote.git A B &&
+ A=$(git -C repo rev-parse A) &&
+ B=$(git -C repo rev-parse B) &&
+ {
+ printf "%s %s refs/heads/foo\0report-status object-format=%s" \
+ $ZERO_OID $A "$(test_oid algo)" |
+ test-tool pkt-line pack-raw-stdin &&
+ printf "%s %s refs/heads/foo" $ZERO_OID $B |
+ test-tool pkt-line pack-raw-stdin &&
+ printf 0000 &&
+ git pack-objects --stdout </dev/null
+ } >request &&
+ git receive-pack remote.git <request >response 2>err &&
+ test_grep "multiple updates for ref ${SQ}refs/heads/foo${SQ} not allowed" err &&
+ test-tool pkt-line unpack <response >report &&
+ sed -n "/^unpack /,\$p" report >actual &&
+ cat >expect <<-\EOF &&
+ unpack ok
+ ng refs/heads/foo failed to update refs
+ ng refs/heads/foo failed to update refs
+ 0000
+ EOF
+ test_cmp expect actual &&
+ test_must_fail git --git-dir=remote.git rev-parse --verify refs/heads/foo
+'
+
test_done
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 09/15] t5408: check refspec order with distinct destinations
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (7 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 08/15] t5408, t5410: test duplicate updates without relying on the client Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 10/15] t5408: expect client-side error for duplicate destinations Jon Simons
` (6 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
26be19ba8d (send-pack: take refspecs over stdin, 2014-08-21) added tests
to assert that send-pack sends command-line refspecs in the order given,
and those read with --stdin after them, by pushing to the same
destination twice, and seeing which update succeeded.
But since 9d2962a7c4 (receive-pack: use batched reference updates,
2025-05-19) such pushes to the same destination now fail completely on
the server-side with "multiple updates for ref '<dst>' not allowed",
and so the ordering is no longer being asserted.
Add two explicit tests to reinstate the ordering assertions.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/t5408-send-pack-stdin.sh | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/t/t5408-send-pack-stdin.sh b/t/t5408-send-pack-stdin.sh
index 3c47be1af8..7af350f01c 100755
--- a/t/t5408-send-pack-stdin.sh
+++ b/t/t5408-send-pack-stdin.sh
@@ -67,6 +67,36 @@ test_expect_success 'stdin mixed with cmdline' '
verify_push B
'
+test_expect_success 'cmdline refs are sent in order' '
+ clear_remote &&
+ test_hook -C remote.git pre-receive <<-\EOF &&
+ cut -d" " -f3 >pushed-refs
+ EOF
+ git send-pack remote.git A:foo B:bar C:baz &&
+ cat >expect <<-\EOF &&
+ refs/heads/foo
+ refs/heads/bar
+ refs/heads/baz
+ EOF
+ test_cmp expect remote.git/pushed-refs
+'
+
+test_expect_success '--stdin refs are sent after cmdline refs' '
+ clear_remote &&
+ test_hook -C remote.git pre-receive <<-\EOF &&
+ cut -d" " -f3 >pushed-refs
+ EOF
+ echo A:bar >input &&
+ git send-pack remote.git --stdin B:foo <input &&
+ cat >expect <<-\EOF &&
+ refs/heads/foo
+ refs/heads/bar
+ EOF
+ test_cmp expect remote.git/pushed-refs &&
+ verify_push B foo &&
+ verify_push A bar
+'
+
test_expect_success 'cmdline refs written in order' '
clear_remote &&
test_must_fail git send-pack remote.git A:foo B:foo 2>err &&
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 10/15] t5408: expect client-side error for duplicate destinations
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (8 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 09/15] t5408: check refspec order with distinct destinations Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 11/15] remote: reject duplicate destinations on an empty remote Jon Simons
` (5 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Update the error message expectation for the existing t5408 tests that
use send-pack to transmit duplicate updates to an empty remote, and
toggle them test_expect_failure.
The tests previously asserted the server-side rejection "multiple
updates for ref '<dst>' not allowed".
The next commit rejects these pushes on the client-side such that
send-pack fails earlier with "dst ref <dst> receives from more than
one src", and never reaches the server. The tests are toggled to
test_expect_success then.
While here, change the test descriptions to note that they are no
longer asserting anything about the order of partial ref updates.
Also included is a fix for a missing "2>err" in the '--stdin refs
come after cmdline' test.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/t5408-send-pack-stdin.sh | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/t/t5408-send-pack-stdin.sh b/t/t5408-send-pack-stdin.sh
index 7af350f01c..0321519f21 100755
--- a/t/t5408-send-pack-stdin.sh
+++ b/t/t5408-send-pack-stdin.sh
@@ -97,25 +97,25 @@ test_expect_success '--stdin refs are sent after cmdline refs' '
verify_push A bar
'
-test_expect_success 'cmdline refs written in order' '
+test_expect_failure 'two cmdline refs for the same destination are rejected' '
clear_remote &&
test_must_fail git send-pack remote.git A:foo B:foo 2>err &&
- test_grep "multiple updates for ref ${SQ}refs/heads/foo${SQ} not allowed" err &&
+ test_grep "dst ref refs/heads/foo receives from more than one src" err &&
test_must_fail git --git-dir=remote.git rev-parse foo
'
-test_expect_success 'cmdline refs with multiple duplicates' '
+test_expect_failure 'three cmdline refs for the same destination are rejected' '
clear_remote &&
test_must_fail git send-pack remote.git A:foo B:foo C:foo 2>err &&
- test_grep "multiple updates for ref ${SQ}refs/heads/foo${SQ} not allowed" err &&
+ test_grep "dst ref refs/heads/foo receives from more than one src" err &&
test_must_fail git --git-dir=remote.git rev-parse foo
'
-test_expect_success '--stdin refs come after cmdline' '
+test_expect_failure 'cmdline and --stdin refs for the same destination are rejected' '
clear_remote &&
echo A:foo >input &&
- test_must_fail git send-pack remote.git --stdin B:foo <input &&
- test_grep "multiple updates for ref ${SQ}refs/heads/foo${SQ} not allowed" err &&
+ test_must_fail git send-pack remote.git --stdin B:foo <input 2>err &&
+ test_grep "dst ref refs/heads/foo receives from more than one src" err &&
test_must_fail git --git-dir=remote.git rev-parse foo
'
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 11/15] remote: reject duplicate destinations on an empty remote
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (9 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 10/15] t5408: expect client-side error for duplicate destinations Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 12/15] remote: use strmap for match_explicit_refs() Jon Simons
` (4 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
match_explicit_refs() passes the head of the remote ref list, 'dst',
to match_explicit() for every refspec. match_explicit() searches that
list via count_refspec_match() and, when nothing matches, appends a
new destination through 'dst_tail'.
This works when the remote already has refs: 'dst' is non-NULL, so each
loop iteration walks from the same head and finds destinations linked
at the tail.
This fails when the remote is empty and 'dst' is NULL. match_push_refs()
computes 'dst_tail' with tail_ref(), which for an empty list is the head
slot itself: so the first link writes through '*dst'. But each loop
iteration still provides the original NULL to match_explicit(), with
the result that newly-linked destinations are not seen.
The behavior dates back to f88395ac23 (Renaming push., 2005-08-03),
where match_explicit_refs() searches the destination list from the
'dst' it was given while appending new destinations through 'dst_tail'.
Pass the remote refs head by pointer in match_explicit_refs() so that
each loop iteration re-reads '*dst' and detects a destination created
by an earlier refspec.
With this, failing tests in t5408, t5516 now pass and are toggled to
test_expect_success.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
remote.c | 6 +++---
t/t5408-send-pack-stdin.sh | 6 +++---
t/t5516-fetch-push.sh | 8 ++++----
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/remote.c b/remote.c
index 91d35b37fe..f4cf63b756 100644
--- a/remote.c
+++ b/remote.c
@@ -1416,12 +1416,12 @@ static int match_explicit(struct ref *src, struct ref *dst,
return ret;
}
-static int match_explicit_refs(struct ref *src, struct ref *dst,
+static int match_explicit_refs(struct ref *src, struct ref **dst,
struct ref ***dst_tail, struct refspec *rs)
{
int i, errs;
for (i = errs = 0; i < rs->nr; i++)
- errs += match_explicit(src, dst, dst_tail, &rs->items[i]);
+ errs += match_explicit(src, *dst, dst_tail, &rs->items[i]);
return errs;
}
@@ -1673,7 +1673,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
if (!rs->nr)
refspec_append(rs, ":");
- errs = match_explicit_refs(src, *dst, &dst_tail, rs);
+ errs = match_explicit_refs(src, dst, &dst_tail, rs);
/* pick the remainder */
for (ref = src; ref; ref = ref->next) {
diff --git a/t/t5408-send-pack-stdin.sh b/t/t5408-send-pack-stdin.sh
index 0321519f21..1e34880323 100755
--- a/t/t5408-send-pack-stdin.sh
+++ b/t/t5408-send-pack-stdin.sh
@@ -97,21 +97,21 @@ test_expect_success '--stdin refs are sent after cmdline refs' '
verify_push A bar
'
-test_expect_failure 'two cmdline refs for the same destination are rejected' '
+test_expect_success 'two cmdline refs for the same destination are rejected' '
clear_remote &&
test_must_fail git send-pack remote.git A:foo B:foo 2>err &&
test_grep "dst ref refs/heads/foo receives from more than one src" err &&
test_must_fail git --git-dir=remote.git rev-parse foo
'
-test_expect_failure 'three cmdline refs for the same destination are rejected' '
+test_expect_success 'three cmdline refs for the same destination are rejected' '
clear_remote &&
test_must_fail git send-pack remote.git A:foo B:foo C:foo 2>err &&
test_grep "dst ref refs/heads/foo receives from more than one src" err &&
test_must_fail git --git-dir=remote.git rev-parse foo
'
-test_expect_failure 'cmdline and --stdin refs for the same destination are rejected' '
+test_expect_success 'cmdline and --stdin refs for the same destination are rejected' '
clear_remote &&
echo A:foo >input &&
test_must_fail git send-pack remote.git --stdin B:foo <input 2>err &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index e150246577..2f46ddc9ba 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -412,25 +412,25 @@ test_expect_success 'push two refspecs targeting the same ref fails' '
test_grep "dst ref refs/heads/frotz receives from more than one src" err
'
-test_expect_failure 'push --dry-run two refspecs creating the same ref fails on empty repo' '
+test_expect_success 'push --dry-run two refspecs creating the same ref fails on empty repo' '
mk_empty testrepo &&
test_must_fail git push --dry-run testrepo main:frotz main:frotz 2>err &&
test_grep "dst ref refs/heads/frotz receives from more than one src" err
'
-test_expect_failure 'push two refspecs creating the same ref fails on empty repo' '
+test_expect_success 'push two refspecs creating the same ref fails on empty repo' '
mk_empty testrepo &&
test_must_fail git push testrepo main:frotz main:frotz 2>err &&
test_grep "dst ref refs/heads/frotz receives from more than one src" err
'
-test_expect_failure 'push --dry-run abbreviated then full refspec creating the same ref fails on empty repo' '
+test_expect_success 'push --dry-run abbreviated then full refspec creating the same ref fails on empty repo' '
mk_empty testrepo &&
test_must_fail git push --dry-run testrepo main:frotz main:refs/heads/frotz 2>err &&
test_grep "dst ref refs/heads/frotz receives from more than one src" err
'
-test_expect_failure 'push abbreviated then full refspec creating the same ref fails on empty repo' '
+test_expect_success 'push abbreviated then full refspec creating the same ref fails on empty repo' '
mk_empty testrepo &&
test_must_fail git push testrepo main:frotz main:refs/heads/frotz 2>err &&
test_grep "dst ref refs/heads/frotz receives from more than one src" err
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 12/15] remote: use strmap for match_explicit_refs()
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (10 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 11/15] remote: reject duplicate destinations on an empty remote Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 13/15] t/perf: measure --force-with-lease in p5516 Jon Simons
` (3 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Optimize match_explicit_refs() by replacing two linear refs traversals
with strmap lookups.
Before this change, matching R explicit refspecs against N local refs
and M remote refs in match_explicit() entails O(R * N) and O(R * M)
calls to refname_match() for sources and destinations respectively.
After this change, we build a strmap for local refs O(N), a strmap for
remote refs O(M), and use them for O(R * rules) lookups of the refspecs.
Refspecs are resolved using count_refspec_match_in_map() introduced
earlier in the series, which issues one strmap lookup for each candidate
rules pattern generated with expand_ref_prefix().
The match_explicit_lhs_map() function introduced in a previous commit
has now been recombined into match_explicit_lhs(): all call sites are
now using the new strmap variation.
Timings show benefit where one or both of the client and remote have
many refs, and multiple refspecs are specified:
Test HEAD~1 HEAD
-----------------------------------------------------------------------
5516.3: empty:refspecs:1 0.13(0.07+0.10) 0.14(0.07+0.10) +7.7%
5516.5: empty:refspecs:10 0.17(0.10+0.11) 0.14(0.07+0.11) -17.6%
5516.7: empty:refspecs:100 0.47(0.41+0.10) 0.15(0.08+0.10) -68.1%
5516.9: mirror:refspecs:1 0.17(0.10+0.11) 0.17(0.10+0.11) +0.0%
5516.11: mirror:refspecs:10 0.23(0.16+0.11) 0.17(0.10+0.11) -26.1%
5516.13: mirror:refspecs:100 0.87(0.80+0.11) 0.18(0.11+0.11) -79.3%
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
remote.c | 54 +++++++++++++++++++++++++-----------------------------
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/remote.c b/remote.c
index f4cf63b756..8074750601 100644
--- a/remote.c
+++ b/remote.c
@@ -1214,12 +1214,12 @@ static char *guess_ref(const char *name, struct ref *peer)
return strbuf_detach(&buf, NULL);
}
-static int match_explicit_lhs_count(const int count,
- struct refspec_item *rs,
- struct ref **match,
- int *allocated_match)
+static int match_explicit_lhs(struct strmap *src,
+ struct refspec_item *rs,
+ struct ref **match,
+ int *allocated_match)
{
- switch (count) {
+ switch (count_refspec_match_in_map(rs->src, src, match)) {
case 1:
if (allocated_match)
*allocated_match = 0;
@@ -1239,24 +1239,6 @@ static int match_explicit_lhs_count(const int count,
}
}
-static int match_explicit_lhs(struct ref *src,
- struct refspec_item *rs,
- struct ref **match,
- int *allocated_match)
-{
- return match_explicit_lhs_count(count_refspec_match(rs->src, src, match),
- rs, match, allocated_match);
-}
-
-static int match_explicit_lhs_map(struct strmap *src,
- struct refspec_item *rs,
- struct ref **match,
- int *allocated_match)
-{
- return match_explicit_lhs_count(count_refspec_match_in_map(rs->src, src, match),
- rs, match, allocated_match);
-}
-
static void show_push_unqualified_ref_name_error(const char *dst_value,
const char *matched_src_name)
{
@@ -1333,7 +1315,7 @@ static bool any_refspec_item_is_explicit(const struct refspec *rs)
return false;
}
-static int match_explicit(struct ref *src, struct ref *dst,
+static int match_explicit(struct strmap *src, struct strmap *dst,
struct ref ***dst_tail,
struct refspec_item *rs)
{
@@ -1367,7 +1349,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
matched_src->name);
}
- switch (count_refspec_match(dst_value, dst, &matched_dst)) {
+ switch (count_refspec_match_in_map(dst_value, dst, &matched_dst)) {
case 1:
break;
case 0:
@@ -1383,6 +1365,9 @@ static int match_explicit(struct ref *src, struct ref *dst,
show_push_unqualified_ref_name_error(dst_value,
matched_src->name);
}
+ /* later refspecs must see the ref we just added to dst */
+ if (matched_dst)
+ strmap_put(dst, matched_dst->name, matched_dst);
break;
default:
matched_dst = NULL;
@@ -1416,12 +1401,23 @@ static int match_explicit(struct ref *src, struct ref *dst,
return ret;
}
-static int match_explicit_refs(struct ref *src, struct ref **dst,
+static int match_explicit_refs(struct ref *src, struct ref *dst,
struct ref ***dst_tail, struct refspec *rs)
{
int i, errs;
+ struct strmap src_map, dst_map;
+
+ if (!any_refspec_item_is_explicit(rs))
+ return 0;
+
+ ref_map_init(&src_map, src);
+ ref_map_init(&dst_map, dst);
for (i = errs = 0; i < rs->nr; i++)
- errs += match_explicit(src, *dst, dst_tail, &rs->items[i]);
+ errs += match_explicit(&src_map, &dst_map, dst_tail,
+ &rs->items[i]);
+ strmap_clear(&dst_map, 0);
+ strmap_clear(&src_map, 0);
+
return errs;
}
@@ -1644,7 +1640,7 @@ int check_push_refs(struct ref *src, struct refspec *rs)
if (!refspec_item_is_explicit(item))
continue;
- ret |= match_explicit_lhs_map(&src_map, item, NULL, NULL);
+ ret |= match_explicit_lhs(&src_map, item, NULL, NULL);
}
strmap_clear(&src_map, 0);
@@ -1673,7 +1669,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
if (!rs->nr)
refspec_append(rs, ":");
- errs = match_explicit_refs(src, dst, &dst_tail, rs);
+ errs = match_explicit_refs(src, *dst, &dst_tail, rs);
/* pick the remainder */
for (ref = src; ref; ref = ref->next) {
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 13/15] t/perf: measure --force-with-lease in p5516
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (11 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 12/15] remote: use strmap for match_explicit_refs() Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 14/15] remote: restructure apply_push_cas() loops Jon Simons
` (2 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Extend p5516 with rows that push the same delete refspecs plus one
explicit --force-with-lease=<refname>:<expect> per refspec.
The new rows are used to demonstrate a speedup in a subsequent commit.
Numbers as of this commit
(`./p5516-push-delete-refspec.sh -r create,lease`):
Test this tree
-------------------------------------------
5516.16: empty:lease:1 0.12(0.06+0.10)
5516.19: empty:lease:10 0.17(0.10+0.10)
5516.22: empty:lease:100 0.45(0.38+0.10)
5516.25: mirror:lease:1 0.15(0.09+0.10)
5516.28: mirror:lease:10 0.18(0.12+0.10)
5516.31: mirror:lease:100 0.52(0.42+0.10)
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
t/perf/p5516-push-delete-refspec.sh | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/t/perf/p5516-push-delete-refspec.sh b/t/perf/p5516-push-delete-refspec.sh
index 0e425193e5..a1e3f74bf7 100755
--- a/t/perf/p5516-push-delete-refspec.sh
+++ b/t/perf/p5516-push-delete-refspec.sh
@@ -6,6 +6,9 @@ Measure client-side matching of explicit delete refspecs with "git push
--dry-run" against a server that advertises lots of refs. An empty client
(no local refs) and a mirror client (full local copy of the server refs)
are tested pushing 1, 10, and 100 refspecs each.
+
+A second set of rows measures one --force-with-lease per refspec for
+timing those paths.
'
. ./perf-lib.sh
@@ -24,6 +27,8 @@ test_expect_success 'create server with many refs and two clients' '
git -C client_mirror config --unset remote.origin.mirror
'
+oid=$(git -C server rev-parse HEAD)
+
for mode in empty mirror
do
client=client_$mode
@@ -39,4 +44,24 @@ do
done
done
+for mode in empty mirror
+do
+ client=client_$mode
+ for nr_refspecs in 1 10 100
+ do
+ test_expect_success "create $mode lease refspecs: $nr_refspecs" '
+ test_seq -f ":refs/heads/b%d" $nr_refspecs >refspecs
+ '
+
+ test_expect_success "create $mode leases: $nr_refspecs" '
+ test_seq -f "refs/heads/b%d:'"$oid"'" $nr_refspecs |
+ sed "s/^/--force-with-lease=/" >leases
+ '
+
+ test_perf "$mode:lease:$nr_refspecs" '
+ git -C '"$client"' push --dry-run origin $(cat refspecs) $(cat leases)
+ '
+ done
+done
+
test_done
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 14/15] remote: restructure apply_push_cas() loops
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (12 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 13/15] t/perf: measure --force-with-lease in p5516 Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 19:29 ` [PATCH 15/15] remote: use strmap for apply_push_cas() Jon Simons
2026-10-09 20:18 ` [PATCH 00/15] push: speed up client-side refspec matching Kristoffer Haugsbakk
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Restructure the apply_push_cas() loops to prepare for replacing
a linear remote_refs traversal with a strmap lookup.
Before this change, apply_push_cas() loops:
for each advertised remote ref,
apply_cas(ref):
for each explicit lease entry until first match,
refname_match(entry, ref)
else if use_tracking_for_rest,
stamp from tracking
After this change the loops are inverted and apply_cas() is split
into apply_one_cas() (one explicit entry, stamp every matching
still-free ref) and apply_cas_tracking() (shared by the explicit
use_tracking path and use_tracking_for_rest):
for each explicit lease entry,
apply_one_cas(entry):
for each advertised remote ref not yet stamped,
refname_match(entry, ref)
if use_tracking_for_rest,
apply_cas_tracking() on remaining refs
In the next commit, apply_one_cas() is updated to use a strmap.
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
remote.c | 66 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/remote.c b/remote.c
index 8074750601..8dd163038d 100644
--- a/remote.c
+++ b/remote.c
@@ -2978,33 +2978,10 @@ static void check_if_includes_upstream(struct ref *remote)
free_one_ref(local);
}
-static void apply_cas(struct push_cas_option *cas,
- struct remote *remote,
- struct ref *ref)
+static void apply_cas_tracking(struct push_cas_option *cas,
+ struct remote *remote,
+ struct ref *ref)
{
- int i;
-
- /* Find an explicit --<option>=<name>[:<value>] entry */
- for (i = 0; i < cas->nr; i++) {
- struct push_cas *entry = &cas->entry[i];
- if (!refname_match(entry->refname, ref->name))
- continue;
- ref->expect_old_sha1 = 1;
- if (!entry->use_tracking)
- oidcpy(&ref->old_oid_expect, &entry->expect);
- else if (remote_tracking(remote, ref->name,
- &ref->old_oid_expect,
- &ref->tracking_ref))
- oidclr(&ref->old_oid_expect, the_repository->hash_algo);
- else
- ref->check_reachable = cas->use_force_if_includes;
- return;
- }
-
- /* Are we using "--<option>" to cover all? */
- if (!cas->use_tracking_for_rest)
- return;
-
ref->expect_old_sha1 = 1;
if (remote_tracking(remote, ref->name,
&ref->old_oid_expect,
@@ -3014,14 +2991,47 @@ static void apply_cas(struct push_cas_option *cas,
ref->check_reachable = cas->use_force_if_includes;
}
+static void apply_one_cas(struct ref *remote_refs,
+ struct remote *remote,
+ struct push_cas_option *cas,
+ struct push_cas *entry)
+{
+ struct ref *ref;
+
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (ref->expect_old_sha1)
+ continue;
+ if (!refname_match(entry->refname, ref->name))
+ continue;
+ if (entry->use_tracking) {
+ apply_cas_tracking(cas, remote, ref);
+ } else {
+ ref->expect_old_sha1 = 1;
+ oidcpy(&ref->old_oid_expect, &entry->expect);
+ }
+ }
+}
+
void apply_push_cas(struct push_cas_option *cas,
struct remote *remote,
struct ref *remote_refs)
{
struct ref *ref;
- for (ref = remote_refs; ref; ref = ref->next) {
- apply_cas(cas, remote, ref);
+ /* Apply each explicit --<option>=<name>[:<value>] entry */
+ for (size_t i = 0; i < cas->nr; i++)
+ apply_one_cas(remote_refs, remote, cas, &cas->entry[i]);
+
+ /* Are we using "--<option>" to cover all? */
+ if (cas->use_tracking_for_rest) {
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (ref->expect_old_sha1)
+ continue;
+ apply_cas_tracking(cas, remote, ref);
+ }
+ }
+
+ for (ref = remote_refs; ref; ref = ref->next) {
/*
* If "compare-and-swap" is in "use_tracking[_for_rest]"
* mode, and if "--force-if-includes" was specified, run
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 15/15] remote: use strmap for apply_push_cas()
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (13 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 14/15] remote: restructure apply_push_cas() loops Jon Simons
@ 2026-10-09 19:29 ` Jon Simons
2026-10-09 20:18 ` [PATCH 00/15] push: speed up client-side refspec matching Kristoffer Haugsbakk
15 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-09 19:29 UTC (permalink / raw)
To: git; +Cc: Jon Simons
Optimize apply_push_cas() by replacing a linear traversal of all
advertised remote refs with strmap lookups.
Before this change, applying N explicit --force-with-lease entries
to M remote advertised refs entails O(N * M) calls to refname_match()
in apply_one_cas().
After this change, we build a strmap of remote refs O(M), and use it
for O(N * rules) lookups of the lease entry refnames.
The refnames are resolved by issuing one strmap lookup for each
candidate rules pattern generated with expand_ref_prefix(), as
with earlier strmap conversions.
p5516 timings show the speedup:
Test HEAD~1 HEAD
--------------------------------------------------------------------
5516.16: empty:lease:1 0.13(0.07+0.10) 0.14(0.08+0.11) +7.7%
5516.19: empty:lease:10 0.16(0.10+0.10) 0.14(0.08+0.11) -12.5%
5516.22: empty:lease:100 0.45(0.39+0.10) 0.16(0.09+0.11) -64.4%
5516.25: mirror:lease:1 0.16(0.09+0.11) 0.17(0.10+0.12) +6.3%
5516.28: mirror:lease:10 0.19(0.12+0.10) 0.17(0.10+0.11) -10.5%
5516.31: mirror:lease:100 0.49(0.42+0.12) 0.19(0.11+0.12) -61.2%
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
remote.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/remote.c b/remote.c
index 8dd163038d..c16f1a2b82 100644
--- a/remote.c
+++ b/remote.c
@@ -2991,17 +2991,17 @@ static void apply_cas_tracking(struct push_cas_option *cas,
ref->check_reachable = cas->use_force_if_includes;
}
-static void apply_one_cas(struct ref *remote_refs,
+static void apply_one_cas(struct strmap *ref_map,
struct remote *remote,
struct push_cas_option *cas,
struct push_cas *entry)
{
- struct ref *ref;
+ struct strvec names = STRVEC_INIT;
- for (ref = remote_refs; ref; ref = ref->next) {
- if (ref->expect_old_sha1)
- continue;
- if (!refname_match(entry->refname, ref->name))
+ expand_ref_prefix(&names, entry->refname);
+ for (size_t i = 0; i < names.nr; i++) {
+ struct ref *ref = strmap_get(ref_map, names.v[i]);
+ if (!ref || ref->expect_old_sha1)
continue;
if (entry->use_tracking) {
apply_cas_tracking(cas, remote, ref);
@@ -3010,6 +3010,7 @@ static void apply_one_cas(struct ref *remote_refs,
oidcpy(&ref->old_oid_expect, &entry->expect);
}
}
+ strvec_clear(&names);
}
void apply_push_cas(struct push_cas_option *cas,
@@ -3019,8 +3020,14 @@ void apply_push_cas(struct push_cas_option *cas,
struct ref *ref;
/* Apply each explicit --<option>=<name>[:<value>] entry */
- for (size_t i = 0; i < cas->nr; i++)
- apply_one_cas(remote_refs, remote, cas, &cas->entry[i]);
+ if (cas->nr) {
+ struct strmap ref_map;
+
+ ref_map_init(&ref_map, remote_refs);
+ for (size_t i = 0; i < cas->nr; i++)
+ apply_one_cas(&ref_map, remote, cas, &cas->entry[i]);
+ strmap_clear(&ref_map, 0);
+ }
/* Are we using "--<option>" to cover all? */
if (cas->use_tracking_for_rest) {
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 00/15] push: speed up client-side refspec matching
2026-10-09 19:29 [PATCH 00/15] push: speed up client-side refspec matching Jon Simons
` (14 preceding siblings ...)
2026-10-09 19:29 ` [PATCH 15/15] remote: use strmap for apply_push_cas() Jon Simons
@ 2026-10-09 20:18 ` Kristoffer Haugsbakk
2026-10-11 2:35 ` Jon Simons
15 siblings, 1 reply; 18+ messages in thread
From: Kristoffer Haugsbakk @ 2026-10-09 20:18 UTC (permalink / raw)
To: Jon Simons, git
On Fri, Oct 9, 2026, at 21:29, Jon Simons wrote:
> Guide to changes:
>
> - Commits are ordered to introduce failing tests for behavioral
> changes and bugfixes, before the code change that fixes them
This project prefers introducing the
regression test and fix in the same commit.
--
Sent from mobile
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 00/15] push: speed up client-side refspec matching
2026-10-09 20:18 ` [PATCH 00/15] push: speed up client-side refspec matching Kristoffer Haugsbakk
@ 2026-10-11 2:35 ` Jon Simons
0 siblings, 0 replies; 18+ messages in thread
From: Jon Simons @ 2026-10-11 2:35 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git
On 10/9/26 4:18 PM, Kristoffer Haugsbakk wrote:
> On Fri, Oct 9, 2026, at 21:29, Jon Simons wrote:
>> Guide to changes:
>>
>> - Commits are ordered to introduce failing tests for behavioral
>> changes and bugfixes, before the code change that fixes them
>
> This project prefers introducing the
> regression test and fix in the same commit.
>
Thanks, I did not know this, and in searching I now find that this is a
common review feedback [1]. And the commit history also shows that it
is uncommon for a series to be merged that introduces a failing test
then fix in a subsequent commit.
I'll look at squashing the commits that introduce new test_expect_failure
cases up into the commits that toggles them passing, for v2, after first
letting the v1 soak for comments.
-Jon
[1] A couple examples of feedback and rationale to not to do this from
August 2025 here:
- https://lore.kernel.org/git/xmqqqzxpzo8b.fsf@gitster.g/
- https://lore.kernel.org/git/xmqqfrdk3aqy.fsf@gitster.g/
^ permalink raw reply [flat|nested] 18+ messages in thread