* Re: [PATCH GSoC v14 09/13] serve: advertise object-info feature
From: Karthik Nayak @ 2026-06-28 22:55 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, chandrapratap3519, chriscool, eric.peijian, gitster,
jltobler, peff, toon, Calvin Wan, Jonathan Tan
In-Reply-To: <CAN5EUNTrdNArd5SX9df6x9bOhRfzE4c7dLOuNu7ONUdn4TLsUA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3367 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> El sáb, 27 jun 2026 a las 0:23, Karthik Nayak
> (<karthik.188@gmail.com>) escribió:
>>
>> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>>
>> > From: Calvin Wan <calvinwan@google.com>
>> >
>> > In order for a client to know what object-info components a server can
>> > provide, advertise supported object-info features. This will allow a
>> > client to decide whether to query the server for object-info or fetch
>> > as a fallback.
>> >
>> > Helped-by: Jonathan Tan <jonathantanmy@google.com>
>> > Helped-by: Christian Couder <chriscool@tuxfamily.org>
>> > Signed-off-by: Calvin Wan <calvinwan@google.com>
>> > Signed-off-by: Eric Ju <eric.peijian@gmail.com>
>> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
>> > ---
>> > serve.c | 5 ++++-
>> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/serve.c b/serve.c
>> > index 49a6e39b1d..2b07d922b3 100644
>> > --- a/serve.c
>> > +++ b/serve.c
>> > @@ -89,7 +89,7 @@ static void session_id_receive(struct repository *r UNUSED,
>> > trace2_data_string("transfer", NULL, "client-sid", client_sid);
>> > }
>> >
>> > -static int object_info_advertise(struct repository *r, struct strbuf *value UNUSED)
>> > +static int object_info_advertise(struct repository *r, struct strbuf *value)
>> > {
>> > if (advertise_object_info == -1 &&
>> > repo_config_get_bool(r, "transfer.advertiseobjectinfo",
>> > @@ -97,6 +97,9 @@ static int object_info_advertise(struct repository *r, struct strbuf *value UNUS
>> > /* disabled by default */
>> > advertise_object_info = 0;
>> > }
>> > + /* Currently only size is supported */
>> > + if (value && advertise_object_info)
>> > + strbuf_addstr(value, "size");
>>
>> So is the plan that further options will be added here to value? If so,
>> whats the format we will follow?
>
> Hi!
> The current documented format is at `gitprotocol-v2.adoc`, however I
> think it could be improved. I have a more complete version in the
> not-yet-sent %(objecttype) support series, but since the question
> comes up here, I will update the format documentation in this series
> for size only:
>
Ah nice, I didn't see that.
> oid <oid>
> Indicates to the server an object which the client wants to obtain
> - information for.
> + information for. They must be full object IDs.
>
> - info = PKT-LINE(attrs) LF)
> + info = PKT-LINE(attrs LF)
> *PKT-LINE(obj-info LF)
>
> attrs = attr | attrs SP attrs
>
> + obj-size = 1*DIGIT
> +
> attr = "size"
>
> - obj-info = obj-id SP obj-size
> + obj-info = obj-id SP [obj-size]
> +
> +If the server does not recognize the object id, the response will be
> +`obj-id SP` regardless of the number of attributes requested.
>
> About the names `size` and future ones `type` they are arbitrarily
> chosen, so for example: `delta:base` could be `delta`. They are
> appended to the buffer so in case of adding `type`, it would look
> like:
>
> strbuf_addstr(value, "size type");
>
> What do you think?
>
I didn't know this part was already documented, so its all good :)
> Thanks for the review,
> Pablo.
>
>>
>> > return advertise_object_info;
>> > }
>> >
>> >
>> > --
>> > 2.54.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH] meson: wire up USE_NSEC build knob
From: brian m. carlson @ 2026-06-29 0:23 UTC (permalink / raw)
To: Jeff King
Cc: Patrick Steinhardt, D. Ben Knoble, git, Junio C Hamano,
Ramsay Jones
In-Reply-To: <20260628084815.GA111587@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]
On 2026-06-28 at 08:48:15, Jeff King wrote:
> Oh, I also ran across this old thread:
>
> https://public-inbox.org/git/5605D88A.20104%40gmail.com/
>
> that implies similar:
>
> * In-core file times may not be properly rounded to on-disk
> precision, causing spurious file time changes when the cache is
> refreshed from disk. This was fixed for typical Unix file systems
> in kernel 2.6.11. The fix for CEPH, CIFS, NTFS, UFS and FUSE will
> be in kernel 4.3. There's no fix for FAT-based file systems yet.
>
> I also tested with CIFS on my system and it is fine. It looks like FAT
> systems were fixed since 2015. ;)
>
> But there is another interesting question raised there, which is how
> different implementations may interact (e.g., two versions of Git
> without and without USE_NSEC, or JGit which may have to use
> millisecond-resolution APIs, etc). It should all work correctly as long
> as each implementation consistently uses its own resolution (so JGit
> would have to compare in millisecond-space and treat ties as racy). And
> I think that is _probably_ what is happening now, since we already store
> nanoseconds unconditionally (and only use them with USE_NSEC).
>
> Though the opposite case is a performance problem but not a correctness
> one: if JGit writes out an index with milliseconds and USE_NSEC Git
> tries to read it, we will consider everything stat-dirty and re-read the
> contents.
>
> I don't know if these would be a problem in practice or not, but it's an
> interesting potential gotcha. And one that nobody may have noticed,
> because probably hardly anybody bothers to build with USE_NSEC now.
I would suggest that we provide a config knob and then build with
USE_NSEC by default. Most people are using Linux with typical Unix file
systems, NTFS, CIFS, or FUSE (e.g., sshfs). In the event someone
detects a problem, there's an easy solution—adjust the knob—and we can
then add a Linux-specific statfs call to determine if the file system is
a safe one in a future version.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] t5551: put many-tags case into its own repo
From: Jeff King @ 2026-06-29 0:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Montalbo, Patrick Steinhardt, git
In-Reply-To: <xmqqh5mm1gsf.fsf@gitster.g>
On Sun, Jun 28, 2026 at 02:44:32PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> > index e236e526f0..cd851f24b8 100755
> > --- a/t/t5551-http-fetch-smart.sh
> > +++ b/t/t5551-http-fetch-smart.sh
> > @@ -397,15 +397,16 @@ create_tags () {
> > }
> >
> > test_expect_success 'create 2,000 tags in the repo' '
> > + git init "$HTTPD_DOCUMENT_ROOT_PATH/many-tags.git" &&
> > (
> > - cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> > + cd "$HTTPD_DOCUMENT_ROOT_PATH/many-tags.git" &&
> > create_tags 1 2000
> > )
> > '
>
> While all the other repositories used in this tests are bare
> repositories, this new one is a non-bare repository.
>
> It shouldn't make any difference, but since I noticed it...
Ah, yeah. It should work either way, but it is slightly confusing for it
to be non-bare. I'll wait to re-send (though if nothing else comes up,
it may be simpler for you to just amend on your side).
-Peff
^ permalink raw reply
* Re: [PATCH v4 6/8] commit-reach: remove unused nonstale_queue dedup wrappers
From: SZEDER Gábor @ 2026-06-29 5:25 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget
Cc: git, Derrick Stolee, Elijah Newren, Kristofer Karlsson
In-Reply-To: <4db485b48aae810eeba28ea4feb47401ab352e88.1782649547.git.gitgitgadget@gmail.com>
On Sun, Jun 28, 2026 at 12:25:44PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
>
> nonstale_queue_put_dedup() and nonstale_queue_get_dedup() became
> unused after the previous commit. The core nonstale_queue functions
> remain in use by ahead_behind().
Please squash this patch into the previous one. Since the last
callers of these static functions went away in that commit, it can't
be built with DEVELOPER=1:
commit-reach.c:91:23: warning: ‘nonstale_queue_get_dedup’ defined but not used [-Wunused-function]
91 | static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
| ^~~~~~~~~~~~~~~~~~~~~~~~
commit-reach.c:82:13: warning: ‘nonstale_queue_put_dedup’ defined but not used [-Wunused-function]
82 | static void nonstale_queue_put_dedup(struct nonstale_queue *queue,
| ^~~~~~~~~~~~~~~~~~~~~~~~
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
> commit-reach.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/commit-reach.c b/commit-reach.c
> index 9ae306f60c..176ffd68d0 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -79,24 +79,6 @@ static void clear_nonstale_queue(struct nonstale_queue *queue)
> queue->max_nonstale = NULL;
> }
>
> -static void nonstale_queue_put_dedup(struct nonstale_queue *queue,
> - struct commit *c)
> -{
> - if (c->object.flags & ENQUEUED)
> - return;
> - c->object.flags |= ENQUEUED;
> - nonstale_queue_put(queue, c);
> -}
> -
> -static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
> -{
> - struct commit *commit = nonstale_queue_get(queue);
> -
> - if (commit)
> - commit->object.flags &= ~ENQUEUED;
> - return commit;
> -}
> -
> /*
> * Priority queue with per-side commit counters for paint_down_to_common().
> * Each non-stale queued commit occupies exactly one bucket: PARENT1-only,
> --
> gitgitgadget
>
^ permalink raw reply
* Re: [PATCH v6 4/4] history: re-edit a squash with every message
From: Junio C Hamano @ 2026-06-29 5:50 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
In-Reply-To: <4edf012b77fd2f2fb2a51eb10863bbf852fffa40.1782635349.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> By default "git history squash" reuses the oldest commit's message.
> When --reedit-message is given it only reopened that one message, so the
> messages of the folded-in commits were lost.
>
> Gather the messages of every commit in the range, oldest first, and use
> them as the editor template when re-editing, mirroring how "git rebase
> -i" presents a squash.
I doubt it would make practical difference, but one thing I notice
is that unlike "git rebase -i", this one does not intersperse
markers like "# This is the 1st commit message" in between the
messages taken from the squashed commits, so it is not exactly
"mirroring".
^ permalink raw reply
* Re: [PATCH v6 3/4] history: add squash subcommand to fold a range
From: Junio C Hamano @ 2026-06-29 5:50 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
In-Reply-To: <811e393ab48e2f79e6f8c78d883a5b92311791b3.1782635349.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> @@ -11,6 +11,7 @@ SYNOPSIS
> 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>...]
> +git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
Not your fault at all, as there are existing violators but the
gitcli tells us that the canonical order of arguments on the command
line is to have dashed options early before any real arguments.
Seeing <commit> followed by --dry-run, --update-refs, etc. with the
existing subcommands should also be corrected and this new command
should not spread the existing violation by mimicking. The revision
range (which is not just a single token A..B, but the usual range
notation that rev-list takes, like ^A B..C) should come after all
these dashed options.
> +The range is given in the usual `<base>..<tip>` form, where _<base>_ is
> +the commit just below the oldest commit to squash. For example, `git
> +history squash @~3..` folds the three most recent commits into one, and
It is OK to use in your personal development, but in the official
documentation, avoid cryptic @ and always spell HEAD (except for a
single place that explain that @ can stand in for HEAD, for obvious
reasons). It is extremely annoying and confusing to read as these
look so similar to reflog notation for the current branch, e.g., @~1
vs @{1}.
So "squash HEAD~3..HEAD" specifies a range of commits HEAD~2, HEAD~1
and HEAD (three commits), these are squashed into one commit on top
of HEAD~3, creating a sibling to HEAD~2 whose tree matches HEAD.
OK. Looking good.
^ permalink raw reply
* Re: [PATCH 3/3] t5551: pack refs after creating many tags
From: Patrick Steinhardt @ 2026-06-29 5:57 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Montalbo, git, Junio C Hamano
In-Reply-To: <20260628080710.GC107826@coredump.intra.peff.net>
On Sun, Jun 28, 2026 at 04:07:10AM -0400, Jeff King wrote:
> We have two tests that create 2,000 and 100,000 tags respectively.
> After doing so, the resulting state can be a bit slow to work with when
> using the "files" ref backend, as each of those refs is in its own file.
>
> This isn't a very realistic scenario, as we'd expect most of those refs
> to be packed. If they accrue over time along with objects, they'd get
> packed by maintenance/gc runs. And if you have a process that creates a
> ton of refs at once (like a big fast-import), the usual recommendation
> is to run maintenance afterwards.
>
> So let's follow that recommendation and pack the refs ourselves.
> Unfortunately, this does not seem to produce an improvement to the
> run-time of the test script! That's because after producing this state,
> we perform only a few fetches of it. And packing the refs costs at least
> as much as serving a ref advertisement (both have to iterate the refs,
> but packing additionally must write .lock files as we pack).
> My wall-clock time was slightly improved (but within the noise) with
> this patch, but my user and system CPU time were slightly worse!
> However, on a loaded system with I/O bottlenecks, it may be a net win.
> That's somewhat of a guess, though.
>
> It would be nice if we had a way to generate all of these refs without
> writing so many individual files. But even if we taught the ref code to
> write large cases directly to the packed-refs file, we'd still need to
> take individual locks. The real solution is a backend like reftable,
> which shaves ~30% off of the test runtime.
We kind of already have this with the `REF_TRANSACTION_FLAG_INITIAL`
flag, but right now it is only used when performing a clone or when
migrating references. Also, it requires an empty repository that has no
references yet.
It raises the question whether we could also extend git-fast-import(1)
to use it, as it would typically be run on an almost-empty repository.
It's the "almost" that kills it though, as we already do have at least
the HEAD reference. So it could be feasible, but it's not as trivial as
just setting the flag and then we're magically faster.
And besides, in this particular test here we run git-fast-import(1)
multiple times in the same repository, so it wouldn't help us.
We could of course extend all of this so that Git is able to write into
the packed-refs directly, even with preexisting refs. But I agree with
your sentiment: it doesn't feel worth it as the reftable backend fixes
scenarios like this anyway.
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I'm iffy on whether this one is worth it.
>
> If you apply just this patch without patch 2, then the run-time does
> improve quite a bit. The cost of packing is amortized by the improved
> performance for all of those subsequent tests (but after patch 2, they
> never even see the unpacked state).
>
> Likewise, I suspect this would make our timeout problems go away even
> without patch 1.
>
> So the whole series _could_ be reduced to just this one patch. But
> hopefully the reasoning given in the earlier patches makes sense, at
> which point this one is kind of superfluous.
Agreed. I'd just merge the first two patches and drop this one here.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values
From: Christian Couder @ 2026-06-29 6:03 UTC (permalink / raw)
To: Tian Yuchen
Cc: Junio C Hamano, git, cirnovskyv, szeder.dev, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <18ad7c1c-5ddc-4f62-ba7c-5cda53f5a48d@malon.dev>
On Sun, Jun 28, 2026 at 2:58 PM Tian Yuchen <cat@malon.dev> wrote:
>
> On 6/28/26 16:40, Junio C Hamano wrote:
> > Tian Yuchen <cat@malon.dev> writes:
> >
> >>> Wouldn't we rather want to try to be more strict and say
> >>>
> >>> if (!repo || !repo->initialized)
> >>> BUG("repo must be an initialied repository");
> >>>
> >>> here? Aren't all the callers of this function supposed to be
> >>> dealing with an already initialized repository?
> >>
> >> That makes sense, but from my point of view...
> >>
> >> 'repo_config_values()' already has a check for 'repo->initialized'. If
> >> we're absolutely certain that the 'repo' is initialized, wouldn't it be
> >> better to simply remove all the checks inside the getter and leave the
> >> judgment to 'repo_config_values()'?
> >
> > Yes, that was what I was getting at ;-).
>
> A lot of CI tests are failing, but that just goes to show that the
> "bugs" are being properly identified, doesn’t it?
>
> It means there are a lot of "invalid" calls in the tests (if the way we
> define a 'valid' call, i.e. repo must be initialized, is correct)... It
> seems that code like 'if (repo != the_repository) return' or something
> similar is inevitably going to end up somewhere, even though, as you
> said, it’s "sweeping problems under the rug."
>
> I’m not sure how to proceed from here either..
I agree that the best end state would be to have no `if (!repo ||
!repo->initialized)` check, but we shouldn't have to get there right
away. I think it's fine to proceed in several steps:
1) `if (!repo || !repo->initialized) return NULL;` allows us to remove
the global variable and use getters which will help us in the next
step.
2) `if (!repo || !repo->initialized) return BUG("repo must be an
initialized repository");` now we want to find and fix callers
(including tests) that haven't properly initialized things.
3) No `if (!repo || !repo->initialized)` check, as we are sure that
all the callers that didn't properly initialized things have been
found and fixed.
So I think 1) is fine for now as long as we properly explain in the
commit messages and in code comments (maybe using NEEDSWORK comments)
that we know there is more work to do on this in the future.
^ permalink raw reply
* Re: [PATCH] meson: wire up USE_NSEC build knob
From: Patrick Steinhardt @ 2026-06-29 6:08 UTC (permalink / raw)
To: Jeff King
Cc: D. Ben Knoble, git, brian m . carlson, Junio C Hamano,
Ramsay Jones
In-Reply-To: <20260628081806.GA3594700@coredump.intra.peff.net>
On Sun, Jun 28, 2026 at 04:18:06AM -0400, Jeff King wrote:
> On Mon, Jun 22, 2026 at 10:13:21AM +0200, Patrick Steinhardt wrote:
>
> > > So I guess if we wanted to go further it would take some digging as to
> > > how each platform behaves, and then flipping the config.make.uname knob
> > > for ones where it can be argued that the behavior is always reasonable.
> >
> > Yeah, it would be nice indeed to figure out whether these concerns still
> > apply. If they do, I would argue that it might even make sense to remove
> > the build option completely. It doesn't really make sense in my opinion
> > to have a build option that nobody uses and that is subtly broken when
> > enabled.
>
> I suspect it works just fine on some platforms and some filesystems
> (i.e., those that actually store nanoseconds on disk). So probably Linux
> with ext4 is OK. That's just guessing, though.
>
> If I understand the original problem correctly, then doing this:
>
> touch foo
> ls --full-time foo
> echo 3 | sudo tee /proc/sys/vm/drop_caches
> ls --full-time foo
>
> should be instructive. If it shows the same time for both "ls" calls,
> then USE_NSEC would be fine. If it doesn't, then the system is losing
> the nanosecond information when it drops the cache and has to reload
> from disk (and thus USE_NSEC would cause spurious stat mismatches).
>
> On my ext4 system, I get the same answers. So far so good.
>
> I get the same answers with a loopback-mounted ext2 system. Which
> surprised me a bit, but even unmounting and remounting the filesystem,
> the nanosecond times are still there. So...I guess ext2 supports
> nanoseconds.
>
> I tried with a vfat mount, and it also works: we don't have nanoseconds
> either before or after. That makes sense, and implies that modern Linux
> will always be OK (because it limits the cached VFS response to what the
> underlying filesystem can handle).
>
> So...maybe this is just a non-issue these days, at least on Linux?
>
> > > But that's all outside the scope of your patch here.
> >
> > Kind of, I guess. If we figure that this mechanism is still subtly broken
> > then I'd argue that it doesn't make sense to expose the option via
> > Meson.
>
> True, but AFAICT it probably is safe these days, at least one some
> platforms.
Hm. That makes me wonder whether it is the completely wrong approach to
make this a build option then. If it works on some systems and only on
some filesystems, then a build option is just too coarse-grained. A
distro wouldn't really be able to ever enable the option, unless it knew
that repositories will only ever exist on a filesystem that works. Which
I guess is an assumption that no distro can make.
So instead, I wonder whether we should treat this the same as for
example "core.ignoreCase", where we only use nanosecond resolution when
opted in by the user. Ideally, if we had a way to detect brokenness, we
could even make git-init(1) set it automatically.
If so, we could unconditionally enable nanoseconds on platforms that
support them, but still have a runtime toggle for filesystems that
don't.
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] odb/source: generalize `reprepare()` callback
From: Patrick Steinhardt @ 2026-06-29 6:16 UTC (permalink / raw)
To: Toon Claes; +Cc: git
In-Reply-To: <87ldc14i4n.fsf@emacs.iotcl.com>
On Fri, Jun 26, 2026 at 02:10:32PM +0200, Toon Claes wrote:
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 8080d1bf5e..7361bf071e 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -1361,10 +1360,8 @@ int cmd_grep(int argc,
> > struct odb_source *source;
> >
> > odb_prepare_alternates(the_repository->objects);
> > - for (source = the_repository->objects->sources; source; source = source->next) {
> > - struct odb_source_files *files = odb_source_files_downcast(source);
>
> So you're downcasting inside the implementation by the backends itself.
> That makes sense, but would it be worth to say something about that in
> the commit message?
Hm. Would that provide much value? I'm probably quite a bit biased here,
but I think that it's implicit that the backends have to eventually cast
the generic structure to their own backend.
So I wouldn't really know how to clarify this. Did you have anything
specific in mind?
> > diff --git a/odb/source-packed.c b/odb/source-packed.c
> > index 42c28fba0e..fa5a072488 100644
> > --- a/odb/source-packed.c
> > +++ b/odb/source-packed.c
> > @@ -15,7 +15,7 @@ static int find_pack_entry(struct odb_source_packed *store,
> > {
> > struct packfile_list_entry *l;
> >
> > - odb_source_packed_prepare(store);
> > + odb_source_prepare(&store->base, 0);
>
> Why are you not using ODB_PREPARE_FLUSH_CACHES here? It used to do
> before?
Because this was calling `odb_source_packed_prepare()` before, not
`odb_source_reprepare()`. So this was calling the non-flushing variant.
> > if (store->midx && fill_midx_entry(store->midx, oid, e))
> > return 1;
> >
> > @@ -47,7 +47,7 @@ static int odb_source_packed_read_object_info(struct odb_source *source,
> > * been added since the last time we have prepared the packfile store.
> > */
> > if (flags & OBJECT_INFO_SECOND_READ)
> > - odb_source_reprepare(source);
> > + odb_source_prepare(source, ODB_PREPARE_FLUSH_CACHES);
>
> I think the new code is correct, but why wasn't `packed` used here in
> the past? The old odb_source_reprepare() expected a downcasted, didn't
> it?
No, `odb_source_reprepare()` is the generic variant. The naming schema
is typically:
- `odb_source_frobnicate()` for the generic variants, which receive a
`struct odb_source` as input.
- `odb_source_<type>_frobnitcate()` for their backend-specific
implementations, which cast down the generic `struct odb_source` to
their backend-specific struct.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH 2/2] odb: introduce `odb_prepare()`
From: Patrick Steinhardt @ 2026-06-29 6:16 UTC (permalink / raw)
To: Toon Claes; +Cc: git
In-Reply-To: <87o6gx4i5w.fsf@emacs.iotcl.com>
On Fri, Jun 26, 2026 at 02:09:47PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/odb.h b/odb.h
> > index c14c9030e4..b1c0f3767b 100644
> > --- a/odb.h
> > +++ b/odb.h
> > @@ -133,9 +133,13 @@ enum odb_prepare_flags {
> > };
> >
> > /*
> > - * Clear caches, reload alternates and then reload object sources so that new
> > - * objects may become accessible.
> > + * Prepare the object database for use. Calling this function is generally not
> > + * needed, but can be useful in case the caller wants to pre-open individual
> > + * sources.
> > */
> > +void odb_prepare(struct object_database *o, enum odb_prepare_flags flags);
> > +
> > +/* Equivalent to `odb_prepare(o, ODB_PREPARE_FLUSH_CACHES)`. */
> > void odb_reprepare(struct object_database *o);
>
> According to my grep results are there 17 callsites for odb_reprepare(),
> then I agree it makes sense to create this wrapper.
Yeah, I was a bit torn myself whether or not to keep the wrapper. I
eventually decided to just keep it because it reduces churn, and it's a
trivial wrapper anyway.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH] reftable: fix unlikely leak on API error
From: Patrick Steinhardt @ 2026-06-29 6:21 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20260628090314.GA661068@coredump.intra.peff.net>
On Sun, Jun 28, 2026 at 05:03:14AM -0400, Jeff King wrote:
> If the reftable writer sees a bogus block size, we return with
> REFTABLE_API_ERROR, leaking the reftable_writer struct we previously
> allocated. Originally this case was a BUG(), but it became a regular
> return in 445f9f4f35 (reftable: stop using `BUG()` in trivial cases,
> 2025-02-18).
>
> We could obviously fix it by calling "reftable_free(wp)". But we can
> observe that we never use the allocated "wp" until after we've validated
> the input options. So let's just bump the allocation down. That fixes
> the leak, and I think makes the flow of the function more logical
> (we validate our inputs before doing any work).
Another alternative would be to create a common exit path where we free
the structure when we're about to return an error. But that might not
even be worth it.
> diff --git a/reftable/writer.c b/reftable/writer.c
> index 0133b64975..1bd4aa388b 100644
> --- a/reftable/writer.c
> +++ b/reftable/writer.c
> @@ -152,16 +152,16 @@ int reftable_writer_new(struct reftable_writer **out,
> struct reftable_write_options opts = {0};
> struct reftable_writer *wp;
>
> - wp = reftable_calloc(1, sizeof(*wp));
> - if (!wp)
> - return REFTABLE_OUT_OF_MEMORY_ERROR;
> -
> if (_opts)
> opts = *_opts;
> options_set_defaults(&opts);
> if (opts.block_size >= (1 << 24))
> return REFTABLE_API_ERROR;
>
> + wp = reftable_calloc(1, sizeof(*wp));
> + if (!wp)
> + return REFTABLE_OUT_OF_MEMORY_ERROR;
> +
> reftable_buf_init(&wp->block_writer_data.last_key);
> reftable_buf_init(&wp->last_key);
> reftable_buf_init(&wp->scratch);
Makes sense. There's another early return in this function, but there we
already know to free the writer.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v5 0/4] history: add squash subcommand to fold a range
From: Patrick Steinhardt @ 2026-06-29 6:26 UTC (permalink / raw)
To: phillip.wood; +Cc: Harald Nordgren via GitGitGadget, git, Harald Nordgren
In-Reply-To: <d37e8f4f-d1f9-45aa-8c95-ebe676d54671@gmail.com>
On Fri, Jun 26, 2026 at 09:52:57AM +0100, Phillip Wood wrote:
> Hi Harald
>
> On 24/06/2026 22:54, Harald Nordgren via GitGitGadget wrote:
> > Adds git history squash <revision-range> to fold a range of commits.
>
> It would be helpful to give a bit more detail here about the command so that
> the reader has an overview of what is actually being implemented.
>
> - what does it do with fixup!, squash! and amend! commits? Can it use
> the message from amend! commits to reword the commit?
> - can the user reword the commit message?
Good things to document/discuss.
> - what happens if a merge commit inside the range has a parent outside
> the range?
Yeah, I agree that we should punt on merge commits for now. They are a
can of worms, and I'm not sure that we should just squash them. I would
at least like the user to ask a flag that tells us that it's fine
squashing those.
> - what happens to branches that point to commits inside the range?
Yeah, this should be documented indeed.
> I had a quick play and found that it accepts ranges that containing a single
> commit (e.g. @^!) where there is nothing to squash. It also accepts ranges
> that are not ancestors of HEAD (e.g. checkout master and run "git history
> squash --dry-run origin/seen^2^!") without printing an error message. Only
> accepting a single argument is quite limiting as one cannot say
>
> git history squash ^:/base :/tip
Note that it is intentional that you can rewrite branches that are not
currently checked out, and the other subcommands work the same. So I'd
argue this should also be the case for "squash".
Patrick
^ permalink raw reply
* Re: [PATCH] history: streamline message preparation and plug file stream leak
From: Patrick Steinhardt @ 2026-06-29 6:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <xmqqecht8df1.fsf@gitster.g>
On Fri, Jun 26, 2026 at 09:38:42AM -0700, Junio C Hamano wrote:
> diff --git a/builtin/history.c b/builtin/history.c
> index 8dcb9a6046..f17ec049c0 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -41,11 +41,6 @@ static int fill_commit_message(struct repository *repo,
> " empty message aborts the commit.\n");
> struct wt_status s;
>
> - strbuf_addstr(out, default_message);
> - strbuf_addch(out, '\n');
> - strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str);
> - write_file_buf(path, out->buf, out->len);
> -
> wt_status_prepare(repo, &s);
> FREE_AND_NULL(s.branch);
> s.ahead_behind_flags = AHEAD_BEHIND_QUICK;
> @@ -57,14 +52,20 @@ static int fill_commit_message(struct repository *repo,
> s.whence = FROM_COMMIT;
> s.committable = 1;
>
> - s.fp = fopen(git_path_commit_editmsg(), "a");
Here we reuse the local `path` variable, which already carries the
result of `git_path_commit_editmsg()`.
> + s.fp = fopen(path, "w");
> if (!s.fp)
> - return error_errno(_("could not open '%s'"), git_path_commit_editmsg());
> + return error_errno(_("could not open '%s'"), path);
Likewise.
> + strbuf_addstr(out, default_message);
> + strbuf_addch(out, '\n');
> + strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str);
> + fwrite(out->buf, 1, out->len, s.fp);
>
> wt_status_collect_changes_trees(&s, old_tree, new_tree);
> wt_status_print(&s);
> wt_status_collect_free_buffers(&s);
> string_list_clear_func(&s.change, change_data_free);
> + fclose(s.fp);
This is fixing the leaked file descriptor.
One thing I wonder though is that we don't perform any error checking on
the file in the new version. Previously, we would have died in case
`write_file_buf()` failed. But now we just `fwrite()` without error
checking. I don't think that "wt-status.c" does error checking either,
so we might end up with a partially-written file without us noticing.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH 05/11] reftable/block: fix OOB write with bogus inflated log size
From: Patrick Steinhardt @ 2026-06-29 7:08 UTC (permalink / raw)
To: Christian Couder; +Cc: git, oxsignal
In-Reply-To: <CAP8UFD0y0GVjdnWYDkOsk6R9-ReGfzr6ZEm8PbyHOHrdAETXzg@mail.gmail.com>
On Fri, Jun 26, 2026 at 09:48:36AM +0200, Christian Couder wrote:
> On Wed, Jun 24, 2026 at 10:24 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> > diff --git a/t/unit-tests/u-reftable-block.c b/t/unit-tests/u-reftable-block.c
> > index f4bded7d26..40274af5c0 100644
> > --- a/t/unit-tests/u-reftable-block.c
> > +++ b/t/unit-tests/u-reftable-block.c
> > @@ -456,3 +456,47 @@ void test_reftable_block__iterator(void)
> > block_writer_release(&writer);
> > reftable_buf_release(&data);
> > }
> > +
> > +void test_reftable_block__corrupt_log_block_size(void)
> > +{
> > + struct reftable_block_source source = { 0 };
> > + struct block_writer writer = {
> > + .last_key = REFTABLE_BUF_INIT,
> > + };
> > + struct reftable_record rec = {
> > + .type = REFTABLE_BLOCK_TYPE_LOG,
> > + .u.log = {
> > + .refname = (char *) "refs/heads/main",
> > + .update_index = 1,
> > + .value_type = REFTABLE_LOG_UPDATE,
> > + },
> > + };
> > + struct reftable_block block = { 0 };
> > + struct reftable_buf data;
> > +
> > + data.len = 1024;
> > + REFTABLE_CALLOC_ARRAY(data.buf, data.len);
> > + cl_assert(data.buf != NULL);
> > +
> > + cl_must_pass(block_writer_init(&writer, REFTABLE_BLOCK_TYPE_LOG,
> > + (uint8_t *) data.buf, data.len,
> > + 0, hash_size(REFTABLE_HASH_SHA1)));
> > + cl_must_pass(block_writer_add(&writer, &rec));
> > + cl_assert(block_writer_finish(&writer) > 0);
>
> It looks like some of the block writing code above could be simplified
> using an helper function like:
>
> int cl_reftable_write_block(struct reftable_buf *buf, uint8_t block_type,
> size_t block_size, uint32_t header_off,
> struct reftable_record *recs, size_t nrecs)
> {
> struct block_writer writer = {
> .last_key = REFTABLE_BUF_INIT,
> };
> int block_end;
>
> REFTABLE_CALLOC_ARRAY(buf->buf, block_size);
> cl_assert(buf->buf != NULL);
> buf->len = block_size;
>
> cl_must_pass(block_writer_init(&writer, block_type, (uint8_t *) buf->buf,
> block_size, header_off,
> hash_size(REFTABLE_HASH_SHA1)));
> for (size_t i = 0; i < nrecs; i++)
> cl_must_pass(block_writer_add(&writer, &recs[i]));
>
> block_end = block_writer_finish(&writer);
> cl_assert(block_end > 0);
>
> block_writer_release(&writer);
>
> return block_end;
> }
>
> This function could be introduced by a preparatory commit in
> t/unit-tests/lib-reftable.{c,h}. It would be kind of similar to the
> existing cl_reftable_write_to_buf() helper in those files.
>
> It looks like it could already simplify existing tests like:
>
> - test_reftable_block__log_read_write
> - test_reftable_block__obj_read_write
> - test_reftable_block__ref_read_write
> - test_reftable_block__iterator
>
> and it could simplify the new tests introduced by other patches in this series:
>
> - 06/11 reftable/block: fix OOB read with bogus block size
> - 07/11 reftable/block: fix OOB read with bogus restart count
> - 09/11 reftable/block: fix OOB read with bogus restart offset
Good point, will do. Thanks!
Patrick
^ permalink raw reply
* receive-pack hangs on zero-object push into promisor-shaped repository
From: Wei Hu @ 2026-06-29 7:10 UTC (permalink / raw)
To: git
[-- Attachment #1.1: Type: text/plain, Size: 860 bytes --]
Hello,
I found a reproducible hang in `git receive-pack` when pushing a ref update
that sends zero objects into a repository that has promisor remote
configuration and `.promisor` pack sidecar files.
The same zero-object ref update returns normally when the receiving
repository
is a normal non-bare repository or a bare repository. It also returns
normally
if I remove either the promisor remote config or the `.promisor` sidecar
files
from the receiving repository.
Check the attached script to reproduce the bug.
Environment:
git version 2.54.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
rust: disabled
gettext: enabled
libcurl: 8.5.0
zlib: 1.3
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
OS: Ubuntu 24.04.4 LTS (Noble Numbat)
[-- Attachment #1.2: Type: text/html, Size: 1024 bytes --]
[-- Attachment #2: git-promisor-zero-object-push-repro.sh --]
[-- Type: text/x-sh, Size: 1723 bytes --]
#!/bin/sh
set -eu
GIT=${GIT:-git}
ROOT=$(mktemp -d "${TMPDIR:-/tmp}/git-promisor-push-hang.XXXXXX")
SRC=$ROOT/src
DST=$ROOT/dst
UPSTREAM=$ROOT/upstream.git
TRACE=$ROOT/trace.log
echo "root: $ROOT"
echo "git: $($GIT --version)"
$GIT init -q "$SRC"
$GIT -C "$SRC" config user.name Repro
$GIT -C "$SRC" config user.email repro@example.invalid
printf A >"$SRC/file"
$GIT -C "$SRC" add file
$GIT -C "$SRC" commit -q -m A
$GIT -C "$SRC" branch topic
OLD=$($GIT -C "$SRC" rev-parse topic)
printf B >"$SRC/file"
$GIT -C "$SRC" commit -q -am B
$GIT -C "$SRC" branch -M main
NEW=$($GIT -C "$SRC" rev-parse main)
$GIT clone -q --bare "$SRC" "$UPSTREAM"
$GIT init -q "$DST"
$GIT -C "$DST" config receive.denycurrentbranch updateInstead
$GIT -C "$SRC" push -q "$DST" main:main topic:topic
$GIT -C "$DST" checkout -q main
$GIT -C "$DST" config remote.origin.url "file://$UPSTREAM"
$GIT -C "$DST" config remote.origin.promisor true
$GIT -C "$DST" config remote.origin.partialclonefilter blob:none
$GIT -C "$DST" gc -q
for pack in "$DST"/.git/objects/pack/*.pack
do
: >"${pack%.pack}.promisor"
done
$GIT -C "$DST" update-ref refs/heads/topic "$OLD"
status=0
timeout --kill-after=2s 8 \
env GIT_TRACE=1 GIT_TRACE_PACKET=1 \
$GIT -C "$SRC" push --porcelain "$DST" HEAD:topic \
>"$TRACE" 2>&1 || status=$?
AFTER=$($GIT -C "$DST" rev-parse refs/heads/topic)
ZERO_PACK=no
grep -q -- '--pack_header=2,0' "$TRACE" && ZERO_PACK=yes
echo "old: $OLD"
echo "new: $NEW"
echo "after: $AFTER"
echo "push status: $status"
echo "zero-object pack observed: $ZERO_PACK"
echo "trace: $TRACE"
if test "$status" = 124 && test "$AFTER" = "$OLD" && test "$ZERO_PACK" = yes
then
echo "BUG REPRODUCED"
exit 0
fi
echo "BUG NOT REPRODUCED"
exit 1
^ permalink raw reply
* Re: [PATCH 0/3] fixing expensive http test timeouts
From: Patrick Steinhardt @ 2026-06-29 7:33 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Montalbo, git, Junio C Hamano
In-Reply-To: <20260628075716.GA3525066@coredump.intra.peff.net>
On Sun, Jun 28, 2026 at 03:57:16AM -0400, Jeff King wrote:
> On Fri, Jun 26, 2026 at 04:26:28PM -0700, Michael Montalbo wrote:
>
> > I think Peff and Patrick's suggestion to just increase the Apache timeout
> > makes sense. I ran some experiments using a really long timeout with an
> > artificially slowed down CI runner and all the jobs made progress
> > (if slowly) without stalling, and eventually completed successfully:
> >
> > https://github.com/mmontalbo/git/actions/runs/28267019651
> >
> > I haven't spent a lot of time trying to figure out what the right timeout
> > value should be. An hour definitely seems like overkill, with something
> > on the order of 5-10 minutes seeming more reasonable, but I don't
> > have a principled number.
>
> Here are some patches to keep things moving along. I arbitrarily picked
> 10 minutes, because multiplying the 1-minute default by 10 felt right. ;)
>
> The first one just bumps the timeout and should make our problems go
> away. The other two are optimizations, but I'm on the fence on whether
> the final patch is worth it.
>
> Thanks again for all of the digging.
>
> [1/3]: t/lib-httpd: bump apache timeout
> [2/3]: t5551: put many-tags case into its own repo
> [3/3]: t5551: pack refs after creating many tags
By the way, the only reason why we at GitLab haven't been feeling the
pain is that we only enable GIT_TEST_LONG for GitHub. So I was wondering
whether we want to have something like the below patch on top.
Patrick
diff --git a/ci/lib.sh b/ci/lib.sh
index b939110a6e..57801586aa 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -215,6 +215,14 @@ then
test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
CI_REPO_SLUG="$GITHUB_REPOSITORY"
CI_JOB_ID="$GITHUB_RUN_ID"
+
+ case "$GITHUB_EVENT_NAME" in
+ pull_request)
+ CI_EVENT=pull_request;;
+ push)
+ CI_EVENT=push;;
+ esac
+
CC="${CC_PACKAGE:-${CC:-gcc}}"
DONT_SKIP_TAGS=t
handle_failed_tests () {
@@ -239,6 +247,13 @@ then
CI_BRANCH="$CI_COMMIT_REF_NAME"
CI_COMMIT="$CI_COMMIT_SHA"
+ case "$CI_PIPELINE_SOURCE" in
+ merge_request_event)
+ CI_EVENT=pull_request;;
+ push)
+ CI_EVENT=push;;
+ esac
+
case "$OS,$CI_JOB_IMAGE" in
Windows_NT,*)
CI_OS_NAME=windows
@@ -319,7 +334,7 @@ export SKIP_DASHED_BUILT_INS=YesPlease
# enable "expensive" tests for PR events.
# In order to catch bugs introduced at integration time by mismerges,
# enable the long tests for pushes to the integration branches as well.
-case "$GITHUB_EVENT_NAME,$CI_BRANCH" in
+case "$CI_EVENT,$CI_BRANCH" in
pull_request,*|push,*next*|push,*master*|push,*main*|push,*maint*)
export GIT_TEST_LONG=YesPlease
;;
^ permalink raw reply related
* Re: [PATCH v6 09/10] builtin/history: split handling of ref updates into two phases
From: Patrick Steinhardt @ 2026-06-29 7:33 UTC (permalink / raw)
To: Christian Couder
Cc: git, Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk,
Phillip Wood
In-Reply-To: <CAP8UFD1evTZqj1ymW9g5g2RmMkYMaE0rPa0Hzt+irH94M6LD6A@mail.gmail.com>
On Thu, Jun 25, 2026 at 03:37:42PM +0200, Christian Couder wrote:
> On Mon, Jun 15, 2026 at 3:56 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> > @@ -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++;
>
> It looks like this duplicates what replay_result_queue_update() from
> replay.c does.
It indeed is. That function is internal to "replay.c" though. We could
expose it, but I wonder whether that's worth it. Goes looking... you
know, let me just do it.
Thanks!
Patrick
^ permalink raw reply
* [PATCH v7 00/11] builtin/history: introduce "drop" subcommand
From: Patrick Steinhardt @ 2026-06-29 7:34 UTC (permalink / raw)
To: git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood,
Christian Couder
In-Reply-To: <20260601-b4-pks-history-drop-v1-0-643e32340d55@pks.im>
Hi,
this small patch series introduces the new "drop" subcommand for
git-history(1). As a reader might guess, the command does exactly that:
given a commit, it will drop that commit from the commit history and
replay descendant branches on top of it.
Changes in v7:
- Expose `replay_result_queue_update()` so that we don't have to
duplicate its functionality.
- Add missing SOB.
- Link to v6: https://patch.msgid.link/20260615-b4-pks-history-drop-v6-0-2e329e536d78@pks.im
Changes in v6:
- Fix bad interactions of DRY_RUN with UPDATE_HEAD
- Link to v5: https://patch.msgid.link/20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im
Changes in v5:
- Reject UPDATE_ORIG_HEAD without UPDATE_HEAD.
- Link to v4: https://patch.msgid.link/20260610-b4-pks-history-drop-v4-0-70d5f0ae8c25@pks.im
Changes in v4:
- Remove the `SKIP_REF_UPDATES` flag in favor of a new `UPDATE_HEAD`
flag, as suggested by Phillip.
- Rename `reset_head()` to `reset_working_tree()`. This better matches
the new scope of the function, and it helps us to catch any
in-flight patches that would now have to set the `UPDATE_HEAD` flag.
- Link to v3: https://patch.msgid.link/20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im
Changes in v3:
- Fix commit message typos.
- Make `update_orig_head` and `skip_ref_updates` mutually exclusive.
- Use fancy revisions to specify the commit to drop in the example
section.
- Detect conflicting changes in the index/working tree in dry-run
mode.
- Consistently use a subshell.
- Rename `RESET_HEAD_ORIG_HEAD` to `RESET_HEAD_UPDATE_ORIG_HEAD`.
-
- Link to v2: https://patch.msgid.link/20260603-b4-pks-history-drop-v2-0-742cb5b5176d@pks.im
Changes in v2:
- Reworked `update_worktree()` to use `reset_head()`, which required a
bunch of changes to `reset_head()`.
- Consistently mention the commit that cannot be dropped as part of
error messages.
- Adapt error message to not use backticks anymore.
- Drop redundant "--graph" flag in a test helper.
- Link to v1: https://patch.msgid.link/20260601-b4-pks-history-drop-v1-0-643e32340d55@pks.im
Thanks!
Patrick
---
Patrick Steinhardt (11):
read-cache: split out function to drop unmerged entries to stage 0
reset: drop `USE_THE_REPOSITORY_VARIABLE`
reset: rename `reset_head()`
reset: modernize flags passed to `reset_working_tree()`
reset: introduce dry-run mode
reset: introduce ability to skip updating HEAD
reset: allow the caller to specify the current HEAD object
reset: stop assuming that the caller passes in a clean index
replay: expose `replay_result_queue_update()`
builtin/history: split handling of ref updates into two phases
builtin/history: implement "drop" subcommand
Documentation/git-history.adoc | 38 ++-
builtin/history.c | 286 +++++++++++++++++++---
builtin/rebase.c | 41 ++--
read-cache-ll.h | 1 +
read-cache.c | 12 +-
replay.c | 8 +-
replay.h | 5 +
reset.c | 102 +++++---
reset.h | 51 ++--
sequencer.c | 17 +-
t/meson.build | 1 +
t/t3454-history-drop.sh | 537 +++++++++++++++++++++++++++++++++++++++++
12 files changed, 978 insertions(+), 121 deletions(-)
Range-diff versus v6:
1: 61c6eb1bdc = 1: 640b51b963 read-cache: split out function to drop unmerged entries to stage 0
2: 50a61da426 = 2: 485dee2858 reset: drop `USE_THE_REPOSITORY_VARIABLE`
3: 96ffa9d2a6 ! 3: fd82a0b592 reset: rename `reset_head()`
@@ Commit message
subsequent commit.
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
+ Signed-off-by: Patrick Steinhardt <ps@pks.im>
## builtin/rebase.c ##
@@ builtin/rebase.c: static int finish_rebase(struct rebase_options *opts)
4: 874c3ecd59 = 4: 7fe6db0459 reset: modernize flags passed to `reset_working_tree()`
5: bba2845f2d = 5: b0fba42b75 reset: introduce dry-run mode
6: ada93af1da = 6: db635dbea4 reset: introduce ability to skip updating HEAD
7: 3cf1dcf549 = 7: 5422a99683 reset: allow the caller to specify the current HEAD object
8: ff28ad814c = 8: a264b72376 reset: stop assuming that the caller passes in a clean index
-: ---------- > 9: 8060e462d1 replay: expose `replay_result_queue_update()`
9: 7b048d5a16 ! 10: 7233b48732 builtin/history: split handling of ref updates into two phases
@@ builtin/history.c: 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++;
++ replay_result_queue_update(result, decoration->name,
++ &original->object.oid,
++ &rewritten->object.oid);
+ }
+
+ return 0;
10: 7389e0432a = 11: 61668ea59a builtin/history: implement "drop" subcommand
---
base-commit: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e
change-id: 20260601-b4-pks-history-drop-28f6c6399e7b
^ permalink raw reply
* [PATCH v7 01/11] read-cache: split out function to drop unmerged entries to stage 0
From: Patrick Steinhardt @ 2026-06-29 7:34 UTC (permalink / raw)
To: git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood,
Christian Couder
In-Reply-To: <20260629-b4-pks-history-drop-v7-0-6e9392a957d8@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.55.0.rc2.803.g1fd1e6609c.dirty
^ permalink raw reply related
* [PATCH v7 02/11] reset: drop `USE_THE_REPOSITORY_VARIABLE`
From: Patrick Steinhardt @ 2026-06-29 7:34 UTC (permalink / raw)
To: git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood,
Christian Couder
In-Reply-To: <20260629-b4-pks-history-drop-v7-0-6e9392a957d8@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.55.0.rc2.803.g1fd1e6609c.dirty
^ permalink raw reply related
* [PATCH v7 03/11] reset: rename `reset_head()`
From: Patrick Steinhardt @ 2026-06-29 7:34 UTC (permalink / raw)
To: git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood,
Christian Couder
In-Reply-To: <20260629-b4-pks-history-drop-v7-0-6e9392a957d8@pks.im>
In a subsequent commit we're about to adapt `reset_head()` so that the
reference update to HEAD is optional, only. At this point the function
starts to feel misnamed, as it doesn't necessarily have anything to do
with the HEAD reference anymore. The gist of the function then is that
we reset the working tree to a specific new commit, updating both the
index and the checked-out files.
Rename it to `reset_working_tree()` to better reflect that.
Note that we don't adjust the flags yet. This will happen in a
subsequent commit.
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rebase.c | 20 ++++++++++----------
reset.c | 5 +++--
reset.h | 4 ++--
sequencer.c | 8 ++++----
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index fa4f5d9306..22fbba3c62 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -592,7 +592,7 @@ static int finish_rebase(struct rebase_options *opts)
static int move_to_original_branch(struct rebase_options *opts)
{
struct strbuf branch_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT;
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
int ret;
if (!opts->head_name)
@@ -610,7 +610,7 @@ static int move_to_original_branch(struct rebase_options *opts)
ropts.flags = RESET_HEAD_REFS_ONLY;
ropts.branch_msg = branch_reflog.buf;
ropts.head_msg = head_reflog.buf;
- ret = reset_head(the_repository, &ropts);
+ ret = reset_working_tree(the_repository, &ropts);
strbuf_release(&branch_reflog);
strbuf_release(&head_reflog);
@@ -685,7 +685,7 @@ static int run_am(struct rebase_options *opts)
status = run_command(&format_patch);
if (status) {
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
unlink(rebased_patches);
free(rebased_patches);
child_process_clear(&am);
@@ -693,7 +693,7 @@ static int run_am(struct rebase_options *opts)
ropts.oid = &opts->orig_head->object.oid;
ropts.branch = opts->head_name;
ropts.default_reflog_action = opts->reflog_action;
- reset_head(the_repository, &ropts);
+ reset_working_tree(the_repository, &ropts);
error(_("\ngit encountered an error while preparing the "
"patches to replay\n"
"these revisions:\n"
@@ -855,7 +855,7 @@ static int rebase_config(const char *var, const char *value,
static int checkout_up_to_date(struct rebase_options *options)
{
struct strbuf buf = STRBUF_INIT;
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
int ret = 0;
strbuf_addf(&buf, "%s: checkout %s",
@@ -866,7 +866,7 @@ static int checkout_up_to_date(struct rebase_options *options)
if (!ropts.branch)
ropts.flags |= RESET_HEAD_DETACH;
ropts.head_msg = buf.buf;
- if (reset_head(the_repository, &ropts) < 0)
+ if (reset_working_tree(the_repository, &ropts) < 0)
ret = error(_("could not switch to %s"), options->switch_to);
strbuf_release(&buf);
@@ -1116,7 +1116,7 @@ int cmd_rebase(int argc,
int reschedule_failed_exec = -1;
int allow_preemptive_ff = 1;
int preserve_merges_selected = 0;
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
struct option builtin_rebase_options[] = {
OPT_STRING(0, "onto", &options.onto_name,
N_("revision"),
@@ -1385,7 +1385,7 @@ int cmd_rebase(int argc,
rerere_clear(the_repository, &merge_rr);
string_list_clear(&merge_rr, 1);
ropts.flags = RESET_HEAD_HARD;
- if (reset_head(the_repository, &ropts) < 0)
+ if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not discard worktree changes"));
remove_branch_state(the_repository, 0);
if (read_basic_state(&options))
@@ -1410,7 +1410,7 @@ int cmd_rebase(int argc,
ropts.head_msg = head_msg.buf;
ropts.branch = options.head_name;
ropts.flags = RESET_HEAD_HARD;
- if (reset_head(the_repository, &ropts) < 0)
+ if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not move back to %s"),
oid_to_hex(&options.orig_head->object.oid));
strbuf_release(&head_msg);
@@ -1880,7 +1880,7 @@ int cmd_rebase(int argc,
RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
ropts.default_reflog_action = options.reflog_action;
- if (reset_head(the_repository, &ropts)) {
+ if (reset_working_tree(the_repository, &ropts)) {
ret = error(_("Could not detach HEAD"));
goto cleanup_autostash;
}
diff --git a/reset.c b/reset.c
index 3b3cb74dab..799596398b 100644
--- a/reset.c
+++ b/reset.c
@@ -12,7 +12,7 @@
#include "hook.h"
static int update_refs(struct repository *repo,
- const struct reset_head_opts *opts,
+ const struct reset_working_tree_options *opts,
const struct object_id *oid,
const struct object_id *head)
{
@@ -85,7 +85,8 @@ static int update_refs(struct repository *repo,
return ret;
}
-int reset_head(struct repository *r, const struct reset_head_opts *opts)
+int reset_working_tree(struct repository *r,
+ const struct reset_working_tree_options *opts)
{
const struct object_id *oid = opts->oid;
const char *switch_to_branch = opts->branch;
diff --git a/reset.h b/reset.h
index a28f81829d..f130152014 100644
--- a/reset.h
+++ b/reset.h
@@ -17,7 +17,7 @@
/* Update ORIG_HEAD as well as HEAD */
#define RESET_ORIG_HEAD (1<<4)
-struct reset_head_opts {
+struct reset_working_tree_options {
/*
* The commit to checkout/reset to. Defaults to HEAD.
*/
@@ -55,6 +55,6 @@ struct reset_head_opts {
const char *default_reflog_action;
};
-int reset_head(struct repository *r, const struct reset_head_opts *opts);
+int reset_working_tree(struct repository *r, const struct reset_working_tree_options *opts);
#endif
diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..d73ecf0384 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4677,7 +4677,7 @@ static void create_autostash_internal(struct repository *r,
if (has_unstaged_changes(r, 1) ||
has_uncommitted_changes(r, 1)) {
struct child_process stash = CHILD_PROCESS_INIT;
- struct reset_head_opts ropts = { .flags = RESET_HEAD_HARD };
+ struct reset_working_tree_options ropts = { .flags = RESET_HEAD_HARD };
struct object_id oid;
strvec_pushl(&stash.args,
@@ -4707,7 +4707,7 @@ static void create_autostash_internal(struct repository *r,
if (!silent)
printf(_("Created autostash: %s\n"), buf.buf);
- if (reset_head(r, &ropts) < 0)
+ if (reset_working_tree(r, &ropts) < 0)
die(_("could not reset --hard"));
discard_index(r->index);
if (repo_read_index(r) < 0)
@@ -4867,7 +4867,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
const char *onto_name, const struct object_id *onto,
const struct object_id *orig_head)
{
- struct reset_head_opts ropts = {
+ struct reset_working_tree_options ropts = {
.oid = onto,
.orig_head = orig_head,
.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
@@ -4876,7 +4876,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
onto_name),
.default_reflog_action = sequencer_reflog_action(opts)
};
- if (reset_head(r, &ropts)) {
+ if (reset_working_tree(r, &ropts)) {
apply_autostash(rebase_path_autostash());
sequencer_remove_state(opts);
return error(_("could not detach HEAD"));
--
2.55.0.rc2.803.g1fd1e6609c.dirty
^ permalink raw reply related
* [PATCH v7 04/11] reset: modernize flags passed to `reset_working_tree()`
From: Patrick Steinhardt @ 2026-06-29 7:34 UTC (permalink / raw)
To: git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood,
Christian Couder
In-Reply-To: <20260629-b4-pks-history-drop-v7-0-6e9392a957d8@pks.im>
The flags passed to `reset_working_tree()` are declared as defines. This
has fallen a bit out of practice nowadays, where we instead prefer to
use enums. Furthermore, the prefix of those flags does not match the
function name anymore after the rename in the preceding commit.
Adapt the code to follow modern best practices and adapt the flag names.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rebase.c | 15 ++++++++-------
reset.c | 12 ++++++------
reset.h | 31 +++++++++++++++++++------------
sequencer.c | 9 ++++++---
4 files changed, 39 insertions(+), 28 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 22fbba3c62..06dcbaf5e8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -607,7 +607,7 @@ static int move_to_original_branch(struct rebase_options *opts)
strbuf_addf(&head_reflog, "%s (finish): returning to %s",
opts->reflog_action, opts->head_name);
ropts.branch = opts->head_name;
- ropts.flags = RESET_HEAD_REFS_ONLY;
+ ropts.flags = RESET_WORKING_TREE_REFS_ONLY;
ropts.branch_msg = branch_reflog.buf;
ropts.head_msg = head_reflog.buf;
ret = reset_working_tree(the_repository, &ropts);
@@ -862,9 +862,9 @@ static int checkout_up_to_date(struct rebase_options *options)
options->reflog_action, options->switch_to);
ropts.oid = &options->orig_head->object.oid;
ropts.branch = options->head_name;
- ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
+ ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
if (!ropts.branch)
- ropts.flags |= RESET_HEAD_DETACH;
+ ropts.flags |= RESET_WORKING_TREE_DETACH;
ropts.head_msg = buf.buf;
if (reset_working_tree(the_repository, &ropts) < 0)
ret = error(_("could not switch to %s"), options->switch_to);
@@ -1384,7 +1384,7 @@ int cmd_rebase(int argc,
rerere_clear(the_repository, &merge_rr);
string_list_clear(&merge_rr, 1);
- ropts.flags = RESET_HEAD_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not discard worktree changes"));
remove_branch_state(the_repository, 0);
@@ -1409,7 +1409,7 @@ int cmd_rebase(int argc,
ropts.oid = &options.orig_head->object.oid;
ropts.head_msg = head_msg.buf;
ropts.branch = options.head_name;
- ropts.flags = RESET_HEAD_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not move back to %s"),
oid_to_hex(&options.orig_head->object.oid));
@@ -1876,8 +1876,9 @@ int cmd_rebase(int argc,
options.reflog_action, options.onto_name);
ropts.oid = &options.onto->object.oid;
ropts.orig_head = &options.orig_head->object.oid;
- ropts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
- RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
+ ropts.flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
+ RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
ropts.default_reflog_action = options.reflog_action;
if (reset_working_tree(the_repository, &ropts)) {
diff --git a/reset.c b/reset.c
index 799596398b..4ca7f23a25 100644
--- a/reset.c
+++ b/reset.c
@@ -16,9 +16,9 @@ static int update_refs(struct repository *repo,
const struct object_id *oid,
const struct object_id *head)
{
- unsigned detach_head = opts->flags & RESET_HEAD_DETACH;
- unsigned run_hook = opts->flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
- unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
+ unsigned detach_head = opts->flags & RESET_WORKING_TREE_DETACH;
+ unsigned run_hook = opts->flags & RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
+ unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
const struct object_id *orig_head = opts->orig_head;
const char *switch_to_branch = opts->branch;
const char *reflog_branch = opts->branch_msg;
@@ -90,9 +90,9 @@ int reset_working_tree(struct repository *r,
{
const struct object_id *oid = opts->oid;
const char *switch_to_branch = opts->branch;
- unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
- unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
- unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
+ unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
+ unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
+ unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
struct object_id *head = NULL, head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
diff --git a/reset.h b/reset.h
index f130152014..2e5826de99 100644
--- a/reset.h
+++ b/reset.h
@@ -6,16 +6,22 @@
#define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
-/* Request a detached checkout */
-#define RESET_HEAD_DETACH (1<<0)
-/* Request a reset rather than a checkout */
-#define RESET_HEAD_HARD (1<<1)
-/* Run the post-checkout hook */
-#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
-/* Only update refs, do not touch the worktree */
-#define RESET_HEAD_REFS_ONLY (1<<3)
-/* Update ORIG_HEAD as well as HEAD */
-#define RESET_ORIG_HEAD (1<<4)
+enum reset_working_tree_flags {
+ /* Request a detached checkout */
+ RESET_WORKING_TREE_DETACH = (1 << 0),
+
+ /* Request a reset rather than a checkout */
+ RESET_WORKING_TREE_HARD = (1 << 1),
+
+ /* Run the post-checkout hook */
+ RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK = (1 << 2),
+
+ /* Only update refs, do not touch the worktree */
+ RESET_WORKING_TREE_REFS_ONLY = (1 << 3),
+
+ /* Update ORIG_HEAD as well as HEAD */
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
+};
struct reset_working_tree_options {
/*
@@ -33,7 +39,7 @@ struct reset_working_tree_options {
/*
* Flags defined above.
*/
- unsigned flags;
+ enum reset_working_tree_flags flags;
/*
* Optional reflog message for branch, defaults to head_msg.
*/
@@ -45,7 +51,8 @@ struct reset_working_tree_options {
const char *head_msg;
/*
* Optional reflog message for ORIG_HEAD, if this omitted and flags
- * contains RESET_ORIG_HEAD then default_reflog_action must be given.
+ * contains RESET_WORKING_TREE_UPDATE_ORIG_HEAD then
+ * default_reflog_action must be given.
*/
const char *orig_head_msg;
/*
diff --git a/sequencer.c b/sequencer.c
index d73ecf0384..4efe831178 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4677,7 +4677,9 @@ static void create_autostash_internal(struct repository *r,
if (has_unstaged_changes(r, 1) ||
has_uncommitted_changes(r, 1)) {
struct child_process stash = CHILD_PROCESS_INIT;
- struct reset_working_tree_options ropts = { .flags = RESET_HEAD_HARD };
+ struct reset_working_tree_options ropts = {
+ .flags = RESET_WORKING_TREE_HARD,
+ };
struct object_id oid;
strvec_pushl(&stash.args,
@@ -4870,8 +4872,9 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
struct reset_working_tree_options ropts = {
.oid = onto,
.orig_head = orig_head,
- .flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
- RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
+ .flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
+ RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK,
.head_msg = reflog_message(opts, "start", "checkout %s",
onto_name),
.default_reflog_action = sequencer_reflog_action(opts)
--
2.55.0.rc2.803.g1fd1e6609c.dirty
^ permalink raw reply related
* [PATCH v7 05/11] reset: introduce dry-run mode
From: Patrick Steinhardt @ 2026-06-29 7:34 UTC (permalink / raw)
To: git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood,
Christian Couder
In-Reply-To: <20260629-b4-pks-history-drop-v7-0-6e9392a957d8@pks.im>
In a subsequent commit we'll add another caller to `reset_working_tree()`
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 4ca7f23a25..99f2c1b012 100644
--- a/reset.c
+++ b/reset.c
@@ -93,11 +93,14 @@ int reset_working_tree(struct repository *r,
unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
+ unsigned dry_run = opts->flags & RESET_WORKING_TREE_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;
@@ -110,7 +113,7 @@ int reset_working_tree(struct repository *r,
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;
}
@@ -125,16 +128,36 @@ int reset_working_tree(struct repository *r,
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;
@@ -142,11 +165,6 @@ int reset_working_tree(struct repository *r,
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));
@@ -163,6 +181,9 @@ int reset_working_tree(struct repository *r,
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));
@@ -182,6 +203,7 @@ int reset_working_tree(struct repository *r,
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 2e5826de99..898e4a1e95 100644
--- a/reset.h
+++ b/reset.h
@@ -21,6 +21,12 @@ enum reset_working_tree_flags {
/* Update ORIG_HEAD as well as HEAD */
RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
+
+ /*
+ * Perform a dry-run by performing the operation without updating
+ * any user-visible state.
+ */
+ RESET_WORKING_TREE_DRY_RUN = (1 << 5),
};
struct reset_working_tree_options {
--
2.55.0.rc2.803.g1fd1e6609c.dirty
^ permalink raw reply related
* [PATCH v7 06/11] reset: introduce ability to skip updating HEAD
From: Patrick Steinhardt @ 2026-06-29 7:34 UTC (permalink / raw)
To: git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood,
Christian Couder
In-Reply-To: <20260629-b4-pks-history-drop-v7-0-6e9392a957d8@pks.im>
In a subsequent commit we'll introduce a new caller to
`reset_working_tree()` that really only wants to update the index and
working tree, without updating any references. Introduce a new flag that
makes the caller opt in to updating HEAD and adapt all callers to set
that flag.
Note that in a previous iteration we instead introduced a flag that made
callers opt out of updating any references. This was somewhat awkward
though because we already have the `UPDATE_ORIG_HEAD` flag, so the
result was somewhat inconsistent.
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rebase.c | 14 ++++++++++----
reset.c | 9 +++++++--
reset.h | 9 ++++++---
sequencer.c | 4 +++-
4 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 06dcbaf5e8..10a306310c 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -607,7 +607,8 @@ static int move_to_original_branch(struct rebase_options *opts)
strbuf_addf(&head_reflog, "%s (finish): returning to %s",
opts->reflog_action, opts->head_name);
ropts.branch = opts->head_name;
- ropts.flags = RESET_WORKING_TREE_REFS_ONLY;
+ ropts.flags = RESET_WORKING_TREE_REFS_ONLY |
+ RESET_WORKING_TREE_UPDATE_HEAD;
ropts.branch_msg = branch_reflog.buf;
ropts.head_msg = head_reflog.buf;
ret = reset_working_tree(the_repository, &ropts);
@@ -693,6 +694,7 @@ static int run_am(struct rebase_options *opts)
ropts.oid = &opts->orig_head->object.oid;
ropts.branch = opts->head_name;
ropts.default_reflog_action = opts->reflog_action;
+ ropts.flags = RESET_WORKING_TREE_UPDATE_HEAD;
reset_working_tree(the_repository, &ropts);
error(_("\ngit encountered an error while preparing the "
"patches to replay\n"
@@ -862,7 +864,8 @@ static int checkout_up_to_date(struct rebase_options *options)
options->reflog_action, options->switch_to);
ropts.oid = &options->orig_head->object.oid;
ropts.branch = options->head_name;
- ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
+ ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK |
+ RESET_WORKING_TREE_UPDATE_HEAD;
if (!ropts.branch)
ropts.flags |= RESET_WORKING_TREE_DETACH;
ropts.head_msg = buf.buf;
@@ -1384,7 +1387,8 @@ int cmd_rebase(int argc,
rerere_clear(the_repository, &merge_rr);
string_list_clear(&merge_rr, 1);
- ropts.flags = RESET_WORKING_TREE_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD |
+ RESET_WORKING_TREE_UPDATE_HEAD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not discard worktree changes"));
remove_branch_state(the_repository, 0);
@@ -1409,7 +1413,8 @@ int cmd_rebase(int argc,
ropts.oid = &options.orig_head->object.oid;
ropts.head_msg = head_msg.buf;
ropts.branch = options.head_name;
- ropts.flags = RESET_WORKING_TREE_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD |
+ RESET_WORKING_TREE_UPDATE_HEAD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not move back to %s"),
oid_to_hex(&options.orig_head->object.oid));
@@ -1877,6 +1882,7 @@ int cmd_rebase(int argc,
ropts.oid = &options.onto->object.oid;
ropts.orig_head = &options.orig_head->object.oid;
ropts.flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_HEAD |
RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
diff --git a/reset.c b/reset.c
index 99f2c1b012..4bde5d8dc6 100644
--- a/reset.c
+++ b/reset.c
@@ -92,6 +92,7 @@ int reset_working_tree(struct repository *r,
const char *switch_to_branch = opts->branch;
unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
+ unsigned update_head = opts->flags & RESET_WORKING_TREE_UPDATE_HEAD;
unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
unsigned dry_run = opts->flags & RESET_WORKING_TREE_DRY_RUN;
struct object_id *head = NULL, head_oid;
@@ -113,6 +114,9 @@ int reset_working_tree(struct repository *r,
if (opts->branch_msg && !opts->branch)
BUG("branch reflog message given without a branch");
+ if (update_orig_head && !update_head)
+ BUG("cannot update ORIG_HEAD without updating HEAD" );
+
if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
@@ -129,7 +133,7 @@ int reset_working_tree(struct repository *r,
oid = &head_oid;
if (refs_only) {
- if (!dry_run)
+ if (!dry_run && update_head)
return update_refs(r, opts, oid, head);
return 0;
}
@@ -197,7 +201,8 @@ int reset_working_tree(struct repository *r,
goto leave_reset_head;
}
- if (oid != &head_oid || update_orig_head || switch_to_branch)
+ if (update_head &&
+ (oid != &head_oid || update_orig_head || switch_to_branch))
ret = update_refs(r, opts, oid, head);
leave_reset_head:
diff --git a/reset.h b/reset.h
index 898e4a1e95..38b2891b53 100644
--- a/reset.h
+++ b/reset.h
@@ -19,14 +19,17 @@ enum reset_working_tree_flags {
/* Only update refs, do not touch the worktree */
RESET_WORKING_TREE_REFS_ONLY = (1 << 3),
- /* Update ORIG_HEAD as well as HEAD */
- RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
+ /* Update HEAD */
+ RESET_WORKING_TREE_UPDATE_HEAD = (1 << 4),
+
+ /* Update ORIG_HEAD */
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 5),
/*
* Perform a dry-run by performing the operation without updating
* any user-visible state.
*/
- RESET_WORKING_TREE_DRY_RUN = (1 << 5),
+ RESET_WORKING_TREE_DRY_RUN = (1 << 6),
};
struct reset_working_tree_options {
diff --git a/sequencer.c b/sequencer.c
index 4efe831178..e905b1b2d9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4678,7 +4678,8 @@ static void create_autostash_internal(struct repository *r,
has_uncommitted_changes(r, 1)) {
struct child_process stash = CHILD_PROCESS_INIT;
struct reset_working_tree_options ropts = {
- .flags = RESET_WORKING_TREE_HARD,
+ .flags = RESET_WORKING_TREE_HARD |
+ RESET_WORKING_TREE_UPDATE_HEAD,
};
struct object_id oid;
@@ -4873,6 +4874,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
.oid = onto,
.orig_head = orig_head,
.flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_HEAD |
RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK,
.head_msg = reflog_message(opts, "start", "checkout %s",
--
2.55.0.rc2.803.g1fd1e6609c.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