The behavior of repack and pack-objects has been updated to avoid
losing objects to a '.keep' file that appears mid-run. Instead of
pack-objects looking for '.keep' files independently, repack now
hands it a snapshot of kept packs via a new --keep-pack-from-file
option.
* qs/repack-kept-pack-fix:
repack: tell pack-objects which packs are kept
pack-objects: add --keep-pack-from-file
pack-objects: sort --keep-pack list for lookup
pack-objects: reset kept-pack cache for cruft walk
pack-objects: keep --keep-pack open when following
Various corner cases in the cruft-less MIDX feature have been fixed
to ensure reachability closure when writing reachability bitmaps,
such as properly retaining cruft packs after incremental repacks or
when containing kept packs.
* tb/repack-cruft-less-midx-corner-cases:
repack: include required packs in incremental MIDX writes
repack: defer allocating the append plan's write step
repack: track the preferred pack explicitly in MIDX write steps
repack: follow kept packs when omitting cruft from the MIDX
repack: use a sorted list for explicitly kept packs
repack: retain cruft packs in MIDXs after incremental repacks
pack-objects: ensure tree/tag closure with '--stdin-packs=follow'
pack-objects: introduce `stdin_packs_context` struct
"--keep-pack" names one pack per occurrence, and there is only so much
room on the command line: ARG_MAX is shared with the environment, and
on Windows the whole line is capped at 32,767 characters, which a few
hundred pack names fill. Past that the spawn fails before pack-objects
has started. fetch-pack grew "--stdin" in 078b895fef (fetch-pack: new
--stdin option to read refs from stdin, 2012-04-02) for the same
reason.
stdin is taken here: every mode repack drives pack-objects in already
uses it, for the revision list under "-a", object names for the
promisor pack, and pack lists for "--stdin-packs" and "--cruft". So
read the names from a file instead, one per line, skipping empty
lines. They go into the same list as the "--keep-pack" names and are
treated exactly alike: matched against local packs, ignored when they
match nothing, and kept open under "--stdin-packs=follow". A relative
path is resolved against the directory the user ran from, as
"--refs-snapshot" of "git multi-pack-index write" is.
The list now holds strings from two sources, so let it own its copies.
repack is about to use this to hand pack-objects its own snapshot of
the packs that have a ".keep" file.
Signed-off-by: Qin ShiCheng <qeesung@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
add_extra_kept_packs() scans the whole "--keep-pack" list once per
pack in the repository. That is fine for the handful of names it gets
today, but the next commit lets a caller name every kept pack in the
repository, and with thousands of them the scan dominates: matching
20,000 kept packs against 20,000 names takes 11 seconds here, against
under a second with "--honor-pack-keep".
Sort the list once and look each pack up in it. The comparison stays
fspathcmp(), so what matches does not change.
Signed-off-by: Qin ShiCheng <qeesung@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When writing a cruft pack with an expiration, pack-objects first
collects the recent objects and then walks from them to rescue
whatever they reach, expired or not. A pack the caller did not list is
marked kept while collecting, so that its objects are not copied into
the cruft pack, and unmarked before the walk, so that the walk can go
through it.
The walk does not see the unmarking. Whether an object sits in a kept
pack is answered from a cache that is built on first use and only
dropped when asked about a different kind of kept pack. Collecting
builds it while the unlisted pack is still marked, the walk asks the
same kind of question, and so the unlisted pack stays in it: the walk
stops there, and whatever lies beyond it in an expired pack is left
out of the cruft pack, to go when that pack is deleted.
This went unnoticed because of "--honor-pack-keep". repack passes it,
and when there is a ".keep" file it makes the collecting side ask
about on-disk and in-core kept packs together while the walk asks
about in-core ones alone; the cache is rebuilt each time the question
changes, and by accident the walk sees the current marks. Take the
".keep" file away and the objects are lost today. A later commit stops
repack from passing "--honor-pack-keep" at all, so fix this first.
Drop the cache after re-marking. The loop over the object sources
that does so lives in packfile.c, as repo_invalidate_kept_pack_caches(),
next to has_object_kept_pack() which reads the cache. Like it, the
loop assumes every source is a files backend; keeping that assumption
in packfile.c rather than adding it to pack-objects means the two can
move together once packfile management is pushed down into that
backend.
The test builds an unreachable chain whose middle commit sits in a
pack pack-objects is not told about and whose oldest objects have
expired; without the fix the cruft pack holds only the recent tip.
Signed-off-by: Qin ShiCheng <qeesung@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"--stdin-packs=follow" distinguishes excluded packs that are closed
under reachability ("^") from those that are not ("!"). The traversal
stops at objects in the former, and goes on through the latter to
rescue whatever they depend on that would otherwise be left out.
A pack named with "--keep-pack" gets the same in-core flag as a "^"
pack, so the traversal stops at it too. Nothing warrants that: the
caller said not to repack it, not that it is self-contained. When it
holds a commit but not that commit's tree, the tree is never rescued,
and writing a bitmap over the result fails for lack of closure.
In follow mode, mark such a pack as kept-open instead, the way repack
already lists the packs it cannot vouch for as "!" on stdin. Its
objects stay out of the result, and the traversal can go through it.
This matters more once repack names its ".keep" packs this way instead
of passing "--honor-pack-keep": on-disk kept packs never were a
boundary, and they should not become one.
Signed-off-by: Qin ShiCheng <qeesung@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Originally, when designing pluggable object databases the goal was that
the object database can have multiple sources, and every source attached
to it could use a different backend. This would have allowed for quite a
lot of flexibility, as you could trivially mix and match different kinds
of object storages in whatever way you like.
But while well-intentioned, this design led to a bunch of conceptual
problems:
- We're now trying to read objects in source order, whereas we
previously tried to read objects via packfiles before trying to read
them via loose objects. This led to a performance regression when
using alternates or when using a quarantine directory.
- Some data structures are supposed to only ever exist once, like for
example bitmaps and commit graphs. At the same time, those data
structures also span across the union of all objects, so they may
cross sources.
- It is unclear how we can extend GIT_OBJECT_DIRECTORY or
GIT_ALTERNATE_OBJECT_DIRECTORIES to become backend-agnostic in a
backwards-compatible way. In general, introducing an object storage
extension into the current status quo where alternates may have to
be extended to become generic was proving to be painful.
- Some mechanisms of alternates assume way too much about how exactly
their backends work. Alternate refs for example assume that the
alternate is backed by a filesystem path, and that this filesystem
path may also allow us to read references. This is not a given
though, as backends may not even have local data at all.
In short, there are a bunch of conceptual mismatches when we have
alternates and pluggable object databases coexist. So while the original
idea was nice, it does not result in a system that is easy to reason
about.
Correct course by moving alternates into the "files" source itself so
that it becomes an implementation detail thereof so that we can avoid
all of these shortcomings. While it's unfortunate that we cannot easily
mix and match sources now, that ability doesn't go away. It's still very
much feasible to introduce a new backend that allows for exactly that
use case, and such a backend may also be a lot more flexible as we can
now add new logic to determine which objects should be stored where. So
the original motivation for having per-source backends can still be
realized with the new architecture.
Note that as part of this move, we also handle the GIT_OBJECT_DIRECTORY
and GIT_ALTERNATE_OBJECT_DIRECTORIES environment variables in the
"files" backend. This may be surprising at first, but object directories
are very much a concept of that backend, too. So these variables would
have bad interactions with other backends, and they create a bit of a
mismatch with the eventual object storage extension that we plan to
introduce.
Note that this commit is way larger than I'd like it to be. I'm sorry, I
couldn't find a way to split it up further. That being said, most of the
changes are straight-forward conversions that go from iterating over
sources to iterating over object directories. The more involved changes
are moving the infrastructure to track individual sources from "odb.c"
and moving them into "odb/source-files.c".
This also serves to show a bit of a who's-who of commands that don't
work properly with pluggable object databases. Almost all of these
commands are related to housekeeping though -- some of them will be
converted eventually, like for example commit graphs or MIDXs. But many
of them will stay incompatible going forward as they are simply too
specific to the "files" backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "files" object database source consists of two separate backends for
storing loose and packed objects. These are managed as somewhat separate
entities even though they derive from the same object directory, whether
it's the primary object directory or that one from an alternate.
In a subsequent commit we'll move the handling of alternates into the
"files" backend completely so that it becomes another implementation
detail thereof. As part of this, we'll want to keep track of both of
these sub-sources as a single entity derived from their respective
object directory.
Prepare for this change by introducing a new `struct odb_files_dir` that
encapsulates them. For now, every "files" source has exactly one such
directory. In a subsequent commit though, we'll make it a linked list of
directories so that we can manage multiple such directories in a single
"files" source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 5ee86c273b (repack: exclude cruft pack(s) from the MIDX where
possible, 2025-06-23), when the 'repack.midxMustContainCruft'
configuration is set to "false", geometric repacks use
'--stdin-packs=follow' to copy needed objects out of cruft packs so the
MIDX can omit those packs.
In cd846bacc7 (pack-objects: introduce '--stdin-packs=follow',
2025-06-23), this behavior changed such that whenever excluded-open
('!') packs are present, the walk stops at objects in excluded-closed
('^') packs. Geometric repacks use '^' for retained packs already in the
MIDX, relying on the indexed object set being closed under reachability.
However, the walk introduced in cd846bacc7 starts only from commit
objects. A geometric repack can therefore produce a MIDX that does not
maintain reachability closure for lone trees (that are not reachable
from any commit otherwise in the closure).
A subsequent repack with '!' packs can stop at that tree in a retained
'^' pack even if a new commit reaches it. If the cruft pack remains
excluded, and the bitmap selection picks one or more commits which reach
that tree, the MIDX cannot generate a bitmap for that commit.
Add trees and tags from included and '!' packs (and loose ones with
'--unpacked') as roots in '--stdin-packs=follow' mode. This rescues
their descendants even when no input commit reaches them. Walk these
roots after the existing traversal, preserving the `SEEN` bit to avoid
redundant traversals. This gives directly enumerated commits priority
for the path prefixes used by name hashes and delta attributes. Tags can
introduce commits in the second walk, so path selection remains
best-effort.
Collect the extra roots in an oidset to avoid queuing duplicates. This
uses memory for each distinct root and walks its unvisited descendants.
Objects in '^' packs remain cutoffs to avoid rewalking packs that are
known to be closed under reachability, provided '!' packs are present.
This does not repair existing MIDXs lacking closure; those need a full
repack.
Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`stdin_packs_read_input()` currently receives a pointer to the
`rev_info` struct and '--stdin-packs' mode separately as arguments, but
the object enumeration callbacks only receive a pointer to the
`rev_info` struct.
Wrap the pair in a new `stdin_packs_context` struct so that a future
change may reference the '--stdin-packs' mode within the various
object enumeration callbacks.
Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The object lookup machinery has been taught to gracefully recover
when a multi-pack-index points to an owning pack that was removed
during a concurrent geometric repack, and 'git replay' has been
fixed to not segfault when reading such missing objects.
* en/midx-missing-pack-fallback:
packfile: recover when a multi-pack-index names a removed pack
mktree: do not use OBJECT_INFO_QUICK when checking objects
mktree: plug per-tree leak in --batch mode
replay: fail gracefully when a merge input is unreadable
The global variable 'fetch_if_missing' has been moved to a member in
'struct repository', continuing the libification process and
allowing per-repository control (such as for submodules).
* ty/repository-fetch-if-missing:
repository: move fetch_if_missing into struct repository
The pack-objects command has been updated to record the total bytes
written to pack files in trace2 output, allowing performance
analysis of different compression settings by comparing the
resulting pack sizes.
* fr/pack-objects-trace-pack-bytes:
pack-objects: trace pack bytes written
The object database layer has been simplified by eagerly loading
alternate object directories upon initialization, instead of
deferring it to the first object lookup. This eliminates the need
for scattered lazy-loading calls throughout the codebase and paves
the way for integrating alternates with the pluggable backends.
* ps/odb-eagerly-load-alternates:
odb: drop `alternates_db` field
odb: drop `loaded_alternates` field
odb: eagerly initialize alternates
odb: decouple source path comparisons from `the_repository`
setup: create ref and object databases after config is written
The object database (odb) API has been refactored to distinguish
between missing objects and corrupt ones by returning more
descriptive error statuses. Both the packed and loose backends now
faithfully propagate error details using a generic strbuf error
mechanism, removing backend-specific leakage from central lookup
paths.
* ps/odb-generic-corrupt-objects:
odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically
odb/source: allow `read_object_info()` to bubble up error messages
odb/source: let callers discern missing and corrupt objects
odb/source: introduce error status when reading objects
odb/source-packed: flag known-bad objects as corrupt and not missing
A geometric repack writes a new pack and multi-pack-index and then
deletes the packs the new one subsumes. A process still using the
previous MIDX keeps seeing a removed pack listed as the owner of some
objects. Since a MIDX attributes each object to exactly one pack, such
an object is served only through its recorded owner; if that owner was
just removed, find_pack_entry() cannot serve it -- the MIDX lookup routes
to the missing pack, and the regular pack fallback deliberately skips
every MIDX-covered pack, so a surviving copy in another covered pack
(e.g. a kept base pack) is never consulted.
Unlike the ordinary "a pack's .idx is mapped but its .pack is gone"
race, the second read does not rescue us. Reloading the on-disk pack set
does not reload the borrowed, cached MIDX (freeing it under the code that
caches the "struct multi_pack_index *" would be a use-after-free), so the
stale MIDX keeps routing to the removed pack and the surviving copy stays
hidden behind the covered-pack skip. cat-file, rev-list and pack-objects
can thus all spuriously fail with "unable to read object".
Teach find_pack_entry() to recover. The MIDX lookup now returns a
tri-state, distinguishing an object absent from the MIDX from one it owns
via a pack that can no longer be opened; in the latter case, once the
regular fallback has also missed, scan the MIDX's packs directly for a
surviving copy. Because the return value is no longer a boolean, rename
fill_midx_entry() to midx_fill_entry() so callers must reckon with the
new enum rather than silently treat MIDX_FILL_OWNER_UNAVAILABLE as a hit.
Do the scan only on the second read (OBJECT_INFO_SECOND_READ): by then
the cheaper on-disk reload has run, so an object merely relocated into a
new (uncovered) pack has already been found by the regular fallback, and
only a genuine hidden duplicate reaches the rescan. A QUICK caller that
skips the second read simply accepts the false negative, as QUICK is
designed to.
Reloading the stale MIDX would be a more complete fix but is much more
involved (the borrowers above need proper invalidation), so leave that
for later.
Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'pack-objects' and delta-encoding code paths have been updated to
use 'size_t' instead of 'unsigned long' for object sizes and offset
limits, avoiding potential truncation issues on 64-bit Windows.
* js/pack-objects-delta-size-t:
packfile: widen `unpack_object_header_buffer()` to `size_t`
git-zlib: widen `git_deflate_bound()` to `size_t`
t/helper/test-pack-deltas: widen `do_compress()`'s maxsize local to `size_t`
http-push: widen `start_put()`'s size local from `ssize_t` to `size_t`
diff: widen `deflate_it()`'s bound local from int to `size_t`
archive-zip: widen `zlib_deflate_raw()`'s maxsize local to `size_t`
packfile, git-zlib: widen `use_pack()` and zstream avail fields to `size_t`
delta: widen `create_delta()` and `diff_delta()` to `size_t`
pack-objects: widen `mem_usage` and `try_delta()`'s out-param to `size_t`
pack-objects: widen `free_unpacked()` return to `size_t`
pack-objects: widen delta-cache accounting to `size_t`
delta: widen `create_delta_index()` parameter to `size_t`
diff-delta: widen `struct delta_index`' size fields to `size_t`
We want to measure how compression settings affect push performance on
the client. Different settings can produce different-sized packs from
the same objects. Trace2 records the object count, but we also need the
pack size to compare those settings.
Add a write_pack_file/wrote_bytes Trace2 datum alongside
write_pack_file/wrote. Count packs written to stdout or disk, including
each pack's header and trailing checksum. When pack.packSizeLimit splits
the output, report the sum of the pack sizes.
Signed-off-by: Friel <friel@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When reading an object fails even though it exists, the sources know
best what exactly went wrong and where the corrupt object is located.
This information is lost though when bubbling up the error to the object
database layer, which forces that layer to reconstruct it after the
fact. This is exactly what `do_oid_object_info_extended()` does via
`has_packed_and_bad()`, but that function only really knows to handle
the "files" backend by reaching into its internals.
Introduce a new `errmsg` parameter for the `read_object_info()` callback
that sources are expected to populate with a human-readable message in
case reading the object has failed. Adapt the packed and loose sources
to populate the buffer with the messages that we ultimately want to
surface to the user.
For now, all callers are adapted to pass a `NULL` pointer. We will add a
user of this new infrastructure in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When reading packed objects we know to tell apart missing objects and
corrupt objects by returning a positive error code in the former case,
and a negative one in the latter case. We do that by distinguishing
between errors returned by `find_pack_entry()`, which yields the offset
of the object, and `packed_object_info()`, which reads the object
contents.
But even though we already distinguish those cases when reading packed
objects, the logic is broken in case a caller tries to read an object
that has been marked as corrupt. In that case, `find_pack_entry()` will
tell us that the object in question does not exist, and consequently
we'll not flag the object as corrupt but as missing.
Fix this issue by bubbling up whether the object is corrupt and, if so,
which packfile contains the corrupted object.
Note that we don't yet need the information about the specific packfile,
so we could've just as well made this a `bool *corrupted` pointer. But
we'll need information about the containing packfile in a subsequent
commit so that we can generate a proper error message telling the user
which packfile contains the broken object.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating the object database we initialize the main object database
source, but we don't yet initialize its alternates. Instead, we have
many calls to `odb_prepare_alternates()` cluttered around the code base
whenever we are about to iterate through the sources.
This lazy loading doesn't really add much value: the moment where we
read any object we _have_ to load the alternates anyway. So given that
most of our commands would access the object database this optimization
is not really buying us much in the first place. Quite on the contrary,
it makes the code harder to understand and is a potential source of bugs
in case any callsite forgot to prepare alternates before we iterate
through the sources.
Historically though there was a reason why we deferred lazy-loading: it
may happen that the repository has "core.ignoreCase" configured, and we
use that to deduplicate the list of alternates in case we had the same
alternate configured multiple times, but with different casing. We used
to initialize the object database before we had fully configured the
owning repository though, and consequently we couldn't access that
configuration yet. This has changed in the preceding commit though where
we started to parse "core.ignoreCase" manually.
Eagerly prepare alternates both when creating the object database and
when flushing its caches. Drop the now-unneeded calls to prepare the
alternates that are scattered across the code base.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'fetch_if_missing' controls whether a missing
object check should prompt a lazy fetch from a promisor remote.
In order to continue the libification effort, move it into
'struct repository' and initialize it to 1 by default to keep the
previous behavior.
builtin/fetch-pack.c, builtin/fsck.c, and builtin/rev-list.c are
entered via commands marked RUN_SETUP in git.c:commands[]. Their
'repo' parameter is only NULL when '-h' is given outside of a
repository, in which case either show_usage_if_asked() or
parse_options()'s own '-h' handling exits the process before
returning. We can therefore drop their UNUSED markers and assign
to 'repo' directly.
builtin/index-pack.c is entered via RUN_SETUP_GENTLY, so its
'repo' pointer can be NULL any time it is run outside of a
repository, not only with '-h'. We keep a NULL check there and fall
back to 'the_repository'.
builtin/pack-objects.c needs two adjustments to make 'repo' reach
every 'fetch_if_missing' call site: 'read_stdin_packs()' now takes a
'struct repository *'; 'option_parse_missing_action()', which
is registered as an OPT_CALLBACK, receives a 'repo' through the
option's 'value' field now.
Additionally, update the partial clone documentation to reflect
that this is now a per-repository flag.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As part of the ongoing effort to replace `unsigned long` data types with
`size_t` wherever appropriate (mainly to fix all those problems on
Windows with objects larger than 4GB), let's also adjust the return type
and the type of the `len` parameter of this function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bundling the two widenings: four call sites pass `&stream.avail_in`
directly to `use_pack()`, and widening either type fencepost alone would
force a bridge variable at each. Doing both together is the simpler end
state and is the prerequisite for the `do_compress()` widening in the
next commit, which is what lets `write_no_reuse_object()` lose its last
`cast_size_t_to_ulong()` shim.
The unsigned-long locals widened at the other `use_pack()` callers
(avail / remaining / left) hold pack-window sizes bounded by
`core.packedGitWindowSize`, so the change is type consistency rather
than a new >4GB capability. `git_zstream.avail_in`/`avail_out` likewise
reach zlib's `uInt` fields only after `zlib_buf_cap()`'s 1 GiB cap, so
the wrapper already accepted `size_t`-shaped inputs in practice.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Last stop in the delta-encoding API widening for >4 GiB blobs on
Windows: with `create_delta_index()` done in the prior commit and
`create_delta()`/`diff_delta()` finished here, every byte count that
crosses delta.h is now `size_t`. The struct fields they store into have
been `size_t` since the diff-delta struct widening.
The API change must move with all callers in the same commit (the build
only passes when every `&delta_size` matches the new `size_t*`). Caller
updates are kept minimal:
* builtin/pack-objects.c `get_delta()` and `try_delta()`: widen only
the local `delta_size` variable; the surrounding unsigned-long
locals and their `cast_size_t_to_ulong()` shims are out of scope
here and will be cleaned up in their own commits.
* builtin/fast-import.c, diff.c, t/helper/test-pack-deltas.c:
keep the local unsigned-long delta size (each feeds a still-
unsigned-long downstream consumer: zlib's `avail_in`,
`deflate_it()`, the test helper's own `do_compress()`), and bridge
via a temporary `size_t` plus `cast_size_t_to_ulong()`. The new
casts are paid back in later topics that widen those consumers.
* t/helper/test-delta.c: widen the local outright (no downstream
consumer beyond the test's own `out_size`, which is already
`size_t`).
Note that GCC struggles a bit to figure out that `deltalen` is always
initialized before it is used; To help it along, we initialize it to 0.
This work-around will go away in a later patch series when `deltalen`
can be widened to `size_t`.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The pair must move together because `find_deltas()` passes `&mem_usage`
to `try_delta()`: widening either alone breaks the type match.
`mem_usage` accumulates per-object byte counts already computed in
`size_t` (`SIZE()` and `sizeof_delta_index()` reach here through
`free_unpacked()`, now `size_t`), and was the last 32-bit-on-Windows
narrowing point in the delta-window memory accounting chain. With this
commit, that chain uses `size_t` consistently except for
`sizeof_delta_index()`'s still-narrow return, whose value is bounded by
`create_delta_index()`'s entries cap.
`window_memory_limit` (config-driven via `git_config_ulong()`) stays
`unsigned long`: it is only compared against `mem_usage` and promotes.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`free_unpacked()` sums two byte counts: `sizeof_delta_index()` and
`SIZE(n->entry)`. The latter has been `size_t` since the prior topic
"More work supporting objects larger than 4GB on Windows" widened
`SIZE()`/`oe_size()` to `size_t`, so accumulating it into an `unsigned
long` return was a silent Windows-only truncation on a packing run with
many large objects.
The sole caller, `find_deltas()`, still holds its own `mem_usage` in an
`unsigned long` for now, and therefore still truncates silently.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These three are a single accounting tuple (the globals tracking
cumulative cached-delta bytes, plus the helper that compares them
against an incoming delta size) and are latently 32-bit on Windows where
`unsigned long` != `size_t`: a pack with many large cached deltas could
wrap silently.
The widening is internally consistent on its own: the additions and
subtractions against delta_cache_size already come from `size_t` sources
(`DELTA_SIZE()` returns `size_t`), and `delta_cacheable()`'s sole caller
in `try_delta()` still passes `unsigned long`, which promotes.
Prerequisite for dropping `try_delta()`'s `cast_size_t_to_ulong()`
shims, which becomes possible once 1create_delta()` and `diff_delta()`
are widened in a later commit.
Note: since `max_delta_cache_size` changes data type to `size_t`, a pair
of new helpers is introduced to parse config values of that type, too.
Assisted-by: Opus 4.7
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Unify the function names to create new streams from different sources so
that they follow a common schema. While at it, document the ownership of
the file descriptor passed to `odb_stream_from_fd()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rename `struct odb_read_stream` to just `struct odb_stream`. This
prepares for unification of the two different types of streams, as these
provide the same functionality with the preceding refactorings.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic to write loose objects has been refactored and moved from
'object-file.c' to the loose backend source file 'odb/source-loose.c',
making the loose backend more self-contained. This is achieved by
first refactoring force_object_loose() to use generic ODB write
interfaces instead of loose-backend internals.
* ps/odb-move-loose-object-writing:
object-file: move logic to write loose objects
object-file: move `force_object_loose()`
object-file: force objects loose via generic interface
object-file: fix memory leak in `force_object_loose()`
odb: support setting mtime when writing objects
odb: lift object existence check out of the "loose" backend
odb: compute object hash in `odb_write_object_ext()`
t/u-odb-inmemory: implement wrapper for writing objects
odb: compute compat object ID in `odb_write_object_ext()`
In the preceding commits we have refactored `force_object_loose()` to
not call internal functions anymore for writing the object. Instead, it
now only uses generic functions that are accessible to all callers.
Consequently, we can now easily move the function to its only caller,
which is git-pack-objects(1). Do so.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `force_object_loose()` is used to loosen packed objects
before repacking. It passes the pack's mtime along so that the newly
written loose object inherits the same timestamp. This matters for
object pruning, which uses the mtime to determine whether an object is
old enough to be pruned.
In a subsequent commit, `force_object_loose()` will be converted to use
the generic `odb_source_write_object()` interface instead of calling
`write_loose_object()` directly. But the generic interface doesn't yet
support setting a specific mtime, which makes it impossible to implement
the logic as of now.
Prepare for the change by introducing a new `mtime` parameter to this
function that we plumb through the stack. If set, the backends are
instructed to set the object's mtime accordingly. If unset, the backends
are expected to use the current time instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'whence' field in 'struct object_info' has been removed. The
backend-specific object information retrieval has been refactored into
an opt-in 'struct object_info_source' structure.
* ps/odb-drop-whence:
odb: document object info fields
odb: drop `whence` field from object info
treewide: convert users of `whence` to the new source field
odb: add `source` field to struct object_info_source
odb: make backend-specific fields optional
packfile: thread odb_source_packed through packed_object_info()
In a subsequent commit we'll lift iteration of bitmapped objects into
the "packed" backend and make it accessible via `odb_for_each_object()`.
The calling convention for that function is that the callback may return
a non-zero exit code, and if so we'll abort iteration. This is currently
impossible to realize though, as `for_each_bitmapped_object()` will
ignore any return value and just churn through all objects completely.
This doesn't matter to the callers of `for_each_bitmapped_object()`, as
there's only one of them in git-cat-file(1), and the callbacks we pass
always return zero. But once we move the logic into the generic
infrastructure it becomes a latent bug waiting to happen.
Refactor the code so that the return value of the `show_reach` callback
is not ignored anymore. Instead, returning a non-zero value will cause
us to abort iteration in both `show_objects_for_type()` and in
`for_each_bitmapped_object()`.
Note though that there's a second user of `show_objects_for_type()` with
`traverse_bitmap_commit_list()`, and that function does indeed invoke
callbacks that may return non-zero. This non-zero return value never had
any effect at all though, and the callbacks that return non-zero values
are only ever invoked via `traverse_bitmap_commit_list()`. Consequently,
we adapt them to always return 0.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function returns 0/1 to its caller to tell them whether we actually
added a new entry (or if we considered it redundant). But nobody has
relied on that behavior since 5379a5c5ee (Thin pack generation:
optimization., 2006-04-05).
The extra return does not hurt much, but it is a bit confusing. We have
a sister function, add_object_entry_from_bitmap(), which has the same
return value semantics. That function is about to change to always return
0 (not void, because it must conform to a callback function interface).
So with that change, we'd have two related functions which both return
an "int" but with different semantics.
Let's drop the unused "int" return from add_object_entry() entirely,
which makes it more clear that the two functions have diverged.
Signed-off-by: Jeff King <peff@peff.net>
[ps: slightly massaged the commit message]
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ps/odb-drop-whence:
odb: document object info fields
odb: drop `whence` field from object info
treewide: convert users of `whence` to the new source field
odb: add `source` field to struct object_info_source
odb: make backend-specific fields optional
packfile: thread odb_source_packed through packed_object_info()
The pack-objects command has been updated to support reachability
bitmaps and delta-islands concurrently with the `--path-walk` option,
allowing faster packaging by falling back to path-walk when bitmaps
cannot fully satisfy the request.
* tb/pack-path-walk-bitmap-delta-islands:
pack-objects: support `--delta-islands` with `--path-walk`
pack-objects: extract `record_tree_depth()` helper
pack-objects: support reachability bitmaps with `--path-walk`
t/perf: drop p5311's lookup-table permutation
The packed object source has been refactored into a proper struct
odb_source.
* ps/odb-source-packed:
odb/source-packed: drop pointer to "files" parent source
midx: refactor interfaces to work on "packed" source
odb/source-packed: stub out remaining functions
odb/source-packed: wire up `freshen_object()` callback
odb/source-packed: wire up `find_abbrev_len()` callback
odb/source-packed: wire up `count_objects()` callback
odb/source-packed: wire up `for_each_object()` callback
odb/source-packed: wire up `read_object_stream()` callback
odb/source-packed: wire up `read_object_info()` callback
packfile: use higher-level interface to implement `has_object_pack()`
odb/source-packed: wire up `reprepare()` callback
odb/source-packed: wire up `close()` callback
odb/source-packed: start converting to a proper `struct odb_source`
odb/source-packed: store pointer to "files" instead of generic source
packfile: move packed source into "odb/" subsystem
packfile: split out packfile list logic
packfile: rename `struct packfile_store` to `odb_source_packed`
The `whence` field has become redundant now that callers can learn about
the exact source an object has been looked up from via the `struct
object_info_source::source` field.
Adapt callers to use the new field. Note that all callsites already set
up the `info.sourcep` request pointer, so the conversion is rather
straight-forward.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit introduced `struct object_info_source` as an opt-in
container for backend-specific information, but for now we only moved
preexisting data into this structure. Most importantly, the caller has
no way yet to learn about which source an object was actually looked up
from. Instead, callers have to rely on the `whence` enum to distinguish
the object type, but cannot use that enum to tell the object source.
Add a `struct odb_source *source` field to the structure and populate it
from each backend's lookup path.
The `whence` enum is still set and used by callers; it will be removed
in a subsequent commit now that `sourcep->source` can identify the
backend on its own.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `struct object_info` carries two pieces of information
about how an object was looked up:
- The `whence` enum identifying the backend.
- The backend-tagged union `u` exposing backend-specific details
(currently only the packed-source case, which records the owning
pack, offset and packed object type).
The union is populated unconditionally, even though most callers don't
care about provenance at all.
Split the backend-specific union out into a new public type, `struct
object_info_source`, and make the object info structure carry it via
just another opt-in request pointer. As with all the other requestable
information, callers that need source info allocate a `struct
object_info_source` on the stack and point `sourcep` at it; callers that
don't care about it simply leave the field as a `NULL` pointer. Adapt
callers accordingly.
Note that the `whence` enum is strictly-speaking also backend-specific
information, so it would be another good candidate to be moved into the
`struct object_info_source`. For now though it is left alone, as it will
be replaced by a `struct odb_source` pointer in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add an optional `struct odb_source_packed *source` parameter to
`packed_object_info()` and `packed_object_info_with_index_pos()`. This
parameter is unused at this point in time, but it will be used in a
follow-up commit so that we can record the source of a specific object.
Note that callers in "odb/source-packed.c" pass the already-available
source, but all other callers pass `NULL` instead. This is fine though,
as we only care about populating this info when called via the packed
store.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ps/odb-source-packed:
odb/source-packed: drop pointer to "files" parent source
midx: refactor interfaces to work on "packed" source
odb/source-packed: stub out remaining functions
odb/source-packed: wire up `freshen_object()` callback
odb/source-packed: wire up `find_abbrev_len()` callback
odb/source-packed: wire up `count_objects()` callback
odb/source-packed: wire up `for_each_object()` callback
odb/source-packed: wire up `read_object_stream()` callback
odb/source-packed: wire up `read_object_info()` callback
packfile: use higher-level interface to implement `has_object_pack()`
odb/source-packed: wire up `reprepare()` callback
odb/source-packed: wire up `close()` callback
odb/source-packed: start converting to a proper `struct odb_source`
odb/source-packed: store pointer to "files" instead of generic source
packfile: move packed source into "odb/" subsystem
packfile: split out packfile list logic
packfile: rename `struct packfile_store` to `odb_source_packed`
* js/objects-larger-than-4gb-on-windows-more:
odb: use size_t for object_info.sizep and the size APIs
packfile,delta: drop the `cast_size_t_to_ulong()` wrappers
pack-objects: use size_t for in-core object sizes
packfile: widen unpack_entry()'s size out-parameter to size_t
pack-objects(check_pack_inflate()): use size_t instead of unsigned long
patch-delta: use size_t for sizes
compat/msvc: use _chsize_s for ftruncate
Since the inception of `--path-walk`, this option has had a documented
incompatibility with `--delta-islands`.
When discussing those original patches on the list, a message from
Stolee in [1] noted the following:
this could be remedied by [...] doing a separate walk to identify
islands using the normal method
In a related portion of the thread, Peff explains[2]:
The delta islands code already does its own tree walk to propagate
the bits down (it does rely on the base walk's show_commit() to
propagate through the commits).
Once each object has its island bitmaps, I think however you
choose to come up with delta candidates [...] you should be able
to use it. It's fundamentally just answering the question of "am
I allowed to delta between these two objects".
That is similar to what this patch does, and it turns out the cheaper
option is sufficient: perform the same island side effects from the
path-walk callback rather than doing a second walk.
Recall how delta-islands are computed during a normal repack:
- `show_commit()` calls `propagate_island_marks()` for each commit,
which merges the commit's island bitset onto its root tree object and
onto each of its parent commits.
- `show_object()` for a tree records the tree's depth derived from the
slash-separated pathname. Subsequent `resolve_tree_islands()` uses
that depth to walk trees in increasing-depth order, propagating each
tree's marks to its children.
- At delta-search time, `in_same_island()` enforces that a delta
target's island bitmap is a subset of its base's: every island that
reaches the target must also reach the base.
Path-walk's enumeration callback is `add_objects_by_path()`. It already
adds objects to `to_pack`, but until now did not perform the
island-related side effects. Two things are needed:
- For each commit batch, call `propagate_island_marks()` on commits,
exactly as `show_commit()` does.
We have to be careful about the order in which we call this function,
and we must see a commit before its parents in order to have
island marks to propagate.
The path-walk batch preserves that order. Path-walk appends commits
to its `OBJ_COMMIT` batch as they come back from the same
`get_revision()` loop the regular traversal uses, and
`add_objects_by_path()` iterates the batch in array order. So every
commit reaches `propagate_island_marks()` in the same sequence that
`show_commit()` would have seen it, and the descendant-first chain
that the algorithm relies on is intact.
Skip island propagation for excluded commits to match the regular
traversal, whose `show_commit()` callback is only invoked for
interesting commits. Boundary commits may still be present in
path-walk's callback so they can serve as thin-pack bases, but they
should not contribute island marks.
- For each tree batch, record the tree's depth from the path. Use the
`record_tree_depth()` helper from the previous commit so both
callbacks behave identically, including the max-depth-wins behavior
when a tree is reached via more than one path. The helper accepts
both the `show_object()` path shape ("foo", "foo/bar") and the
path-walk shape with a trailing slash ("foo/", "foo/bar/"), so depths
recorded from either traversal mode are directly comparable.
This is implicit in the implementation sketch from Peff above.
`resolve_tree_islands()` sorts trees by `oe->tree_depth` in
increasing-depth order before propagating marks down, so that a
parent tree's marks are finalized before its children inherit them.
Without recording the depth at path-walk time, every
path-walk-discovered tree would land at depth 0 in `to_pack`, the
sort would lose its ordering, and children could inherit marks from
parents whose own contributions had not yet been merged in.
With those two pieces in place, `resolve_tree_islands()` receives the
same island inputs from path-walk as it would from the regular
traversal, so the existing island checks can be reused unchanged.
Drop the documented incompatibility between `--path-walk` and
`--delta-islands`, and add t5320 coverage for path-walk island repacks
with and without bitmap writing, as well as the same-island case where a
delta remains allowed.
[1]: https://lore.kernel.org/git/9aa2471b-0850-4707-9733-d3b33609f5f2@gmail.com/
[2]: https://lore.kernel.org/git/20240911063203.GA1538586@coredump.intra.peff.net/
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prepare for a subsequent change that needs to record tree depths from a
second call site by factoring the delta-islands tree-depth bookkeeping
out of `show_object()` and into a helper, `record_tree_depth()`.
The helper looks up the object in `to_pack`, returns early when the
object was not added there, computes the depth from the slash count in
the supplied name, and preserves the existing max-depth-wins behavior
when a tree is reached by more than one path.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When 'pack-objects' is invoked with '--path-walk', it prevents us from
using reachability bitmaps.
This behavior dates back to 70664d2865 (pack-objects: add --path-walk
option, 2025-05-16), which included a comment in the relevant portion of
the command-line arguments handling that read as follows:
/*
* We must disable the bitmaps because we are removing
* the --objects / --objects-edge[-aggressive] options.
*/
In fb2c309b7d3 (pack-objects: pass --objects with --path-walk,
2026-05-02), path-walk learned to pass '--objects' again, but still
kept bitmap traversal disabled. That leaves two useful cases
unsupported:
* A path-walk repack that writes bitmaps does not give the bitmap
selector any commits, because path-walk reveals commits through
`add_objects_by_path()` rather than through `show_commit()`, where
`index_commit_for_bitmap()` is normally called.
* An invocation like "git pack-objects --use-bitmap-index --path-walk"
never tries an existing bitmap, even when one is available and could
answer the request.
Fortunately for us, neither restriction is required.
* On the writing side: teach the path-walk object callback to call
`index_commit_for_bitmap()` for commits that it adds to the pack.
That gives the bitmap selector the commit candidates it would have
seen from the regular traversal.
* For bitmap reading, keep passing '--objects' to the internal rev_list
machinery, but stop clearing `use_bitmap_index`. If an existing
bitmap can answer the request, use it; otherwise fall back to
path-walk's own enumeration.
As a result, we can see significantly reduced pack generation times from
p5311 (with our `GIT_PERF_REPO` set to a recent clone of the fluentui
repository) before this commit:
Test HEAD^ HEAD
----------------------------------------------------------------------------------------
5311.40: server (1 days, --path-walk) 1.43(1.39+0.04) 0.01(0.01+0.00) -99.3%
5311.41: size (1 days, --path-walk) 139.6K 139.7K +0.0%
5311.42: client (1 days, --path-walk) 0.02(0.02+0.00) 0.02(0.02+0.00) +0.0%
5311.44: server (2 days, --path-walk) 1.43(1.39+0.04) 0.01(0.00+0.00) -99.3%
5311.45: size (2 days, --path-walk) 139.6K 139.7K +0.0%
5311.46: client (2 days, --path-walk) 0.02(0.02+0.00) 0.02(0.02+0.00) +0.0%
5311.48: server (4 days, --path-walk) 1.44(1.39+0.04) 0.01(0.01+0.00) -99.3%
5311.49: size (4 days, --path-walk) 238.1K 238.1K +0.0%
5311.50: client (4 days, --path-walk) 0.03(0.03+0.00) 0.03(0.03+0.00) +0.0%
5311.52: server (8 days, --path-walk) 1.43(1.39+0.03) 0.01(0.00+0.00) -99.3%
5311.53: size (8 days, --path-walk) 344.9K 344.9K +0.0%
5311.54: client (8 days, --path-walk) 0.07(0.07+0.00) 0.07(0.08+0.00) +0.0%
5311.56: server (16 days, --path-walk) 1.47(1.44+0.03) 0.10(0.08+0.01) -93.2%
5311.57: size (16 days, --path-walk) 844.0K 844.0K +0.0%
5311.58: client (16 days, --path-walk) 0.09(0.09+0.00) 0.09(0.09+0.00) +0.0%
5311.60: server (32 days, --path-walk) 1.52(1.50+0.05) 0.14(0.15+0.02) -90.8%
5311.61: size (32 days, --path-walk) 4.2M 4.2M +0.1%
5311.62: client (32 days, --path-walk) 0.34(0.48+0.02) 0.34(0.45+0.05) +0.0%
5311.64: server (64 days, --path-walk) 1.55(1.52+0.06) 0.15(0.15+0.04) -90.3%
5311.65: size (64 days, --path-walk) 6.4M 6.4M -0.0%
5311.66: client (64 days, --path-walk) 0.51(0.79+0.05) 0.51(0.80+0.06) +0.0%
5311.68: server (128 days, --path-walk) 1.59(1.57+0.06) 0.16(0.21+0.01) -89.9%
5311.69: size (128 days, --path-walk) 8.4M 8.4M -0.0%
5311.70: client (128 days, --path-walk) 0.72(1.44+0.08) 0.71(1.47+0.09) -1.4%
We get the same size of output pack, but this commit allows us to do so
in a significantly shorter amount of time. Intuitively, we're generating
the same pack (hence the unchanged 'test_size' output from run to run),
but varying how we get there. Before this commit, pack-objects prefers
'--path-walk' to '--use-bitmap-index', so we generate the output pack by
performing a normal '--path-walk' traversal. With this commit, we are
operating over a *repacked* state (that itself was done with a
'--path-walk' traversal), but are able to perform pack-reuse on that
repacked state via bitmaps.
When comparing the size of the repacked pack with/without '--path-walk'
on the previous commit versus this one, we see that (a) the repacked size
improves significantly with '--path-walk', and that (b) writing bitmaps
during repacking does not regress this improvement:
Test HEAD^ HEAD
----------------------------------------------------------------------------------------
5311.3: size of bitmapped pack 558.4M 558.5M +0.0%
5311.38: size of bitmapped pack (--path-walk) 164.4M 164.4M +0.0%
(Note that to observe an improvement here, we must repack with '-F' in
order to avoid reusing non-'--path-walk' deltas, which would otherwise
skew our results.)
There is one wrinkle when it comes to '--boundary', which we must not
pass into the bitmap walk in the presence of both '--path-walk' and
'--use-bitmap-index'. Path-walk needs boundary commits when it performs
its own traversal, in order to discover bases for thin packs, but the
bitmap traversal does not expect this. Work around this by setting
`revs->boundary` as late as possible within the '--path-walk' traversal,
after any bitmap attempt has either succeeded or declined to answer the
request.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our interfaces used to interact with MIDXs all work on top of the
generic `struct odb_source`. This doesn't make much sense though: a MIDX
is strictly tied to the "packed" source, so passing in a generic source
gives the false sense that it may also work with a different type of
source.
Fix this conceptual weirdness and instead require the caller to pass in
a "packed" source explicitly. This also makes the next commit easier to
implement, where we drop the pointer to the "files" source in the
"packed" source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move `packfile_store_for_each_object()` and its associated helpers from
"packfile.c" into "odb/source-packed.c" and wire it up as the
`for_each_object()` callback of the "packed" source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano