* Re: [PATCH] ls-files: filter pathspec before lstat
From: Junio C Hamano @ 2026-06-08 13:06 UTC (permalink / raw)
To: Tamir Duberstein; +Cc: git, René Scharfe, Patrick Steinhardt
In-Reply-To: <20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com>
On Sun, Jun 7, 2026 at 11:40, Tamir Duberstein wrote:
> show_files() checks whether each index entry is deleted or modified
> before show_ce() applies the pathspec. prune_index() avoids most of this
> work for pathspecs with a common directory prefix, but a top-level name
> or leading wildcard leaves every entry to be checked.
>
> Match the pathspec before lstat() for the deleted and modified modes.
> Keep the later match in show_ce() so --error-unmatch is satisfied only
> by entries that are actually shown.
Adding an extra early `match_pathspec()` check before making slow
system calls like `lstat()` makes sense, especially when most of the
index entries need to be skipped. But if most of them would match,
then we would end up doing the same match_pathspec() calls twice for
each path, and run lstat() anyway, so you may also be able to
construct a perf test that demonstrates a case where this approach
is not a clear win (or even degradation), perhaps?
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index e1a22b41b9..702c607183 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -450,6 +450,13 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
> continue;
> if (ce_skip_worktree(ce))
> continue;
> + /* Only entries shown by show_ce() satisfy --error-unmatch. */
> + if (pathspec.nr &&
> + !match_pathspec(repo->index, &pathspec, fullname.buf,
> + fullname.len, max_prefix_len, NULL,
> + S_ISDIR(ce->ce_mode) ||
> + S_ISGITLINK(ce->ce_mode)))
> + continue;
> stat_err = lstat(fullname.buf, &st);
> if (stat_err && (errno != ENOENT && errno != ENOTDIR))
> error_errno("cannot lstat '%s'", fullname.buf);
Hmph. In the current code, because there is no such pre-filtering,
show_ce() would unconditionally recurse into active submodules when
told to with the "--recurse-submodules" flag, even if your pathspec
coes not match the submodule. With this change, such a submodule
whose path does not match the pathspec would not even be seen by
show_ce(). Would it cause a change in behaviour?
^ permalink raw reply
* [PATCH v3 2/2] compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
From: Dominik Loidolt @ 2026-06-08 12:44 UTC (permalink / raw)
To: ps; +Cc: git, gitster, asedeno, asedeno, avarab, Dominik Loidolt
In-Reply-To: <20260608124419.38905-1-dominik.loidolt@univie.ac.at>
Replace the glibc-style bit-shift version comparison with an explicit
major/minor comparison. This is easier to read and is consistent with
the format already used by GIT_CLANG_PREREQ() and many BSD
<sys/cdefs.h> headers.
This has no runtime impact, as the macro is evaluated at compile time.
It is also more future-proof, as it no longer assumes that GCC version
components stay below 65536.
Signed-off-by: Dominik Loidolt <dominik.loidolt@univie.ac.at>
---
compat/posix.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/compat/posix.h b/compat/posix.h
index ffdfd91c7b..deefc43f28 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -4,22 +4,24 @@
#define _FILE_OFFSET_BITS 64
/*
- * Derived from Linux "Features Test Macro" header
- * Convenience macros to test the versions of gcc (or
- * a compatible compiler).
+ * Convenience macros to test the versions of GCC (or a compatible compiler).
* Use them like this:
* #if GIT_GNUC_PREREQ (2,8)
- * ... code requiring gcc 2.8 or later ...
+ * ... code requiring GCC 2.8 or later ...
* #endif
*
+ * Note that Clang and other compilers define __GNUC__ for compatibility; use
+ * GIT_CLANG_PREREQ() to check for specific Clang versions.
+ *
* This macro of course is not part of POSIX, but we need it for the UNUSED
* macro which is used by some of our POSIX compatibility wrappers.
-*/
+ */
#if defined(__GNUC__) && defined(__GNUC_MINOR__)
# define GIT_GNUC_PREREQ(maj, min) \
- ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
+ ((__GNUC__ > (maj)) || \
+ (__GNUC__ == (maj) && __GNUC_MINOR__ >= (min)))
#else
- #define GIT_GNUC_PREREQ(maj, min) 0
+# define GIT_GNUC_PREREQ(maj, min) 0
#endif
/* Similar for Clang. */
--
2.54.0
^ permalink raw reply related
* [PATCH v3 1/2] compat/posix.h: enable UNUSED warning messages for Clang
From: Dominik Loidolt @ 2026-06-08 12:44 UTC (permalink / raw)
To: ps; +Cc: git, gitster, asedeno, asedeno, avarab, Dominik Loidolt
In-Reply-To: <20260605094647.94805-1-dominik.loidolt@univie.ac.at>
Use a dedicated Clang version check for the UNUSED macro.
Commit 7c07f36ad2 (git-compat-util.h: GCC deprecated message arg only in
GCC 4.5+, 2022-10-05) restricted use of the deprecated attribute's
message argument in the UNUSED macro to GCC 4.5 or newer.
Clang identifies itself as GNUC 4.2.1 for compatibility, so
GIT_GNUC_PREREQ(4, 5) does not detect whether Clang supports the
deprecated("...") form. Add GIT_CLANG_PREREQ() macro and use it to
enable the UNUSED warning message for Clang 2.9 and newer.
Signed-off-by: Dominik Loidolt <dominik.loidolt@univie.ac.at>
---
v3:
- fix comment style nit
- remove unnecessary parentheses around __clang_minor__ >= (min)
compat/posix.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/compat/posix.h b/compat/posix.h
index faaae1b655..ffdfd91c7b 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -22,6 +22,15 @@
#define GIT_GNUC_PREREQ(maj, min) 0
#endif
+/* Similar for Clang. */
+#if defined(__clang__) && defined(__clang_minor__) && defined(__clang_major__)
+# define GIT_CLANG_PREREQ(maj, min) \
+ ((__clang_major__ > (maj)) || \
+ (__clang_major__ == (maj) && __clang_minor__ >= (min)))
+#else
+# define GIT_CLANG_PREREQ(maj, min) 0
+#endif
+
/*
* UNUSED marks a function parameter that is always unused. It also
* can be used to annotate a function, a variable, or a type that is
@@ -35,7 +44,7 @@
* When a parameter may be used or unused, depending on conditional
* compilation, consider using MAYBE_UNUSED instead.
*/
-#if GIT_GNUC_PREREQ(4, 5)
+#if GIT_GNUC_PREREQ(4, 5) || GIT_CLANG_PREREQ(2, 9)
#define UNUSED __attribute__((unused)) \
__attribute__((deprecated ("parameter declared as UNUSED")))
#elif defined(__GNUC__)
base-commit: a89346e34a937f001e5d397ee62224e3e9852040
--
2.54.0
^ permalink raw reply related
* [PATCH v2] parse-options: introduce die_for_missing_opt()
From: Siddharth Shrimali @ 2026-06-08 12:44 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, siddharthasthana31, toon, jn.avila,
r.siddharth.shrimali
In-Reply-To: <20260603111044.39116-1-r.siddharth.shrimali@gmail.com>
Introduce die_for_missing_opt() to check if a dependent option is
present without its required prerequisite. This provides a centralized
API for simple option dependencies (X requires Y), inspired by and
matching the style of die_for_incompatible_opt{2,3,4}().
Use the new helper in builtin/add.c to replace the manual prerequisite
check for '--pathspec-file-nul' (requires '--pathspec-from-file'). This
case is already exercised by existing tests in t3704-add-pathspec-file.sh
and several other pathspec-file test scripts, ensuring the new helper is
verified without additional test code.
Suggested-by: Christian Couder <christian.couder@gmail.com>
Suggested-by: Jean-Noël AVILA <jn.avila@free.fr>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
Changes since v1:
- Squashed the implementation patch and the caller patch into a single,
unified patch as suggested by Christian.
- Renamed the helper function from die_for_require_opt() to
die_for_missing_opt() to improve clarity.
- Updated the argument names and logic order to better match the style of
die_for_incompatible_opt*().
- Dropped the conversion of the '--ignore-missing' check in builtin/add.c
to keep this initial iteration strictly focused on a single, clean
example ('--pathspec-file-nul').
builtin/add.c | 4 ++--
parse-options.c | 7 +++++++
parse-options.h | 3 +++
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index c859f66519..505834ad3f 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -462,6 +462,8 @@ int cmd_add(int argc,
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, argv);
+ die_for_missing_opt(pathspec_file_nul, "--pathspec-file-nul",
+ !!pathspec_from_file, "--pathspec-from-file");
if (pathspec_from_file) {
if (pathspec.nr)
die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
@@ -470,8 +472,6 @@ int cmd_add(int argc,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, pathspec_from_file, pathspec_file_nul);
- } else if (pathspec_file_nul) {
- die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
}
if (require_pathspec && pathspec.nr == 0) {
diff --git a/parse-options.c b/parse-options.c
index a676da86f5..11e40669eb 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1558,3 +1558,10 @@ void die_for_incompatible_opt4(int opt1, const char *opt1_name,
break;
}
}
+
+void die_for_missing_opt(int dependent_opt, const char *dependent_opt_name,
+ int required_opt, const char *required_opt_name)
+{
+ if (dependent_opt && !required_opt)
+ die(_("the option '%s' requires '%s'"), dependent_opt_name, required_opt_name);
+}
diff --git a/parse-options.h b/parse-options.h
index 0d1f738f8d..5b41d2fd39 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -460,6 +460,9 @@ static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
0, "");
}
+void die_for_missing_opt(int dependent_opt, const char *dependent_opt_name,
+ int required_opt, const char *required_opt_name);
+
/*
* Use these assertions for callbacks that expect to be called with NONEG and
* NOARG respectively, and do not otherwise handle the "unset" and "arg"
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] describe: limit default ref iteration to tags
From: Junio C Hamano @ 2026-06-08 12:36 UTC (permalink / raw)
To: Tamir Duberstein; +Cc: git, Jeff King, Patrick Steinhardt
In-Reply-To: <20260607-describe-tag-ref-scope-v1-1-653d232b86b5@gmail.com>
Tamir Duberstein <tamird@gmail.com> writes:
[jc: Removing Shawn from CC who passed away quite a while ago, RIP].
> Unless --all is given, get_name() rejects every ref outside refs/tags/.
> The rejection happens only after the ref backend has enumerated the ref,
> so repositories with many other refs spend most of a simple describe
> invocation visiting refs which cannot affect its result.
> ...
> Both revisions were built with -O3, -mcpu=native, and ThinLTO 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.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> builtin/describe.c | 3 +++
> t/perf/p6100-describe.sh | 20 ++++++++++++++++++++
> 2 files changed, 23 insertions(+)
Interesting. How would this relate to and work well with
<20260601233727.43558-1-jacob.e.keller@intel.com>?
> +test_lazy_prereq PERF_REFFILES '
> + test "$(git rev-parse --show-ref-format)" = files
> +'
> +
> +ref_count=10000
> +
> # clear out old tags and give us a known state
> test_expect_success 'set up tags' '
> git for-each-ref --format="delete %(refname)" refs/tags >to-delete &&
> @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '
> git describe --match=new HEAD
> '
>
> +test_expect_success PERF_REFFILES 'set up many unrelated refs' '
> + git tag -m tip tip HEAD &&
> + for i in $(test_seq $ref_count)
> + do
> + printf "create refs/heads/describe-perf/%05d HEAD\n" $i ||
> + return 1
> + done >instructions &&
> + git update-ref --stdin <instructions
> +'
> +
> +test_perf 'describe exact tag with many loose refs' --prereq PERF_REFFILES '
> + git describe --exact-match HEAD
> +'
> +
Is there a strong reason to guard this new test behind
`PERF_REFFILES`?
Even though the penalty of enumerating 10,000 unrelated loose
references may be most pronounced in the `files` backend, skipping
unnecessary reference enumeration is an architectural win for other
backends (like `reftable` or a fully packed repository) as well.
If we drop `PERF_REFFILES` and retitle the test to "describe exact
tag with many unrelated refs", we could run it unconditionally to
benchmark the improvement across all storage formats.
^ permalink raw reply
* Re: [PATCH v3 4/6] diff: add long-running diff process via diff.<driver>.process
From: Junio C Hamano @ 2026-06-08 12:26 UTC (permalink / raw)
To: Michael Montalbo
Cc: Johannes Schindelin, Michael Montalbo via GitGitGadget, git
In-Reply-To: <CAC2QwmJwxpnrPNW6YLm2uXKaYjkUwjVsPN_U+c52m0rNe95_Nw@mail.gmail.com>
Michael Montalbo <mmontalbo@gmail.com> writes:
> On Sun, Jun 7, 2026 at 7:36 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>>
>> Hi Michael,
>>
>> I stumbled about this patch when it broke CI in Git for Windows, where we
>> do _not_ use `NO_PYTHON`, even though Python is unavailable in the
>> build/test CI jobs. The existing tests handle this situation gracefully,
>> this here patch does not:
>> ...
>> Given the complexity of what t4080 tries to test (error, abort, crash,
>> bad-sync, no-hunks, multiple files in one session, capability
>> negotiation), it would unfortunately be infeasible to use `test-tool
>> pkt-line` from a shell script implementing that `diff.*.process` protocol.
>>
>> So I've spiked a demo how the `test-tool diff-process-backend` could look
>> like (letting Opus do the menial typing, so that I can enjoy at least part
>> of a sunny Sunday outside), which also passes the CI build and test:
>> https://github.com/dscho/git/commit/b6e3c93381b00929476c3a00155f7cf7334a22e6
>>
>> That commit is of course not intended to be used as-is; Feel free to pick
>> code parts of it and integrate them into your topic branch. Or write your
>> own test-tool helper from scratch if that's more your jam.
>>
>
> Johannes, thank you for the great feedback. The historical context is
> really helpful and
> the concerns you raise make a lot of sense. I will take a look at your
> spike and also work
> on removing Python from the test.
Another request.
Please do not force readers to scroll through a ~800 line message
just to read only 5 lines of response from you. Keep relevant parts
of the message you are responding to in your message to help readers
understand the context in which your response was made, but trim
everything else that is not relevant from your quote.
Thanks.
^ permalink raw reply
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message
From: Junio C Hamano @ 2026-06-08 12:16 UTC (permalink / raw)
To: Pablo Sabater; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam
In-Reply-To: <20260607-ps-history-reword-v1-1-ba43a3cbb81b@gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> When using `git history reword` if the new message is the same as the
> original it continues anyway creating a new commit with the same
> message and updates its descendants, modifying the history after this
> 'reworded' commit even though there was no actual change.
>
> `git commit --amend` and `git rebase -i` + reword share this behavior,
> however `git history reword` is different:
> 1. Works in-memory without touching the index or the worktree [1], so
> there are no side effects like staged files that could justify
> rewriting the history when the commit message is the same.
> 2. `git history` by default updates all the branches [2] that contain the
> original commit making it more costly than `git rebase -i` that only
> updates the current branch.
I think the reasoning is flawed.
Both "git commit --amend" and "git rebase -i", even with no changes
to the tree, parents, or the message, update the committer timestamp
(and perhaps the committer identity running the command may be
different from the original). Updating this info is one of the
important effects of the command.
And "history" being more capable than "rebase" is a wrong excuse to
make the system behave inconsistently between commands that have
similar features [*1*]. In a situation where letting 'history'
update all the relevant branches, if a command behaves differently
from the way the user likes (and if the way 'rebase -i' works is the
one the user likes), you'd end up forcing the user to use 'rebase
-i' when 'history' would have been more appropriate.
Having said that, I personally think that the current behaviour of
`commit --amend` and `history reword` are both _wrong_ [*2*].
You may start `git commit --amend`, and after staring at the
existing commit log message for some time in your editor, it is
quite natural for you to decide that leaving the commit as-is is the
right thing [*3*] in your situation. It may have been a better
design for the system to notice this situation and leave the commit
as-is, with an override option `--force` to allow users to forcibly
update the committer ident and timestamp in the commit header. I am
not a `history reword` user (yet), but from the motivation you
described for this patch, I sense that the story is the same there.
`git rebase -i A`, when A is truly an ancestor at the bottom of a
linear history leading to HEAD, behaves slightly better. It gives
you a todo list with a bunch of `pick` insns, and when you do not
edit earliest 'pick's the todo list, these earliest commits are left
as-is. It may still share the same issue that a 'reword' that you
ended up not rewording (or 'edit' that you ended up not touching its
tree or log message) does still recreate a new commit object, though.
`git rebase -i` may have an excuse that because it, unlike "git
commit --amend", operates on multiple commits by design. A single
"--force" option given to the command would not have worked as an
escape hatch to allow the user to tell the command "in this reword
of this particular commit, I ended up doing nothing, but I still
want an updated committer log timestamp". Perhaps giving the
"--force" (or --force-rewrite") option at "rebase --continue" time
may work, but in any case, unless we plan to transition to these
"better" default behaviour at a big version boundary, speculating
what a "better" behaviour would have been may be fun but not very
productive.
[Footnote]
*1* Besides, doesn't "--update-refs" in "rebase -i" allow you to
adjust the branches?
*2* But it is an established behaviour people _rely_ on, so even
though it may have been better if these commands behaved
differently, it probably is a bit too late to change it now.
*3* This includes the case where the original author is especially
difficult to work with and would complain any change to their
commits, even if the only change you made for them is a
typofix. Fixing a small typo/grammo may not be worth your time
and unpleasant exchanges with them after touching their commit.
^ permalink raw reply
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword
From: Junio C Hamano @ 2026-06-08 12:16 UTC (permalink / raw)
To: Pablo Sabater; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam
In-Reply-To: <20260607-ps-history-reword-v1-2-ba43a3cbb81b@gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> Unlike `git commit --amend` and `git rebase -i`, `git history reword`
> doesn't print anything, this makes it feel empty for a porcelain command
> and hard to tell if the command did anything without using other
> commands like `git log <commit>` to check if the reword was done.
>
> Print a message on successful rewords so the user has feedback about it.
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> builtin/history.c | 4 ++++
> t/t3451-history-reword.sh | 14 ++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/builtin/history.c b/builtin/history.c
> index 51a22a9a1c..0f1ba3b531 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc,
> goto out;
> }
>
> + fprintf(stderr, _("Successfully reworded commit %s to %s\n"),
> + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV),
> + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV));
> +
> ret = 0;
>
> out:
Do other commands in "git history" (split is in 'master', drop and
fixup are cooking) behave with similar verbosity? Consistency within
the same "history" umbrella matters more than being similar with
other commands that can be used for similar purposes.
> diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
> index 54ea8a7207..4b22d761e3 100755
> --- a/t/t3451-history-reword.sh
> +++ b/t/t3451-history-reword.sh
> @@ -416,4 +416,18 @@ test_expect_success 'aborts if the commit message is the same' '
> )
> '
>
> +test_expect_success 'prints feedback on successful reword' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit first &&
> +
> + reword_with_message HEAD 2>err <<-EOF &&
> + first reworded
> + EOF
> + test_grep "Successfully reworded" err
> + )
> +'
> +
> test_done
^ permalink raw reply
* Re: [PATCH v3 4/6] diff: add long-running diff process via diff.<driver>.process
From: Junio C Hamano @ 2026-06-08 12:06 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Michael Montalbo via GitGitGadget, git, Michael Montalbo
In-Reply-To: <c7987f11-9181-3975-552c-14e74abb2c97@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> So the conscious project direction has been: fold pkt-line test backends
> into `test-tool` and drop the scripting-language prereq. Reintroducing the
> same shape in Python would walk this back.
> ...
> The `PYTHON` prereq exists in exactly five files today, all `git p4`
> related (where Python is an intrinsic prerequisite given that `git-p4.py`
> _is_ written in Python): `t/lib-git-p4.sh`, `t/t9802-git-p4-filetype.sh`,
> `t/t9810-git-p4-rcs.sh`, `t/t9835-git-p4-metadata-encoding-python2.sh`,
> and `t/t9836-git-p4-metadata-encoding-python3.sh`.
> ...
> That commit is of course not intended to be used as-is; Feel free to pick
> code parts of it and integrate them into your topic branch. Or write your
> own test-tool helper from scratch if that's more your jam.
Showing better direction to new folks with such a clear thinking is
very much appreciated. Even though it is natural and perfectly OK
for tests that interacts with parts of Git that are written in these
languages (e.g., we are OK for gitweb tests to require Perl), we
should consciously keep ourselves clean and not adding unnecessary
dependencies.
^ permalink raw reply
* Re: [PATCH v3 0/8] setup: centralize object database creation
From: Junio C Hamano @ 2026-06-08 12:06 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Patrick Steinhardt, git, Kristoffer Haugsbakk
In-Reply-To: <CAOLa=ZQwVbLsOcajaxQwtkTPm=4St7EiGEEyL6_B0o3Tt1v1pw@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> Hi,
>>
>> this small patch series refactors the logic for how we discover and
>> configure repositories. Most importantly, this involves the following
>> two steps:
>>
>> 1. We unify the logic to apply the repository format, which is
>> currently open-coded across multiple sites. These sites have
>> already diverged, where some repository extensions are not
>> consistently applied.
>>
>> 2. We then centralize creation of the object database to happen at the
>> same time we apply the repository format.
>>
>> The end result is that we apply the repository format exactly once, and
>> that's also the point in time where we can finalize the setup of the
>> repo's data structures as we know about all details of the repo at that
>> time. Ultimately, this makes it trivial to introduce the "objectStorage"
>> extension, even though that's not part of this patch series.
>> ...
>> 4: 81b92bca7f = 4: b0d7c11fe6 repository: stop initializing the object database in `repo_set_gitdir()`
>> 5: 807fc56353 = 5: d0af56fdae setup: stop creating the object database in `setup_git_env()`
>> 6: 96563ff99f = 6: 3e75c5b0a6 setup: stop initializing object database without repository
>> 7: c14f45169c = 7: 50fa2fdb3c repository: stop reading loose object map twice on repo init
>> 8: e67c6e66d6 = 8: 4dff9d1794 setup: construct object database in `apply_repository_format()`
>>
>
> The range-diff looks good and as expected. Thanks!
Thanks, both of you. Let me mark the topic for 'next', then.
^ permalink raw reply
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
From: Junio C Hamano @ 2026-06-08 11:56 UTC (permalink / raw)
To: René Scharfe
Cc: Kristofer Karlsson, Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <1aa5b755-0f74-46d5-bd6e-a9cb7f3fbb12@web.de>
René Scharfe <l.s.r@web.de> writes:
> I think I mostly understand it now: cascade is better in prio_queue_get()
> because the sift-down item is from the bottom and will likely end up back
> at the bottom, just of a different branch of the heap. Thus a sift-down
> costs 3 compares times the number of levels, while a cascade costs just
> 2 compares times the number of levels and there is likely little to no
> need to sift it back up.
>
> For prio_queue_replace() we sift down a random item, though; we don't
> know where it will end up. If it belongs at the very top then sift-down
> just needs 3 compares, while cascade needs 2 compares times the number
> of levels to bring the hole down and the same to bring the item up.
An excellent observation, showing clear and analytic mind. This is
one of the reasons why I love reading review messages from you (and
also explanation in the proposed commit log messages in your
patches).
Thanks.
^ permalink raw reply
* inconsistent order of --diff-algorithm variants in man pages
From: Vincent Lefevre @ 2026-06-08 11:26 UTC (permalink / raw)
To: git
In Documentation/diff-algorithm-option.adoc, which is used by the
git-blame(1) and git-diff(1) man pages:
`--diff-algorithm=(patience|minimal|histogram|myers)`::
Choose a diff algorithm. The variants are as follows:
+
--
`default`;;
`myers`;;
The basic greedy diff algorithm. Currently, this is the default.
`minimal`;;
Spend extra time to make sure the smallest possible diff is
produced.
`patience`;;
Use "patience diff" algorithm when generating patches.
`histogram`;;
This algorithm extends the patience algorithm to "support
low-occurrence common elements".
--
I think that using the same order in the --diff-algorithm line and
in the description that follows would be better, i.e.
--diff-algorithm=(myers|minimal|patience|histogram)
FYI, the text was added in 07924d4d50e5304fb53eb60aaba8aef31d4c4e5e
in 2013, but without any explanation on this difference.
--
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / Pascaline project (LIP, ENS-Lyon)
^ permalink raw reply
* Re: [BUG] "git diff --word-diff" gives a diff while they are only space changes
From: Vincent Lefevre @ 2026-06-08 10:58 UTC (permalink / raw)
To: Michael Montalbo; +Cc: Junio C Hamano, Chris Torek, Johannes Sixt, git
In-Reply-To: <CAC2QwmKjr2eiFNPPmERq7n-UjE-SF2vE4eHDanYE-4heWxzQVw@mail.gmail.com>
On 2026-05-28 12:25:01 -0700, Michael Montalbo wrote:
> > Thanks for the ideas, Chris. Here is my attempt at synthesizing Chris'
> > suggestions and Junio's feedback:
> >
> > The `--word-diff` option operates by taking the same line-by-line
> > diff that is produced without the option and computing
> > word-by-word changes within each hunk. This may produce a
> > larger diff than a dedicated word-diff tool would. If Git
> > acquires a different implementation in the future, the output
> > may change. Note that this is similar to the `--diff-algorithm`
> > option, which may also change the output.
> >
> > Does this work?
>
> Updated the patch with the revised wording:
> https://lore.kernel.org/git/pull.2113.git.1778686956622.gitgitgadget@gmail.com/T/#t
>
> Please feel free to pick up, modify, or drop as appropriate.
Just to say that this new text is fine for me.
--
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / Pascaline project (LIP, ENS-Lyon)
^ permalink raw reply
* Re: [PATCH v3] doc: fix typos via codespell
From: Junio C Hamano @ 2026-06-08 10:56 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Andrew Kreimer, git
In-Reply-To: <3398ef40-1547-4324-2cfc-97b9e2b24854@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> I'll squash the fix-up I already had into [v2] that I have queued,
>> which should be sufficient to get to the state this [v3] should have
>> been, I think.
>
> The mechanical nature of these fixes explains another issue: One typo fix
> touched two test fixtures which might seem harmless at first, but those
> fixtures are littered with checksums that relied on the original
> (misspelled) form.
>
> Please adopt this follow-up into ak/typofixes:
Thanks. You often keep your eyes peeled to spot these forgotten
bits, which is very very much appreciated.
I briefly wondered if this was to be done by me or it was an request
to Andrew, but since I've promised to squash the update into what I
have myself, I'll do another squashing into the result, instead of
asking Andrew to update the v2+v3 with these fixes.
Luckily, b8b38eee85 is *not* yet in 'next', so we can just squash
the [v3] from Andrew and this fixup from you into it to keep the
test passing with or without the "typo fix", to maintain
bisectability.
Somebody has to come up with a bit of tweak to the log message to
explain what has been done in all these three pieces when it
happens. I may ask some agent to prepare a draft, review it myself,
and perhaps redo it myself from the originals without taking
anything from agent output, as I am still skeptical about all these
AI hype ;-).
> -- snipsnap --
> From 54aa4f7f7adf0c0e02b5463b5f7f64547e80cbce Mon Sep 17 00:00:00 2001
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Sat, 6 Jun 2026 22:09:04 +0200
> Subject: [PATCH] svn-test-dumps: restore checksums after the `hapenning` typo
> fix
>
> b8b38eee85 (doc: fix typos via codespell, 2026-05-31) ran codespell
> against the entire tree and rewrote `hapenning` to `happening`
> inside the body of `t/t9150/svk-merge.dump` and
> `t/t9151/svn-mergeinfo.dump`. Both files are Subversion dump
> files: each `Node-path:` block embeds `Text-content-md5` /
> `Text-content-sha1` for the new content and, on copy operations,
> `Text-copy-source-md5` / `Text-copy-source-sha1` for the source
> content as observed at the cited revision. None of those
> checksums were updated, so loading the dumps with svnadmin 1.14.5
> (present in `ubuntu:rolling`'s CI image) fails immediately with
> `E200014: Checksum mismatch for '/trunk/Makefile'` and the two
> tests stop before any of the assertions they actually exercise can
> run. The CI failure has been visible on every `seen`-based
> linux-sha256 / linux-reftable build since 2026-06-02 (the first
> run that picked up b8b38eee85).
>
> Because `happening` and `hapenning` have the same length, no
> header byte counts need updating; only the embedded checksums do.
> Recompute the MD5 and SHA1 of every text body in the two dumps,
> and for every `Node-copyfrom-path` consult the path's most
> recently defined content to refresh the corresponding
> `Text-copy-source-md5` / `Text-copy-source-sha1`. After this,
> `svnadmin load -q` accepts both dumps cleanly and t9150 and t9151
> get past their setup steps.
>
> This commit only touches the two dump files; the typo correction
> in their surrounding human-readable comment is preserved.
>
> Assisted-by: Opus 4.7
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> t/t9150/svk-merge.dump | 10 ++++----
> t/t9151/svn-mergeinfo.dump | 48 +++++++++++++++++++-------------------
> 2 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/t/t9150/svk-merge.dump b/t/t9150/svk-merge.dump
> index 6a8ac81b11e6..3c46afc18a65 100644
> --- a/t/t9150/svk-merge.dump
> +++ b/t/t9150/svk-merge.dump
> @@ -71,7 +71,7 @@ Node-kind: file
> Node-action: add
> Prop-content-length: 10
> Text-content-length: 2401
> -Text-content-md5: bfd8ff778d1492dc6758567373176a89
> +Text-content-md5: d6a3917748b0c09ad85c2783f1d4dac1
> Content-length: 2411
>
> PROPS-END
> @@ -201,7 +201,7 @@ Node-path: branches/left/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2465
> -Text-content-md5: 16e38d9753b061731650561ce01b1195
> +Text-content-md5: 3f413450a7a26596d9e512ee385a9b19
> Content-length: 2465
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -305,7 +305,7 @@ Node-path: trunk/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2521
> -Text-content-md5: 0668418a621333f4aa8b6632cd63e2a0
> +Text-content-md5: 89788781014278d76ff23648b8b08b2d
> Content-length: 2521
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -412,7 +412,7 @@ Node-path: branches/left/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2593
> -Text-content-md5: 5ccff689fb290e00b85fe18ee50c54ba
> +Text-content-md5: 706d73919e6f319a0e624aa50c8b8b38
> Content-length: 2593
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -529,7 +529,7 @@ Node-path: trunk/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2713
> -Text-content-md5: 0afbe34f244cd662b1f97d708c687f90
> +Text-content-md5: 1c05266da99e8f01a5ccf816be47a484
> Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> diff --git a/t/t9151/svn-mergeinfo.dump b/t/t9151/svn-mergeinfo.dump
> index d5e169563745..ad741400104e 100644
> --- a/t/t9151/svn-mergeinfo.dump
> +++ b/t/t9151/svn-mergeinfo.dump
> @@ -80,8 +80,8 @@ Node-kind: file
> Node-action: add
> Prop-content-length: 10
> Text-content-length: 2401
> -Text-content-md5: bfd8ff778d1492dc6758567373176a89
> -Text-content-sha1: 103205ce331f7d64086dba497574734f78439590
> +Text-content-md5: d6a3917748b0c09ad85c2783f1d4dac1
> +Text-content-sha1: 9ffe895eb95d4a7c2ee2712dcf7a13637edee6a9
> Content-length: 2411
>
> PROPS-END
> @@ -194,8 +194,8 @@ Node-kind: file
> Node-action: add
> Node-copyfrom-rev: 2
> Node-copyfrom-path: trunk/Makefile
> -Text-copy-source-md5: bfd8ff778d1492dc6758567373176a89
> -Text-copy-source-sha1: 103205ce331f7d64086dba497574734f78439590
> +Text-copy-source-md5: d6a3917748b0c09ad85c2783f1d4dac1
> +Text-copy-source-sha1: 9ffe895eb95d4a7c2ee2712dcf7a13637edee6a9
>
>
> Revision-number: 4
> @@ -228,8 +228,8 @@ Node-kind: file
> Node-action: add
> Node-copyfrom-rev: 2
> Node-copyfrom-path: trunk/Makefile
> -Text-copy-source-md5: bfd8ff778d1492dc6758567373176a89
> -Text-copy-source-sha1: 103205ce331f7d64086dba497574734f78439590
> +Text-copy-source-md5: d6a3917748b0c09ad85c2783f1d4dac1
> +Text-copy-source-sha1: 9ffe895eb95d4a7c2ee2712dcf7a13637edee6a9
>
>
> Revision-number: 5
> @@ -254,8 +254,8 @@ Node-path: branches/left/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2465
> -Text-content-md5: 16e38d9753b061731650561ce01b1195
> -Text-content-sha1: 36da4b84ea9b64218ab48171dfc5c48ae025f38b
> +Text-content-md5: 3f413450a7a26596d9e512ee385a9b19
> +Text-content-sha1: b3cd389d63c5e3af4fe22b7464cf97968662ad1a
> Content-length: 2465
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -359,8 +359,8 @@ Node-path: branches/right/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2521
> -Text-content-md5: 0668418a621333f4aa8b6632cd63e2a0
> -Text-content-sha1: 4f29afd038e52f45acb5ef8c41acfc70062a741a
> +Text-content-md5: 89788781014278d76ff23648b8b08b2d
> +Text-content-sha1: f52afb2d6230e5a418416b77c3c9ad610edfd202
> Content-length: 2521
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -467,8 +467,8 @@ Node-path: branches/left/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2529
> -Text-content-md5: f6b197cc3f2e89a83e545d4bb003de73
> -Text-content-sha1: 2f656677cfec0bceec85e53036ffb63e25126f8e
> +Text-content-md5: abcac8d04eb061b0a3053e359e44a2a0
> +Text-content-sha1: 866caf95e04809a5ed897aea41075b24833612ea
> Content-length: 2529
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -572,8 +572,8 @@ Node-path: branches/left/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2593
> -Text-content-md5: 5ccff689fb290e00b85fe18ee50c54ba
> -Text-content-sha1: a13de8e23f1483efca3e57b2b64b0ae6f740ce10
> +Text-content-md5: 706d73919e6f319a0e624aa50c8b8b38
> +Text-content-sha1: 9992d5a9aea960c7856ef6a9364aedd5b710ef53
> Content-length: 2593
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -689,8 +689,8 @@ Node-kind: file
> Node-action: add
> Node-copyfrom-rev: 8
> Node-copyfrom-path: branches/left/Makefile
> -Text-copy-source-md5: 5ccff689fb290e00b85fe18ee50c54ba
> -Text-copy-source-sha1: a13de8e23f1483efca3e57b2b64b0ae6f740ce10
> +Text-copy-source-md5: 706d73919e6f319a0e624aa50c8b8b38
> +Text-copy-source-sha1: 9992d5a9aea960c7856ef6a9364aedd5b710ef53
>
>
>
> @@ -761,8 +761,8 @@ Node-path: trunk/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2593
> -Text-content-md5: 5ccff689fb290e00b85fe18ee50c54ba
> -Text-content-sha1: a13de8e23f1483efca3e57b2b64b0ae6f740ce10
> +Text-content-md5: 706d73919e6f319a0e624aa50c8b8b38
> +Text-content-sha1: 9992d5a9aea960c7856ef6a9364aedd5b710ef53
> Content-length: 2593
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -942,8 +942,8 @@ Node-path: trunk/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2713
> -Text-content-md5: 0afbe34f244cd662b1f97d708c687f90
> -Text-content-sha1: 46d9377d783e67a9b581da110352e799517c8a14
> +Text-content-md5: 1c05266da99e8f01a5ccf816be47a484
> +Text-content-sha1: 0cba212974e2b288389d73317f3220be11158e00
> Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -1166,8 +1166,8 @@ Node-path: branches/left-sub/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2713
> -Text-content-md5: 0afbe34f244cd662b1f97d708c687f90
> -Text-content-sha1: 46d9377d783e67a9b581da110352e799517c8a14
> +Text-content-md5: 1c05266da99e8f01a5ccf816be47a484
> +Text-content-sha1: 0cba212974e2b288389d73317f3220be11158e00
> Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
> @@ -1408,8 +1408,8 @@ Node-path: branches/left/Makefile
> Node-kind: file
> Node-action: change
> Text-content-length: 2713
> -Text-content-md5: 0afbe34f244cd662b1f97d708c687f90
> -Text-content-sha1: 46d9377d783e67a9b581da110352e799517c8a14
> +Text-content-md5: 1c05266da99e8f01a5ccf816be47a484
> +Text-content-sha1: 0cba212974e2b288389d73317f3220be11158e00
> Content-length: 2713
>
> # -DCOLLISION_CHECK if you believe that SHA1's
^ permalink raw reply
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message
From: Pablo Sabater @ 2026-06-08 10:52 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Kaartic Sivaraam
In-Reply-To: <aiaLxNwGPko5HS2G@pks.im>
El lun, 8 jun 2026 a las 11:30, Patrick Steinhardt (<ps@pks.im>) escribió:
>
> On Sun, Jun 07, 2026 at 10:07:20PM +0200, Pablo Sabater wrote:
> > When using `git history reword` if the new message is the same as the
> > original it continues anyway creating a new commit with the same
> > message and updates its descendants, modifying the history after this
> > 'reworded' commit even though there was no actual change.
> >
> > `git commit --amend` and `git rebase -i` + reword share this behavior,
> > however `git history reword` is different:
> > 1. Works in-memory without touching the index or the worktree [1], so
> > there are no side effects like staged files that could justify
> > rewriting the history when the commit message is the same.
> > 2. `git history` by default updates all the branches [2] that contain the
> > original commit making it more costly than `git rebase -i` that only
> > updates the current branch.
> >
> > Add a check if the original commit message is the same as the new one
> > and abort if so.
> >
> > [1]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/
> > [2]: https://git-scm.com/docs/git-history#_description
>
> Nit: I feel like both of the links don't really add much value.
I'll just drop em.
>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> > builtin/history.c | 10 ++++++++++
> > t/t3451-history-reword.sh | 20 ++++++++++++++++++++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/builtin/history.c b/builtin/history.c
> > index 0fc06fb204..51a22a9a1c 100644
> > --- a/builtin/history.c
> > +++ b/builtin/history.c
> > @@ -135,6 +135,13 @@ static int commit_tree_ext(struct repository *repo,
> > original_body, action, &commit_message);
> > if (ret < 0)
> > goto out;
> > +
> > + if (!strcmp(original_body, commit_message.buf)) {
> > + fprintf(stderr, _("Message unchanged,"
> > + " aborting reword.\n"));
> > + ret = 1;
> > + goto out;
> > + }
> > } else {
> > strbuf_addstr(&commit_message, original_body);
> > }
>
> We also execute this logic via "git history fixup --reedit-message", and
> here it wouldn't make sense to abort the commit in case the message is
> unchanged.
True I hadn't thought that, I made it here because we have both the
original and new message before creating the new commit. We could let
ret = 1 mean that the commit message is the same and then
cmd_history_fixup ignores ret = 1 and for cmd_history_reword handle
the abort.
What do you think?
>
> Patrick
--
Pablo
^ permalink raw reply
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword
From: Pablo Sabater @ 2026-06-08 10:45 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Kaartic Sivaraam
In-Reply-To: <aiaLyQvo8kqfv4js@pks.im>
El lun, 8 jun 2026 a las 11:30, Patrick Steinhardt (<ps@pks.im>) escribió:
>
> On Sun, Jun 07, 2026 at 10:07:21PM +0200, Pablo Sabater wrote:
> > Unlike `git commit --amend` and `git rebase -i`, `git history reword`
> > doesn't print anything, this makes it feel empty for a porcelain command
> > and hard to tell if the command did anything without using other
> > commands like `git log <commit>` to check if the reword was done.
> >
> > Print a message on successful rewords so the user has feedback about it.
>
> I dunno about this one. My take here is that a command should be silent
> unless it has something to say, for example when it couldn't honor the
> user's request [1].
But neither `git commit --amend` nor `git rebase -i` follow this rule
of silence.
>
> > diff --git a/builtin/history.c b/builtin/history.c
> > index 51a22a9a1c..0f1ba3b531 100644
> > --- a/builtin/history.c
> > +++ b/builtin/history.c
> > @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc,
> > goto out;
> > }
> >
> > + fprintf(stderr, _("Successfully reworded commit %s to %s\n"),
> > + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV),
> > + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV));
> > +
>
> Seeing the implementation also raises a couple of questions:
>
> - Why do we mention the rewritten commit, only? Shouldn't we also
> print the changed HEAD?
Because `git history reword <commit>` is for a single commit. After
the reword the hash changes and the original hash is no longer useful
to check the rewritten message. If I want to see how it is now:
$ git history reword aabb
$ git log aabb <- I can't check how it is now because this is the old one
So to check the new one I have to search the new hash. Imagine if it's
the first of 20 long commit messages, I have to git log --oneline, get
the hash and then git log new_hash, which IMO is unnecessary when git
history reword can output the new hash.
>
> - Why don't we print any of the other rewritten branches?
Haven't thought of that, it's nice that it does modify all branches, I
just assumed that the most relevant is the current branch new commit
hash. The other rewritten branches have the same commit message, just
different hashes.
>
> - What makes "git history reword" so special as compared to for
> example "git history fixup" or "git history split" so that it needs
> a message while the others don't?
Nothing, I just wanted this specifically for reword and sent this very
simple as an RFC to discuss the idea, I could extend this where it
fits.
>
> It might make sense to maybe introduce a verbose mode where we do print
> such information. But if so, we should have good answers to the above
> questions and implement this in a way that makes sense for the other
> subcommands, too, so that we can apply the same principle to all of
> them.
I like the verbose mode idea but I still think that on non-verbose
something should be printed, on verbose it could be printed
additionally all the rewritten commits (though it could get very
noisy), the changed HEAD, etc.
>
> Thanks!
>
> Patrick
>
> [1]: https://www.linfo.org/rule_of_silence.html
--
Pablo
^ permalink raw reply
* [PATCH v3 9/9] builtin/history: implement "drop" subcommand
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
A common operation when editing the commit history is to drop a specific
commit from the history entirely, but this operation is not currently
covered by git-history(1).
A couple of noteworthy bits:
- This is the first git-history(1) command that will ultimately result
in changes to both the index and the working tree. We thus have to
add logic to merge resulting changes into those.
- It is still not possible to replay merge commits, so this limitation
is inherited for the new "drop" command.
- For now we refuse to drop root commits. While we _can_ indeed drop
root commits in the general case, there are edge cases where the
resulting history would become completely empty. This is thus left
to a subsequent patch series.
Other than that, most of the logic is rather straight-forward as we can
continue to build on the preexisting logic in git-history(1) for most of
the part.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-history.adoc | 38 ++-
builtin/history.c | 187 ++++++++++++++
t/meson.build | 1 +
t/t3454-history-drop.sh | 537 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 762 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 2ba8121795..28b477cd37 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -8,6 +8,7 @@ git-history - EXPERIMENTAL: Rewrite history
SYNOPSIS
--------
[synopsis]
+git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]
git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
@@ -51,13 +52,28 @@ be stateful operations. The limitation can be lifted once (if) Git learns about
first-class conflicts.
When using `fixup` with `--empty=drop`, dropping the root commit is not yet
-supported.
+supported. Likewise, `drop` cannot remove the root commit or a merge commit.
COMMANDS
--------
The following commands are available to rewrite history in different ways:
+`drop <commit>`::
+ Remove the specified commit from the history. All descendants of the
+ commit are replayed directly onto its parent.
++
+The root commit cannot be dropped as that may lead to edge cases where refs
+end up with no commits anymore. Merge commits cannot be dropped either; see
+LIMITATIONS.
++
+If `HEAD` points at a commit that is to be rewritten, the index and working
+tree are updated to match the new `HEAD`. The command aborts before any
+references are updated in case local modifications would be overwritten.
++
+If replaying any descendant would result in a conflict, the command aborts
+with an error.
+
`fixup <commit>`::
Apply the currently staged changes to the specified commit. This is
similar in nature to `git commit --fixup=<commit>` followed by `git
@@ -170,6 +186,26 @@ The staged addition of `unrelated.txt` has been incorporated into the `first`
commit. All descendant commits have been replayed on top of the rewritten
history.
+Drop a commit
+~~~~~~~~~~~~~
+
+----------
+$ git log --oneline
+abc1234 (HEAD -> main) third
+def5678 second
+ghi9012 first
+
+$ git history drop 'main^{/second}'
+
+$ git log --oneline
+jkl3456 (HEAD -> main) third
+ghi9012 first
+----------
+
+The `second` commit has been removed from the history, and `third` has been
+replayed directly on top of `first`. All branches that pointed at the dropped
+commit have been moved to its parent.
+
Split a commit
~~~~~~~~~~~~~~
diff --git a/builtin/history.c b/builtin/history.c
index 4fadf38c32..5c2cdc673b 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -17,13 +17,17 @@
#include "read-cache.h"
#include "refs.h"
#include "replay.h"
+#include "reset.h"
#include "revision.h"
#include "sequencer.h"
#include "strvec.h"
#include "tree.h"
+#include "tree-walk.h"
#include "unpack-trees.h"
#include "wt-status.h"
+#define GIT_HISTORY_DROP_USAGE \
+ N_("git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]")
#define GIT_HISTORY_FIXUP_USAGE \
N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]")
#define GIT_HISTORY_REWORD_USAGE \
@@ -1001,12 +1005,194 @@ static int cmd_history_split(int argc,
return ret;
}
+static int update_worktree(struct repository *repo,
+ const struct commit *old_head,
+ const struct commit *new_head,
+ bool dry_run)
+{
+ struct reset_head_opts opts = {
+ .oid_from = &old_head->object.oid,
+ .oid = &new_head->object.oid,
+ .flags = RESET_HEAD_SKIP_REF_UPDATES,
+ };
+ if (dry_run)
+ opts.flags |= RESET_HEAD_DRY_RUN;
+ return reset_head(repo, &opts);
+}
+
+static int find_head_tree_change(struct repository *repo,
+ const struct replay_result *result,
+ struct commit **old_head,
+ struct commit **new_head,
+ bool *changed)
+{
+ const struct replay_ref_update *head_update = NULL;
+ struct commit *old_head_commit, *new_head_commit;
+ struct tree *old_head_tree, *new_head_tree;
+ const char *head_target;
+ int head_flags;
+
+ *changed = false;
+
+ head_target = refs_resolve_ref_unsafe(get_main_ref_store(repo),
+ "HEAD", RESOLVE_REF_NO_RECURSE,
+ NULL, &head_flags);
+ if (!head_target)
+ return error(_("cannot look up HEAD"));
+ if (!(head_flags & REF_ISSYMREF))
+ head_target = "HEAD";
+
+ for (size_t i = 0; i < result->updates_nr; i++) {
+ if (!strcmp(result->updates[i].refname, head_target)) {
+ head_update = &result->updates[i];
+ break;
+ }
+ }
+
+ if (!head_update)
+ return 0;
+
+ old_head_commit = lookup_commit_reference(repo, &head_update->old_oid);
+ new_head_commit = lookup_commit_reference(repo, &head_update->new_oid);
+ if (!old_head_commit || !new_head_commit)
+ return error(_("cannot resolve HEAD commit"));
+
+ old_head_tree = repo_get_commit_tree(repo, old_head_commit);
+ new_head_tree = repo_get_commit_tree(repo, new_head_commit);
+ if (!old_head_tree || !new_head_tree)
+ return error(_("cannot resolve tree for HEAD"));
+
+ if (oideq(&old_head_tree->object.oid, &new_head_tree->object.oid))
+ return 0;
+
+ *old_head = old_head_commit;
+ *new_head = new_head_commit;
+ *changed = true;
+
+ return 0;
+}
+
+static int cmd_history_drop(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo)
+{
+ const char * const usage[] = {
+ GIT_HISTORY_DROP_USAGE,
+ NULL,
+ };
+ enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
+ enum ref_action action = REF_ACTION_DEFAULT;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+ N_("control which refs should be updated"),
+ PARSE_OPT_NONEG, parse_ref_action),
+ OPT_BOOL('n', "dry-run", &dry_run,
+ N_("perform a dry-run without updating any refs")),
+ OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
+ N_("how to handle descendants that become empty"),
+ PARSE_OPT_NONEG, parse_opt_empty),
+ OPT_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
+ struct commit *original, *rewritten;
+ struct rev_info revs = { 0 };
+ struct replay_result result = { 0 };
+ struct commit *old_head, *new_head;
+ bool head_moves = false;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single revision"));
+ goto out;
+ }
+ repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+
+ original = lookup_commit_reference_by_name(argv[0]);
+ if (!original) {
+ ret = error(_("commit cannot be found: %s"), argv[0]);
+ goto out;
+ }
+
+ if (!original->parents) {
+ ret = error(_("cannot drop root commit %s: "
+ "it has no parent to replay onto"),
+ argv[0]);
+ goto out;
+ } else if (original->parents->next) {
+ ret = error(_("cannot drop merge commit: %s"), argv[0]);
+ goto out;
+ }
+
+ ret = setup_revwalk(repo, action, original, &revs);
+ if (ret)
+ goto out;
+
+ rewritten = original->parents->item;
+
+ ret = compute_pending_ref_updates(&revs, action, original, rewritten,
+ empty, &result);
+ if (ret) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+ }
+
+ /*
+ * If HEAD will move as a result of the rewrite then we'll have to
+ * merge in the changes into the worktree and index. This merge can of
+ * course conflict, which will cause the whole operation to abort.
+ *
+ * If we had already updated the refs at that point then we'd have an
+ * inconsistent repository state. So we first perform a dry-run merge
+ * here before updating refs.
+ */
+ if (!is_bare_repository()) {
+ ret = find_head_tree_change(repo, &result, &old_head,
+ &new_head, &head_moves);
+ if (ret < 0)
+ goto out;
+
+ if (head_moves && update_worktree(repo, old_head, new_head, true) < 0) {
+ ret = error(_("dropping this commit would "
+ "overwrite local changes; aborting"));
+ goto out;
+ }
+ }
+
+ strbuf_addf(&reflog_msg, "drop: dropping %s", argv[0]);
+ ret = apply_pending_ref_updates(repo, &result, reflog_msg.buf, dry_run);
+ if (ret < 0) {
+ ret = error(_("failed to update references"));
+ goto out;
+ }
+
+ if (!dry_run && head_moves && update_worktree(repo, old_head, new_head, false) < 0) {
+ ret = error(_("could not update working tree to new commit %s"),
+ oid_to_hex(&new_head->object.oid));
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ replay_result_release(&result);
+ strbuf_release(&reflog_msg);
+ release_revisions(&revs);
+ return ret;
+}
+
int cmd_history(int argc,
const char **argv,
const char *prefix,
struct repository *repo)
{
const char * const usage[] = {
+ GIT_HISTORY_DROP_USAGE,
GIT_HISTORY_FIXUP_USAGE,
GIT_HISTORY_REWORD_USAGE,
GIT_HISTORY_SPLIT_USAGE,
@@ -1014,6 +1200,7 @@ int cmd_history(int argc,
};
parse_opt_subcommand_fn *fn = NULL;
struct option options[] = {
+ OPT_SUBCOMMAND("drop", &fn, cmd_history_drop),
OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
OPT_SUBCOMMAND("split", &fn, cmd_history_split),
diff --git a/t/meson.build b/t/meson.build
index 2af8d01279..d5e71056b2 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -399,6 +399,7 @@ integration_tests = [
't3451-history-reword.sh',
't3452-history-split.sh',
't3453-history-fixup.sh',
+ 't3454-history-drop.sh',
't3500-cherry.sh',
't3501-revert-cherry-pick.sh',
't3502-cherry-pick-merge.sh',
diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
new file mode 100755
index 0000000000..0f33247212
--- /dev/null
+++ b/t/t3454-history-drop.sh
@@ -0,0 +1,537 @@
+#!/bin/sh
+
+test_description='tests for git-history drop subcommand'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-log-graph.sh"
+
+expect_graph () {
+ cat >expect &&
+ lib_test_cmp_graph --format=%s "$@"
+}
+
+expect_log () {
+ git log --format="%s" "$@" >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+test_expect_success 'errors on missing commit argument' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history drop 2>err &&
+ test_grep "command expects a single revision" err
+ )
+'
+
+test_expect_success 'errors on too many arguments' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history drop HEAD HEAD 2>err &&
+ test_grep "command expects a single revision" err
+ )
+'
+
+test_expect_success 'errors on unknown revision' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history drop does-not-exist 2>err &&
+ test_grep "commit cannot be found: does-not-exist" err
+ )
+'
+
+test_expect_success 'errors with invalid --empty= value' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_commit second &&
+ test_must_fail git history drop --empty=bogus HEAD 2>err &&
+ test_grep "unrecognized.*--empty.*bogus" err
+ )
+'
+
+test_expect_success 'drops a commit in the middle and replays descendants' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git symbolic-ref HEAD >expect &&
+ git history drop HEAD~ &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
+
+ expect_log <<-\EOF &&
+ third
+ first
+ EOF
+
+ test_must_fail git show HEAD:second.t &&
+ test_path_is_missing second.t &&
+
+ git reflog >reflog &&
+ test_grep "drop: dropping HEAD~" reflog
+ )
+'
+
+test_expect_success 'drops the HEAD commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+
+ git history drop HEAD &&
+
+ expect_log <<-\EOF
+ first
+ EOF
+ )
+'
+
+test_expect_success 'drops a commit on detached HEAD' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+ git checkout --detach HEAD &&
+
+ git history drop HEAD~ &&
+
+ expect_log <<-\EOF
+ third
+ first
+ EOF
+ )
+'
+
+# Note: in this case it would actually be fine to drop the root commit, as we
+# do have a descendant commit, and no reference points to the root commit
+# directly. So this is something that we may relax eventually.
+test_expect_success 'refuses to drop the root commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+
+ test_must_fail git history drop HEAD~ 2>err &&
+ test_grep "cannot drop root commit" err
+ )
+'
+
+# In contrast to the above case, we actually don't want to drop the root commit
+# here as that would cause us to end up with an empty commit graph.
+test_expect_success 'refuses to drop the root commit when branch becomes empty' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+
+ test_must_fail git history drop HEAD 2>err &&
+ test_grep "cannot drop root commit" err
+ )
+'
+
+test_expect_success 'refuses to drop a merge commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch branch &&
+ test_commit ours &&
+ git switch branch &&
+ test_commit theirs &&
+ git switch - &&
+ git merge theirs &&
+
+ test_must_fail git history drop HEAD 2>err &&
+ test_grep "cannot drop merge commit" err
+ )
+'
+
+test_expect_success 'refuses when descendants contain a merge commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit middle &&
+ git branch branch &&
+ test_commit ours &&
+ git switch branch &&
+ test_commit theirs &&
+ git switch - &&
+ git merge theirs &&
+
+ test_must_fail git history drop middle 2>err &&
+ test_grep "replaying merge commits is not supported yet" err
+ )
+'
+
+test_expect_success 'works in a bare repository' '
+ test_when_finished "rm -rf repo repo.git" &&
+
+ git init repo &&
+ test_commit -C repo first &&
+ test_commit -C repo second &&
+ test_commit -C repo third &&
+
+ git clone --bare repo repo.git &&
+ (
+ cd repo.git &&
+
+ git history drop HEAD~ &&
+ expect_log <<-\EOF
+ third
+ first
+ EOF
+ )
+'
+
+test_expect_success 'updates branches on other lines of descent' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit target &&
+ git branch theirs &&
+ test_commit ours &&
+ git switch theirs &&
+ test_commit theirs &&
+
+ expect_graph --branches <<-\EOF &&
+ * theirs
+ | * ours
+ |/
+ * target
+ * base
+ EOF
+
+ git history drop target &&
+
+ expect_graph --branches <<-\EOF
+ * ours
+ | * theirs
+ |/
+ * base
+ EOF
+ )
+'
+
+test_expect_success 'moves branch pointing at dropped commit to its parent' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ git branch points-at-second &&
+ test_commit third &&
+
+ git rev-parse first >expect &&
+ git history drop second &&
+ git rev-parse points-at-second >actual &&
+ test_cmp expect actual &&
+
+ expect_log --format="%s %D" --branches <<-\EOF
+ third HEAD -> main
+ first tag: first, points-at-second
+ EOF
+ )
+'
+
+test_expect_success '--dry-run prints ref updates without modifying repo' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch branch &&
+ test_commit middle &&
+ test_commit ours &&
+ git switch branch &&
+ test_commit theirs &&
+
+ git refs list >refs-expect &&
+ git history drop --dry-run main~ >updates &&
+ git refs list >refs-actual &&
+ test_cmp refs-expect refs-actual &&
+ test_grep "update refs/heads/main" updates &&
+
+ git update-ref --stdin <updates &&
+ expect_log main <<-\EOF
+ ours
+ base
+ EOF
+ )
+'
+
+test_expect_success '--dry-run detects conflicts with modified working tree' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second modify-me &&
+ echo modified >modify-me &&
+
+ git refs list >refs-expect &&
+ git diff >diff-expect &&
+ test_must_fail git history drop --dry-run HEAD 2>err &&
+ test_grep "dropping this commit would overwrite local changes" err &&
+ git diff >diff-actual &&
+ git refs list >refs-actual &&
+
+ test_cmp diff-expect diff-actual &&
+ test_cmp refs-expect refs-actual
+ )
+'
+
+test_expect_success '--update-refs=head updates only HEAD' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit target &&
+ git branch theirs &&
+ test_commit ours &&
+ git switch theirs &&
+ test_commit theirs &&
+
+ # When told to update HEAD only, the command refuses to
+ # rewrite commits that are not an ancestor of HEAD.
+ test_must_fail git history drop --update-refs=head main 2>err &&
+ test_grep "rewritten commit must be an ancestor of HEAD" err &&
+
+ expect_graph --branches <<-\EOF &&
+ * theirs
+ | * ours
+ |/
+ * target
+ * base
+ EOF
+
+ git switch main &&
+ git history drop --update-refs=head target &&
+
+ expect_graph --branches <<-\EOF
+ * ours
+ | * theirs
+ | * target
+ |/
+ * base
+ EOF
+ )
+'
+
+test_expect_success 'conflict with replayed commit aborts cleanly' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit conflict-a file &&
+ test_commit conflict-b file &&
+
+ git refs list >refs-expect &&
+ test_must_fail git history drop HEAD~ 2>err &&
+ test_grep "failed replaying descendants" err &&
+ git refs list >refs-actual &&
+ test_cmp refs-expect refs-actual
+ )
+'
+
+# Build a history where a descendant of the drop target reverts the change
+# introduced by the drop target. After dropping, the descendant's diff applies
+# against a tree that already lacks the change, so it becomes empty.
+setup_empty_descendant_repo () {
+ git init "$1" &&
+ (
+ cd "$1" &&
+ echo C1 >file &&
+ git add file &&
+ git commit -m "base" &&
+ git tag base &&
+ echo C2 >file &&
+ git add file &&
+ git commit -m "drop-me" &&
+ git tag drop-me &&
+ test_commit middle &&
+ echo C1 >file &&
+ git add file &&
+ git commit -m "revert-drop-me" &&
+ git tag revert-drop-me
+ )
+}
+
+test_expect_success '--empty=drop drops descendants that become empty' '
+ test_when_finished "rm -rf repo" &&
+ setup_empty_descendant_repo repo &&
+ (
+ cd repo &&
+
+ git history drop --empty=drop drop-me &&
+
+ expect_log <<-\EOF
+ middle
+ base
+ EOF
+ )
+'
+
+test_expect_success '--empty=keep keeps descendants that become empty' '
+ test_when_finished "rm -rf repo" &&
+ setup_empty_descendant_repo repo &&
+ (
+ cd repo &&
+
+ git history drop --empty=keep drop-me &&
+
+ expect_log <<-\EOF &&
+ revert-drop-me
+ middle
+ base
+ EOF
+ git diff HEAD~ HEAD >diff &&
+ test_must_be_empty diff
+ )
+'
+
+test_expect_success '--empty=abort errors out when a descendant becomes empty' '
+ test_when_finished "rm -rf repo" &&
+ setup_empty_descendant_repo repo &&
+ (
+ cd repo &&
+
+ test_must_fail git history drop --empty=abort drop-me 2>err &&
+ test_grep "became empty after replay" err
+ )
+'
+
+test_expect_success 'updates index and worktree when HEAD moves' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git history drop second &&
+
+ # Worktree should no longer contain second.t.
+ test_path_is_missing second.t &&
+ test_path_is_file first.t &&
+ test_path_is_file third.t &&
+
+ # Index and worktree should both match the new HEAD.
+ git status --porcelain --untracked-files=no >status &&
+ test_must_be_empty status
+ )
+'
+
+test_expect_success 'updates worktree when dropping HEAD itself' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+
+ git history drop HEAD &&
+
+ test_path_is_missing second.t &&
+ test_path_is_file first.t &&
+
+ git status --porcelain --untracked-files=no >status &&
+ test_must_be_empty status
+ )
+'
+
+test_expect_success 'preserves unrelated unstaged modifications' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ echo first-content >unrelated.txt &&
+ git add unrelated.txt &&
+ git commit -m "add unrelated" &&
+ test_commit second &&
+ test_commit third &&
+
+ echo locally-modified >unrelated.txt &&
+
+ git diff >diff-expect &&
+ git history drop second &&
+ git diff >diff-actual &&
+ test_cmp diff-expect diff-actual &&
+ test_path_is_missing second.t
+ )
+'
+
+test_expect_success 'preserves unrelated staged changes' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ echo first-content >unrelated.txt &&
+ git add unrelated.txt &&
+ git commit -m "add unrelated" &&
+ test_commit second &&
+ test_commit third &&
+
+ echo staged-change >unrelated.txt &&
+ git add unrelated.txt &&
+
+ git diff --cached >diff-expect &&
+ git history drop second &&
+ git diff --cached >diff-actual &&
+ test_cmp diff-expect diff-actual &&
+ test_path_is_missing second.t
+ )
+'
+
+test_expect_success 'aborts when local modifications would be overwritten' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit conflict &&
+
+ echo local-edit >conflict.t &&
+ git diff >diff-expect &&
+ test_must_fail git history drop HEAD 2>err &&
+ test_grep "would overwrite local changes" err &&
+ git diff >diff-actual &&
+ test_cmp diff-expect diff-actual
+ )
+'
+
+test_done
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 8/9] builtin/history: split handling of ref updates into two phases
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
The function `handle_reference_updates()` is used by git-history(1) to
update all references that refer to commits that have been rewritten. As
such, it performs two steps:
- It gathers the references that need to be updated in the first
place.
- It prepares and commits the reference transaction.
In a subsequent commit we'll want to handle those two steps separately.
Prepare for this by splitting up the function into two.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/history.c | 102 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 64 insertions(+), 38 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 0fc06fb204..4fadf38c32 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -333,21 +333,17 @@ static int handle_ref_update(struct ref_transaction *transaction,
NULL, NULL, 0, reflog_msg, err);
}
-static int handle_reference_updates(struct rev_info *revs,
- enum ref_action action,
- struct commit *original,
- struct commit *rewritten,
- const char *reflog_msg,
- int dry_run,
- enum replay_empty_commit_action empty)
+static int compute_pending_ref_updates(struct rev_info *revs,
+ enum ref_action action,
+ struct commit *original,
+ struct commit *rewritten,
+ enum replay_empty_commit_action empty,
+ struct replay_result *result)
{
const struct name_decoration *decoration;
struct replay_revisions_options opts = {
.empty = empty,
};
- struct replay_result result = { 0 };
- struct ref_transaction *transaction = NULL;
- struct strbuf err = STRBUF_INIT;
char hex[GIT_MAX_HEXSZ + 1];
bool detached_head;
int head_flags = 0;
@@ -359,34 +355,13 @@ static int handle_reference_updates(struct rev_info *revs,
opts.onto = oid_to_hex_r(hex, &rewritten->object.oid);
- ret = replay_revisions(revs, &opts, &result);
+ ret = replay_revisions(revs, &opts, result);
if (ret)
- goto out;
+ return ret;
if (action != REF_ACTION_BRANCHES && action != REF_ACTION_HEAD)
BUG("unsupported ref action %d", action);
- if (!dry_run) {
- transaction = ref_store_transaction_begin(get_main_ref_store(revs->repo), 0, &err);
- if (!transaction) {
- ret = error(_("failed to begin ref transaction: %s"), err.buf);
- goto out;
- }
- }
-
- for (size_t i = 0; i < result.updates_nr; i++) {
- ret = handle_ref_update(transaction,
- result.updates[i].refname,
- &result.updates[i].new_oid,
- &result.updates[i].old_oid,
- reflog_msg, &err);
- if (ret) {
- ret = error(_("failed to update ref '%s': %s"),
- result.updates[i].refname, err.buf);
- goto out;
- }
- }
-
/*
* `replay_revisions()` only updates references that are
* ancestors of `rewritten`, so we need to manually
@@ -414,14 +389,43 @@ static int handle_reference_updates(struct rev_info *revs,
!detached_head)
continue;
+ ALLOC_GROW(result->updates, result->updates_nr + 1, result->updates_alloc);
+ result->updates[result->updates_nr].refname = xstrdup(decoration->name);
+ result->updates[result->updates_nr].old_oid = original->object.oid;
+ result->updates[result->updates_nr].new_oid = rewritten->object.oid;
+ result->updates_nr++;
+ }
+
+ return 0;
+}
+
+static int apply_pending_ref_updates(struct repository *repo,
+ const struct replay_result *result,
+ const char *reflog_msg,
+ int dry_run)
+{
+ struct ref_transaction *transaction = NULL;
+ struct strbuf err = STRBUF_INIT;
+ int ret;
+
+ if (!dry_run) {
+ transaction = ref_store_transaction_begin(get_main_ref_store(repo),
+ 0, &err);
+ if (!transaction) {
+ ret = error(_("failed to begin ref transaction: %s"), err.buf);
+ goto out;
+ }
+ }
+
+ for (size_t i = 0; i < result->updates_nr; i++) {
ret = handle_ref_update(transaction,
- decoration->name,
- &rewritten->object.oid,
- &original->object.oid,
+ result->updates[i].refname,
+ &result->updates[i].new_oid,
+ &result->updates[i].old_oid,
reflog_msg, &err);
if (ret) {
ret = error(_("failed to update ref '%s': %s"),
- decoration->name, err.buf);
+ result->updates[i].refname, err.buf);
goto out;
}
}
@@ -435,11 +439,33 @@ static int handle_reference_updates(struct rev_info *revs,
out:
ref_transaction_free(transaction);
- replay_result_release(&result);
strbuf_release(&err);
return ret;
}
+static int handle_reference_updates(struct rev_info *revs,
+ enum ref_action action,
+ struct commit *original,
+ struct commit *rewritten,
+ const char *reflog_msg,
+ int dry_run,
+ enum replay_empty_commit_action empty)
+{
+ struct replay_result result = { 0 };
+ int ret;
+
+ ret = compute_pending_ref_updates(revs, action, original, rewritten,
+ empty, &result);
+ if (ret)
+ goto out;
+
+ ret = apply_pending_ref_updates(revs->repo, &result, reflog_msg, dry_run);
+
+out:
+ replay_result_release(&result);
+ return ret;
+}
+
static int commit_became_empty(struct repository *repo,
struct commit *original,
struct tree *result)
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 7/9] reset: stop assuming that the caller passes in a clean index
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
In 652bd0211d (rebase: use 'skip_cache_tree_update' option, 2022-11-10),
we updated `reset_head()` to stop updating the index tree cache. This
was done as a performance optimization: the function is only called by
"sequencer.c" and "rebase.c", both of which assume a clean index before
they perform their operation, so we know that the end result will be a
clean index, too. Consequently, we can skip recomputing the cache as we
can instead use `prime_cache_tree()` directly.
In a subsequent commit we're about to add a new caller though where the
assumption doesn't hold anymore: the index may be dirty before calling
`reset_head()`, and consequently we cannot prime the cache with a given
tree anymore as the index and tree will mismatch.
Adapt the logic so that we only skip the cache tree update in case we're
doing a hard reset. While we could introduce logic that only skips the
update in case the incoming index was dirty already, that doesn't really
feel worth it: after all, the mentioned commit says itself that the
performance improvement was negligible anyway.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/reset.c b/reset.c
index 5ba9a3a574..2fabc54d9b 100644
--- a/reset.c
+++ b/reset.c
@@ -166,10 +166,11 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
unpack_tree_opts.dry_run = dry_run;
unpack_tree_opts.merge = 1;
unpack_tree_opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
- unpack_tree_opts.skip_cache_tree_update = 1;
init_checkout_metadata(&unpack_tree_opts.meta, switch_to_branch, oid, NULL);
- if (reset_hard)
+ if (reset_hard) {
+ unpack_tree_opts.skip_cache_tree_update = 1;
unpack_tree_opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
+ }
if (!reset_hard && !fill_tree_descriptor(r, &desc[nr++], &head_oid)) {
ret = error(_("failed to find tree of %s"),
@@ -196,7 +197,8 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- prime_cache_tree(r, r->index, tree);
+ if (reset_hard)
+ prime_cache_tree(r, r->index, tree);
if (write_locked_index(r->index, &lock, COMMIT_LOCK) < 0) {
ret = error(_("could not write index"));
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 6/9] reset: allow the caller to specify the current HEAD object
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
When calling `reset_head()` we automatically derive the commit that the
callers wants to move from by reading the HEAD commit. Some callers may
already have resolved it, or they may want to move from a different
commit that doesn't match HEAD.
Introduce a new `oid_from` option that lets the caller specify the
commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 5 ++++-
reset.h | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/reset.c b/reset.c
index f88f32d563..5ba9a3a574 100644
--- a/reset.c
+++ b/reset.c
@@ -121,7 +121,10 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- if (!repo_get_oid(r, "HEAD", &head_oid)) {
+ if (opts->oid_from) {
+ oidcpy(&head_oid, opts->oid_from);
+ head = &head_oid;
+ } else if (!repo_get_oid(r, "HEAD", &head_oid)) {
head = &head_oid;
} else if (!oid || !reset_hard) {
ret = error(_("could not determine HEAD revision"));
diff --git a/reset.h b/reset.h
index d2f8546844..9387fc7dce 100644
--- a/reset.h
+++ b/reset.h
@@ -37,6 +37,11 @@ struct reset_head_opts {
* The commit to checkout/reset to. Defaults to HEAD.
*/
const struct object_id *oid;
+ /*
+ * The commit to checkout/reset from when doing a two-way merge. This
+ * is used as one of the sides to merge.
+ */
+ const struct object_id *oid_from;
/*
* Optional value to set ORIG_HEAD. Defaults to HEAD.
*/
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 5/9] reset: introduce ability to skip reference updates
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
In a subsequent commit we'll introduce a new caller to `reset_head()`
that really only wants to update the index and working tree, without
updating any references. Introduce a new flag that lets the caller
perform this operation.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 7 ++++++-
reset.h | 3 +++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/reset.c b/reset.c
index 8fb39d4c51..f88f32d563 100644
--- a/reset.c
+++ b/reset.c
@@ -93,6 +93,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
unsigned update_orig_head = opts->flags & RESET_HEAD_UPDATE_ORIG_HEAD;
unsigned dry_run = opts->flags & RESET_HEAD_DRY_RUN;
+ unsigned skip_ref_updates = opts->flags & RESET_HEAD_SKIP_REF_UPDATES;
struct object_id *head = NULL, head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
@@ -112,6 +113,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (opts->branch_msg && !opts->branch)
BUG("branch reflog message given without a branch");
+ if (skip_ref_updates && (opts->branch || refs_only || update_orig_head))
+ BUG("asked to perform ref updates and skip them at the same time");
+
if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
@@ -196,7 +200,8 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- if (oid != &head_oid || update_orig_head || switch_to_branch)
+ if (!skip_ref_updates &&
+ (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 cc9fd4378a..d2f8546844 100644
--- a/reset.h
+++ b/reset.h
@@ -27,6 +27,9 @@ enum reset_head_flags {
* any user-visible state.
*/
RESET_HEAD_DRY_RUN = (1 << 5),
+
+ /* Skip updating any references, only update the worktree and index. */
+ RESET_HEAD_SKIP_REF_UPDATES = (1 << 6),
};
struct reset_head_opts {
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 4/9] reset: introduce dry-run mode
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
In a subsequent commit we'll add another caller to `reset_head()` that
wants to perform a dry-run check of whether it would be possible to
update the index and working tree when moving to a new commit. Introduce
a new flag that lets the caller perform this operation.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 44 +++++++++++++++++++++++++++++++++-----------
reset.h | 6 ++++++
2 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/reset.c b/reset.c
index 228cad5f42..8fb39d4c51 100644
--- a/reset.c
+++ b/reset.c
@@ -92,11 +92,14 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
unsigned update_orig_head = opts->flags & RESET_HEAD_UPDATE_ORIG_HEAD;
+ unsigned dry_run = opts->flags & RESET_HEAD_DRY_RUN;
struct object_id *head = NULL, head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
struct unpack_trees_options unpack_tree_opts = { 0 };
struct tree *tree;
+ struct index_state scratch_index = INDEX_STATE_INIT(r);
+ struct index_state *istate;
const char *action;
int ret = 0, nr = 0;
@@ -109,7 +112,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (opts->branch_msg && !opts->branch)
BUG("branch reflog message given without a branch");
- if (!refs_only && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
+ if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
}
@@ -124,16 +127,36 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (!oid)
oid = &head_oid;
- if (refs_only)
- return update_refs(r, opts, oid, head);
+ if (refs_only) {
+ if (!dry_run)
+ return update_refs(r, opts, oid, head);
+ return 0;
+ }
+
+ if (dry_run) {
+ if (read_index_from(&scratch_index, r->index_file, r->gitdir) < 0 ||
+ index_state_unmerged_to_stage0(&scratch_index) < 0) {
+ ret = error(_("could not read index"));
+ goto leave_reset_head;
+ }
+
+ istate = &scratch_index;
+ } else {
+ if (repo_read_index_unmerged(r) < 0) {
+ ret = error(_("could not read index"));
+ goto leave_reset_head;
+ }
+ istate = r->index;
+ }
action = reset_hard ? "reset" : "checkout";
setup_unpack_trees_porcelain(&unpack_tree_opts, action);
unpack_tree_opts.head_idx = 1;
- unpack_tree_opts.src_index = r->index;
- unpack_tree_opts.dst_index = r->index;
+ unpack_tree_opts.src_index = istate;
+ unpack_tree_opts.dst_index = istate;
unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
- unpack_tree_opts.update = 1;
+ unpack_tree_opts.update = !dry_run;
+ unpack_tree_opts.dry_run = dry_run;
unpack_tree_opts.merge = 1;
unpack_tree_opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
unpack_tree_opts.skip_cache_tree_update = 1;
@@ -141,11 +164,6 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (reset_hard)
unpack_tree_opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
- if (repo_read_index_unmerged(r) < 0) {
- ret = error(_("could not read index"));
- goto leave_reset_head;
- }
-
if (!reset_hard && !fill_tree_descriptor(r, &desc[nr++], &head_oid)) {
ret = error(_("failed to find tree of %s"),
oid_to_hex(&head_oid));
@@ -162,6 +180,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
+ if (dry_run)
+ goto leave_reset_head;
+
tree = repo_parse_tree_indirect(r, oid);
if (!tree) {
ret = error(_("unable to read tree (%s)"), oid_to_hex(oid));
@@ -181,6 +202,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
leave_reset_head:
rollback_lock_file(&lock);
clear_unpack_trees_porcelain(&unpack_tree_opts);
+ release_index(&scratch_index);
while (nr)
free((void *)desc[--nr].buffer);
return ret;
diff --git a/reset.h b/reset.h
index 0bf25c51de..cc9fd4378a 100644
--- a/reset.h
+++ b/reset.h
@@ -21,6 +21,12 @@ enum reset_head_flags {
/* Update ORIG_HEAD as well as HEAD */
RESET_HEAD_UPDATE_ORIG_HEAD = (1 << 4),
+
+ /*
+ * Perform a dry-run by performing the operation without updating
+ * any user-visible state.
+ */
+ RESET_HEAD_DRY_RUN = (1 << 5),
};
struct reset_head_opts {
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 3/9] reset: modernize flags passed to `reset_head()`
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
The flags passed to `reset_head()` are declared as defines. This has
fallen a bit out of practice nowadays, where we instead prefer to use
enums.
Modernize the code accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rebase.c | 2 +-
reset.c | 4 ++--
reset.h | 30 ++++++++++++++++++------------
sequencer.c | 2 +-
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index fa4f5d9306..4c20bd50cb 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1876,7 +1876,7 @@ 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 |
+ ropts.flags = RESET_HEAD_DETACH | RESET_HEAD_UPDATE_ORIG_HEAD |
RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
ropts.default_reflog_action = options.reflog_action;
diff --git a/reset.c b/reset.c
index 3b3cb74dab..228cad5f42 100644
--- a/reset.c
+++ b/reset.c
@@ -18,7 +18,7 @@ static int update_refs(struct repository *repo,
{
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 update_orig_head = opts->flags & RESET_HEAD_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;
@@ -91,7 +91,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
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 update_orig_head = opts->flags & RESET_HEAD_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 a28f81829d..0bf25c51de 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_head_flags {
+ /* Request a detached checkout */
+ RESET_HEAD_DETACH = (1 << 0),
+
+ /* Request a reset rather than a checkout */
+ RESET_HEAD_HARD = (1 << 1),
+
+ /* Run the post-checkout hook */
+ RESET_HEAD_RUN_POST_CHECKOUT_HOOK = (1 << 2),
+
+ /* Only update refs, do not touch the worktree */
+ RESET_HEAD_REFS_ONLY = (1 << 3),
+
+ /* Update ORIG_HEAD as well as HEAD */
+ RESET_HEAD_UPDATE_ORIG_HEAD = (1 << 4),
+};
struct reset_head_opts {
/*
@@ -33,7 +39,7 @@ struct reset_head_opts {
/*
* Flags defined above.
*/
- unsigned flags;
+ enum reset_head_flags flags;
/*
* Optional reflog message for branch, defaults to head_msg.
*/
@@ -45,7 +51,7 @@ struct reset_head_opts {
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_HEAD_UPDATE_ORIG_HEAD then default_reflog_action must be given.
*/
const char *orig_head_msg;
/*
diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..c46c606b3e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4870,7 +4870,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
struct reset_head_opts ropts = {
.oid = onto,
.orig_head = orig_head,
- .flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
+ .flags = RESET_HEAD_DETACH | RESET_HEAD_UPDATE_ORIG_HEAD |
RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
.head_msg = reflog_message(opts, "start", "checkout %s",
onto_name),
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 2/9] reset: drop `USE_THE_REPOSITORY_VARIABLE`
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
In "reset.c" we still have references to `the_repository`, even though
the only entry point into the file already receives a repository as
parameter.
Update all uses of `the_repository` to instead use the passed-in repo
and drop `USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/reset.c b/reset.c
index 46e30e6394..3b3cb74dab 100644
--- a/reset.c
+++ b/reset.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "cache-tree.h"
#include "gettext.h"
@@ -13,7 +11,8 @@
#include "unpack-trees.h"
#include "hook.h"
-static int update_refs(const struct reset_head_opts *opts,
+static int update_refs(struct repository *repo,
+ const struct reset_head_opts *opts,
const struct object_id *oid,
const struct object_id *head)
{
@@ -42,19 +41,19 @@ static int update_refs(const struct reset_head_opts *opts,
prefix_len = msg.len;
if (update_orig_head) {
- if (!repo_get_oid(the_repository, "ORIG_HEAD", &oid_old_orig))
+ if (!repo_get_oid(repo, "ORIG_HEAD", &oid_old_orig))
old_orig = &oid_old_orig;
if (head) {
if (!reflog_orig_head) {
strbuf_addstr(&msg, "updating ORIG_HEAD");
reflog_orig_head = msg.buf;
}
- refs_update_ref(get_main_ref_store(the_repository),
+ refs_update_ref(get_main_ref_store(repo),
reflog_orig_head, "ORIG_HEAD",
orig_head ? orig_head : head,
old_orig, 0, UPDATE_REFS_MSG_ON_ERR);
} else if (old_orig)
- refs_delete_ref(get_main_ref_store(the_repository),
+ refs_delete_ref(get_main_ref_store(repo),
NULL, "ORIG_HEAD", old_orig, 0);
}
@@ -64,23 +63,23 @@ static int update_refs(const struct reset_head_opts *opts,
reflog_head = msg.buf;
}
if (!switch_to_branch)
- ret = refs_update_ref(get_main_ref_store(the_repository),
+ ret = refs_update_ref(get_main_ref_store(repo),
reflog_head, "HEAD", oid, head,
detach_head ? REF_NO_DEREF : 0,
UPDATE_REFS_MSG_ON_ERR);
else {
- ret = refs_update_ref(get_main_ref_store(the_repository),
+ ret = refs_update_ref(get_main_ref_store(repo),
reflog_branch ? reflog_branch : reflog_head,
switch_to_branch, oid, NULL, 0,
UPDATE_REFS_MSG_ON_ERR);
if (!ret)
- ret = refs_update_symref(get_main_ref_store(the_repository),
+ ret = refs_update_symref(get_main_ref_store(repo),
"HEAD", switch_to_branch,
reflog_head);
}
if (!ret && run_hook)
- run_hooks_l(the_repository, "post-checkout",
- oid_to_hex(head ? head : null_oid(the_hash_algo)),
+ run_hooks_l(repo, "post-checkout",
+ oid_to_hex(head ? head : null_oid(repo->hash_algo)),
oid_to_hex(oid), "1", NULL);
strbuf_release(&msg);
return ret;
@@ -126,7 +125,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
oid = &head_oid;
if (refs_only)
- return update_refs(opts, oid, head);
+ return update_refs(r, opts, oid, head);
action = reset_hard ? "reset" : "checkout";
setup_unpack_trees_porcelain(&unpack_tree_opts, action);
@@ -163,7 +162,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- tree = repo_parse_tree_indirect(the_repository, oid);
+ tree = repo_parse_tree_indirect(r, oid);
if (!tree) {
ret = error(_("unable to read tree (%s)"), oid_to_hex(oid));
goto leave_reset_head;
@@ -177,7 +176,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
}
if (oid != &head_oid || update_orig_head || switch_to_branch)
- ret = update_refs(opts, oid, head);
+ ret = update_refs(r, opts, oid, head);
leave_reset_head:
rollback_lock_file(&lock);
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 1/9] read-cache: split out function to drop unmerged entries to stage 0
From: Patrick Steinhardt @ 2026-06-08 10:23 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im>
In `repo_read_index_unmerged()` we read the index and then drop any
unmerged entries to stage 0. In a subsequent commit we'll want to
perform this operation on arbitrary indexes, not only the one of the
given repository.
Prepare for this by splitting out the functionality into a new function
that can act on an arbitrary index.
While at it, fix a signedness mismatch when iterating through the index
cache entries.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
read-cache-ll.h | 1 +
read-cache.c | 12 +++++++-----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 2c8b4b21b1..71b87615eb 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -309,6 +309,7 @@ int write_locked_index(struct index_state *, struct lock_file *lock, unsigned fl
void discard_index(struct index_state *);
void move_index_extensions(struct index_state *dst, struct index_state *src);
int unmerged_index(const struct index_state *);
+int index_state_unmerged_to_stage0(struct index_state *istate);
/**
* Returns 1 if istate differs from tree, 0 otherwise. If tree is NULL,
diff --git a/read-cache.c b/read-cache.c
index 21829102ae..799a5bc719 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3403,13 +3403,15 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
*/
int repo_read_index_unmerged(struct repository *repo)
{
- struct index_state *istate;
- int i;
+ repo_read_index(repo);
+ return index_state_unmerged_to_stage0(repo->index);
+}
+
+int index_state_unmerged_to_stage0(struct index_state *istate)
+{
int unmerged = 0;
- repo_read_index(repo);
- istate = repo->index;
- for (i = 0; i < istate->cache_nr; i++) {
+ for (unsigned int i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce = istate->cache[i];
struct cache_entry *new_ce;
int len;
--
2.54.0.1136.gdb2ca164c4.dirty
^ 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