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>
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
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>
The function `cache_tree_fully_valid()` verifies whether the cache tree
owned by the index is valid or not. As part of that, the function checks
whether the objects referenced by the cache all exist. But because the
function has no repository available, it is using the object database of
`the_repository` instead.
We could of course adapt callers to pass in a repository as parameter
explicitly to get rid of this implicit dependency on global state. But
all of them pass the cache tree owned by a `struct index_state`, and
that structure already has a reference to its owning repository.
So instead, adapt the function to accept a `struct index_state`, which
ensures that callers will implicitly always pass the correct repository.
Adapt callers accordingly.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
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 'git replay' command has been taught the '--linearize' option to
drop merge commits and linearize the replayed history, mimicking 'git
rebase --no-rebase-merges'.
* tc/replay-linearize:
replay: offer an option to linearize the commit topology
replay: resolve the replay base outside pick_regular_commit()
replay: add helper to put entry into replayed_commits
The string extraction logic for the branch name and worktree name
from the given path in 'git worktree add' has been corrected and
simplified to avoid out-of-bounds reads and improper handling of
trailing slashes.
* rs/worktree-add-basename-fixes:
worktree add: let worktree_basename() return string copy
worktree add: trim slashes when deriving branch name from path
worktree add: reject separator-only path
worktree add: don't read out of bounds in worktree_basename()
The application of the edited patch in 'git add -e' has been
refactored to use the internal apply API directly, avoiding the need
to spawn a 'git apply' subprocess.
* gr/add-e-use-apply-api:
builtin/add.c: replace run_command() with direct apply_all_patches() call
'git checkout' and 'git worktree add' makes guesses based on a name
of a remote-tracking branch, but does not give an error when such a
remote-tracking branch cannot be uniquely identified, which has
been corrected.
* yn/worktree-ambiguous-remote-advice:
worktree add: treat multiple matches with --guess-remote as an error
worktree add: improve message for ambiguous remote branch name
checkout: improve message for ambiguous remote branch name
checkout: extract function to display advice for ambiguous remotes
The "rerere-gc" maintenance task is responsible for pruning rerere
entries older than a certain configurable cutoff point. Whether or not
the task gets run during auto-maintenance can be configured via
"maintenance.rerere-gc.auto":
- A negative value indicates that maintenance should always run.
- A zero value indicates that maintenance should never run.
- Otherwise, a positive value indicates that maintenance should always
run in case we have at least a single rerere entry.
While the first two conditions are sensible, the last one is less so as
it does not account for whether we would even prune old entries in the
first place. Instead, it effectively implies that we unconditionally
spawn "git rerere gc" when rerere is enabled. Chances are high though
that there is nothing to prune, as the default cutoff dates are 60 days
for resolved rerere entries and 15 days for unresolved ones.
Besides being a waste of compute, it also obstructs concurrent processes
that want to write new resolutions as garbage collection takes a central
lock file, as reported in [1]. That race is a longstanding one that
existed even before we introduced fine-grained maintenance tasks, and
the proper fix is to use a locking timeout in the writing processes. But
the race is made worse by us performing garbage collection a lot more
often.
Refine the heuristic to take into account whether any entries can be
pruned in the first place. This ensures that we'll only ever run this
task in situations where it will do anything, and should thus result in
a lot less frequent invocations of "git rerere gc".
Furthermore, tweak the meaning of "maintenance.rerere-gc.auto" so that
positive values allow the user to configure the number of prunable
entries that need to exist before we run it and set the default value to
512. This number is pulled out of thin air, but it ensures that we know
to batch-delete entries instead of pruning every single entry that is
older than the cutoff point.
Note that this now requires us to actually open the rerere-entry
directories and stat the individual files in there, which does add a bit
of overhead when one has lots of rerere entries. To counteract this
overhead, we thus use the same sampling heuristic as we do for loose
objects, where we only consider those entries that start with a "17".
[1]: <pull.2214.git.1788337897490.gitgitgadget@gmail.com>
Reported-by: Thomas Bachem <mail@thomasbachem.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When working with partial clones, callers often need only the missing
object IDs. Today that means post-processing --missing=print to drop
present objects and strip the leading '?':
git rev-list --objects --all --missing=print | perl -ne 'print if s/^[?]//'
This is for a one-shot walk, not a fetch loop. Callers already have
--missing=print and strip the leading '?'. Gitaly does that when packing
a quarantine: '?' lines are objects that must already exist in the main
repo. Tests do the same (is this blob still missing). --missing-only is
just that list without the prefix.
Add --missing-only. Use it with --missing=print or --missing=print-info
to print only missing objects. --missing= still picks the format;
--missing-only only filters. The leading '?' is omitted. With
print-info, path= and type= are still shown.
Require --missing=print or --missing=print-info. Reject --count and
--disk-usage.
Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git checkout -m" stashes the user's local changes when it cannot
perform the checkout, and then applies the stash. When applying the
stash results in conflicts, the advice on how to deal with them is
printed directly on top of the branch-switch message ("Switched to
branch ..."), making the two hard to tell apart. Print a blank line
in between so that the advice and the branch-switch message are
visually distinct.
apply_autostash_ref() reports whether applying the stash resulted in
conflicts via its enum stash_apply_result return value, so only print
the blank line in the conflicted case.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git stash apply", "pop" and "branch" exit with status 1 both when
applying the stash entry resulted in conflicts and when they fail for
other reasons, so callers cannot tell the two apart.
Follow the convention of "git merge-tree" and the merge strategies,
which exit with status 1 to indicate conflicts and with a different
non-zero status for errors: those subcommands now exit with status 1
only when applying the stash entry resulted in conflicts, in which
case the stash entry is left in place, and exit with status 128, the
status die() uses, when they fail for other reasons. Document the
exit statuses.
The only subcommand implementations that can return a positive value
are "apply", "pop" and "branch", which return the value of
do_apply_stash(): "apply" returns it directly, and "pop" and "branch"
drop the stash entry, via do_drop_stash(), which always returns 0,
only when the application succeeded. do_apply_stash() only returns a
positive value when the three-way merge was unclean. cmd_stash() now
maps negative values to 128 and passes positive values through as the
exit status, so exit status 1 unambiguously indicates conflicts.
enum stash_apply_result makes the convention explicit, and the
autostash helpers use it to tell users that their stashed changes
were saved when applying them fails.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
write_ondisk_index() dereferences the return value of
repo_parse_tree_indirect() unconditionally. If the parent commit's
tree object is missing from the object store (corrupt repository,
object removed by tooling, or incomplete restore), the function
returns NULL and "git history split" crashes with a SIGSEGV.
Guard the parse result and error out gracefully, following the
codebase convention for objects that cannot be loaded.
Signed-off-by: Jinbao Chen <zkd18cjb@mail.ustc.edu.cn>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
If repo setup fails, we'll return an error without freeing the allocated
url string, leaking the memory. The test suite does trigger this error,
but never with the leak. We only allocate a url if submodule_from_path()
returned something, but our tests use other situations, like totally
nonexistent submodules.
We can cover this case by asking about a submodule that exists but which
has not been initialized. The new test fails with SANITIZE=leak.
The smallest fix would just be a call to free(url), but I think it's a
little nicer to set up a dedicated out-path for cleanup here. The
previous commit made it safe to call repo_clear() even if
repo_submodule_init() fails.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to the previous commit, just as `git commit --amend` is a
foot-gun during conflict resolution, so is a partial commit (`git commit
<paths>`). Recording a conflict resolution is about capturing the state
of the entire tree on top of HEAD, not a subset of paths. For many years
we have rejected partial commits in the middle of
- a merge
- a cherry-pick
but, just like amending, this was never extended to the other operations
that can also leave conflicts to resolve:
- an `am` operation
- a revert
- a rebase that stopped for conflict resolution
Reuse sequencer_ongoing_operation(), introduced for the analogous
`--amend` check, to detect these and refuse the partial commit. A rebase
that stopped because a pick became empty is not conflict resolution and,
as an earlier patch established, is deliberately left permitted.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running `git commit --amend` during conflict resolution is an ugly
foot-gun. For many years, we have rejected amending during conflict
resolution in the middle of
- a merge
- a cherry-pick
However, this was never extended to other operations that can also
produce conflicts:
- an `am` operation
- a revert
- a rebase
Extend it to handle these other cases now.
Extending to `am`, revert, and the apply backend of rebase are fairly
straightforward. However, with the merge backend of rebase we have to
be more careful, since it powers interactive rebases and
- the interactive machinery internally uses `git commit --amend` for
`squash` and `reword` directives
- users are expected to `git commit --amend` after hitting an `edit`
or `break` directive
So, we need to be careful with rebase to only reject amending when doing
conflict resolution.
A few files under the rebase-merge/ directory provide us the necessary
information:
- stopped-sha is written only when the rebase stops and hands control
back to the user, so its presence marks a genuine stop -- as opposed
to the sequencer's own internal `git commit --amend` while applying
a squash, fixup, or reword, during which no stopped-sha exists.
- amend is written only when the rebase stops with HEAD already
pointing at the commit the user is meant to amend: a clean `edit`,
or a fast-forward `reword`. Its absence at a stop therefore means
the commit did not apply, so HEAD is the previously-applied commit
rather than the one being rebased -- exactly the case we refuse.
So for the merge backend we die when stopped-sha exists and amend does
not. This covers a plain conflicted pick as well as a conflicted `edit`
(both leave HEAD on the previously-applied commit), while still allowing
a clean `edit` or `reword` stop and a `break` stop (no stopped-sha).
stopped-sha is unlinked at the start of the resume loop, so a resumed
squash's internal amend is unaffected.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a rebase applies a commit that becomes empty, it stops and asks the
user to decide whether to keep it or drop it. HEAD still points at the
previously-applied commit at that point, so amending is refused, with:
You are in the middle of a rebase -- cannot amend.
That message would suggest that amending is not allowed during an 'edit'
or 'break' stop, which is misleading, plus it lacks the specificity that
might help the user know why their particular case is a problem: the
commit they intended to amend became empty and was dropped, so amending
would affect the wrong commit. Reword the error accordingly.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For years, we disallowed partial commits during merges or cherry-picks.
In commit 430b75f720 (commit: give correct advice for empty commit
during a rebase, 2019-12-06) it was noted that the "cannot do a partial
commit during a cherry-pick" message was also printed when rebasing a
commit that became empty, and rather than drop the check in that case,
that commit opted to make the message print the actual operation that
was in progress.
Since a commit that has become empty comes without conflicts, a new
partial commit poses no problems; remove the error in that case.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 430b75f720 (commit: give correct advice for empty commit during
a rebase, 2019-12-06) introduced a FROM_REBASE_PICK enum value and an
is_from_rebase() function. Those names failed to convey that they were
specifically about hitting a commit that becomes empty when rebasing.
Clarify their names now.
While at it, change `whence == FROM_REBASE_NOW_EMPTY` to use
`is_from_rebase_now_empty(whence)`.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of the stated goals of git-replay(1) is to allow implementing the
git-rebase(1) functionality on the server side.
The default mode of git-rebase(1) is to act as if `--no-rebase-merges`
was given. This mode drops merge commits instead of replaying them, and
linearizes the history into a sequence of regular (single-parent)
commits.
Add option `--linearize` to git-replay(1) to do the same. Each replayed
commit is stacked on top of the previously replayed one. When a merge is
encountered, the commits reachable from all of its sides are replayed
into the single line and the merge itself is dropped.
If a ref was pointing to a merge commit, that ref is updated to the
merge's last replayed ancestor.
git-replay(1) accepts multiple branches, for example:
$ git replay --onto main topic1 topic2
Without `--linearize` this replays 'topic1' and 'topic2' onto 'main'
(keeping shared portions of history shared and divergent parts
divergent) and updates both refs.
Due to current implementation limitations, replaying multiple branches
with `--linearize` is disallowed to avoid concatenating unrelated
histories into a single line. For the same reason disallow the use of
`--contained` with `--linearize`.
Users who want to linearize multiple branches are advised to do this in
separate git-replay(1) invocations. Linearizing multiple branches at
once might be added later.
Note that `--linearize` is not modeled after git-rebase(1)'s
`--rebase-merges[=<mode>]` interface. Recreating merges, by preserving
their topology, is a distinct operation that would be a separate mode.
`--linearize` only drops merges and replays commits linearly. So
git-replay(1) uses its own option rather than reusing that interface.
Based-on-patches-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 mechanism to generate a packfile corresponding to the result of
a fetch/push has been made pluggable through a set of object
database callback functions, removing hardcoded references to
'pack-objects' and enabling alternative ODBs to serve packfiles
themselves.
* ps/odb-pluggable-pack-generation:
bundle: generate packfiles via the object database
bundle: get (mostly) rid of `the_repository`
builtin/bundle: refactor option handling for progress meter
send-pack: generate packfiles via the object database
upload-pack: generate packfiles via the object database
odb: introduce interface to generate packfiles
The 'git receive-pack' command has been updated to use a new ODB
transaction interface for writing incoming packfiles, making it more
backend-agnostic.
* jt/receive-pack-pluggable-writes:
odb/transaction: add transaction interface to write packfiles
odb: return temporary ODB source when set
builtin/receive-pack: explicitly pass packfile fd
builtin/receive-pack: report unpack errors via strbuf
builtin/receive-pack: lift global state out of unpack()
builtin/receive-pack: read unpack limit config lazily
builtin/receive-pack: pass shallow file explicitly
odb/transaction: add transaction finalize interface
builtin/receive-pack: properly clean up keep files
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
The DWIM logic in 'git worktree add' sometimes tried to infer a
remote-tracking branch when an explicit '-b' or '-B' option was
given to create a new branch, causing the explicit branch name to
be ignored, which has been corrected.
* yn/worktree-add-no-dwim-with-b:
worktree add: shouldn't dwim if -b or -B is given
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>
Junio C Hamano