The 'git var' command has been updated to output individual components
of the author and committer identities, take multiple keys on the
command-line, and support NUL-delimited output with the '-z' option.
* ap/var-broken-down-idents:
var: add broken-out identity variables
var: accept more than one variable
var: add "-z" output mode
var: represent multi-valued variables with a string_list
The handling of index conflicts in 'git stash apply --index' has
been optimized. Instead of relying on expensive external
subprocesses using 'git diff-tree' and 'git apply', the index is now
merged in-core using the merge-ort machinery, which fixes a
concurrent lock file issue with autostash.
* dk/stash-apply-index-incore:
builtin/stash: merge index in-core
t3903: test failed "stash apply --index"
stash: prepare merge options earlier
builtin/stash: remove unused header
The internal diff and merge machinery has been updated to use
standard size_t types rather than signed longs for buffer
allocations and offsets, preventing subtle overflow issues on LLP64
architectures, and plugging uninitialized memory leaks along error
paths.
* jk/xdiff-size_t:
merge-ll: report an error when reading external merge results fails
merge-ll: handle external driver status before reading result
merge-ll: use read_mmfile() to read external merge results
xdiff: NUL-terminate buffers read by read_mmfile()
xdiff: use size_t for buffer sizes
xdiff: replace mmbuffer_t with mmfile_t
xdiff: clean up read_mmfile() allocations on error
The sequencer machinery (used by 'git rebase', 'git cherry-pick', and
'git revert') has been updated to defer automatic maintenance tasks
until the end of the operation, preventing nested 'git commit', 'git
merge', and 'exec' commands from triggering GC operations that could
contend for locks or delete open packs while the sequence is in
progress.
* tb/rerere-lock-grace:
sequencer: disable auto maintenance in spawned commands
rebase, cherry-pick, revert: run auto maintenance when done
config: add git_config_append_parameter()
'git pull' has been taught to fail more gracefully instead of
segfaulting when lookup_commit_reference() fails on unparseable
objects.
* jk/pull-null-merge-head:
pull: avoid segfault when commit lookup fails
The build-time knob 'USE_NSEC' for nanosecond stat precision has been
converted to a runtime configuration 'core.useNanosec', allowing
distributions to bundle one binary that adapts to filesystem
capabilities dynamically.
* dk/use-nsec-runtime:
core: convert build-time USE_NSEC into runtime core.useNanosec
environment: align repo_config_values_init with struct declaration
meson: expose knob for xmlto relative links in manuals
The terminology regarding reference storage formats has been unified
across command-line options, environment variables, configuration
variables, and source code, standardizing on the phrase "ref storage
format" (e.g., `--ref-storage-format`, `'GIT_REF_STORAGE_FORMAT'`).
Additionally, the `--ref-storage-format` option has been updated to
accept payloads in the form `<format>://<payload>`.
* ps/ref-storage-format:
setup: allow "--ref-storage-format=" to specify a payload
setup: rename "init.defaultRefFormat" to "init.defaultRefStorageFormat"
t: rename GIT_TEST_DEFAULT_REF_FORMAT
setup: rename ref storage format environment variables
setup: refactor how we configure the ref storage format
refs: expose function to parse reference URIs
help: rename "default-ref-format" to "default-ref-storage-format"
builtin/rev-parse: rename "--show-ref-format" to "--show-ref-storage-format"
builtin/submodule: rename "--ref-format=" to "--ref-storage-format="
builtin/refs: rename "--ref-format=" to "--ref-storage-format="
builtin/clone: rename "--ref-format=" to "--ref-storage-format="
builtin/init: rename "--ref-format=" to "--ref-storage-format="
parse-options: allow for hidden aliases
A new hook 'report' is added to 'git receive-pack', which runs after
reference updates and allows the server to filter or modify the
packet-line status report sent back to the client.
* kn/receive-report-hook:
receive-pack: coccinelle fix
hook: introduce the receive-report hook
receive-pack: move message generation to separate function
receive-pack: drop static variables to track report status version
doc: add proc-receive hook info in 'git-receive-pack.adoc'
The experimental 'git history' command has been taught a new 'squash'
subcommand to fold a range of commits into a single commit, with any
descendants replayed on top.
* hn/history-squash:
history: support editing squashed commit messages
history: create squashed commits without editing
history: protect branches when squashing a range
history: validate squash revision ranges
history: add skeleton for squash subcommand
sequencer: share the squash message marker helpers and flags
history: give commit_tree_ext a message template
history: extract helper for a commit's parent tree
The setup of alternates has been deferred to object database
creation time during clone, which drops the unused ad-hoc alternate
writing API, simplifying the object database backend interface.
* ps/odb-alternates-at-creation:
odb/source: remove the ability to write alternates
builtin/clone: write alternates via `odb_create_on_disk()`
odb/source: support writing alternates when creating the database
builtin/clone: move setup of alternates for non-shared local clones
builtin/clone: move setup of alternates for shared local clones
builtin/clone: refactor handling of "--reference{,-if-able}"
builtin/clone: move around `setup_reference()`
builtin/clone: defer setup of the object database
setup: split up concerns of `init_db()`
"git stash apply --index" does a 2-step dance to report index conflicts
before carrying out the main unstash: first, attempt to merge the index
(and remember the name of the resulting tree). If that succeeds, reset
the index and carry on unstashing the working tree, then use the
remembered index tree to unstash the index.
The "merge the index" step is performed on the actual index by a
combination of git-diff-tree(1) and git-apply(1), which incurs an extra
cost to git-reset(1) to cleanup. This also introduces an autostash bug
when stash.index is true: "git reset" eventually wants to
remove_merge_branch_state(), which calls save_autostash() due to
a03b55530a (merge: teach --autostash option, 2020-04-07). This can
happen from a "git merge --autostash", which itself calls
save_autostash(). Operating on the file-system in this way is not
re-entrant, so we end up trying to lock a now-deleted MERGE_AUTOSTASH
ref [1]. This bug has lurked for a while, but it would have been
impossible to trigger without the availability of stash.index to force
the autostash apply into index mode.
[1]: https://lore.kernel.org/git/CALO-guvbk2TcrVwzdNQ3yRpzHr0HHZ3h1wite0Xp0sUyAT4otA@mail.gmail.com/
Fortunately, we can achieve 2 goals at once: avoid round-tripping to the
file-system (and invoking expensive subprocesses) by performing the
merge in-core. If there are conflicts, we discard the resulting tree, so
we don't see the usual branch and ancestor labels, but the merge
subroutines insist on their presence, so use something simple.
We need to take care to get the order of trees right when merging. Add a
test that covers this case.
We *could* swap just the git-reset(1) subprocess with our internal
reset_tree() and refresh_index(), which would fix the bug. We'd much
prefer to clean up these vestiges of the shell-based git-stash, though.
Reported-by: Eli Barzilay <eli@barzilay.org>
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a future commit, we will reuse these options for the index merge of
"apply --index", not just for the worktree.
Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Clang complains that oid-array.h is unused. Certainly none of the
oid_array* functions, types, etc., are used, and the
transitively-included hash.h declarations are used but covered by a
pre-existing direct #include of hash.h.
Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our import of xdiff has two identical buffer structures: mmfile_t and
mmbuffer_t. In upstream xdiff these were actually different, but the
import in 3443546f6e (Use a *real* built-in diff generator, 2006-03-24)
simplified mmfile_t to a simple buffer.
In xdiff we usually use mmfile_t for input and mmbuffer_t for output,
but they are really both just a ptr/len pair. I don't think that having
different types is buying us anything in terms of type safety or
semantics, and having two makes it awkward to use the same helpers for
both. In particular, an external merge driver's output is read from a
file, but we can't easily use read_mmfile(), since we want the result in
an mmbuffer_t.
Let's use mmfile_t for both cases and drop mmbuffer_t. The latter is
probably a more descriptive name, but we have many more uses of
mmfile_t (and helpers like read_mmfile). So let's consolidate using that
name; we can always change it to something more sensible later.
There should be no behavior change here; this is just consolidating the
types.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When read_mmfile() returns an error, it may or may not have allocated a
buffer in the passed-in mmfile_t. So callers must initialize the pointer
to NULL and free it even on error.
Most callers do this already, but rerere's diff_two() does not, and
would leak the buffer after a read error. We could fix it directly, but
let's instead try to make the interface less error-prone by freeing the
memory when returning failure from read_mmfile().
This fixes (part of) the leak in diff_two(). In theory it also lets us
simplify other callers to skip initializing the mmfile. But in practice
most still need zero-initialization because they may jump to free()
before even calling read_mmfile (e.g., in try_merge()). But we can at
least simplify rerere_forget_one_path() a bit.
I said "part of" earlier. There's a related leak in diff_two(): if
reading the first file succeeds but reading the second fails, we return
early and leak the first buffer. We can fix that by checking each
individually.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A script that wants only the author's name has to ask for
GIT_AUTHOR_IDENT and take the result apart itself, which means
reimplementing the rules split_ident_line() already knows, and getting
them subtly wrong for names that contain an e-mail address or other
awkward characters.
Add GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE, together
with their GIT_COMMITTER_* counterparts, which give the individual
pieces of the corresponding *_IDENT variable. They are derived from
the same ident string, so they are strict in the same way: asking for
one of them fails if the identity cannot be determined.
Signed-off-by: Andrew Pleeter <andrewpleeter@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git var" takes exactly one variable and shows its value, so a script
that needs several of them has to run the command once per variable,
paying process startup and configuration parsing each time.
Teach "git var" to take any number of variables and show them the way
"git var -l" does, i.e. as "VARIABLE=value" (or "VARIABLE\nvalue\0"
under "-z"), but limited to the variables the user asked for. A single
variable still shows its bare value, and still exits with status 1 when
it has no value, so existing users are unaffected.
When more than one variable is asked for, a variable that has no value
is simply left out of the output and the rest are still shown. That a
variable is unset is not an error in itself: a caller asking for both
GIT_CONFIG_SYSTEM and GIT_CONFIG_GLOBAL should not be told that
something went wrong merely because the user set GIT_CONFIG_NOSYSTEM.
Keeping a non-zero status for genuine errors, such as naming a variable
that does not exist, also lets callers detect those by looking at the
exit code alone.
Signed-off-by: Andrew Pleeter <andrewpleeter@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Scripts that read from "git var" have no unambiguous way to parse its
output. A value that itself contains a newline, and a variable that
has more than one value, both run together with the newline that
terminates each entry.
Teach "git var" a "-z" option that terminates each entry with a NUL
byte instead. With "-l", the variable name and its value are separated
by a newline and each entry is terminated by NUL, which is the same
convention "git config list -z" already uses. When a single variable
is asked for, its bare value is terminated by NUL.
Parsing of the command line is switched over to parse_options() in
order to accept the new option; as a side effect, options now have to
come before the variable name.
Signed-off-by: Andrew Pleeter <andrewpleeter@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A variable whose value is a list, of which GIT_CONFIG_GLOBAL is
currently the only one, is read into a single string whose elements are
joined with newlines, and "git var -l" splits that string apart again
before showing each element. Round-tripping through a delimiter that
can legitimately appear in a value is fragile, and it forces every
future caller to know which variables need splitting.
Give "struct git_var" a separate "multiread" function that appends the
elements to a string_list, and let the single "read" function stay for
the variables that have exactly one value. A variable uses one or the
other, never both, so which one is set also says whether the variable
is multi-valued.
The output is unchanged. While at it, say in the documentation that
GIT_CONFIG_GLOBAL can have multiple values, so that callers know when
to expect a list.
Signed-off-by: Andrew Pleeter <andrewpleeter@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_can_ff() and already_up_to_date() pass the result of
lookup_commit_reference() straight to commit_list_insert() and
repo_is_descendant_of() without checking it. When the object
behind HEAD or one of the merge heads cannot be parsed, e.g. because
a loose object was left truncated by a fetch or gc racing on the
same repository, lookup_commit_reference() returns NULL and
"git pull" segfaults instead of reporting the corruption.
Treat a failed lookup as "cannot fast-forward" and "not up to date",
so that the caller falls through to the normal merge path, which
already diagnoses the broken object and fails cleanly.
An alternative would be to report the breakage at each lookup site,
which could give a more precise diagnosis. The minimal guards are
preferred because they do no more than is needed to avoid the
crash, and will be easy to drop once "git pull" is reworked to
resolve object names into commit objects early and pass those
around, at which point there will not be multiple lookups of the
same object name to guard in the first place.
The test corrupts the loose object in place rather than removing
it: a missing object that is still recorded in the commit-graph is
caught by the consistency check in fetch-pack before "git pull"
reaches the fast-forward check, so removing it would not exercise
the crash.
Signed-off-by: Jiri Kuncar <jiri.kuncar@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commands that use the sequencer with the "merge" backend, like
git-cherry-pick(1) or git-rebase(1) with "--merge", create their
commits in-process. Consequently, these commands typically don't
execute auto maintenance at all. Only the commands they spawn on the
way run it, like git-commit(1) for a resolved conflict or an edited
message.
In contrast to that, the "apply" backend of git-rebase(1) _does_ run
auto maintenance after it has processed the sequence of commits. And
this is a sensible thing to do: after all, we may just have written
lots of objects, so chances are high that we have something to clean
up now.
Adapt users of the "merge" backend to do the same.
The sequencer has no single exit where a call to run_auto_maintenance()
could go. A sequence ends in pick_commits(), but a single pick never
gets there: it returns from sequencer_pick_revisions() via
single_pick(), its "--continue" from sequencer_continue() via
continue_single_pick(), and its "--skip" from sequencer_skip(). So
call it from the sequencer's two callers instead: run_specific_rebase()
for the merge backend, once its state directory is gone, and
run_sequencer() in builtin/revert.c after a successful pick,
"--continue" or "--skip".
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
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
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()`
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>
Let's not check the nullness of cmd->error_string_owned before
calling FREE_AND_NULL(cmd->error_string_owned). It is cheap and
safe to call FREE_AND_NULL(variable) for a variable that has NULL
in it.
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
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 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
When running 'git-receive-pack(1)', there is no way for the server to
intercept and modify the status report before it is sent back to the
client. Servers with custom logic may need to transform or gate the
report based on the outcome of external logic post reference updates.
This is specially needed for our usecase at GitLab where we have custom
MVCC logic on top of Git which creates a new version for each push
operation. The new version is only committed when certain external
operations post reference transaction succeed. So reporting the correct
message based on the outcome of these operations is important.
The outcome of these operations is only known after `execute_commands()`
has returned and before the report is written. There is no point in
receive-pack where the server can act on that.
We cannot use any of the existing hooks as:
- The pre-receive hook runs too early, as we haven't updated
references at that point yet and we need to have the full view of
all resulting updates (both objects and references).
- The update hook is too inefficient as it runs once per reference,
and we cannot trivially determine the last update.
- The reference-transaction hook is not suited for this. It fires from
within `ref_transaction_commit()`, which is before the outcome we
need to report is known, so there is no phase at which it could give
us the answer. It also does not contain any knowledge regarding the
push and cannot communicate with the clients.
- The proc-receive hook replaces execute_commands() for references
matching 'receive.procReceiveRefs'. We need to gate the report for
the push as a whole.
- The post-receive and post-update hooks cannot be used as they run
too late, at the point where we have already reported success to the
client.
Introduce a new 'receive-report' hook. The hook receives the complete
pkt-line encoded status report on standard input, after all ref updates
have been applied to the repository by execute_commands() but before the
report is sent to the client. See linkgit:gitprotocol-pack[5] details on
the protocol structure.
The hook's stdout fully replaces the report sent to the client.
receive-pack fully buffers the hook's stdout before acting on the exit
status, so the exit code is known before the client receives anything.
This gives two distinct behaviors depending on exit status:
- Exit 0: the hook's stdout is used as the report. The hook can
rewrite 'ok' lines to 'ng' lines to signal per-ref rejection to the
client while receive-pack itself exits cleanly. The client marks
rejected refs as '[remote rejected]' and exits with a non-zero
status if any ref is 'ng'.
- Non-zero exit: the hook's stdout is discarded, receive-pack modifies
all references to be rejected with a 'receive-report hook failed'
error.
In both cases, any output the hook writes to standard error is
forwarded to the client over the sideband channel and appears as
'remote:' lines on the client terminal. Writing to stderr alone does
not affect the push outcome.
Reference updates applied by execute_commands() are not rolled back in
either failure mode. The hook can cause the client to perceive the push
as failed, but cannot undo server-side changes. This creates a
divergence that the server cannot resolve: the client leaves its
remote-tracking reference at the old value while the update is in fact
applied, and a later fetch may reveal the update that the push reported
as rejected.
The hook is therefore only appropriate for servers which can guarantee
that a rejected update is not observable by any reader. In our case the
transaction committed by execute_commands() produces a candidate version
which is not visible to other readers and is only published once the
subsequent operations succeed, so a report of 'ng' corresponds to a
version that is discarded rather than published. On a repository where a
committed reference update is immediately visible, rejecting a push from
this hook would instead leave the pusher with a view that does not match
the server.
This hook does not use the config-based hook infrastructure, which
supports running multiple scripts per hook event. This hook is a
bidirectional filter: it receives the report on stdin and writes a
modified version to stdout. Running multiple such scripts sequentially
would require piping the output of one into the input of the next,
which the current hook infrastructure does not support. A single-script
design is therefore a natural fit, and is consistent with how
'proc-receive' is structured for the same reason.
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After git-receive-pack(1) has committed the reference updates, we call
either `report()` or `report_v2()` to report to the client which of the
references we have updated successfully and which updates have failed.
The only difference between those two functions is that the latter also
knows to provide a more detailed report about how exactly a given
reference was updated.
With this, also drop `report_v2()` as both report functions now are
similar in structure with only the `report_status_version`
differentiating them.
In the next commit we're about to add another site that wants to
generate these reports. Refactor the logic into a shared function that
can easily be reused.
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 'git-receive-pack(1)', to track the report status version, we use the
static variables `report_status` and `report_status_v2`. As the report
status version is mutually exclusive, using an enum better suits the
requirement. switch to using a new `enum report_status_version`, while
also dropping the static variable to make the flow easier to understand.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Racy Git problems persist today, manifesting themselves in the
performance of commands like "git diff" in new worktrees [1]. We have
long had a build knob "USE_NSEC" to tell Git to use in-core nanosecond
precision when available, which mitigates most if not all racy issues,
but most builds we know about don't use it. In part, that's because
someone distributing Git can't safely enable it at compile-time if they
don't know exactly what platforms their distribution will be used on.
[1]: https://lore.kernel.org/git/CALnO6CADMJSixqYvL1Yo8qKX5rWhKQ+2OoSEuPUh-yoeK9TseQ@mail.gmail.com
These days, most platforms are likely to be safe for the USE_NSEC code.
Regardless, we want to give users the ability to benefit from it. This
requires exposing the compile-time gated code as a runtime option.
In addition, update the Racy Git documentation and other mentions of
USE_NSEC in the code.
Due to the conversion from #ifdef to runtime check, using the flag
"--ignore-space-change" may be particularly helpful when viewing changes
from this patch.
Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
When checking loose objects we manually parse the object buffer we have
read from the on-disk file, mark the object and then call `fsck_obj()`.
The exact same steps are also performed by `fsck_obj_buffer()`.
Stop open-coding this logic and call `fsck_obj_buffer()` instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Users can tell git-multi-pack-index(1) to access multi-pack indices that
are stored in a different object directory via the "--object-dir="
option. This allows them to for example write or verify a multi-pack
index other than the one located in the main object directory in case a
repository has alternates with multiple multi-pack indices.
But while the documentation explicitly points out that the specified
object directory must be an alternate of the current repository, we
never verify that property. Instead, starting with 017db7bb14 (midx:
load multi-pack indices via their source, 2025-08-11), we now construct
an ad-hoc source and link it to the main object directory.
Besides contradicting the documentation, it's dubious that this really
ought to work in the first place: creating a multi-pack index (and
potentially a bitmap) for a completely foreign object directory is of
questionable value, as bitmap commit selection operates on the invoking
repository's refs. Furthermore, this is the only remaining caller
outside of our test helpers that constructs an ad-hoc source and links
it to the database, and we want to get rid of this mechanism as part of
this series.
Stop constructing the ad-hoc source and instead refuse the operation.
While this results in a change in behaviour, this restriction has been
documented as such ever since f57a739691 (midx: avoid opening multiple
MIDXs when writing, 2021-09-01).
Note that this change requires us to adapt one test chain in t5319, as
it creates an object directory that is not connected to any repository
and then uses it via "--object-dir=". The setup itself already documents
this and does the necessary gymnastics to link the object directory to a
temporary repository, but subsequent tests don't. Adapt those tests to
retain and reuse the temporary repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Same as with the preceding commit, git-grep(1) registers each
submodule's object database as an in-memory source of the main object
database before grepping it. This was introduced as an eager alternate
registration and converted into the lazy mechanism via 8d33c3af0b (grep:
use submodule-ODB-as-alternate lazy-addition, 2021-08-16).
Starting with 0693806bf8 (grep: add repository to OID grep sources,
2021-08-16), the command instead knows to pass submodule repositories to
our workers, which means that those now use that repository to look up
objects, too. As a consequence, registering submodule sources as
alternates is not required anymore.
Remove the logic to register submodule sources. Unfortunately, this does
not allow us to get rid of the object read lock as initializing the
subrepository is still racy.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Several functions in the submodule-config subsystem implicitly depend
on `the_repository`. Refactor these to take a `struct repository` as
parameter and adapt callers accordingly.
Note that as usual with these refactorings, callers simply pass
`the_repository` even if they already have a different repository
available in the calling context. This simplifies the migration and
ensures that we don't have a change in behaviour.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano