`rerere` can mark a conflict variant as resolved even when writing its
preimage or postimage fails. A later invocation may then replay
incomplete data from the cache, turning a local filesystem failure into
an incorrect working-tree change.
629716d256 (rerere: do use multiple variants, 2015-07-30) introduced
the code paths without checks for those I/O results. Treat such failures
as failures, report them, and leave the rerere status unchanged unless
the corresponding data was recorded successfully.
The defect has been latent since 2015. Git for Windows' Coverity run
only reported it after merging v2.56.0-rc0, for reasons that could not
be figured out in a reasonable amount of time.
Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Incremental MIDX support made object-offset pack IDs local to each layer
and then converted them to chain-global IDs by adding
`num_packs_in_base`. The conversion was introduced by 19419821ba
(midx: teach `nth_midxed_pack_int_id()` about incremental MIDXs,
2024-08-06). Chain-aware pack preparation followed in 1820bd878c
(midx: teach `prepare_midx_pack()` about incremental MIDXs, 2024-08-06),
but the final `midx_fill_entry()` lookup remained tied to the original
layer. Only with 8f909ff4e9 (packfile: recover when a multi-pack-index
names a removed pack, 2026-08-29) did Coverity point out this issue: a
local ID such as `UINT32_MAX` could wrap when the base-pack count was
added, producing a plausible but incorrect global ID. After
`prepare_midx_pack()` resolved the chain, `midx_fill_entry()` could then
underflow or address the wrong layer while indexing the current layer's
pack array, causing an invalid memory access and crashing Git.
Validate each local pack ID against its layer's pack count before adding
the base count, and obtain the final pack through `nth_midxed_pack()`,
which resolves the correct MIDX layer. This prevents an invalid local ID
from wrapping during conversion and ensures that the lookup uses the
layer identified by the resolved chain-global ID.
Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After merging v2.56.0-rc0 into Git for Windows, its Coverity run
reported the following issue: The `parse_signed_buffer()` function
accepts object buffers with an explicit size, while
`get_format_by_sig()` uses `starts_with()`, i.e. it expects a
NUL-terminated buffer. A tag object with a non-NUL-terminated payload
ending in a partial signature prefix, such as a final '-' byte, could
therefore cause an invalid read past the object buffer.
The observable consequences are limited to reading past the allocation.
In practice it can crash Git if the read enters an unmapped page. It can
also misplace the payload/signature split, corrupting the compat-hash
object being written.
The older unbounded matcher predates this path, but c8762c30df
(object-file-convert: convert tag objects when writing, 2023-10-01)
exposed the defect by passing exact-sized converted tag buffers to
`parse_signed_buffer()`. That commit first shipped in v2.45.0, so the
defect has been latent in every release since.
This pattern was noticed on the mailing list in February 2024. Reviewing
a patch for a very similar issue in commit.c's find_header_mem(), Jeff
King observed in
https://lore.kernel.org/git/20240208214137.GB1090198@coredump.intra.peff.net/:
But more interestingly: even though we pass a buf/len pair to
parse_signed_buffer(), it then calls get_format_by_sig() which takes
only a NUL-terminated string. [...] That raises the question of
whether parse_signed_buffer() has a similar walk-too-far problem. ;)
The answer is no, because we feed it from a strbuf. But it's not a
great pattern overall.
That reasoning surveyed the callers that existed at the time and missed
c8762c30df (object-file-convert: convert tag objects when writing,
2023-10-01), which was four months old at that time, and does not feed
from a strbuf; `convert_tag_object()` hands `parse_signed_buffer()` an
exact-sized `xmalloc()` buffer, and the concern flagged and dismissed in
that thread is exactly the defect Coverity now reports.
Jeff went on to add `starts_with_mem()` a month later, in
https://lore.kernel.org/git/20240307092638.GK2080210@coredump.intra.peff.net/,
precisely for "cases where the buffer is not NUL-terminated (and we
instead have an explicit size or end pointer)", so the tool for this fix
has been in the tree since v2.45.0.
Even though the issue had been latent, it most likely surfaced via
Coverity because of 215d305f45 (odb: compute compat object ID in
`odb_write_object_ext()`, 2026-07-17), which moved
`convert_object_file()` out of the `source->write_object` function
pointer into a direct call in `odb_write_object_ext()`.
Preserve the existing NUL-terminated behavior for callers that provide
strings while making signature-prefix matching honor the known buffer
lengths, via the `starts_with_mem()` helper. This keeps reads within the
object data without implying exploitability beyond the observed invalid
read.
Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As Git for Windows' Coverity run after merging v2.56.0-rc0 reported,
`writev_in_full()` keeps its cumulative successful output in an
`ssize_t`. Although `xwritev()` limits each individual write to a
syscall-sized amount, repeated successful writes can still exceed
`SSIZE_MAX`. The unchecked accumulation was introduced by d70eb7f360
(wrapper: introduce writev(3p) wrappers, 2026-08-07).
Treat an aggregate that would overflow the signed total as an I/O
failure.
Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The memory leak caused by not unusing the commit buffer returned by
repo_logmsg_reencode() during the rewording operation in 'git
history' has been plugged.
* ks/history-commit-leakfix:
builtin/history: unuse the commit buffer after use
When cross-compiling with Cargo, the output artifact is placed in a
target-specific subdirectory, which causes the build system to fail
to locate it. The build system has been updated to respect the
'CARGO_BUILD_TARGET' environment variable.
* jc/rust-cargo-build-target:
rust: respect CARGO_BUILD_TARGET when locating build output
Update Windows CI build to support Rust.
* js/rust-in-windows-ci:
ci(windows): build with Rust
rust: pick a GCC-compatible Cargo target under MSYS2/MinGW
The consistency checks for the object database (fsck) have been
decoupled from the generic builtin implementation and moved into the
backend-specific object source layers, making them pluggable for
different object storage formats.
* ps/odb-pluggable-fsck:
builtin/fsck: move loose object verification into the loose source
builtin/fsck: move multi-pack index verification into the packed source
builtin/fsck: move bitmap verification into the packed source
builtin/fsck: move reverse index verification into the packed source
builtin/fsck: move packfile verification into the packed source
odb: provide infrastructure for pluggable fsck checks
builtin/fsck: don't check alternates with "--no-full"
builtin/fsck: de-globalize option handling
builtin/fsck: merge `fsck_obj_buffer()` and `fsck_obj()`
builtin/fsck: use `fsck_obj_buffer()` when checking loose objects
The mechanism to register in-memory alternate object sources has
been removed, as submodule object databases are now accessed
natively via their own repository structures. This simplifies
object database management and prepares the codebase for migrating
alternate tracking into the files backend.
* ps/odb-stop-registering-in-memory-sources:
odb: remove the ability to link sources ad-hoc
t/helper: stop registering alternates in "ref-store" command
t/helper: adapt read-midx to not link ad-hoc source anymore
builtin/multi-pack-index: refuse unknown sources with "--object-dir="
odb/packed: fix memory leaks when freeing source
tmp-objdir: drop unused function to register alternate
odb: remove infrastructure to register submodule sources
builtin/grep: stop registering submodule ODB as source
submodule-config: stop registering submodule sources
submodule-config: stop using `the_hash_algo`
submodule-config: remove uses of `the_repository`
cache-tree: remove dependency on `the_repository`
cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
Windows build switches from MINGW64 to URCR64 runtime starting Git
2.56.0; switch the cmake based build at the same time.
* js/win-cmake-use-ucrt64:
cmake(windows): accommodate for Git for Windows' migration to UCRT64
Correct tests that depend on Perl running on MinGW
* js/mingw-test-fixes-around-perl:
t9129: skip UTF-8 tests on Windows
t9700: accommodate for MSYS2 Perl reporting as `cygwin`
Every call to repo_logmsg_reencode() must be paired with a call to
repo_unuse_commit_buffer(), or we may leak an allocated buffer. We
have such a leak in "git history", which we can fix by adding an unuse
call.
The leak-checking tests don't detect this because we only allocate a
fresh buffer sometimes: when the message is reencoded, or when we had
to load it fresh from the odb (e.g., because the commit was parsed
from the commit graph rather than the object contents). But you can
see it by running:
make SANITIZE=leak
cd t
GIT_TEST_COMMIT_GRAPH=1 ./t3451-history-reword.sh
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The git rev-list command has been augmented with a '--missing-only'
option that filters the output to only show missing objects,
stripping the leading '?' character and suppressing present objects,
which is useful when used in combination with '--missing=print' or
'--missing=print-info'.
* sa/rev-list-missing-only:
rev-list: add --missing-only option to filter output
"git maintenance" triggered "rerere gc" in unappropriate times and
interfered with "git rebase" etc. too much. The conditions "rerere
gc" gets triggered have been tweaked.
* ps/tune-rerere-gc:
builtin/maintenance: improve heuristic for "rerere gc"
rerere: extract logic to determine whether entries are stale
The autostash fallback in 'git checkout -m' has been refined to only
retry when there are local changes. Additionally, a blank line now
visually separates autostash conflict advice from the subsequent
branch-switch message.
* hn/checkout-m-autostash-refine:
checkout: separate autostash conflict advice from branch-switch message
stash: reserve exit status 1 for conflicts
The 'git imap-send' command has been taught to take the '--draft'
option to mark uploaded messages as drafts, which helps some email
clients render them properly for editing and sending.
* wf/imap-send-draft:
imap-send: add --draft to set IMAP \Draft flag
CGI helper scripts used by HTTP-related test scripts have been updated
to use atomic filesystem operations, preventing race conditions when
Apache handles concurrent requests.
* mm/lib-httpd-cgi-safe:
t/lib-httpd: document writing concurrency-safe CGI helpers
t/lib-httpd: make http-429 first-request check atomic
t/lib-httpd: fix apply-one-time-script race under concurrent requests
The development helper script to lint gitlink references in the
documentation has been updated to avoid a newer Perl regular
expression syntax that breaks on older Perl versions.
* ta/lint-gitlink-older-perl-fix:
lint-gitlink: don't use empty lower bound in .{0,8}
Running "git history" in a corrupt repository can (unsurprisingly)
segfault when a necessary tree object is not found.
* jc/history-missing-tree-errorfix:
history: do not dereference NULL when parent tree is missing
The documentation for 'git cherry-pick' has been updated to clarify
that the '--no-commit' option intentionally skips setting the
'CHERRY_PICK_HEAD' ref. A test has also been added to ensure this
behavior holds even when the operation stops for conflicts.
* as/cherry-pick-no-commit-doc:
doc: cherry-pick: note --no-commit skips CHERRY_PICK_HEAD
t3507: check no CHERRY_PICK_HEAD after conflicting --no-commit
The gitdatamodel documentation page has been linked from a handful
of key documentaiton pages.
* kh/doc-datamodel:
doc: datamodel: link to the glossary
doc: glossary: link four of the terms to gitdatamodel(7)
doc: git: link to the gitdatamodel(7) tutorial
doc: git: list gitdatamodel(7) as a concept guide
The process of downloading packfile URIs in protocol v2 has been
instrumented with a Trace2 region. This visibility allows tracking
the cumulative time spent downloading external packs and the number
of advertised URIs without emitting a separate event per pack.
* tn/fetch-pack-trace-packfile-uri:
fetch-pack: trace packfile URI downloads
Two members in "struct pathspec_item" were of type "char *", but
nobody updated the string through these pointers. They have been
made "const char *" instead.
* jc/pathspec-match-const:
pathspec: match and original in pathspec_item are const
The error path in 'git submodule--helper' has been updated to plug a
memory leak when a repository handle could not be obtained,
leveraging an updated idempotent repo_clear().
* jk/submodule-error-leak:
submodule--helper: free URL when repository setup fails
repository: make repo_clear() idempotent
Teach 'am', 'revert', and 'rebase' that running 'commit --amend' or a
partial 'commit <paths>' makes no sense during operations that stop
and return control to the user to resolve conflicts left in the
working tree, just like 'cherry-pick' and 'merge' do.
* en/no-amend-during-conflicts:
commit: refuse partial commits during conflict resolution
commit: refuse to amend during conflict resolution
commit: reword the empty-commit rebase amend error
commit: allow a partial commit when a rebase pick becomes empty
commit: clarify FROM_REBASE_PICK and is_from_rebase() names
The CI script to install dependencies for the documentation build
has been updated to install asciidoctor directly via the system
package manager instead of pinning to an older version via gem.
Additionally, an obsolete variable used for retired Azure Pipelines
environments has been removed.
* jk/ci-use-system-asciidoctor:
ci: use system asciidoctor
Git for Windows needs to ship with a lot of Unix tools that Git takes
for granted, such as `sed`, `awk`, a C compiler and a Unix shell, just
to name a few. In Git for Windows, these are provided by the MSYS2
project.
Part of these tools (such as `bash.exe`) use a POSIX emulation layer
("MSYS2 runtime", a friendly fork of the Cygwin runtime), but others
target a native Win32 toolchain, e.g. `git.exe`. There are multiple
flavors of that toolchain, and historically Git for Windows used MINGW64
on x64 Windows. This toolchain uses the old MSVC runtime, and therefore
the MSYS2 project deprecated it.
As a consequence, Git for Windows switches to UCRT64 with v2.56.0. That
flavor still uses GCC to compile native Win32 binaries, but targets the
Universal C Runtime ("UCRT"). Internally, this means that the new
`git.exe` is installed into a new prefix, `/ucrt64/`, whereas the old
`git.exe` was installed into `/mingw64/`.
A recently-upstreamed commit hard-codes this expectation even into the
CMake-based build, so that the built `git.exe` "knows where it lives"
and can ensure that the tools it expects on the `PATH` are found.
Naturally, this hard-coded MINGW64 needs to change to UCRT64 now, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The assumption of this test is that Perl and Git have the same idea how
to perform encoding conversions.
However, in Git for Windows, Git is a native Win32 program, and such
programs have a very different concept of encodings (called "Code
Pages", and they are not controlled via environment variables at all),
whereas the Perl interpreter used in Git for Windows is a pseudo-Unix
one that uses the MSYS2 runtime (which _does_ try very much to abide by
Unix' `LC_ALL` and friends, and totally ignores Windows' current or
active code pages).
As such, these test cases _cannot_ work with Git for Windows. So let's
just skip them on that platform.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As of a year or two ago, there is this push to align MSYS2 more closely
with Cygwin, so as to benefit from a closer collaboration. Part of that
is that the triplet `x86_64-pc-cygwin` is used nowadays, whereas it had
been `x86_64-pc-msys` previously. Likewise, Perl now reports `$^O` as
`cygwin` instead of `msys`.
The Perl module test used `msys` as tell-tale when to accommodate for a
native Windows version of `git.exe` which would report absolute
_Windows_ paths rather than those pseudo-Unix paths. We cannot use that
tell-tale anymore, and we also cannot adjust it to `cygwin` because that
would break in Cygwin (where `git.exe` reports absolute pseudo-Unix
paths).
Let's use the environment variable `MSYSTEM` instead (being mindful that
the `MSYSTEM=MSYS` variant would _also_ reflect a setup where `git.exe`
won't report absolute _Windows_ paths).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The Windows runners used by Git's GitHub workflow's `windows-build` job
ship `rustup` plus a `*-pc-windows-msvc` default toolchain (see
https://github.com/actions/runner-images/blob/main/images/windows/Windows2022-Readme.md
and
https://github.com/actions/partner-runner-images/blob/main/images/arm-windows-11-image.md),
but no precompiled `std` for `*-pc-windows-gnu` or
`*-pc-windows-gnullvm`. With the Makefile now picking a GCC-compatible
target triple based on `$(MSYSTEM)`, the build step needs that
precompiled `std` to be installed before invoking `make`, otherwise
`cargo build --target <triple>` fails to find a usable `std` for the
chosen target.
Add a step between the SDK setup and the `make` invocation that selects
the matching triple from `$MSYSTEM` (which
`git-for-windows/setup-git-for-windows-sdk` exports for every subsequent
step) and runs `rustup target add` for it. The mapping mirrors what
`config.mak.uname` derives from `$(MSYSTEM)` and `$(HOST_CPU)`, just
enumerated explicitly here since CI has direct knowledge of which MSYS2
subsystems the matrix actually exercises (`CLANGARM64` for the ARM64
runner, `MINGW64` for the x86_64 runner). Technically, we only need to
handle MINGW64 at present, but the switch to UCRT64 is imminent, and the
other case arms serve as a very fine documentation of what people should
do for other MSYSTEM values.
For a `staticlib` crate-type `cargo build` does not invoke an external
linker, so no further toolchain components (e.g. the `gnullvm` LLVM
linker) need to be installed; `rustup target add` alone is sufficient.
Assisted-by: Claude Opus 4.7
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When Git is built under MSYS2/MinGW with Rust support enabled, the
Makefile expects `cargo build` to drop a `target/release/libgitcore.a`
that is linkable by the same MinGW GCC used for every other object. With
Rust installed via `rustup` (the way it ships on the GitHub-hosted
`windows-2022` and `windows-11-arm` runners that build git/git and its
forks), the default toolchain targets the MSVC ABI; cargo then writes
`target/release/gitcore.lib` instead, which the MinGW `ld.exe` cannot
consume:
LINK git-shell.exe
D:\git-sdk-64-minimal\mingw64\bin/ld.exe: cannot find target/release/libgitcore.a: No such file or directory
collect2.exe: error: ld returned 1 exit status
See https://github.com/microsoft/git/actions/runs/27341625000 for a
full example log.
Let's define the correct target, using the `CARGO_BUILD_TARGET` variable
that will be picked up by Rust, see
https://dirname.github.io/rust-std-doc/cargo/reference/environment-variables.html#:~:text=CARGO%5FBUILD%5FTARGET
Re-use (and fix) the existing `HOST_CPU` variable to determine the
correct value. Avoid relying on environment variables that are simply
not defined in Git for Windows' minimal SDK that Git uses in its CI
runs.
Assisted-by: Claude Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our CI Documentation builds have pinned asciidoctor since 615a6c37e1
(ci: stick with Asciidoctor v1.5.8 for now, 2019-03-29). Back then a few
changes were needed to build with the then-new v2.0.0.
We've since made those changes, in f6461b82b9 (Documentation: fix build
with Asciidoctor 2, 2019-09-15) and other commits. So it is not only
safe to use newer versions of asciidoctor, but preferable: it's what
people are likely to use for actually building the documentation in
practice!
I don't think there's any need to pin to a specific version. We can just
use what ships in our Ubuntu image, which should give us an arbitrary
representative version (and if it changes and something breaks, we'd
want to know).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A collection of patches from Git for Windows has been upstreamed,
mostly focusing on simplifying and robustifying build configurations
for MinGW/MSYS2, dropping obsolete compatibility options, and allowing
the main 'git.exe' to be used directly without the extra wrapper
process on Windows.
* js/mingw-build-updates:
t0060: adjust the code style
mingw: allow `git.exe` to be used instead of the "Git wrapper"
mingw: ensure valid CTYPE
mingw: always define `ETC_*` for MSYS2 environments
windows: skip linking `git-<command>` for built-ins
mingw: rely on MSYS2's metadata instead of hard-coding it
mingw: only enable the MSYS2-specific stuff when compiling in MSYS2
mingw: set the prefix and HOST_CPU as per MSYS2's settings
mingw: avoid over-specifying `--pic-executable`
mingw: only use -Wl,--large-address-aware for 32-bit builds
mingw: drop the -D_USE_32BIT_TIME_T option
mingw: stop hard-coding `CC = gcc`
mingw: include the Python parts in the build
The CI script to install dependencies for the documentation build
has been updated to install asciidoctor directly via the system
package manager instead of pinning to an older version via gem.
Additionally, an obsolete variable used for retired Azure Pipelines
environments has been removed.
* jk/ci-use-system-asciidoctor:
ci: drop ALREADY_HAVE_ASCIIDOCTOR variable
ci: fix missing Ruby dependency in "documentation" job
The CI job for Debian 11 has been updated to use Debian 12, as the
former is now out of the LTS period.
* jk/ci-bump-debian-to-12:
ci: bump debian-11 job to debian-12
The consistency checks for loose objects are hosted by "builtin/fsck.c".
These checks are obviously specific to the "loose" backend.
Move the logic into `odb_source_loose_fsck()`. Introduce a new "verbose"
flag so that we can properly retain semantics around whether or not we
want to print some status messages.
Note that this fixes a bug as a side effect: the progress meter was
captured in the callback data before `start_progress()` was even called,
so the per-subdirectory progress updates always operated on a NULL
pointer and the meter jumped straight from 0 to 256 upon completion. The
new code only sets up the callback data's progress meter after it has
been created, so the progress display now advances incrementally again.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for multi-pack indexes are hosted in `cmd_fsck()` directly.
These checks are obviously specific to the "packed" backend.
Move the logic into `odb_source_packed_fsck()`. As in preceding commits,
this means that we now properly honor both "--connectivity-only" and
"--no-full". Furthermore, we drop the dedicated `ERROR_MULTI_PACK_INDEX`
bit and instead use the generic `ERROR_OBJECT` bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for bitmaps live in `verify_bitmap_files()`, which is called
by "builtin/fsck.c". These checks are obviously specific to the "packed"
backend.
Move the logic into `odb_source_packed_fsck()`. As in preceding commits,
this means that we now properly honor both "--connectivity-only" and
"--no-full". Furthermore, we drop the dedicated `ERROR_BITMAP` bit and
instead use the generic `ERROR_OBJECT` bit.
Note that this change also adapts `verify_bitmap_files()` to be
focused on a single "packed" source instead of verifying bitmaps from
all sources. This change is required as we already know to loop around
the sources in `odb_fsck()` itself.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for reverse indexes live in `check_pack_rev_indexes()`, which
is hosted in "builtin/fsck.c". These checks are obviously specific to
the "packed" backend.
Move the logic into `odb_source_packed_fsck()`. As in the preceding
commit, drop the dedicated `ERROR_PACK_REV_INDEX` bit and instead use
the generic `ERROR_OBJECT` bit.
Note that this changes behaviour in two ways:
- The checks are now skipped when "--connectivity-only" was passed.
This is because we don't even run `odb_fsck()` at all when that
flag has been passed by the user, and not verifying data structures
of the object database matches the documented intent of that flag,
which is to only check the connectivity of reachable objects.
- The checks are now skipped for non-local sources when "--no-full"
was passed. This is, again, in line with the documented intent of
that flag.
Add a test to cast these semantics into stone.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the packfile verification out of `cmd_fsck()` and into the "packed"
source. While doing so, thread the progress meter and object callback
through the newly introduced `struct odb_fsck_options` so that the
caller's preferences are honoured without exposing those details at the
"builtin/fsck.c" level.
Note that the old code reported failures when verifying packfiles with
the `ERROR_PACK` bit, which gets returned to the caller via the exit
code. This bit is neither exercised in our test suite nor is it
documented anywhere in our codebase. Furthermore, this bit is highly
specific to the object storage backend, which makes it a bad fit for the
new pluggable infrastructure. So instead of retaining these semantics,
we drop them and return the generic `ERROR_OBJECT` bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The on-disk consistency checks in git-fsck(1) are conceptually
backend-specific: while connectivity checks and object-level parsing
checks are generic, verifying the physical integrity of packfiles and
loose objects is meaningful only to backends that use these formats:
Having these checks live in "builtin/fsck.c" violates that layering,
because it forces the command to reach directly into format-specific
internals.
Provide new infrastructure to make these format-specific checks
pluggable and implement stubs for the different source types we already
have. In subsequent commits we'll move functionality over piece by
piece.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
According to git-fsck(1), the "--full" option behaves in the following
way:
Check not just objects in GIT_OBJECT_DIRECTORY ($GIT_DIR/objects), but
also the ones found in alternate object pools listed in
GIT_ALTERNATE_OBJECT_DIRECTORIES or $GIT_DIR/objects/info/alternates,
and in packed Git archives found in $GIT_DIR/objects/pack and
corresponding pack subdirectories in alternate object pools.
So ultimately, it is supposed to control two things: (1) whether we only
check the main object directory, and (2) whether we check packfiles.
In its current state though, the flag only controls whether we check
packfiles or not, and if so we verify packfiles of all attached sources.
But we also have checks for loose objects in git-fsck(1), and here we
unconditionally check them in all sources.
The flag is arguably conflating two unrelated concerns with one another,
and it really should be split up into two flags: one that controls how
thorough we want to check individual sources, and one that controls
which sources we want to check in the first place. So ideally, we would
have:
- "--include-alternates": check all sources, not only the local one.
- "--include-optimized-objects": check not only loose objects, but
also those that have been packed. Note that we explicitly don't say
"--include-packed-objects" here to be more backend-agnostic.
- "--full": implies both of the above flags.
This feels out of scope for this series though. So for now, simply fix
the code by honoring locality of the sources for loose objects.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In subsequent commits we're about to rework some of the option handling
in git-fsck(1) a bit. It is currently a bit of a mess though due to lots
of global state that makes it hard to see which flags are used where
exactly.
Refactor the code by moving the fsck options into `cmd_fsck()`. This
allows us to convert some of the options into function-local variables.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The interfaces of the functions `fsck_obj()` and `fsck_obj_buffer()` are
somewhat similar to one another. The only difference between those two
is that `fsck_obj()` takes an already-parsed object as input, whereas
`fsck_obj_buffer()` parses the buffer and then calls `fsck_obj()`.
Furthermore, `fsck_obj()` has no callers other than `fsck_obj_buffer()`.
Refactor the code by merging those two functions. This makes it obvious
which function does what, and it allows us to get rid of the early
return in `fsck_obj()` in case `SEEN` is set as the only caller
unconditionally clears that bit before calling it anyway.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin