* [PATCH v2 2/3] read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
From: Tian Yuchen @ 2026-06-10 9:36 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260610093635.139719-1-cat@malon.dev>
The ce_mode_from_stat() function is declared as a static inline function
in 'read-cache.h'. As we want to migrate configuration variables, this
helper function will need access to corresponding repository-specific
configuration logic. Move the implementation to 'read-cache.c' to
cleanly encapsulate its dependencies.
Note that the 'extern int trust_executable_bit, has_symlinks;' line is
discarded because it's not necessary when the function lives in
"read-cache.c".
At present, this change has no visible impact, but it is crucial
for our future plans to pass in the repo context. Comment
has been added whilst we are at it.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
read-cache.c | 20 ++++++++++++++++++++
read-cache.h | 16 ++--------------
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index c44e4d128f..cb4f4878c8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -202,6 +202,26 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
}
}
+/*
+ * Determine the appropriate index mode for a file based on its stat()
+ * information and the existing cache entry (if any).
+ *
+ * This function handles degradation for filesystems that lack
+ * symlink support or reliable executable bits.
+ */
+unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
+{
+ if (!has_symlinks && S_ISREG(mode) &&
+ ce && S_ISLNK(ce->ce_mode))
+ return ce->ce_mode;
+ if (!trust_executable_bit && S_ISREG(mode)) {
+ if (ce && S_ISREG(ce->ce_mode))
+ return ce->ce_mode;
+ return create_ce_mode(0666);
+ }
+ return create_ce_mode(mode);
+}
+
static unsigned int st_mode_from_ce(const struct cache_entry *ce)
{
switch (ce->ce_mode & S_IFMT) {
diff --git a/read-cache.h b/read-cache.h
index 043da1f1aa..3c4af2faeb 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -5,20 +5,8 @@
#include "object.h"
#include "pathspec.h"
-static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
- unsigned int mode)
-{
- extern int trust_executable_bit, has_symlinks;
- if (!has_symlinks && S_ISREG(mode) &&
- ce && S_ISLNK(ce->ce_mode))
- return ce->ce_mode;
- if (!trust_executable_bit && S_ISREG(mode)) {
- if (ce && S_ISREG(ce->ce_mode))
- return ce->ce_mode;
- return create_ce_mode(0666);
- }
- return create_ce_mode(mode);
-}
+unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+ unsigned int mode);
static inline int ce_to_dtype(const struct cache_entry *ce)
{
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/3] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-06-10 9:36 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260610093635.139719-1-cat@malon.dev>
Move the global 'trust_executable_bit' configurations
into the repository-specific 'repo_config_values'
struct. To ensure code readability, the getter functions
'repo_trust_executable_bit()' has been introduced.
For now, associated functions access this configuration by
explicitly falling back to 'the_repository'.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
apply.c | 2 +-
environment.c | 11 +++++++++--
environment.h | 9 ++++++++-
read-cache.c | 8 ++++----
4 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..fbb907d3c0 100644
--- a/apply.c
+++ b/apply.c
@@ -3893,7 +3893,7 @@ static int check_preimage(struct apply_state *state,
if (*ce && !(*ce)->ce_mode)
BUG("ce_mode == 0 for path '%s'", old_name);
- if (trust_executable_bit || !S_ISREG(st->st_mode))
+ if (repo_trust_executable_bit(the_repository) || !S_ISREG(st->st_mode))
st_mode = ce_mode_from_stat(*ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..75069a884d 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,6 @@
static int pack_compression_seen;
static int zlib_compression_seen;
-int trust_executable_bit = 1;
int trust_ctime = 1;
int check_stat = 1;
int has_symlinks = 1;
@@ -142,6 +141,13 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+int repo_trust_executable_bit(struct repository *repo)
+{
+ return repo->gitdir?
+ repo_config_values(repo)->trust_executable_bit :
+ 1;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -305,7 +311,7 @@ int git_default_core_config(const char *var, const char *value,
/* This needs a better name */
if (!strcmp(var, "core.filemode")) {
- trust_executable_bit = git_config_bool(var, value);
+ cfg->trust_executable_bit = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.trustctime")) {
@@ -720,5 +726,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
+ cfg->trust_executable_bit = 1;
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 123a71cdc8..44b97be654 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
int apply_sparse_checkout;
+ int trust_executable_bit;
/* section "branch" config values */
enum branch_track branch_track;
@@ -123,6 +124,13 @@ int git_default_config(const char *, const char *,
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb);
+/*
+ * Getters for the `repo_trust_executable_bit` fields of `struct repo_config_values`.
+ * They check `repo->gitdir` to prevent calling repo_config_values()
+ * before the configuration is loaded or in bare environments.
+ */
+int repo_trust_executable_bit(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -160,7 +168,6 @@ int is_bare_repository(void);
extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
-extern int trust_executable_bit;
extern int trust_ctime;
extern int check_stat;
extern int has_symlinks;
diff --git a/read-cache.c b/read-cache.c
index cb4f4878c8..89f5c88c58 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -214,7 +214,7 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
if (!has_symlinks && S_ISREG(mode) &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
- if (!trust_executable_bit && S_ISREG(mode)) {
+ if (!repo_trust_executable_bit(the_repository) && S_ISREG(mode)) {
if (ce && S_ISREG(ce->ce_mode))
return ce->ce_mode;
return create_ce_mode(0666);
@@ -228,7 +228,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
case S_IFLNK:
return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
case S_IFREG:
- return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
+ return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
case S_IFGITLINK:
return S_IFDIR | 0755;
case S_IFDIR:
@@ -338,7 +338,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
/* We consider only the owner x bit to be relevant for
* "mode changes"
*/
- if (trust_executable_bit &&
+ if (repo_trust_executable_bit(the_repository) &&
(0100 & (ce->ce_mode ^ st->st_mode)))
changed |= MODE_CHANGED;
break;
@@ -759,7 +759,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (trust_executable_bit && has_symlinks) {
+ if (repo_trust_executable_bit(the_repository) && has_symlinks) {
ce->ce_mode = create_ce_mode(st_mode);
} else {
/* If there is an existing entry, pick the mode bits and type
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] ref-filter: restore prefix-scoped iteration
From: Karthik Nayak @ 2026-06-10 10:50 UTC (permalink / raw)
To: Tamir Duberstein, git
Cc: Patrick Steinhardt, Junio C Hamano, Victoria Dye, ZheNing Hu
In-Reply-To: <20260608-fix-git-branch-regression-v2-1-fd82075a8520@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8571 bytes --]
Tamir Duberstein <tamird@gmail.com> writes:
> Commit dabecb9db2 (for-each-ref: introduce a '--start-after' option,
> 2025-07-15) changed single-kind branch, remote-tracking branch, and tag
> enumeration in do_filter_refs() from constructing an iterator with the
> namespace prefix to constructing an unscoped iterator and applying the
> prefix with ref_iterator_seek().
>
> Before that change, refs_for_each_fullref_in() passed the namespace
> prefix during iterator construction. That helper has since been
> replaced by refs_for_each_ref_ext().
>
> The files backend primes its loose-ref cache for the construction
> prefix before it opens packed refs. An empty construction prefix
> therefore reads every loose ref, and a later seek cannot undo that I/O.
> Consequently, git branch, git branch --remotes, and git tag scale with
> unrelated loose refs.
>
And this is the crux of the issue. Currently we do
- refs_ref_iterator_begin()
- ref_iterator_seek()
And between the two `cache_ref_iterator_set_prefix()` is already called
which caches all the loose refs. This is the IO intensive operation this
patch tries to avoid.
I think it would be worthwhile to add this information in the commit
message.
>
> Patrick Steinhardt observed during review that iterator construction
> and seeking accepted similar strings but assigned them different state
> semantics. Junio C Hamano then pointed out that no current command can
> combine start_after with this single-kind path, but future branch or
> tag support would need to keep the namespace while moving the cursor.
>
> Keep the existing start_after path unchanged. The iterator API cannot
> currently seek to one string while retaining another as its prefix:
> an unflagged seek clears the prefix, while REF_ITERATOR_SEEK_SET_PREFIX
> replaces it with the seek string.
>
> For the commands affected by this regression, which do not set
> start_after, pass the namespace prefix during iterator construction so
> that loose refs are scoped before the packed-refs snapshot is opened.
> This fixes the current regression without deleting the ref-filter state
> discussed during review or changing its dormant behavior.
>
> Add REFFILES-gated performance cases with one branch, one
> remote-tracking branch, one tag, and 10,000 unrelated loose refs. The
> benchmarks were run with:
>
> GIT_PERF_REPEAT_COUNT=5 GIT_PERF_MAKE_OPTS=-j8 \
> t/perf/run a89346e34a . -- p6300-for-each-ref.sh
>
> The following are the best of five runs, with each run invoking the
> command ten times. Times are elapsed seconds with user and system CPU
> seconds in parentheses:
>
> a89346e34a this commit
> branch 2.74(0.13+2.56) 0.11(0.04+0.04)
> branch --remotes 2.81(0.13+2.62) 0.12(0.04+0.04)
> tag 3.01(0.14+2.82) 0.11(0.04+0.04)
>
> Both revisions used the default -O2 build flags and a config.mak
> containing only "NO_REGEX = NeedsStartEnd". They were built with Apple
> clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro (Mac16,6)
> with a 16-core Apple M4 Max (12 performance and four efficiency cores)
> and 128 GB RAM.
>
> Link: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
> Link: https://lore.kernel.org/git/xmqqikjq7s16.fsf@gitster.g/
> Fixes: dabecb9db2b2 ("for-each-ref: introduce a '--start-after' option")
> Assisted-by: Codex gpt-5.5
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> The series is based on a89346e34a (maint) because the regression has
> been present in released versions since Git 2.51.0.
> ---
> Changes in v2:
> - Extract local variable `store`.
> - Link to v1: https://patch.msgid.link/20260605-fix-git-branch-regression-v1-1-02f40ad40929@gmail.com
> ---
> ref-filter.c | 28 +++++++++++++++++++---------
> t/perf/p6300-for-each-ref.sh | 39 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 57 insertions(+), 10 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 1da4c0e60d..5cbc007d64 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -3315,19 +3315,29 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
> prefix = "refs/tags/";
>
> if (prefix) {
> - struct ref_iterator *iter;
> + struct ref_store *store = get_main_ref_store(the_repository);
>
> - iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
> - "", NULL, 0, 0);
> + if (filter->start_after) {
> + struct ref_iterator *iter;
> +
> + iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
>
> - if (filter->start_after)
> ret = start_ref_iterator_after(iter, filter->start_after);
> - else
> - ret = ref_iterator_seek(iter, prefix,
> - REF_ITERATOR_SEEK_SET_PREFIX);
> + if (!ret)
> + ret = do_for_each_ref_iterator(iter, fn,
> + cb_data);
> + } else {
> + /*
> + * Pass the prefix during construction because the files
> + * backend primes loose refs before a later seek can
> + * narrow the iterator.
> + */
> + struct refs_for_each_ref_options opts = {
> + .prefix = prefix,
> + };
>
> - if (!ret)
> - ret = do_for_each_ref_iterator(iter, fn, cb_data);
> + ret = refs_for_each_ref_ext(store, fn, cb_data, &opts);
> + }
This would work, as now we separate out the regular path to use
`do_for_each_ref_iterator()` instead.
But this causes a bit of confusion, why do we need to use
`do_for_each_ref_iterator()` and why not simply provide the prefix to
`refs_ref_iterator_begin()`, like before?
On top of master, the below diff seems to fix the issue and works with
the benchmarks provided in this patch. (I haven't tested it with out
test suite though).
modified ref-filter.c
@@ -3316,15 +3316,16 @@ static int do_filter_refs(struct ref_filter
*filter, unsigned int type, refs_for
if (prefix) {
struct ref_iterator *iter;
+ struct ref_store *store;
- iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
- "", NULL, 0, 0);
+ store = get_main_ref_store(the_repository);
- if (filter->start_after)
+ if (filter->start_after) {
+ iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
ret = start_ref_iterator_after(iter, filter->start_after);
- else
- ret = ref_iterator_seek(iter, prefix,
- REF_ITERATOR_SEEK_SET_PREFIX);
+ } else {
+ iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0);
+ }
if (!ret)
ret = do_for_each_ref_iterator(iter, fn, cb_data);
I would say something like this would make more sense, since it still
keeps the current structure without introducing a new command.
> } else if (filter->kind & FILTER_REFS_REGULAR) {
> ret = for_each_fullref_in_pattern(filter, fn, cb_data);
> }
> diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
> index fa7289c752..ed9c1c6a19 100755
> --- a/t/perf/p6300-for-each-ref.sh
> +++ b/t/perf/p6300-for-each-ref.sh
> @@ -1,6 +1,6 @@
> #!/bin/sh
>
> -test_description='performance of for-each-ref'
> +test_description='performance of ref-filter users'
> . ./perf-lib.sh
>
> test_perf_fresh_repo
> @@ -84,4 +84,41 @@ test_expect_success 'pack refs' '
> '
> run_tests "packed"
>
> +test_expect_success REFFILES 'setup many unrelated loose refs' '
> + git init scoped &&
> + test_commit -C scoped --no-tag base &&
> + test_seq $ref_count_per_type |
> + sed "s,.*,update refs/custom/unrelated_& HEAD," |
> + git -C scoped update-ref --stdin &&
> + git -C scoped update-ref refs/remotes/origin/main HEAD &&
> + git -C scoped update-ref refs/tags/only HEAD
> +'
> +
> +test_perf "branch (many unrelated loose refs)" --prereq REFFILES "
> + (
> + cd scoped &&
> + for i in \$(test_seq $test_iteration_count); do
> + git branch --format='%(refname)' >/dev/null
> + done
> + )
> +"
> +
> +test_perf "branch --remotes (many unrelated loose refs)" --prereq REFFILES "
> + (
> + cd scoped &&
> + for i in \$(test_seq $test_iteration_count); do
> + git branch --remotes --format='%(refname)' >/dev/null
> + done
> + )
> +"
> +
> +test_perf "tag (many unrelated loose refs)" --prereq REFFILES "
> + (
> + cd scoped &&
> + for i in \$(test_seq $test_iteration_count); do
> + git tag --format='%(refname)' >/dev/null
> + done
> + )
> +"
> +
> test_done
>
> ---
> base-commit: a89346e34a937f001e5d397ee62224e3e9852040
> change-id: 20260605-fix-git-branch-regression-9e4236f18091
>
> Best regards,
> --
> Tamir Duberstein <tamird@gmail.com>
Thanks for the patch, this is indeed a regression we must fix and the
benchmarks are a clear indication of it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [BUG] rebase --update-refs emits unqualified "update-ref HEAD" into the todo
From: betel_taxis4h @ 2026-06-10 11:00 UTC (permalink / raw)
To: git
What did you do before the bug happened? (Steps to reproduce your issue)
With rebase.updateRefs=true, an interactive rebase of the checked-out branch generates a todo containing the literal line "update-ref HEAD”, which git's own todo parser then rejects.
Minimal reproduction (plain repo, no worktrees, no remotes required):
git init -b main repro && cd repro
git -c user.email=t@t.t -c user.name=t commit --allow-empty -m base
git checkout -b feat
git -c user.email=t@t.t -c user.name=t commit --allow-empty -m c1
git -c user.email=t@t.t -c user.name=t commit --allow-empty -m c2
git -c rebase.updateRefs=true rebase -i feat~2
The generated todo contains:
pick <c1> c1
pick <c2> c2
update-ref HEAD <-- emitted for HEAD, a symref to the branch being rebased
update-ref refs/heads/feat (correctly placed; this one is fine)
Letting the editor save the auto-generated todo verbatim (or running `git rebase --continue`) fails immediately with:
error: update-ref requires a fully qualified refname e.g. refs/heads/HEAD
error: invalid line 3: update-ref HEAD
You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.
What did you expect to happen? (Expected behavior)
--update-refs should not emit an "update-ref HEAD" line. HEAD is a symbolic alias of the branch being rebased; the branch ref itself is (correctly) excluded from the update-ref set, so its HEAD alias should be excluded too. The todo should contain only fully-qualified refs/heads/... lines.
What happened instead? (Actual behavior)
git emits a todo line ("update-ref HEAD") that its own sequencer parser rejects as not fully qualified, breaking the rebase. The only recovery is `git rebase --edit-todo` to manually delete the line.
What's different between what you expected and what actually happened?
git generated a todo command it refuses to execute. The unqualified "HEAD" should either be expanded to its target ref or omitted entirely.
Anything else you want to add:
- Reproduces identically in a plain single-worktree repo and in a bare-repo + linked-worktree layout, so it is not worktree-specific.
- An in-sync remote-tracking ref (origin/feat) on the tip adds a second, valid "update-ref refs/remotes/origin/feat" line but is not required to trigger the fatal "update-ref HEAD".
- Workaround: unset rebase.updateRefs (or pass -c rebase.updateRefs=false), or delete the "update-ref HEAD" line via `git rebase --edit-todo`.
[System Info]
git version:
git version 2.54.0
cpu: aarch64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
rust: disabled
gettext: enabled
libcurl: 8.14.1
OpenSSL: OpenSSL 3.5.6 7 Apr 2026
zlib: 1.3.1
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
uname: Linux 7.0.11-orbstack-00360-gc9bc4d96ac70 #1 SMP PREEMPT Thu Jun 4 16:40:25 UTC 2026 aarch64
compiler info: gnuc: 14.2
libc info: glibc: 2.41
$SHELL (typically, interactive shell): /usr/bin/zsh
^ permalink raw reply
* Re: [PATCH v3 1/3] MyFirstContribution: recommend shallow threading of cover letters
From: Karthik Nayak @ 2026-06-10 11:08 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones,
SZEDER Gábor, Kristoffer Haugsbakk, Toon Claes
In-Reply-To: <20260608-pks-b4-v3-1-f5e497d10c56@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3654 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> The "MyFirstContribution" document recommends the use of deep threading
> of cover letters: every cover letter of subsequent iterations shall be
> linked to the cover letter of the preceding version. The result of this
> is that eventually, threads with many versions are getting nested so
> deep that it becomes hard to follow.
>
> Adapt the recommendation to instead propose shallow threading of cover
> letters: instead of linking the cover letter to the previous cover
> letter, the user is supposed to always link it to the first cover
> letter. This still makes it easy to follow the iterations, but has the
> benefit of nesting to a much shallower level.
Should we also modify 'Documentation/SubmittingPatches'? Which states:
All subsequent versions of a patch series and other related patches
should be grouped into their own e-mail thread to help readers find
all parts of the series. To that end, send them as replies to either
an additional "cover letter" message (see below), the first patch, or
the respective preceding patch. Here is a
link:MyFirstContribution.html#v2-git-send-email[step-by-step guide] on
how to submit updated versions of a patch series.
Personally, I find it a bit awkward when new versions are sent as a new
separate thread, especially when the subject is changed over versions.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Documentation/MyFirstContribution.adoc | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index b9fdefce02..984b7f5aa8 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -790,7 +790,7 @@ We can note a few things:
> v3", etc. in place of "PATCH". For example, "[PATCH v2 1/3]" would be the first of
> three patches in the second iteration. Each iteration is sent with a new cover
> letter (like "[PATCH v2 0/3]" above), itself a reply to the cover letter of the
> - previous iteration (more on that below).
> + first iteration (more on that below).
>
> NOTE: A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without
> _i_/_n_ numbering (in the above thread overview, no single-patch topic appears,
> @@ -1214,7 +1214,7 @@ between your last version and now, if it's something significant. You do not
> need the exact same body in your second cover letter; focus on explaining to
> reviewers the changes you've made that may not be as visible.
>
> -You will also need to go and find the Message-ID of your previous cover letter.
> +You will also need to go and find the Message-ID of your first cover letter.
> You can either note it when you send the first series, from the output of `git
> send-email`, or you can look it up on the
> https://lore.kernel.org/git[mailing list]. Find your cover letter in the
> @@ -1227,8 +1227,8 @@ Message-ID: <foo.12345.author@example.com>
>
> Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
> below as well; make sure to replace it with the correct Message-ID for your
> -**previous cover letter** - that is, if you're sending v2, use the Message-ID
> -from v1; if you're sending v3, use the Message-ID from v2.
> +**first cover letter** - that is, for any subsequent version that you send,
> +always use the Message-ID from v1.
>
> While you're looking at the email, you should also note who is CC'd, as it's
> common practice in the mailing list to keep all CCs on a thread. You can add
>
> --
> 2.54.0.1136.gdb2ca164c4.dirty
The patch looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/3] b4: introduce configuration for the Git project
From: Karthik Nayak @ 2026-06-10 11:13 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones,
SZEDER Gábor, Kristoffer Haugsbakk, Toon Claes
In-Reply-To: <20260608-pks-b4-v3-3-f5e497d10c56@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> We're about to extend our documentation to recommend b4 for sending
Nit: This is in the past now
> patch series to the mailing list. Prepare for this by introducing a b4
> configuration so that the tool knows to honor our preferences. For now,
> this configuration does two things:
>
> - It configures "send-same-thread = shallow", which tells b4 to always
> send subsequent versions of the same patch series as a reply to the
> cover letter of the first version.
>
> - It configures "prep-cover-template", which tells b4 to use a custom
> template for the cover letter. The most important change compared to
> the default template is that our custom template also includes a
> range-diff.
>
> There's potentially more things that we may want to configure going
> forward, like for example auto-configuration of folks to Cc on certain
> patches. But these two tweaks feel like a good place to start.
>
> Note that these values only serve as defaults, and users may want to
> tweak those defaults based on their own preference. Luckily, users can
> do that without having to touch `.b4-config` at all, as b4 allows them
> to override values via Git configuration:
>
> ```
> $ git config set b4.prep-cover-template /does/not/exist
> $ b4 send --dry-run
> ERROR: prep-cover-template says to use x, but it does not exist
> ```
>
> So this gives users an easy way to override our defaults without having
> to touch ".b4-config", which would dirty the tree.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> .b4-config | 6 ++++++
> .b4-cover-template | 11 +++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/.b4-config b/.b4-config
> new file mode 100644
> index 0000000000..fd4fb56b6d
> --- /dev/null
> +++ b/.b4-config
> @@ -0,0 +1,6 @@
> +# Note that these are default values that you can tweak via the typical
> +# git-config(1) machinery. You thus shouldn't ever have to change this file.
> +# See also https://b4.docs.kernel.org/en/latest/config.html.
> +[b4]
> +send-same-thread = shallow
> +prep-cover-template = ./.b4-cover-template
> diff --git a/.b4-cover-template b/.b4-cover-template
> new file mode 100644
> index 0000000000..ab864933b5
> --- /dev/null
> +++ b/.b4-cover-template
> @@ -0,0 +1,11 @@
> +${cover}
> +
> +---
> +${shortlog}
> +
> +${diffstat}
> +
> +${range_diff}
> +---
> +base-commit: ${base_commit}
> +${prerequisites}
>
This looks similar to what I have locally too, happy to see this land.
> --
> 2.54.0.1136.gdb2ca164c4.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] ref-filter: memoize --contains with generations
From: Karthik Nayak @ 2026-06-10 11:47 UTC (permalink / raw)
To: Tamir Duberstein, git
Cc: Jeff King, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren
In-Reply-To: <20260608-ref-filter-memoized-contains-v2-2-e72720344a7c@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4109 bytes --]
Tamir Duberstein <tamird@gmail.com> writes:
> git branch and git for-each-ref call repo_is_descendant_of() for
> each candidate selected by --contains or --no-contains. Each call
> starts a new graph walk, so refs with shared history repeatedly
> traverse the same commits.
>
> ffc4b8012d (tag: speed up --contains calculation, 2011-06-11)
> introduced a depth-first walk for git tag that caches positive and
> negative answers across candidates. ee2bd06b0f (ref-filter: implement
> '--contains' option, 2015-07-07) preserved both implementations when
> ref-filter learned --contains.
>
> The memoized walk is not always faster. Without generation numbers,
> a negative check can walk to the root even when the breadth-first
> merge-base walk finds a nearby divergence. With generation numbers,
> the depth-first walk can stop below the oldest target while still
> reusing answers across candidates.
>
> Keep the existing memoized selection for git tag. Select it for other
> ref-filter callers when generation numbers are enabled, and retain
> the breadth-first walk otherwise.
>
> When generation numbers are unavailable, repo_is_descendant_of() can
> return -1 if ancestry cannot be read. The ref-filter Boolean interface
> treated that error as a match. Check it and exit instead. The memoized
> path already dies on the same parse failure, so both selected paths now
> fail rather than return a result.
>
> Add p1500 cases for up to 8,192 packed refs along one first-parent
> history and for sibling refs near the tip with generation numbers
> forced off.
>
> On a checkout with 62,174 remote-tracking refs and generation numbers
> enabled, I ran:
>
> hyperfine --warmup 0 --runs 3 \
> --command-name parent \
> '"$parent" branch -r --contains c78ae85f3ce7e >/dev/null' \
> --command-name this-commit \
> '"$this" branch -r --contains c78ae85f3ce7e >/dev/null'
>
> The results were:
>
> parent this commit
> elapsed 104.365 s 467.7 ms
> user 93.702 s 220.2 ms
> system 0.723 s 182.7 ms
>
> The wall-time standard deviations were 11.356 seconds and 133.8
> milliseconds, respectively. Separate runs without redirection produced
> the same output with SHA-256
> 2466f6e2b72aa16b1a2126eddb81c8a1b2764ee251204ac034c191a925aa896f.
>
> Both revisions were built with the default -O2 flags using Apple
> clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro (Mac16,6)
> with a 16-core Apple M4 Max (12 performance and four efficiency
> cores) and 128 GB RAM.
>
> Link: https://lore.kernel.org/git/1445163904-24611-1-git-send-email-Karthik.188@gmail.com/
> Link: https://lore.kernel.org/r/20230324191009.GA536967@coredump.intra.peff.net
> Link: https://lore.kernel.org/git/20260527070510.3510836-1-krka@spotify.com/
> Link: https://lore.kernel.org/r/20260608223430.GA340696@coredump.intra.peff.net
> Suggested-by: Jeff King <peff@peff.net>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> commit-reach.c | 13 +++++++++--
> commit-reach.h | 7 ++++++
> t/perf/p1500-graph-walks.sh | 49 +++++++++++++++++++++++++++++++++++++++++-
> t/t6301-for-each-ref-errors.sh | 22 +++++++++++++++++++
> 4 files changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/commit-reach.c b/commit-reach.c
> index 65b618959b..83a48004ef 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -821,9 +821,18 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
> int commit_contains(struct ref_filter *filter, struct commit *commit,
> struct commit_list *list, struct contains_cache *cache)
> {
> - if (filter->with_commit_tag_algo)
> + int result;
> +
> + if (!list)
> + return 1;
> + if (filter->with_commit_tag_algo ||
> + generation_numbers_enabled(the_repository))
What's stopping us from dropping `filter->with_commit_tag_algo`
completely and then doing?
if (generation_numbers_enabled(the_repository))
return contains_algo(commit, list, cache) == CONTAINS_YES;
return repo_is_descendant_of(the_repository, commit, list);
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [GSoC PATCH v2 3/4] repo: add path.gitdir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-10 12:11 UTC (permalink / raw)
To: Justin Tobler
Cc: git, a3205153416, gitster, kumarayushjha123, lucasseikioshiro,
phillip.wood, sandals
In-Reply-To: <aighAZXRtLaz6sg8@denethor>
>
> Intercepting PATH_FORMAT_DEFAULT in print_path() and overriding it to
> the appropriate default needed for the specific path printed by
> git-rev-parse(1), as shown above, seems reasonable to me.
>
> But I do think that PATH_FORMAT_DEFAULT should have an actual default in
> format_path(). Otherwise we would have an enum value that requires
> callers to explicitly handle prior to invoking format_path() which would
> also be rather awkward. IMO, it probably wouldn't be a big deal to just
> say PATH_FORMAT_DEFAULT is treated as PATH_FORMAT_UNMODIFIED when passed
> to format_path() and document it. In practice, our rev-parse use-case
> would always replace PATH_FORMAT_DEFAULT with the appropriate value
> prior to invoking format_path().
>
Makes sense. I'll have format_path() treat PATH_FORMAT_DEFAULT as
PATH_FORMAT_UNMODIFIED
internally and document it, so there's no unhandled enum value.
print_path() in rev-parse will still intercept PATH_FORMAT_DEFAULT
and replace it with the path-specific default before calling format_path(),
so in practice it won't fall through to the unmodified behavior.
Almost done with v3 this was the only one left.
Thanks for taking time!
^ permalink raw reply
* Re: [PATCH v2 2/2] ref-filter: memoize --contains with generations
From: Tamir Duberstein @ 2026-06-10 12:20 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, Jeff King, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren
In-Reply-To: <CAOLa=ZRFSuGrqFXhTuQ7Dk5GCQQGHom++78xwONoiNdt1h_gWQ@mail.gmail.com>
On Wed, Jun 10, 2026 at 4:47 AM Karthik Nayak <karthik.188@gmail.com> wrote:
>
> Tamir Duberstein <tamird@gmail.com> writes:
>
> > git branch and git for-each-ref call repo_is_descendant_of() for
> > each candidate selected by --contains or --no-contains. Each call
> > starts a new graph walk, so refs with shared history repeatedly
> > traverse the same commits.
> >
> > ffc4b8012d (tag: speed up --contains calculation, 2011-06-11)
> > introduced a depth-first walk for git tag that caches positive and
> > negative answers across candidates. ee2bd06b0f (ref-filter: implement
> > '--contains' option, 2015-07-07) preserved both implementations when
> > ref-filter learned --contains.
> >
> > The memoized walk is not always faster. Without generation numbers,
> > a negative check can walk to the root even when the breadth-first
> > merge-base walk finds a nearby divergence. With generation numbers,
> > the depth-first walk can stop below the oldest target while still
> > reusing answers across candidates.
> >
> > Keep the existing memoized selection for git tag. Select it for other
> > ref-filter callers when generation numbers are enabled, and retain
> > the breadth-first walk otherwise.
> >
> > When generation numbers are unavailable, repo_is_descendant_of() can
> > return -1 if ancestry cannot be read. The ref-filter Boolean interface
> > treated that error as a match. Check it and exit instead. The memoized
> > path already dies on the same parse failure, so both selected paths now
> > fail rather than return a result.
> >
> > Add p1500 cases for up to 8,192 packed refs along one first-parent
> > history and for sibling refs near the tip with generation numbers
> > forced off.
> >
> > On a checkout with 62,174 remote-tracking refs and generation numbers
> > enabled, I ran:
> >
> > hyperfine --warmup 0 --runs 3 \
> > --command-name parent \
> > '"$parent" branch -r --contains c78ae85f3ce7e >/dev/null' \
> > --command-name this-commit \
> > '"$this" branch -r --contains c78ae85f3ce7e >/dev/null'
> >
> > The results were:
> >
> > parent this commit
> > elapsed 104.365 s 467.7 ms
> > user 93.702 s 220.2 ms
> > system 0.723 s 182.7 ms
> >
> > The wall-time standard deviations were 11.356 seconds and 133.8
> > milliseconds, respectively. Separate runs without redirection produced
> > the same output with SHA-256
> > 2466f6e2b72aa16b1a2126eddb81c8a1b2764ee251204ac034c191a925aa896f.
> >
> > Both revisions were built with the default -O2 flags using Apple
> > clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro (Mac16,6)
> > with a 16-core Apple M4 Max (12 performance and four efficiency
> > cores) and 128 GB RAM.
> >
> > Link: https://lore.kernel.org/git/1445163904-24611-1-git-send-email-Karthik.188@gmail.com/
> > Link: https://lore.kernel.org/r/20230324191009.GA536967@coredump.intra.peff.net
> > Link: https://lore.kernel.org/git/20260527070510.3510836-1-krka@spotify.com/
> > Link: https://lore.kernel.org/r/20260608223430.GA340696@coredump.intra.peff.net
> > Suggested-by: Jeff King <peff@peff.net>
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > commit-reach.c | 13 +++++++++--
> > commit-reach.h | 7 ++++++
> > t/perf/p1500-graph-walks.sh | 49 +++++++++++++++++++++++++++++++++++++++++-
> > t/t6301-for-each-ref-errors.sh | 22 +++++++++++++++++++
> > 4 files changed, 88 insertions(+), 3 deletions(-)
> >
> > diff --git a/commit-reach.c b/commit-reach.c
> > index 65b618959b..83a48004ef 100644
> > --- a/commit-reach.c
> > +++ b/commit-reach.c
> > @@ -821,9 +821,18 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
> > int commit_contains(struct ref_filter *filter, struct commit *commit,
> > struct commit_list *list, struct contains_cache *cache)
> > {
> > - if (filter->with_commit_tag_algo)
> > + int result;
> > +
> > + if (!list)
> > + return 1;
> > + if (filter->with_commit_tag_algo ||
> > + generation_numbers_enabled(the_repository))
>
> What's stopping us from dropping `filter->with_commit_tag_algo`
> completely and then doing?
>
> if (generation_numbers_enabled(the_repository))
> return contains_algo(commit, list, cache) == CONTAINS_YES;
> return repo_is_descendant_of(the_repository, commit, list);
Jeff raised this distinction during the v1 review:
https://lore.kernel.org/r/20260608223430.GA340696@coredump.intra.peff.net/
`with_commit_tag_algo` preserves the existing behavior of `git tag` when
generation numbers are unavailable. `git tag --contains` has used the
memoized walk since ffc4b8012d (tag: speed up --contains calculation,
2011-06-11). Dropping the flag would send it back through repeated
`repo_is_descendant_of()` walks in repositories without usable generation
numbers.
The condition in v2 implements the rule discussed there: retain the
existing memoized path for `git tag`, and use it for other ref-filter
callers when generation numbers make the depth-first walk reliably
advantageous.
This is probably my fault for breaking the threading between this and
v1. Sorry about that.
^ permalink raw reply
* Re: [PATCH v2] ref-filter: restore prefix-scoped iteration
From: Tamir Duberstein @ 2026-06-10 12:25 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, Patrick Steinhardt, Junio C Hamano, Victoria Dye, ZheNing Hu
In-Reply-To: <CAOLa=ZRHKNNymXGk31YgECjUmF9nZ8GsPUdQb7aKBH5DKMz7=w@mail.gmail.com>
On Wed, Jun 10, 2026 at 3:50 AM Karthik Nayak <karthik.188@gmail.com> wrote:
>
> Tamir Duberstein <tamird@gmail.com> writes:
>
> > Commit dabecb9db2 (for-each-ref: introduce a '--start-after' option,
> > 2025-07-15) changed single-kind branch, remote-tracking branch, and tag
> > enumeration in do_filter_refs() from constructing an iterator with the
> > namespace prefix to constructing an unscoped iterator and applying the
> > prefix with ref_iterator_seek().
> >
> > Before that change, refs_for_each_fullref_in() passed the namespace
> > prefix during iterator construction. That helper has since been
> > replaced by refs_for_each_ref_ext().
> >
> > The files backend primes its loose-ref cache for the construction
> > prefix before it opens packed refs. An empty construction prefix
> > therefore reads every loose ref, and a later seek cannot undo that I/O.
> > Consequently, git branch, git branch --remotes, and git tag scale with
> > unrelated loose refs.
> >
>
> And this is the crux of the issue. Currently we do
>
> - refs_ref_iterator_begin()
> - ref_iterator_seek()
>
> And between the two `cache_ref_iterator_set_prefix()` is already called
> which caches all the loose refs. This is the IO intensive operation this
> patch tries to avoid.
>
> I think it would be worthwhile to add this information in the commit
> message.
Agreed. I will explain that `cache_ref_iterator_set_prefix()` primes
the loose-ref cache during iterator construction, before the later
seek can narrow it.
>
> >
> > Patrick Steinhardt observed during review that iterator construction
> > and seeking accepted similar strings but assigned them different state
> > semantics. Junio C Hamano then pointed out that no current command can
> > combine start_after with this single-kind path, but future branch or
> > tag support would need to keep the namespace while moving the cursor.
> >
> > Keep the existing start_after path unchanged. The iterator API cannot
> > currently seek to one string while retaining another as its prefix:
> > an unflagged seek clears the prefix, while REF_ITERATOR_SEEK_SET_PREFIX
> > replaces it with the seek string.
> >
> > For the commands affected by this regression, which do not set
> > start_after, pass the namespace prefix during iterator construction so
> > that loose refs are scoped before the packed-refs snapshot is opened.
> > This fixes the current regression without deleting the ref-filter state
> > discussed during review or changing its dormant behavior.
> >
> > Add REFFILES-gated performance cases with one branch, one
> > remote-tracking branch, one tag, and 10,000 unrelated loose refs. The
> > benchmarks were run with:
> >
> > GIT_PERF_REPEAT_COUNT=5 GIT_PERF_MAKE_OPTS=-j8 \
> > t/perf/run a89346e34a . -- p6300-for-each-ref.sh
> >
> > The following are the best of five runs, with each run invoking the
> > command ten times. Times are elapsed seconds with user and system CPU
> > seconds in parentheses:
> >
> > a89346e34a this commit
> > branch 2.74(0.13+2.56) 0.11(0.04+0.04)
> > branch --remotes 2.81(0.13+2.62) 0.12(0.04+0.04)
> > tag 3.01(0.14+2.82) 0.11(0.04+0.04)
> >
> > Both revisions used the default -O2 build flags and a config.mak
> > containing only "NO_REGEX = NeedsStartEnd". They were built with Apple
> > clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro (Mac16,6)
> > with a 16-core Apple M4 Max (12 performance and four efficiency cores)
> > and 128 GB RAM.
> >
> > Link: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
> > Link: https://lore.kernel.org/git/xmqqikjq7s16.fsf@gitster.g/
> > Fixes: dabecb9db2b2 ("for-each-ref: introduce a '--start-after' option")
> > Assisted-by: Codex gpt-5.5
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > The series is based on a89346e34a (maint) because the regression has
> > been present in released versions since Git 2.51.0.
> > ---
> > Changes in v2:
> > - Extract local variable `store`.
> > - Link to v1: https://patch.msgid.link/20260605-fix-git-branch-regression-v1-1-02f40ad40929@gmail.com
> > ---
> > ref-filter.c | 28 +++++++++++++++++++---------
> > t/perf/p6300-for-each-ref.sh | 39 ++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 57 insertions(+), 10 deletions(-)
> >
> > diff --git a/ref-filter.c b/ref-filter.c
> > index 1da4c0e60d..5cbc007d64 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -3315,19 +3315,29 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
> > prefix = "refs/tags/";
> >
> > if (prefix) {
> > - struct ref_iterator *iter;
> > + struct ref_store *store = get_main_ref_store(the_repository);
> >
> > - iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
> > - "", NULL, 0, 0);
> > + if (filter->start_after) {
> > + struct ref_iterator *iter;
> > +
> > + iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
> >
> > - if (filter->start_after)
> > ret = start_ref_iterator_after(iter, filter->start_after);
> > - else
> > - ret = ref_iterator_seek(iter, prefix,
> > - REF_ITERATOR_SEEK_SET_PREFIX);
> > + if (!ret)
> > + ret = do_for_each_ref_iterator(iter, fn,
> > + cb_data);
> > + } else {
> > + /*
> > + * Pass the prefix during construction because the files
> > + * backend primes loose refs before a later seek can
> > + * narrow the iterator.
> > + */
> > + struct refs_for_each_ref_options opts = {
> > + .prefix = prefix,
> > + };
> >
> > - if (!ret)
> > - ret = do_for_each_ref_iterator(iter, fn, cb_data);
> > + ret = refs_for_each_ref_ext(store, fn, cb_data, &opts);
> > + }
>
> This would work, as now we separate out the regular path to use
> `do_for_each_ref_iterator()` instead.
>
> But this causes a bit of confusion, why do we need to use
> `do_for_each_ref_iterator()` and why not simply provide the prefix to
> `refs_ref_iterator_begin()`, like before?
We do not. Your version is simpler and preserves the existing iterator
flow. I have adopted it for v3. Thanks!
> [...]
>
> Thanks for the patch, this is indeed a regression we must fix and the
> benchmarks are a clear indication of it.
Thank you! I'll try not to break threading on the next roll.
^ permalink raw reply
* [PATCH v3] ref-filter: restore prefix-scoped iteration
From: Tamir Duberstein @ 2026-06-10 12:29 UTC (permalink / raw)
To: git
Cc: Karthik Nayak, Patrick Steinhardt, Junio C Hamano, Victoria Dye,
ZheNing Hu, Tamir Duberstein
In-Reply-To: <20260608-fix-git-branch-regression-v2-1-fd82075a8520@gmail.com>
dabecb9db2 (for-each-ref: introduce a '--start-after' option,
2025-07-15) changed branch, remote-tracking branch, and tag enumeration
from constructing an iterator with the namespace prefix to constructing
an unscoped iterator and seeking to the prefix.
The files backend constructs its loose-ref iterator with cache priming
enabled. cache_ref_iterator_begin() immediately applies the construction
prefix through cache_ref_iterator_set_prefix(), reading loose refs
beneath it before packed refs are opened. An empty prefix therefore
reads every loose ref, and a later seek cannot undo that I/O.
For these single-kind filters, construct the iterator with the namespace
prefix when start_after is not set. Keep the existing unscoped
construction for start_after, whose seek position may differ from the
namespace prefix.
With 10,000 unrelated loose refs, the p6300 tests improve as follows:
before after
branch 2.74 s 0.11 s
branch --remotes 2.81 s 0.12 s
tag 3.01 s 0.11 s
Link: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
Link: https://lore.kernel.org/git/xmqqikjq7s16.fsf@gitster.g/
Link: https://lore.kernel.org/r/CAOLa=ZRHKNNymXGk31YgECjUmF9nZ8GsPUdQb7aKBH5DKMz7=w@mail.gmail.com
Fixes: dabecb9db2b2 ("for-each-ref: introduce a '--start-after' option")
Suggested-by: Karthik Nayak <karthik.188@gmail.com>
Assisted-by: Codex gpt-5.5
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
The series is based on a89346e34a (maint) because the regression has
been present in released versions since Git 2.51.0.
---
Changes in v3:
- Construct the iterator directly with the namespace prefix.
- Explain when the files backend primes its loose-ref cache.
- Condense the commit message and performance results.
- Link to v2: https://patch.msgid.link/20260608-fix-git-branch-regression-v2-1-fd82075a8520@gmail.com
Changes in v2:
- Extract local variable `store`.
- Link to v1: https://patch.msgid.link/20260605-fix-git-branch-regression-v1-1-02f40ad40929@gmail.com
---
ref-filter.c | 13 ++++++-------
t/perf/p6300-for-each-ref.sh | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1da4c0e60d..9b04e3af85 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3316,15 +3316,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
if (prefix) {
struct ref_iterator *iter;
+ struct ref_store *store = get_main_ref_store(the_repository);
- iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
- "", NULL, 0, 0);
-
- if (filter->start_after)
+ if (filter->start_after) {
+ iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
ret = start_ref_iterator_after(iter, filter->start_after);
- else
- ret = ref_iterator_seek(iter, prefix,
- REF_ITERATOR_SEEK_SET_PREFIX);
+ } else {
+ iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0);
+ }
if (!ret)
ret = do_for_each_ref_iterator(iter, fn, cb_data);
diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
index fa7289c752..ed9c1c6a19 100755
--- a/t/perf/p6300-for-each-ref.sh
+++ b/t/perf/p6300-for-each-ref.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='performance of for-each-ref'
+test_description='performance of ref-filter users'
. ./perf-lib.sh
test_perf_fresh_repo
@@ -84,4 +84,41 @@ test_expect_success 'pack refs' '
'
run_tests "packed"
+test_expect_success REFFILES 'setup many unrelated loose refs' '
+ git init scoped &&
+ test_commit -C scoped --no-tag base &&
+ test_seq $ref_count_per_type |
+ sed "s,.*,update refs/custom/unrelated_& HEAD," |
+ git -C scoped update-ref --stdin &&
+ git -C scoped update-ref refs/remotes/origin/main HEAD &&
+ git -C scoped update-ref refs/tags/only HEAD
+'
+
+test_perf "branch (many unrelated loose refs)" --prereq REFFILES "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git branch --format='%(refname)' >/dev/null
+ done
+ )
+"
+
+test_perf "branch --remotes (many unrelated loose refs)" --prereq REFFILES "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git branch --remotes --format='%(refname)' >/dev/null
+ done
+ )
+"
+
+test_perf "tag (many unrelated loose refs)" --prereq REFFILES "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git tag --format='%(refname)' >/dev/null
+ done
+ )
+"
+
test_done
---
base-commit: a89346e34a937f001e5d397ee62224e3e9852040
change-id: 20260605-fix-git-branch-regression-9e4236f18091
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply related
* Re: [GSoC PATCH v2 0/4] teach git repo info to handle path keys
From: Lucas Seiki Oshiro @ 2026-06-10 12:42 UTC (permalink / raw)
To: K Jayatheerth
Cc: Junio C Hamano, git, a3205153416, jltobler, kumarayushjha123,
phillip.wood, sandals
In-Reply-To: <CA+rGoLf39iQH9X-xKW7HeTS3sMv-N-QzGiqm0Y=RYGOAqDcaoA@mail.gmail.com>
Junio has a good point here.
This is a plumbing command and we should design it
for machines instead of humans.
^ permalink raw reply
* [PATCH v2 0/1] environment: move protect_hfs and protect_ntfs into repo_config_values
From: Tian Yuchen @ 2026-06-10 12:43 UTC (permalink / raw)
To: git
Cc: phillip.wood123, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260606143412.15443-1-cat@malon.dev>
Hi everyone,
This series continues the ongoing libification effort by moving the
global filesystem variables, 'protect_hfs' and 'protect_ntfs', into
'struct repo_config_values'.
Place them within the per-repository configuration structure
aligns with our goal of removing global states.
For reviewers familiar with previous libification efforts, Derrick Stolee
attempted to wrap this kind of filesystem-level variable using a
lazy-loaded global accessor get_int_config_global() [1].
However, as Glen Choo pointed out in his review of that series [2],
it is strongly preferred to use plain fields in a repository-scoped
struct over global lazy-loaders, provided those fields are properly
initialized during the setup process.
By moving these variables into repo_config_values and parsing
them eagerly, we successfully tie the filesystem security flags
to the specific repository instance without altering the timing
of configuration warnings or introducing new global states.
Thanks!
Recent related patch (environment.c: migrate 'trust_executable_bit' into 'repo_config_values'): [3]
[1] https://lore.kernel.org/git/a42dd9397d07b2dc4a0d7e75bfe1af2e46cad262.1685716420.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/kl6lbkhpzujf.fsf@chooglen-macbookpro.roam.corp.google.com/
[3] https://lore.kernel.org/git/20260610093635.139719-1-cat@malon.dev/
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
Tian Yuchen (1):
environment.c: move 'protect_hfs' and 'protect_ntfs' into
'repo_config_values'
compat/mingw.c | 2 +-
environment.c | 22 ++++++++++++++++++----
environment.h | 12 ++++++++++--
read-cache.c | 7 ++++---
t/helper/test-path-utils.c | 24 +++++++++++++++---------
5 files changed, 48 insertions(+), 19 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'
From: Tian Yuchen @ 2026-06-10 12:43 UTC (permalink / raw)
To: git
Cc: phillip.wood123, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260610124353.149874-1-cat@malon.dev>
Move the global 'protect_hfs' and 'protect_ntfs' configurations
into the repository-specific 'repo_config_values' struct.
This will help with the elimination of 'the_repository'
To ensure code readability, the getter functions
'repo_protect_hfs()' and 'repo_protect_ntfs()'
have been introduced.
For now, associated functions access this configuration by
explicitly falling back to 'the_repository', which needs to
be addressed in the future.
Note: In 't/helper/test-path-utils.c', there is a function
'protect_ntfs_hfs_benchmark()' where these two global
variables are used as loop iterators. New local variables
have been created to replace them.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
compat/mingw.c | 2 +-
environment.c | 22 ++++++++++++++++++----
environment.h | 12 ++++++++++--
read-cache.c | 7 ++++---
t/helper/test-path-utils.c | 24 +++++++++++++++---------
5 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419..af87df77fd 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3392,7 +3392,7 @@ int is_valid_win32_path(const char *path, int allow_literal_nul)
const char *p = path;
int preceding_space_or_period = 0, i = 0, periods = 0;
- if (!protect_ntfs)
+ if (!repo_protect_ntfs(the_repository))
return 1;
skip_dos_drive_prefix((char **)&path);
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..683fe1b4d3 100644
--- a/environment.c
+++ b/environment.c
@@ -82,12 +82,10 @@ unsigned long pack_size_limit_cfg;
#ifndef PROTECT_HFS_DEFAULT
#define PROTECT_HFS_DEFAULT 0
#endif
-int protect_hfs = PROTECT_HFS_DEFAULT;
#ifndef PROTECT_NTFS_DEFAULT
#define PROTECT_NTFS_DEFAULT 1
#endif
-int protect_ntfs = PROTECT_NTFS_DEFAULT;
/*
* The character that begins a commented line in user-editable file
@@ -142,6 +140,20 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+int repo_protect_ntfs(struct repository *repo)
+{
+ return repo->gitdir ?
+ repo_config_values(repo)->protect_ntfs :
+ PROTECT_NTFS_DEFAULT;
+}
+
+int repo_protect_hfs(struct repository *repo)
+{
+ return repo->gitdir ?
+ repo_config_values(repo)->protect_hfs :
+ PROTECT_HFS_DEFAULT;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -541,12 +553,12 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.protecthfs")) {
- protect_hfs = git_config_bool(var, value);
+ cfg->protect_hfs = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.protectntfs")) {
- protect_ntfs = git_config_bool(var, value);
+ cfg->protect_ntfs = git_config_bool(var, value);
return 0;
}
@@ -720,5 +732,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
+ cfg->protect_hfs = PROTECT_HFS_DEFAULT;
+ cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 9eb97b3869..fdd9775900 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,8 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
int apply_sparse_checkout;
+ int protect_hfs;
+ int protect_ntfs;
/* section "branch" config values */
enum branch_track branch_track;
@@ -123,6 +125,14 @@ int git_default_config(const char *, const char *,
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb);
+/*
+ * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
+ * They check `repo->gitdir` to prevent calling repo_config_values()
+ * before the configuration is loaded or in bare environments.
+ */
+int repo_protect_hfs(struct repository *repo);
+int repo_protect_ntfs(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -173,8 +183,6 @@ extern int pack_compression_level;
extern unsigned long pack_size_limit_cfg;
extern int precomposed_unicode;
-extern int protect_hfs;
-extern int protect_ntfs;
extern int core_sparse_checkout_cone;
extern int sparse_expect_files_outside_of_patterns;
diff --git a/read-cache.c b/read-cache.c
index 21829102ae..2c6a60c756 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1002,7 +1002,7 @@ static enum verify_path_result verify_path_internal(const char *path,
return PATH_OK;
if (is_dir_sep(c)) {
inside:
- if (protect_hfs) {
+ if (repo_protect_hfs(the_repository)) {
if (is_hfs_dotgit(path))
return PATH_INVALID;
@@ -1011,7 +1011,7 @@ static enum verify_path_result verify_path_internal(const char *path,
return PATH_INVALID;
}
}
- if (protect_ntfs) {
+ if (repo_protect_ntfs(the_repository)) {
#if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__
if (c == '\\')
return PATH_INVALID;
@@ -1035,7 +1035,8 @@ static enum verify_path_result verify_path_internal(const char *path,
if (c == '\0')
return S_ISDIR(mode) ? PATH_DIR_WITH_SEP :
PATH_INVALID;
- } else if (c == '\\' && protect_ntfs) {
+ } else if (c == '\\' &&
+ repo_protect_ntfs(the_repository)) {
if (is_ntfs_dotgit(path))
return PATH_INVALID;
if (S_ISLNK(mode)) {
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 15eb44485c..f77b3f9d70 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -250,6 +250,7 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
double m[3][2], v[3][2];
uint64_t cumul;
double cumul2;
+ int ntfs, hfs;
if (argc > 1 && !strcmp(argv[1], "--with-symlink-mode")) {
file_mode = 0120000;
@@ -276,8 +277,13 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
names[i][--len] = (char)(' ' + (my_random() % ('\x7f' - ' ')));
}
- for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
- for (protect_hfs = 0; protect_hfs < 2; protect_hfs++) {
+ if (!the_repository->gitdir)
+ the_repository->gitdir = xstrdup(".git");
+
+ for (ntfs = 0; ntfs < 2; ntfs++)
+ for (hfs = 0; hfs < 2; hfs++) {
+ repo_config_values(the_repository)->protect_ntfs = ntfs;
+ repo_config_values(the_repository)->protect_hfs = hfs;
cumul = 0;
cumul2 = 0;
for (i = 0; i < repetitions; i++) {
@@ -285,18 +291,18 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
for (j = 0; j < nr; j++)
verify_path(names[j], file_mode);
end = getnanotime();
- printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);
+ printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", ntfs, hfs, (end-begin) / (double)1e6);
cumul += end - begin;
cumul2 += (end - begin) * (end - begin);
}
- m[protect_ntfs][protect_hfs] = cumul / (double)repetitions;
- v[protect_ntfs][protect_hfs] = my_sqrt(cumul2 / (double)repetitions - m[protect_ntfs][protect_hfs] * m[protect_ntfs][protect_hfs]);
- printf("mean: %lfms, stddev: %lfms\n", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);
+ m[ntfs][hfs] = cumul / (double)repetitions;
+ v[ntfs][hfs] = my_sqrt(cumul2 / (double)repetitions - m[ntfs][hfs] * m[ntfs][hfs]);
+ printf("mean: %lfms, stddev: %lfms\n", m[ntfs][hfs] / (double)1e6, v[ntfs][hfs] / (double)1e6);
}
- for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
- for (protect_hfs = 0; protect_hfs < 2; protect_hfs++)
- printf("ntfs=%d/hfs=%d: %lf%% slower\n", protect_ntfs, protect_hfs, (m[protect_ntfs][protect_hfs] - m[0][0]) * 100 / m[0][0]);
+ for (ntfs = 0; ntfs < 2; ntfs++)
+ for (hfs = 0; hfs < 2; hfs++)
+ printf("ntfs=%d/hfs=%d: %lf%% slower\n", ntfs, hfs, (m[ntfs][hfs] - m[0][0]) * 100 / m[0][0]);
return 0;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] read_gitfile_gently(): return non-repo path on error
From: Junio C Hamano @ 2026-06-10 13:01 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Jeff King, git, Tian Yuchen
In-Reply-To: <ah6WEtk2pXyViEQA@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> On Tue, Jun 02, 2026 at 02:11:59AM -0400, Jeff King wrote:
> [snip]
>> Two other points of interest.
>>
>> One, I'm not sure how useful printing the pointed-to directory is. We
>> _could_ just say:
>>
>> fatal: gitfile does not point to a valid repository: /path/to/.git
>>
>> which is enough for somebody to investigate themselves. That would
>> certainly make the patch smaller.
>
> I have to agree that the patch is somewhat gross, and I myself don't
> really see much of an issue to move to an error message like the above
> if it ends up simplifying the logic.
So we are in agreement among three of us that simplifying the code
to lose error message with dubious value would be a good way
forward.
Peff, can we have a formal [v2] then?
>> diff --git a/setup.c b/setup.c
>> index 075bf89fa9..2df6fbf595 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -1641,9 +1650,11 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
>> return GIT_DIR_INVALID_GITFILE;
>> default:
>> if (die_on_error)
>> - read_gitfile_error_die(error_code, dir->buf, NULL);
>> - else
>> + read_gitfile_error_die(error_code, dir->buf, error_dst);
>> + else {
>> + free(error_dst);
>> return GIT_DIR_INVALID_GITFILE;
>> + }
>
> The `if` branch should also gain some curly braces here.
True.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 03/10] reset: rename `reset_head()`
From: Phillip Wood @ 2026-06-10 13:10 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260610-b4-pks-history-drop-v4-3-70d5f0ae8c25@pks.im>
Hi Patrick
On 10/06/2026 09:52, Patrick Steinhardt wrote:
> In a subsequent commit we're about to adapt `reset_head()` so that the
> reference update to HEAD is optional, only. At this point the function
> starts to feel misnamed, as it doesn't necessarily have anything to do
> with the HEAD reference anymore. The gist of the function then is that
> we reset the working tree to a specific new commit, updating both the
> index and the checked-out files.
>
> Rename it to `reset_working_tree()` to better reflect that.
That sounds good. Because we defer renaming the flags this patch is very
straight forward.
Thanks
Phillip
> Note that we don't adjust the flags yet. This will happen in a
> subsequent commit.
>
> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> ---
> builtin/rebase.c | 20 ++++++++++----------
> reset.c | 5 +++--
> reset.h | 4 ++--
> sequencer.c | 8 ++++----
> 4 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index fa4f5d9306..22fbba3c62 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -592,7 +592,7 @@ static int finish_rebase(struct rebase_options *opts)
> static int move_to_original_branch(struct rebase_options *opts)
> {
> struct strbuf branch_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT;
> - struct reset_head_opts ropts = { 0 };
> + struct reset_working_tree_options ropts = { 0 };
> int ret;
>
> if (!opts->head_name)
> @@ -610,7 +610,7 @@ static int move_to_original_branch(struct rebase_options *opts)
> ropts.flags = RESET_HEAD_REFS_ONLY;
> ropts.branch_msg = branch_reflog.buf;
> ropts.head_msg = head_reflog.buf;
> - ret = reset_head(the_repository, &ropts);
> + ret = reset_working_tree(the_repository, &ropts);
>
> strbuf_release(&branch_reflog);
> strbuf_release(&head_reflog);
> @@ -685,7 +685,7 @@ static int run_am(struct rebase_options *opts)
>
> status = run_command(&format_patch);
> if (status) {
> - struct reset_head_opts ropts = { 0 };
> + struct reset_working_tree_options ropts = { 0 };
> unlink(rebased_patches);
> free(rebased_patches);
> child_process_clear(&am);
> @@ -693,7 +693,7 @@ static int run_am(struct rebase_options *opts)
> ropts.oid = &opts->orig_head->object.oid;
> ropts.branch = opts->head_name;
> ropts.default_reflog_action = opts->reflog_action;
> - reset_head(the_repository, &ropts);
> + reset_working_tree(the_repository, &ropts);
> error(_("\ngit encountered an error while preparing the "
> "patches to replay\n"
> "these revisions:\n"
> @@ -855,7 +855,7 @@ static int rebase_config(const char *var, const char *value,
> static int checkout_up_to_date(struct rebase_options *options)
> {
> struct strbuf buf = STRBUF_INIT;
> - struct reset_head_opts ropts = { 0 };
> + struct reset_working_tree_options ropts = { 0 };
> int ret = 0;
>
> strbuf_addf(&buf, "%s: checkout %s",
> @@ -866,7 +866,7 @@ static int checkout_up_to_date(struct rebase_options *options)
> if (!ropts.branch)
> ropts.flags |= RESET_HEAD_DETACH;
> ropts.head_msg = buf.buf;
> - if (reset_head(the_repository, &ropts) < 0)
> + if (reset_working_tree(the_repository, &ropts) < 0)
> ret = error(_("could not switch to %s"), options->switch_to);
> strbuf_release(&buf);
>
> @@ -1116,7 +1116,7 @@ int cmd_rebase(int argc,
> int reschedule_failed_exec = -1;
> int allow_preemptive_ff = 1;
> int preserve_merges_selected = 0;
> - struct reset_head_opts ropts = { 0 };
> + struct reset_working_tree_options ropts = { 0 };
> struct option builtin_rebase_options[] = {
> OPT_STRING(0, "onto", &options.onto_name,
> N_("revision"),
> @@ -1385,7 +1385,7 @@ int cmd_rebase(int argc,
> rerere_clear(the_repository, &merge_rr);
> string_list_clear(&merge_rr, 1);
> ropts.flags = RESET_HEAD_HARD;
> - if (reset_head(the_repository, &ropts) < 0)
> + if (reset_working_tree(the_repository, &ropts) < 0)
> die(_("could not discard worktree changes"));
> remove_branch_state(the_repository, 0);
> if (read_basic_state(&options))
> @@ -1410,7 +1410,7 @@ int cmd_rebase(int argc,
> ropts.head_msg = head_msg.buf;
> ropts.branch = options.head_name;
> ropts.flags = RESET_HEAD_HARD;
> - if (reset_head(the_repository, &ropts) < 0)
> + if (reset_working_tree(the_repository, &ropts) < 0)
> die(_("could not move back to %s"),
> oid_to_hex(&options.orig_head->object.oid));
> strbuf_release(&head_msg);
> @@ -1880,7 +1880,7 @@ int cmd_rebase(int argc,
> RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
> ropts.head_msg = msg.buf;
> ropts.default_reflog_action = options.reflog_action;
> - if (reset_head(the_repository, &ropts)) {
> + if (reset_working_tree(the_repository, &ropts)) {
> ret = error(_("Could not detach HEAD"));
> goto cleanup_autostash;
> }
> diff --git a/reset.c b/reset.c
> index 3b3cb74dab..799596398b 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -12,7 +12,7 @@
> #include "hook.h"
>
> static int update_refs(struct repository *repo,
> - const struct reset_head_opts *opts,
> + const struct reset_working_tree_options *opts,
> const struct object_id *oid,
> const struct object_id *head)
> {
> @@ -85,7 +85,8 @@ static int update_refs(struct repository *repo,
> return ret;
> }
>
> -int reset_head(struct repository *r, const struct reset_head_opts *opts)
> +int reset_working_tree(struct repository *r,
> + const struct reset_working_tree_options *opts)
> {
> const struct object_id *oid = opts->oid;
> const char *switch_to_branch = opts->branch;
> diff --git a/reset.h b/reset.h
> index a28f81829d..f130152014 100644
> --- a/reset.h
> +++ b/reset.h
> @@ -17,7 +17,7 @@
> /* Update ORIG_HEAD as well as HEAD */
> #define RESET_ORIG_HEAD (1<<4)
>
> -struct reset_head_opts {
> +struct reset_working_tree_options {
> /*
> * The commit to checkout/reset to. Defaults to HEAD.
> */
> @@ -55,6 +55,6 @@ struct reset_head_opts {
> const char *default_reflog_action;
> };
>
> -int reset_head(struct repository *r, const struct reset_head_opts *opts);
> +int reset_working_tree(struct repository *r, const struct reset_working_tree_options *opts);
>
> #endif
> diff --git a/sequencer.c b/sequencer.c
> index 1ee4b2875b..d73ecf0384 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4677,7 +4677,7 @@ static void create_autostash_internal(struct repository *r,
> if (has_unstaged_changes(r, 1) ||
> has_uncommitted_changes(r, 1)) {
> struct child_process stash = CHILD_PROCESS_INIT;
> - struct reset_head_opts ropts = { .flags = RESET_HEAD_HARD };
> + struct reset_working_tree_options ropts = { .flags = RESET_HEAD_HARD };
> struct object_id oid;
>
> strvec_pushl(&stash.args,
> @@ -4707,7 +4707,7 @@ static void create_autostash_internal(struct repository *r,
>
> if (!silent)
> printf(_("Created autostash: %s\n"), buf.buf);
> - if (reset_head(r, &ropts) < 0)
> + if (reset_working_tree(r, &ropts) < 0)
> die(_("could not reset --hard"));
> discard_index(r->index);
> if (repo_read_index(r) < 0)
> @@ -4867,7 +4867,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
> const char *onto_name, const struct object_id *onto,
> const struct object_id *orig_head)
> {
> - struct reset_head_opts ropts = {
> + struct reset_working_tree_options ropts = {
> .oid = onto,
> .orig_head = orig_head,
> .flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
> @@ -4876,7 +4876,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
> onto_name),
> .default_reflog_action = sequencer_reflog_action(opts)
> };
> - if (reset_head(r, &ropts)) {
> + if (reset_working_tree(r, &ropts)) {
> apply_autostash(rebase_path_autostash());
> sequencer_remove_state(opts);
> return error(_("could not detach HEAD"));
>
^ permalink raw reply
* Re: [PATCH v4 04/10] reset: modernize flags passed to `reset_working_tree()`
From: Phillip Wood @ 2026-06-10 13:10 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260610-b4-pks-history-drop-v4-4-70d5f0ae8c25@pks.im>
Hi Patrick
On 10/06/2026 09:52, Patrick Steinhardt wrote:
> The flags passed to `reset_working_tree()` are declared as defines. This
> has fallen a bit out of practice nowadays, where we instead prefer to
> use enums. Furthermore, the prefix of those flags does not match the
> function name anymore after the rename in the preceding commit.
>
> Adapt the code to follow modern best practices and adapt the flag names.
This looks good
Thanks
Phillip
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/rebase.c | 15 ++++++++-------
> reset.c | 12 ++++++------
> reset.h | 31 +++++++++++++++++++------------
> sequencer.c | 9 ++++++---
> 4 files changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 22fbba3c62..06dcbaf5e8 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -607,7 +607,7 @@ static int move_to_original_branch(struct rebase_options *opts)
> strbuf_addf(&head_reflog, "%s (finish): returning to %s",
> opts->reflog_action, opts->head_name);
> ropts.branch = opts->head_name;
> - ropts.flags = RESET_HEAD_REFS_ONLY;
> + ropts.flags = RESET_WORKING_TREE_REFS_ONLY;
> ropts.branch_msg = branch_reflog.buf;
> ropts.head_msg = head_reflog.buf;
> ret = reset_working_tree(the_repository, &ropts);
> @@ -862,9 +862,9 @@ static int checkout_up_to_date(struct rebase_options *options)
> options->reflog_action, options->switch_to);
> ropts.oid = &options->orig_head->object.oid;
> ropts.branch = options->head_name;
> - ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
> + ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
> if (!ropts.branch)
> - ropts.flags |= RESET_HEAD_DETACH;
> + ropts.flags |= RESET_WORKING_TREE_DETACH;
> ropts.head_msg = buf.buf;
> if (reset_working_tree(the_repository, &ropts) < 0)
> ret = error(_("could not switch to %s"), options->switch_to);
> @@ -1384,7 +1384,7 @@ int cmd_rebase(int argc,
>
> rerere_clear(the_repository, &merge_rr);
> string_list_clear(&merge_rr, 1);
> - ropts.flags = RESET_HEAD_HARD;
> + ropts.flags = RESET_WORKING_TREE_HARD;
> if (reset_working_tree(the_repository, &ropts) < 0)
> die(_("could not discard worktree changes"));
> remove_branch_state(the_repository, 0);
> @@ -1409,7 +1409,7 @@ int cmd_rebase(int argc,
> ropts.oid = &options.orig_head->object.oid;
> ropts.head_msg = head_msg.buf;
> ropts.branch = options.head_name;
> - ropts.flags = RESET_HEAD_HARD;
> + ropts.flags = RESET_WORKING_TREE_HARD;
> if (reset_working_tree(the_repository, &ropts) < 0)
> die(_("could not move back to %s"),
> oid_to_hex(&options.orig_head->object.oid));
> @@ -1876,8 +1876,9 @@ int cmd_rebase(int argc,
> options.reflog_action, options.onto_name);
> ropts.oid = &options.onto->object.oid;
> ropts.orig_head = &options.orig_head->object.oid;
> - ropts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
> - RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
> + ropts.flags = RESET_WORKING_TREE_DETACH |
> + RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
> + RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
> ropts.head_msg = msg.buf;
> ropts.default_reflog_action = options.reflog_action;
> if (reset_working_tree(the_repository, &ropts)) {
> diff --git a/reset.c b/reset.c
> index 799596398b..4ca7f23a25 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -16,9 +16,9 @@ static int update_refs(struct repository *repo,
> const struct object_id *oid,
> const struct object_id *head)
> {
> - unsigned detach_head = opts->flags & RESET_HEAD_DETACH;
> - unsigned run_hook = opts->flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
> - unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
> + unsigned detach_head = opts->flags & RESET_WORKING_TREE_DETACH;
> + unsigned run_hook = opts->flags & RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
> + unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
> const struct object_id *orig_head = opts->orig_head;
> const char *switch_to_branch = opts->branch;
> const char *reflog_branch = opts->branch_msg;
> @@ -90,9 +90,9 @@ int reset_working_tree(struct repository *r,
> {
> const struct object_id *oid = opts->oid;
> const char *switch_to_branch = opts->branch;
> - unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
> - unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
> - unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
> + unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
> + unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
> + unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
> struct object_id *head = NULL, head_oid;
> struct tree_desc desc[2] = { { NULL }, { NULL } };
> struct lock_file lock = LOCK_INIT;
> diff --git a/reset.h b/reset.h
> index f130152014..2e5826de99 100644
> --- a/reset.h
> +++ b/reset.h
> @@ -6,16 +6,22 @@
>
> #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
>
> -/* Request a detached checkout */
> -#define RESET_HEAD_DETACH (1<<0)
> -/* Request a reset rather than a checkout */
> -#define RESET_HEAD_HARD (1<<1)
> -/* Run the post-checkout hook */
> -#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
> -/* Only update refs, do not touch the worktree */
> -#define RESET_HEAD_REFS_ONLY (1<<3)
> -/* Update ORIG_HEAD as well as HEAD */
> -#define RESET_ORIG_HEAD (1<<4)
> +enum reset_working_tree_flags {
> + /* Request a detached checkout */
> + RESET_WORKING_TREE_DETACH = (1 << 0),
> +
> + /* Request a reset rather than a checkout */
> + RESET_WORKING_TREE_HARD = (1 << 1),
> +
> + /* Run the post-checkout hook */
> + RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK = (1 << 2),
> +
> + /* Only update refs, do not touch the worktree */
> + RESET_WORKING_TREE_REFS_ONLY = (1 << 3),
> +
> + /* Update ORIG_HEAD as well as HEAD */
> + RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
> +};
>
> struct reset_working_tree_options {
> /*
> @@ -33,7 +39,7 @@ struct reset_working_tree_options {
> /*
> * Flags defined above.
> */
> - unsigned flags;
> + enum reset_working_tree_flags flags;
> /*
> * Optional reflog message for branch, defaults to head_msg.
> */
> @@ -45,7 +51,8 @@ struct reset_working_tree_options {
> const char *head_msg;
> /*
> * Optional reflog message for ORIG_HEAD, if this omitted and flags
> - * contains RESET_ORIG_HEAD then default_reflog_action must be given.
> + * contains RESET_WORKING_TREE_UPDATE_ORIG_HEAD then
> + * default_reflog_action must be given.
> */
> const char *orig_head_msg;
> /*
> diff --git a/sequencer.c b/sequencer.c
> index d73ecf0384..4efe831178 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4677,7 +4677,9 @@ static void create_autostash_internal(struct repository *r,
> if (has_unstaged_changes(r, 1) ||
> has_uncommitted_changes(r, 1)) {
> struct child_process stash = CHILD_PROCESS_INIT;
> - struct reset_working_tree_options ropts = { .flags = RESET_HEAD_HARD };
> + struct reset_working_tree_options ropts = {
> + .flags = RESET_WORKING_TREE_HARD,
> + };
> struct object_id oid;
>
> strvec_pushl(&stash.args,
> @@ -4870,8 +4872,9 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
> struct reset_working_tree_options ropts = {
> .oid = onto,
> .orig_head = orig_head,
> - .flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
> - RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
> + .flags = RESET_WORKING_TREE_DETACH |
> + RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
> + RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK,
> .head_msg = reflog_message(opts, "start", "checkout %s",
> onto_name),
> .default_reflog_action = sequencer_reflog_action(opts)
>
^ permalink raw reply
* Re: [PATCH v4 06/10] reset: introduce ability to skip updating HEAD
From: Phillip Wood @ 2026-06-10 13:11 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260610-b4-pks-history-drop-v4-6-70d5f0ae8c25@pks.im>
Hi Patrick
On 10/06/2026 09:52, Patrick Steinhardt wrote:
> In a subsequent commit we'll introduce a new caller to
> `reset_working_tree()` that really only wants to update the index and
> working tree, without updating any references. Introduce a new flag that
> makes the caller opt in to updating HEAD and adapt all callers to set
> that flag.
>
> Note that in a previous iteration we instead introduced a flag that made
> callers opt out of updating any references. This was somewhat awkward
> though because we already have the `UPDATE_ORIG_HEAD` flag, so the
> result was somewhat inconsistent.
Thanks for doing this. I've grepped for all the callers of reset_head()
to confirm this patch adds RESET_HEAD_UPDATE_HEAD to them all.
I wonder if we should add a check for passing
RESET_HEAD_UPDATE_ORIG_HEAD without RESET_HEAD_UPDATE_HEAD that calls
BUG() as we don't support that. Everything else looks good.
Thanks
Phillip
> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/rebase.c | 14 ++++++++++----
> reset.c | 6 ++++--
> reset.h | 9 ++++++---
> sequencer.c | 4 +++-
> 4 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 06dcbaf5e8..10a306310c 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -607,7 +607,8 @@ static int move_to_original_branch(struct rebase_options *opts)
> strbuf_addf(&head_reflog, "%s (finish): returning to %s",
> opts->reflog_action, opts->head_name);
> ropts.branch = opts->head_name;
> - ropts.flags = RESET_WORKING_TREE_REFS_ONLY;
> + ropts.flags = RESET_WORKING_TREE_REFS_ONLY |
> + RESET_WORKING_TREE_UPDATE_HEAD;
> ropts.branch_msg = branch_reflog.buf;
> ropts.head_msg = head_reflog.buf;
> ret = reset_working_tree(the_repository, &ropts);
> @@ -693,6 +694,7 @@ static int run_am(struct rebase_options *opts)
> ropts.oid = &opts->orig_head->object.oid;
> ropts.branch = opts->head_name;
> ropts.default_reflog_action = opts->reflog_action;
> + ropts.flags = RESET_WORKING_TREE_UPDATE_HEAD;
> reset_working_tree(the_repository, &ropts);
> error(_("\ngit encountered an error while preparing the "
> "patches to replay\n"
> @@ -862,7 +864,8 @@ static int checkout_up_to_date(struct rebase_options *options)
> options->reflog_action, options->switch_to);
> ropts.oid = &options->orig_head->object.oid;
> ropts.branch = options->head_name;
> - ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
> + ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK |
> + RESET_WORKING_TREE_UPDATE_HEAD;
> if (!ropts.branch)
> ropts.flags |= RESET_WORKING_TREE_DETACH;
> ropts.head_msg = buf.buf;
> @@ -1384,7 +1387,8 @@ int cmd_rebase(int argc,
>
> rerere_clear(the_repository, &merge_rr);
> string_list_clear(&merge_rr, 1);
> - ropts.flags = RESET_WORKING_TREE_HARD;
> + ropts.flags = RESET_WORKING_TREE_HARD |
> + RESET_WORKING_TREE_UPDATE_HEAD;
> if (reset_working_tree(the_repository, &ropts) < 0)
> die(_("could not discard worktree changes"));
> remove_branch_state(the_repository, 0);
> @@ -1409,7 +1413,8 @@ int cmd_rebase(int argc,
> ropts.oid = &options.orig_head->object.oid;
> ropts.head_msg = head_msg.buf;
> ropts.branch = options.head_name;
> - ropts.flags = RESET_WORKING_TREE_HARD;
> + ropts.flags = RESET_WORKING_TREE_HARD |
> + RESET_WORKING_TREE_UPDATE_HEAD;
> if (reset_working_tree(the_repository, &ropts) < 0)
> die(_("could not move back to %s"),
> oid_to_hex(&options.orig_head->object.oid));
> @@ -1877,6 +1882,7 @@ int cmd_rebase(int argc,
> ropts.oid = &options.onto->object.oid;
> ropts.orig_head = &options.orig_head->object.oid;
> ropts.flags = RESET_WORKING_TREE_DETACH |
> + RESET_WORKING_TREE_UPDATE_HEAD |
> RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
> RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
> ropts.head_msg = msg.buf;
> diff --git a/reset.c b/reset.c
> index 99f2c1b012..3ac99a51c0 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -92,6 +92,7 @@ int reset_working_tree(struct repository *r,
> const char *switch_to_branch = opts->branch;
> unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
> unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
> + unsigned update_head = opts->flags & RESET_WORKING_TREE_UPDATE_HEAD;
> unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
> unsigned dry_run = opts->flags & RESET_WORKING_TREE_DRY_RUN;
> struct object_id *head = NULL, head_oid;
> @@ -129,7 +130,7 @@ int reset_working_tree(struct repository *r,
> oid = &head_oid;
>
> if (refs_only) {
> - if (!dry_run)
> + if (update_head)
> return update_refs(r, opts, oid, head);
> return 0;
> }
> @@ -197,7 +198,8 @@ int reset_working_tree(struct repository *r,
> goto leave_reset_head;
> }
>
> - if (oid != &head_oid || update_orig_head || switch_to_branch)
> + if (update_head &&
> + (oid != &head_oid || update_orig_head || switch_to_branch))
> ret = update_refs(r, opts, oid, head);
>
> leave_reset_head:
> diff --git a/reset.h b/reset.h
> index 898e4a1e95..38b2891b53 100644
> --- a/reset.h
> +++ b/reset.h
> @@ -19,14 +19,17 @@ enum reset_working_tree_flags {
> /* Only update refs, do not touch the worktree */
> RESET_WORKING_TREE_REFS_ONLY = (1 << 3),
>
> - /* Update ORIG_HEAD as well as HEAD */
> - RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
> + /* Update HEAD */
> + RESET_WORKING_TREE_UPDATE_HEAD = (1 << 4),
> +
> + /* Update ORIG_HEAD */
> + RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 5),
>
> /*
> * Perform a dry-run by performing the operation without updating
> * any user-visible state.
> */
> - RESET_WORKING_TREE_DRY_RUN = (1 << 5),
> + RESET_WORKING_TREE_DRY_RUN = (1 << 6),
> };
>
> struct reset_working_tree_options {
> diff --git a/sequencer.c b/sequencer.c
> index 4efe831178..e905b1b2d9 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4678,7 +4678,8 @@ static void create_autostash_internal(struct repository *r,
> has_uncommitted_changes(r, 1)) {
> struct child_process stash = CHILD_PROCESS_INIT;
> struct reset_working_tree_options ropts = {
> - .flags = RESET_WORKING_TREE_HARD,
> + .flags = RESET_WORKING_TREE_HARD |
> + RESET_WORKING_TREE_UPDATE_HEAD,
> };
> struct object_id oid;
>
> @@ -4873,6 +4874,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
> .oid = onto,
> .orig_head = orig_head,
> .flags = RESET_WORKING_TREE_DETACH |
> + RESET_WORKING_TREE_UPDATE_HEAD |
> RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
> RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK,
> .head_msg = reflog_message(opts, "start", "checkout %s",
>
^ permalink raw reply
* Re: [PATCH v4] git-gui: silence install recipes under "make -s"
From: Harald Nordgren @ 2026-06-10 13:19 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <950f70ea-1615-402f-9cd4-3317bf177c5c@kdbg.org>
On Sat, Jun 6, 2026 at 1:47 PM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Thanks, queued.
>
> -- Hannes
>
Hi!
Thanks for the help!
What does it mean for it to be queued here, should I expect it to show
up on seen or next?
Harald
^ permalink raw reply
* Re: [PATCH v2] Makefile: dedup archives in $(LIBS) so link recipes don't repeat them
From: Harald Nordgren @ 2026-06-10 13:24 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2314.v2.git.git.1780610623006.gitgitgadget@gmail.com>
Hi Johannes!
Maybe this could be interesting for you to look at too.
Harald
On Fri, Jun 5, 2026 at 12:03 AM Harald Nordgren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> A handful of link recipes listed archive files twice: once explicitly
> via $(filter %.a,$^) and again implicitly through $(LIBS), which
> expanded to $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the
> linker warned about the duplicates:
>
> ld: warning: ignoring duplicate libraries: 'libgit.a', 'target/release/libgitcore.a'
>
> Redefine $(LIBS) to list archive prerequisites from $^ first, then
> the rest of the library list with those archives filtered out so each
> appears only once.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> Makefile: drop duplicate %.a from test-helper link rule
>
> Redefine $(LIBS) to list archive prerequisites from $^ first, then the
> rest of the library list to avoid brittleness in the future.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2314%2FHaraldNordgren%2Fmakefile-test-helper-dedup-libs-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2314/HaraldNordgren/makefile-test-helper-dedup-libs-v2
> Pull-Request: https://github.com/git/git/pull/2314
>
> Range-diff vs v1:
>
> 1: f6166450b0 ! 1: 0ef442ea05 Makefile: drop duplicate %.a from link recipes
> @@ Metadata
> Author: Harald Nordgren <haraldnordgren@gmail.com>
>
> ## Commit message ##
> - Makefile: drop duplicate %.a from link recipes
> + Makefile: dedup archives in $(LIBS) so link recipes don't repeat them
>
> - Three link recipes list archive files twice on the link line: once
> - via $(filter %.a,$^) and again through $(LIBS), which expands to
> - $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the linker warns
> - about the duplicates:
> + A handful of link recipes listed archive files twice: once explicitly
> + via $(filter %.a,$^) and again implicitly through $(LIBS), which
> + expanded to $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the
> + linker warned about the duplicates:
>
> ld: warning: ignoring duplicate libraries: 'libgit.a', 'target/release/libgitcore.a'
>
> - Drop the redundant filter from the test-helper, fuzz-program, and
> - unit-test recipes so they match the pattern used by other link
> - recipes in the file.
> + Redefine $(LIBS) to list archive prerequisites from $^ first, then
> + the rest of the library list with those archives filtered out so each
> + appears only once.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> ## Makefile ##
> +@@ Makefile: endif
> + #
> + # where we use it as a dependency. Since we also pull object files
> + # from the dependency list, that would make each entry appear twice.
> +-LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
> ++# Archives from $^ come first, then the rest with those archives
> ++# filtered out so each appears only once.
> ++LIBS = $(filter %.a,$^) $(filter-out $(filter %.a,$^),$(filter-out %.o,$(GITLIBS)) $(EXTLIBS))
> +
> + BASIC_CFLAGS += $(COMPAT_CFLAGS)
> + LIB_OBJS += $(COMPAT_OBJS)
> @@ Makefile: perf: all
> t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o
>
>
>
> Makefile | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b31ecb0756..a828a66f28 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2503,7 +2503,9 @@ endif
> #
> # where we use it as a dependency. Since we also pull object files
> # from the dependency list, that would make each entry appear twice.
> -LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
> +# Archives from $^ come first, then the rest with those archives
> +# filtered out so each appears only once.
> +LIBS = $(filter %.a,$^) $(filter-out $(filter %.a,$^),$(filter-out %.o,$(GITLIBS)) $(EXTLIBS))
>
> BASIC_CFLAGS += $(COMPAT_CFLAGS)
> LIB_OBJS += $(COMPAT_OBJS)
> @@ -3392,7 +3394,7 @@ perf: all
> t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o
>
> t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> check-sha1:: t/helper/test-tool$X
> t/helper/test-sha1.sh
> @@ -4015,13 +4017,13 @@ fuzz-all: $(FUZZ_PROGRAMS)
> $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS
> $(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \
> -Wl,--allow-multiple-definition \
> - $(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
> + $(filter %.o,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
>
> $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_OBJS) \
> $(GITLIBS) GIT-LDFLAGS
> $(call mkdir_p_parent_template)
> $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> - $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> + $(filter %.o,$^) $(LIBS)
>
> GIT-TEST-SUITES: FORCE
> @FLAGS='$(CLAR_TEST_SUITES)'; \
>
> base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
> --
> gitgitgadget
^ permalink raw reply
* Re: trailers: --only-trailers normalizes URLs to trailers
From: Kristoffer Haugsbakk @ 2026-06-10 14:21 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20260609004340.GF358144@coredump.intra.peff.net>
On Tue, Jun 9, 2026, at 02:43, Jeff King wrote:
> On Thu, Jun 04, 2026 at 11:27:51PM +0200, Kristoffer Haugsbakk wrote:
>
>> The following is a bug that follows straightforwardly from the documented
>> or discussed behavior. In that sense it is not a bug. But it is a bug in
>> the sense that it makes things inconvenient and violates a design goal.
>
> Yeah, though if you'll allow me to nitpick your subject a moment: I
> don't think --only-trailers is really the culprit here. It demonstrates
> the problem because it normalizes the "trailer" it found. But the loose
> trailer matching is the more fundamental issue. For example:
>
>[snip]
Yeah, this is more precise. I focused a ton on the normalized output
because that’s what makes it obvious. But the fundamental problem is
interpreting URLs like trailers.
>
>> > What's different between what you expected and what actually happened?
>>
>> In an ideal world to have some special-casing of URLs so that they are
>> not detected as trailers. Does anyone realistically want trailers like
>> this?:
>>
>> file: //...
>> http: //...
>> https: //...
>
> I could even see those as trailers, if somebody really wanted to allow
> arbitrary values that might just happen to start with "//". But without
> the whitespace after the colon, it is quite questionable.
>
>> Just special-casing `https` would go a long way.
>
> Agreed, though I think a rule like: ":// (with no whitespace)" is not a
> valid separator. Something like this:
Yes, matching on `://` strictly is a better proposal. No need to care
about `http`, `https`, `file`, etc. And both of these would *still* have
to be true for this change to be a false negative w.r.t. the user’s
intentions:
• They really input a trailer that looks like a URL, but it’s not meant
to be a URL
• They really wanted the value to start with `//`
And again I don’t think that is likely to ever happen (with a knock
on wood).
Thanks!
>
> diff --git a/trailer.c b/trailer.c
> index 6d8ec7fa8d..342ed81c78 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -635,8 +635,12 @@ static ssize_t find_separator(const char *line,
> const char *separators)
> int whitespace_found = 0;
> const char *c;
> for (c = line; *c; c++) {
> - if (strchr(separators, *c))
> + if (strchr(separators, *c)) {
> + /* special case to avoid accidental URL matches */
> + if (*c == ':' && c[1] == '/' && c[2] == '/')
> + return -1;
> return c - line;
> + }
> if (!whitespace_found && (isalnum(*c) || *c == '-'))
> continue;
> if (c != line && (*c == ' ' || *c == '\t')) {
^ permalink raw reply
* Re: [PATCH 3/3] replay: offer an option to linearize the commit topology
From: Toon Claes @ 2026-06-10 14:26 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
In-Reply-To: <xmqqtsrcvnjw.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Toon Claes <toon@iotcl.com> writes:
>
>> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>>
>> One of the stated goals of git-replay(1) is to allow implementing the
>> git-rebase(1) functionality on the server side.
>>
>> The default mode of git-rebase(1) is to act as if `--no-rebase-merges`
>> was given. This mode drops merge commits instead of replaying them, and
>> linearized the commit history into a sequence of the
>> regular (single-parent) commits.
>
> "linearized" -> "linearizes"?
Thanks.
>>
>> Add option `--linearize` to git-replay(1) do the same.
>
> "do the same" -> "to do the same"?
Ack.
>> Co-authored-by: Toon Claes <toon@iotcl.com>
>
> There is no sign-off by any of the authors?
My bad. I'll add mine.
@Johannes, can I re-add yours? I've removed it because I've made some
changes on top of the patch you wrote, but if you agree, I'll add your
Sign-off back.
>> @@ -430,12 +435,20 @@ int replay_revisions(struct rev_info *revs,
>> while ((commit = get_revision(revs))) {
>> const struct name_decoration *decoration;
>>
>> - if (commit->parents && commit->parents->next)
>> + if (opts->linearize && (!commit->parents || commit->parents->next))
>> + ; /* map current commit to the same as the previous commit */
>
> This uses the same treatment on either root commits or merge
> commits? If this were a mistake and this wants to handle merges but
> not roots, shouldn't it be more like
>
> if (opts->linearize && (commit->parents && commit->parents->next))
> ; /* map the merge to the previous */
>
>> + else if (commit->parents && commit->parents->next)
>> die(_("replaying merge commits is not supported yet!"));
>
> And because the next one is also about merges, perhaps the early
> part of this if/else if cascade can be written
>
> if (commit->parents && commit->parents->next) {
> /* We have a merge */
> if (!opts->linearize)
> die(_("can't replay a merge (yet)"));
> ; /* map current to the previous */
> } else {
> ...
>
> wouldn't it?
The way it was written in v1 was maybe a bit too smart and hard to
follow. I agree with your suggestion and will adopt this (with some
tweaks) in the next version.
> If the "map current to prev" is applicable to root, any root are
> mapped to the last_commit in the above, and if we saw a root as the
> first thing in the loop, last_commit is NULL, we do not do anything
> here, and after the if/else if/else cascade, we see last_commit is
> NULL and break out of the loop.
Yes, good observation. I did not test this.
> Perhaps we would want to have a test that replays all the way down
> to the root commit?
I'll add it.
--
Cheers,
Toon
^ permalink raw reply
* [PATCH] bash-completions: add --max-count-oldest
From: Mirko Faina @ 2026-06-10 14:38 UTC (permalink / raw)
To: git; +Cc: Mirko Faina
Add missing completion for log --max-count-oldest
Signed-off-by: Mirko Faina <mroik@delayed.space>
---
Unfortunately I forgot to add bash completions.
This is built upon 1ff279f340 (The 13th batch, 2026-06-09) with
jch/mf/revision-max-count-oldest bb4ce23284 (revision.c: implement
--max-count-oldest, 2026-05-19) merged into it.
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbf..e875787710 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2195,7 +2195,7 @@ __git_log_common_options="
--not --all
--branches --tags --remotes
--first-parent --merges --no-merges
- --max-count=
+ --max-count= --max-count-oldest=
--max-age= --since= --after=
--min-age= --until= --before=
--min-parents= --max-parents=
--
2.54.0.505.ga804828a04
^ permalink raw reply related
* [PATCH v2 0/3] Teach git-replay(1) to linearize merge commits
From: Toon Claes @ 2026-06-10 14:49 UTC (permalink / raw)
To: git; +Cc: Toon Claes, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <20260608-toon-git-replay-drop-merges-v1-0-e3ee71fce7b4@iotcl.com>
As an alternative to dscho's patch series to replay merges[1], add
option to git-replay(1) to linearize merges. This mimics wath
git-rebase(1) does too with --no-rebase-merges (the default).
The first two patches do some refactoring. The third patch implements
the actual change. I was kindly helped by dscho to implement this
change.
The --linearize option is only added to git-replay(1) and not to
git-history(1) because in my opinion doesn't make much sense to do so,
but I'm happy to hear if anyone disagrees.
This series might conflict with Kristoffer's series to make
documentation changes[2], but should be trivial to resolve. And I don't
think there's a conflict with Patrick's series on adding "drop" to
git-history(1)[3].
dscho's series to replay merges[1] need a bit of rework to fit on top of
this, but I'm happy to help figuring that out. We've been discussing to
either name the option --flatten or --linearize, but I've decided on
"linearize" because the documentation of git-rebase(1) also mentions
"linearize".
[1]: <pull.2106.git.1778107405.gitgitgadget@gmail.com>
[2]: <V2_CV_doc_replay_config.767@msgid.xyz>
[3]: <20260603-b4-pks-history-drop-v2-0-742cb5b5176d@pks.im>
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Changes in v2:
- Restructured the conditions to detect merge commits and added a line
of comment why the loop continues.
- Rewrote tests to use the history from the setup step and added a few
test cases.
- Re-added Johannes's Signed-off-by trailer. Johannes gave me the
patches with this trailer, and if I understand correctly, I can keep
it. Please let me know if that wrong.
- Link to v1: https://patch.msgid.link/20260608-toon-git-replay-drop-merges-v1-0-e3ee71fce7b4@iotcl.com
---
Johannes Schindelin (1):
replay: offer an option to linearize the commit topology
Toon Claes (2):
replay: refactor enum replay_mode into a bool
replay: add helper to put entry into mapped_commits
Documentation/git-replay.adoc | 5 ++
builtin/replay.c | 4 ++
replay.c | 114 ++++++++++++++++++++++++------------------
replay.h | 5 ++
t/t3650-replay-basics.sh | 26 ++++++++++
5 files changed, 105 insertions(+), 49 deletions(-)
Range-diff versus v1:
1: 7f3bc6f425 ! 1: 0975b142e3 replay: refactor enum replay_mode into a bool
@@ Commit message
- The value `REPLAY_MODE_REVERT` is used when option `--revert` is
passed to git-replay(1). When using this value the commits are
- possible in reverse order and the inverse of the changes are applied.
+ processed in reverse order and the inverse of the changes are
+ applied.
- The value `REPLAY_MODE_PICK` is used when either option `--onto` or
- `--advance` is used. In both cases the commits are pocessed in normal
- order, and the changes are applied as-is.
+ `--advance` is used. In both cases the commits are processed in
+ normal order, and the changes are applied as-is.
Since there are only two possible values of this enum, simplify the code
- by converting the enum into a bool. This avoid adding code paths that
- check for invalid vaues of the enum, and shortens code where the value
+ by converting the enum into a bool. This avoids adding code paths that
+ check for invalid values of the enum, and shortens code where the value
is checked with a ternary operator.
Signed-off-by: Toon Claes <toon@iotcl.com>
2: 0868871c78 ! 2: db88193624 replay: add helper to put entry into mapped_commits
@@ Commit message
replay: add helper to put entry into mapped_commits
The function replay_revisions() in replay.c is rather lengthy. Extract
- the logic to put commit entry into mapped_commits into a helper
- function.
+ the logic to put a commit entry into mapped_commits into a helper
+ function put_mapped_commit().
+
+ While at it, rename mapped_commit() to get_mapped_commit() to pair with
+ this new function.
Signed-off-by: Toon Claes <toon@iotcl.com>
3: a432ae753b ! 3: d0c220ec8e replay: offer an option to linearize the commit topology
@@ Commit message
The default mode of git-rebase(1) is to act as if `--no-rebase-merges`
was given. This mode drops merge commits instead of replaying them, and
- linearized the commit history into a sequence of the
+ linearizes the commit history into a sequence of the
regular (single-parent) commits.
- Add option `--linearize` to git-replay(1) do the same.
+ Add option `--linearize` to git-replay(1) to do the same.
Co-authored-by: Toon Claes <toon@iotcl.com>
+ Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Toon Claes <toon@iotcl.com>
## Documentation/git-replay.adoc ##
@@ Documentation/git-replay.adoc: incompatible with `--contained` (which is a modifier for `--onto` only).
@@ replay.c: int replay_revisions(struct rev_info *revs,
const struct name_decoration *decoration;
- if (commit->parents && commit->parents->next)
-+ if (opts->linearize && (!commit->parents || commit->parents->next))
-+ ; /* map current commit to the same as the previous commit */
-+ else if (commit->parents && commit->parents->next)
- die(_("replaying merge commits is not supported yet!"));
-+ else {
+- die(_("replaying merge commits is not supported yet!"));
++ if (commit->parents && commit->parents->next) {
++ if (!opts->linearize)
++ die(_("replaying merge commits is not supported yet!"));
++ /*
++ * When linearizing, a merge commit itself is not picked,
++ * but refs that point to it might need updating.
++ */
++ } else {
+ struct commit *to_pick = reverse ? last_commit : onto;
+ last_commit =
+ pick_regular_commit(revs->repo, commit,
@@ t/t3650-replay-basics.sh: test_expect_success '--onto with --ref rejects multipl
test_grep "cannot be used with multiple revision ranges" err
'
-+test_expect_success 'linearize the commit topology' '
-+ test_tick &&
-+ N=$(git commit-tree -m N -p L -p I L:) &&
-+ N=$(git commit-tree -m N-child -p $N L:) &&
-+ git update-ref refs/heads/N $N &&
++test_expect_success 'replay merge commit fails' '
++ echo "fatal: replaying merge commits is not supported yet!" >expect &&
++ test_must_fail git replay --ref-action=print --onto main I..P 2>actual &&
++ test_cmp expect actual
++'
++
++test_expect_success 'replay to rebase merge commit with --linearize' '
++ git replay --ref-action=print --linearize --onto main I..topic-with-merge >result &&
++
++ test_line_count = 1 result &&
++
++ git log --format=%s $(cut -f 3 -d " " result) >actual &&
++ test_write_lines O N J M L B A >expect &&
++ test_cmp expect actual
++'
+
-+ git replay --ref-action=print --linearize \
-+ --onto A B..refs/heads/N >out &&
++test_expect_success 'replay to rebase merge commit with --linearize down to root commit' '
++ git replay --ref-action=print --linearize --onto main A..topic-with-merge >result &&
+
-+ test_line_count = 1 out &&
-+ read N1 N2 N3 N4 <out &&
++ test_line_count = 1 result &&
+
-+ cat >expect <<-EOF &&
-+ * N-child
-+ * I
-+ * L
-+ o A
-+ EOF
-+ git log --format=%s --graph --boundary A...$N3 >actual &&
++ git log --format=%s $(cut -f 3 -d " " result) >actual &&
++ test_write_lines O N J I M L B A >expect &&
+ test_cmp expect actual
+'
+
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260604-toon-git-replay-drop-merges-807fa008d395
^ permalink raw reply
* [PATCH v2 1/3] replay: refactor enum replay_mode into a bool
From: Toon Claes @ 2026-06-10 14:49 UTC (permalink / raw)
To: git; +Cc: Toon Claes
In-Reply-To: <20260610-toon-git-replay-drop-merges-v2-0-5714a71c6d83@iotcl.com>
In 2760ee4983 (replay: add --revert mode to reverse commit changes,
2026-03-26) the enum `replay_mode` was introduced. This has two possible
values:
- The value `REPLAY_MODE_REVERT` is used when option `--revert` is
passed to git-replay(1). When using this value the commits are
processed in reverse order and the inverse of the changes are
applied.
- The value `REPLAY_MODE_PICK` is used when either option `--onto` or
`--advance` is used. In both cases the commits are processed in
normal order, and the changes are applied as-is.
Since there are only two possible values of this enum, simplify the code
by converting the enum into a bool. This avoids adding code paths that
check for invalid values of the enum, and shortens code where the value
is checked with a ternary operator.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
replay.c | 59 +++++++++++++++++++++++++----------------------------------
1 file changed, 25 insertions(+), 34 deletions(-)
diff --git a/replay.c b/replay.c
index 4ef8abb607..1f8e5b083b 100644
--- a/replay.c
+++ b/replay.c
@@ -18,11 +18,6 @@
*/
#define the_repository DO_NOT_USE_THE_REPOSITORY
-enum replay_mode {
- REPLAY_MODE_PICK,
- REPLAY_MODE_REVERT,
-};
-
static const char *short_commit_name(struct repository *repo,
struct commit *commit)
{
@@ -81,7 +76,7 @@ static struct commit *create_commit(struct repository *repo,
struct tree *tree,
struct commit *based_on,
struct commit *parent,
- enum replay_mode mode)
+ bool reverse)
{
struct object_id ret;
struct object *obj = NULL;
@@ -98,15 +93,13 @@ static struct commit *create_commit(struct repository *repo,
commit_list_insert(parent, &parents);
extra = read_commit_extra_headers(based_on, exclude_gpgsig);
- if (mode == REPLAY_MODE_REVERT) {
+ if (reverse) {
generate_revert_message(&msg, based_on, repo);
/* For revert, use current user as author (NULL = use default) */
- } else if (mode == REPLAY_MODE_PICK) {
+ } else {
find_commit_subject(message, &orig_message);
strbuf_addstr(&msg, orig_message);
author = get_author(message);
- } else {
- BUG("unexpected replay mode %d", mode);
}
reset_ident_date();
if (commit_tree_extended(msg.buf, msg.len, &tree->object.oid, parents,
@@ -269,7 +262,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
struct commit *onto,
struct merge_options *merge_opt,
struct merge_result *result,
- enum replay_mode mode,
+ bool reverse,
enum replay_empty_commit_action empty)
{
struct commit *base, *replayed_base;
@@ -287,7 +280,21 @@ static struct commit *pick_regular_commit(struct repository *repo,
replayed_base_tree = repo_get_commit_tree(repo, replayed_base);
pickme_tree = repo_get_commit_tree(repo, pickme);
- if (mode == REPLAY_MODE_PICK) {
+ if (reverse) {
+ /* Revert: swap base and pickme to reverse the diff */
+ const char *pickme_name = short_commit_name(repo, pickme);
+ merge_opt->branch1 = short_commit_name(repo, replayed_base);
+ merge_opt->branch2 = xstrfmt("parent of %s", pickme_name);
+ merge_opt->ancestor = pickme_name;
+
+ merge_incore_nonrecursive(merge_opt,
+ pickme_tree,
+ replayed_base_tree,
+ base_tree,
+ result);
+
+ free((char *)merge_opt->branch2);
+ } else {
/* Cherry-pick: normal order */
merge_opt->branch1 = short_commit_name(repo, replayed_base);
merge_opt->branch2 = short_commit_name(repo, pickme);
@@ -303,22 +310,6 @@ static struct commit *pick_regular_commit(struct repository *repo,
result);
free((char *)merge_opt->ancestor);
- } else if (mode == REPLAY_MODE_REVERT) {
- /* Revert: swap base and pickme to reverse the diff */
- const char *pickme_name = short_commit_name(repo, pickme);
- merge_opt->branch1 = short_commit_name(repo, replayed_base);
- merge_opt->branch2 = xstrfmt("parent of %s", pickme_name);
- merge_opt->ancestor = pickme_name;
-
- merge_incore_nonrecursive(merge_opt,
- pickme_tree,
- replayed_base_tree,
- base_tree,
- result);
-
- free((char *)merge_opt->branch2);
- } else {
- BUG("unexpected replay mode %d", mode);
}
merge_opt->ancestor = NULL;
merge_opt->branch2 = NULL;
@@ -341,7 +332,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
}
}
- return create_commit(repo, result->tree, pickme, replayed_base, mode);
+ return create_commit(repo, result->tree, pickme, replayed_base, reverse);
}
void replay_result_release(struct replay_result *result)
@@ -381,13 +372,13 @@ int replay_revisions(struct rev_info *revs,
char *revert;
const char *ref;
struct object_id old_oid;
- enum replay_mode mode = REPLAY_MODE_PICK;
+ bool reverse;
int ret;
advance = xstrdup_or_null(opts->advance);
revert = xstrdup_or_null(opts->revert);
- if (revert)
- mode = REPLAY_MODE_REVERT;
+ reverse = !!revert;
+
set_up_replay_mode(revs->repo, &revs->cmdline, opts->onto,
&detached_head, &advance, &revert, &onto, &update_refs);
@@ -430,8 +421,8 @@ int replay_revisions(struct rev_info *revs,
die(_("replaying merge commits is not supported yet!"));
last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
- mode == REPLAY_MODE_REVERT ? last_commit : onto,
- &merge_opt, &result, mode, opts->empty);
+ reverse ? last_commit : onto,
+ &merge_opt, &result, reverse, opts->empty);
if (!last_commit)
break;
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox