As the last commit deleted the only user of VERBATIM_MSG remove
it. This reverts remaining parts of commit f7d42ceec52 (rebase -i:
do leave commit message intact in fixup! chains, 2021-01-28) that
were not deleted by the last commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rebase -i: respect commit.cleanup when picking fixups
If the user uses a prepare-commit-msg hook to add comments to the
commit message template and sets commit.cleanup to remove them when the
commit is created then the comments will not be removed when rebase
commits the final command in a chain of "fixup" commands[1]. This
happens because f7d42ceec52 (rebase -i: do leave commit message intact
in fixup! chains, 2021-01-28) started passing the VERBATIM_MSG flag
when committing the final command in a chain of "fixup" commands. That
change was added in response to a bug report[2] where the commit
message was being cleaned up when it should not be. The cause of that
bug was that before f7d42ceec52 the sequencer passed CLEANUP_MSG
when committing the final fixup. That commit should have simply
removed the CLEANUP_MSG flag, not changed it to VERBATIM_MSG. Using
VERBATIM_MSG ignores the user's commit.cleanup config when committing
the final fixup which means it behaves differently to an ordinary
"pick" command which respects commit.cleanup.
Fix this by not setting an explicit cleanup flag when committing the
final fixup which matches the way "pick" commands behave. The test
added in f7d42ceec52 is replaced with one that checks that "fixup"
and "pick" commands do not clean up the message when commit.cleanup
is not set and do clean up the message when it is set.
Reported-by: Simon Cheng <cyqsimon@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
last-modified: fix bug when some paths remain unhandled
The recently introduced new subcommand git-last-modified(1) runs into an
error in some scenarios. It then would exit with the message:
BUG: paths remaining beyond boundary in last-modified
This seems to happens for example when criss-cross merges are involved.
In that scenario, the function diff_tree_combined() gets called.
The function diff_tree_combined() copies the `struct diff_options` from
the input `struct rev_info` to override some flags. One flag is
`recursive`, which is always set to 1. This has been the case since the
inception of this function in af3feefa1d (diff-tree -c: show a merge
commit a bit more sensibly., 2006-01-24).
This behavior is incompatible with git-last-modified(1), when called
non-recursive (which is the default).
The last-modified machinery uses a hashmap for all the paths it wants to
get the last-modified commit for. Through log_tree_commit() the callback
mark_path() is called. The diff machinery uses diff_tree_combined()
internally, and due to it's recursive behavior the callback receives
entries inside subtrees, but not the subtree entries themselves. So a
directory is never expelled from the hashmap, and the BUG() statement
gets hit.
Because there are many callers calling into diff_tree_combined(), both
directly and indirectly, we cannot simply change it's behavior.
Instead, add a flag `no_recursive_diff_tree_combined` which supresses
the behavior of diff_tree_combined() to override `recursive` and set
this flag in builtin/last-modified.c.
Signed-off-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
BreakingChanges: remove claim about whatchanged reports
This was written in e836757e14b (whatschanged: list it in
BreakingChanges document, 2025-05-12) which was on the same
topic that added the `--i-still-use-this` requirement.[1]
Maybe it was a work-in-progress comment/status.
[1]: jc/you-still-use-whatchanged
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There have been quite a few `--i-still-use-this` user reports since Git
2.51.0 was released.[1][2] And it doesn’t seem like they are reading
the man page about the git-log(1) equivalent.
Tell them what options to plug into git-log(1), either as a replacement
command or as an alias.[3] That template produces almost the same
output[4] and is arguably a plug-in replacement. Concretely, add
an optional `hint` argument so that we can use it right after the
initial error line.
Also mention the same concrete options in the documentation while we’re
at it.
[1]: E.g.,
• https://lore.kernel.org/git/e1a69dea-bcb6-45fc-83d3-9e50d32c410b@5y5.one/
• https://lore.kernel.org/git/1011073f-9930-4360-a42f-71eb7421fe3f@chrispalmer.uk/#t
• https://lore.kernel.org/git/9fcbfcc4-79f9-421f-b9a4-dc455f7db485@acm.org/#t
• https://lore.kernel.org/git/83241BDE-1E0D-489A-9181-C608E9FCC17B@gmail.com/
[2]: The error message on 2.51.0 does tell them to report it, unconditionally
[3]: We allow aliasing deprecated builtins now for people who are very
used to the command name or just like it a lot
[4]: You only get different outputs if you happen to have empty
commits (no changes)[4]
[5]: https://lore.kernel.org/git/20250825085428.GA367101@coredump.intra.peff.net/
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
you-still-use-that??: help the user help themselves
Give the user a list of suggestions for what to do when they run a
deprecated command.
The first order of action will be to check the breaking changes
document;[1] this short error message says nothing about why this
command is deprecated, and in any case going into any kind of detail
might overwhelm the user.
Then they can find out if this has been discussed on the mailing list.
Then users who e.g. are using git-whatchanged(1) can learn that this is
arguably a plug-in replacement:
git log <opts> --raw --no-merges
Finally they are invited to send an email to the mailing list.
Also drop the “please add” part in favor of just using the “refusing”
die-message; these two would have been right after each other in this
new version.
Also drop “Thanks” since it now would require a new paragraph.
[1]: www.git-scm.com has a disclaimer for these internal documents that
says that “This information is specific to the Git project”. That’s
misleading in this particular case. But users are unlikely to get
discouraged from reading about why they (or their programs) cannot run a
command any more; it clearly concerns them.
Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t0014: test shadowing of aliases for a sample of builtins
The previous commit added tests for shadowing deprecated builtins.
Let’s make the test suite more complete by exercising a sample of
the builtins and in turn test the documentation for git-config(1):
To avoid confusion and troubles with script usage, aliases that hide
existing Git commands are ignored except for deprecated commands.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-whatchanged(1) is deprecated and you need to pass
`--i-still-use-this` in order to force it to work as before.
There are two affected users, or usages:
1. people who use the command in scripts; and
2. people who are used to using it interactively.
For (1) the replacement is straightforward.[1] But people in (2) might
like the name or be really used to typing it.[3]
An obvious first thought is to suggest aliasing `whatchanged` to the
git-log(1) equivalent.[1] But this doesn’t work and is awkward since you
cannot shadow builtins via aliases.
Now you are left in an uncomfortable limbo; your alias won’t work until
the command is removed for good.
Let’s lift this limitation by allowing *deprecated* builtins to be
shadowed by aliases.
The only observed demand for aliasing has been for git-whatchanged(1),
not for git-pack-redundant(1). But let’s be consistent and treat all
deprecated commands the same.
[1]:
git log --raw --no-merges
With a minor caveat: you get different outputs if you happen to
have empty commits (no changes)[2]
[2]: https://lore.kernel.org/git/20250825085428.GA367101@coredump.intra.peff.net/
[3]: https://lore.kernel.org/git/BL3P221MB0449288C8B0FA448A227FD48833AA@BL3P221MB0449.NAMP221.PROD.OUTLOOK.COM/
Based-on-patch-by: Jeff King <peff@peff.net> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git: move seen-alias bookkeeping into handle_alias(...)
We are about to complicate the command handling by allowing *deprecated*
builtins to be shadowed by aliases. We need to organize the code in
order to facilitate that.[1]
The code in the `while(1)` speculatively adds commands to the list
before finding out if it’s an alias. Let’s instead move it inside
`handle_alias(...)`—where it conceptually belongs anyway—and in turn
only run this logic when we have found an alias.[2]
[1]: We will do that with an additional call to `handle_alias(1)` inside
the loop. *Not* moving this code leaves a blind spot; we will miss
alias looping crafted via deprecated builtin names
[2]: Also rename the list to a more descriptive name
Based-on-patch-by: Jeff King <peff@peff.net> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
With 145 builtin commands (according to `git --list-cmds=builtins`),
users are probably not keeping on top of which ones (if any) are
deprecated.
Let’s expand the experimental `--list-cmds`[1] to allow users and
programs to query for this information. We will also use this in an
upcoming commit to implement `is_deprecated_command`.
[1]: Using something which is experimental to query for deprecations is
perhaps not the most ideal approach, but it is simple to implement
and better than having to scan the documentation
Acked-by: Patrick Steinhardt <ps@pks.im> Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Makefile: don’t add whatchanged after it has been removed
07572f220a8 (whatchanged: remove when built with WITH_BREAKING_CHANGES,
2025-05-12) set up the removal of git-whatchanged(1) when
`WITH_BREAKING_CHANGES` is active. Part of that work was removing it
from `commands` in `git.c`. But the Makefile still lists it as a
builtin. That leaves it in the limbo of being linked but not being
callable; you get the generic error about not being able to call it as
a *builtin*:
$ git whatchanged
fatal: cannot handle whatchanged as a builtin
instead of the expected:
$ git whatchanged
git: 'whatchanged' is not a git command. See 'git --help'.
Based-on-patch-by: Jeff King <peff@peff.net> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Christian Couder [Wed, 17 Sep 2025 18:14:27 +0000 (20:14 +0200)]
fast-import: add '--signed-commits=<mode>' option
A '--signed-commits=<mode>' option is already available when using
`git fast-export` to decide what should be done at export time about
commit signatures. At import time though, there is no option, or
other way, in `git fast-import` to decide about commit signatures.
To remediate that, let's add a '--signed-commits=<mode>' option to
`git fast-import` too.
For now the supported <mode>s are the same as those supported by
`git fast-export`.
The code responsible for consuming a signature is refactored into
the import_one_signature() and discard_one_signature() functions,
which makes it easier to follow the logic and add new modes in the
future.
In the 'strip' and 'warn-strip' modes, we deliberately use
discard_one_signature() to discard the signature without parsing it.
This ensures that even malformed signatures, which would cause the
parser to fail, can be successfully stripped from a commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Christian Couder [Wed, 17 Sep 2025 18:14:26 +0000 (20:14 +0200)]
gpg-interface: refactor 'enum sign_mode' parsing
The definition of 'enum sign_mode' as well as its parsing code are in
"builtin/fast-export.c". This was fine because `git fast-export` was the
only command with '--signed-tags=<mode>' or '--signed-commits=<mode>'
options.
In a following commit, we are going to add a similar option to `git
fast-import`, which will be simpler, easier and cleaner if we can reuse
the 'enum sign_mode' defintion and parsing code.
So let's move that definition and parsing code from
"builtin/fast-export.c" to "gpg-interface.{c,h}".
While at it, let's fix a small indentation issue with the arguments of
parse_opt_sign_mode().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit added the necessary validation and checks for F/D
conflicts in the files backend when working on case insensitive systems.
There is still a possibility for D/F conflicts. This is a different from
the F/D since for F/D conflicts, there would not be a conflict during
the lock creation phase:
refs/heads/foo.lock
refs/heads/foo/bar.lock
However there would be a conflict when the locks are committed, since we
cannot have 'refs/heads/foo/bar' and 'refs/heads/foo'. These kinds of
conflicts are checked and resolved in
`refs_verify_refnames_available()`, so the previous commit ensured that
for case-insensitive filesystems, we would lowercase the inputs to that
function.
For D/F conflicts, there is a conflict during the lock creation phase
itself:
refs/heads/foo/bar.lock
refs/heads/foo.lock
As in `lock_raw_ref()` after creating the lock, we also check for D/F
conflicts. This can occur in case-insensitive filesystems when trying to
fetch case-conflicted references like:
refs/heads/Foo/new
refs/heads/foo
D/F conflicts can also occur in case-sensitive filesystems, when the
repository already contains a directory with a lock file
'refs/heads/foo/bar.lock' and trying to fetch 'refs/heads/foo'. This
doesn't concern directories containing garbage files as those are
handled on a higher level.
To fix this, simply categorize the error as a name conflict. Also remove
this reference from the list of valid refnames for availability checks.
By categorizing the error and removing it from the list of valid
references, batched updates now knows to reject such reference updates
and apply the other reference updates.
Fix a small typo in `ref_transaction_maybe_set_rejected()` while here.
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>
refs/files: handle F/D conflicts in case-insensitive FS
When using the files-backend on case-insensitive filesystems, there is
possibility of hitting F/D conflicts when creating references within a
single transaction, such as:
- 'refs/heads/foo'
- 'refs/heads/Foo/bar'
Ideally such conflicts are caught in `refs_verify_refnames_available()`
which is responsible for checking F/D conflicts within a given
transaction. This utility function is shared across the reference
backends. As such, it doesn't consider the issues of using a
case-insensitive file system, which only affects the files-backend.
While one solution would be to make the function aware of such issues,
this feels like leaking implementation details of file-backend specific
issues into the utility function. So opt for the more simpler option, of
lowercasing all references sent to this function when on a
case-insensitive filesystem and operating on the files-backend.
To do this, simply use a `struct strbuf` to convert the refname to
lowercase and append it to the list of refnames to be checked. Since we
use a `struct strbuf` and the memory is cleared right after, make sure
that the string list duplicates all provided string.
Without this change, the user would simply be left with a repository
with '.lock' files which were created in the 'prepare' phase of the
transaction, as the 'commit' phase would simply abort and not do the
necessary cleanup.
Reported-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>
refs/files: use correct error type when lock exists
When fetching references into a repository, if a lock for a particular
reference exists, then `lock_raw_ref()` throws:
- REF_TRANSACTION_ERROR_CASE_CONFLICT: when there is a conflict
because the transaction contains conflicting references while being
on a case-insensitive filesystem.
- REF_TRANSACTION_ERROR_GENERIC: for all other errors.
The latter causes the entire set of batched updates to fail, even in
case sensitive filessystems.
Instead, return a 'REF_TRANSACTION_ERROR_CREATE_EXISTS' error. This
allows batched updates to reject the individual update which conflicts
with the existing file, while updating the rest of the references.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs/files: catch conflicts on case-insensitive file-systems
During the 'prepare' phase of a reference transaction in the files
backend, we create the lock files for references to be created. When
using batched updates on case-insensitive filesystems, the entire
batched updates would be aborted if there are conflicting names such as:
refs/heads/Foo
refs/heads/foo
This affects all commands which were migrated to use batched updates in
Git 2.51, including 'git-fetch(1)' and 'git-receive-pack(1)'. Before
that, reference updates would be applied serially with one transaction
used per update. When users fetched multiple references on
case-insensitive systems, subsequent references would simply overwrite
any earlier references. So when fetching:
This is buggy behavior since the user is never informed about the
overrides performed and missing references. Nevertheless, the user is
left with a working repository with a subset of the references. Since
Git 2.51, in such situations fetches would simply fail without updating
any references. Which is also buggy behavior and worse off since the
user is left without any references.
The error is triggered in `lock_raw_ref()` where the files backend
attempts to create a lock file. When a lock file already exists the
function returns a 'REF_TRANSACTION_ERROR_GENERIC'. When this happens,
the entire batched updates, not individual operation, is aborted as if
it were in a transaction.
Change this to return 'REF_TRANSACTION_ERROR_CASE_CONFLICT' instead to
aid the batched update mechanism to simply reject such errors. The
change only affects batched updates since batched updates will reject
individual updates with non-generic errors. So specifically this would
only affect:
This bubbles the error type up to `files_transaction_prepare()` which
tries to lock each reference update. So if the locking fails, we check
if the rejection type can be ignored, which is done by calling
`ref_transaction_maybe_set_rejected()`.
As the error type is now 'REF_TRANSACTION_ERROR_CASE_CONFLICT',
the specific reference update would simply be rejected, while other
updates in the transaction would continue to be applied. This allows
partial application of references in case-insensitive filesystems when
fetching colliding references.
While the earlier implementation allowed the last reference to be
applied overriding the initial references, this change would allow the
first reference to be applied while rejecting consequent collisions.
This should be an okay compromise since with the files backend, there is
no scenario possible where we would retain all colliding references.
Let's also be more proactive and notify users on case-insensitive
filesystems about such problems by providing a brief about the issue
while also recommending using the reftable backend, which doesn't have
the same issue.
Reported-by: Joe Drew <joe.drew@indexexchange.com> 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>
and edit the intro message, then it will get two copies of the Reply-To
field. gmail.com rejects such messages.
This happens because send-email reads the edited message examining the
headers. For recognised headers the content is extracted to use in
constructing the final message and for possible inclusion in the patch
emails. Unrecognised headers are gathered (in @xh) to be passed through
uninterpreted.
Unfortunately "Reply-To" is not recognised in this process so it is
added to @xh as an uninterpreted header, but also generated from the
$reply_to variable in gen_header(), resulting in two copies
Add parsing to the loop in pre_process_file() to recognise a Reply-to
header and to store the result in $reply_to. This means that the
intro message will not get a second header and also means that
any changes made to the Reply-To header during editing will be
incorporated in the $reply_to variable and so included in all the
generated email messages.
Signed-off-by: NeilBrown <neil@brown.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:26:37 +0000 (16:26 -0400)]
config: store want_color() result in a separate bool
The "git config --get-colorbool foo.bar" command not only digs in the
config to find the value of foo.bar, it evaluates the result using
want_color() to check the tty-ness of stdout.
But it stores the bool result of want_color() in the same git_colorbool
that we found in the config. This works in practice because the
git_colorbool enum is a superset of the bool values. But it is an oddity
from a type system perspective.
Let's instead store the result in a separate bool and use that.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:26:24 +0000 (16:26 -0400)]
add-interactive: retain colorbool values longer
Most of the diff code stores the decision about whether to show color as
a git_colorbool, and evaluates it at point-of-use with want_color().
This timing is important for reasons explained in daa0c3d971 (color:
delay auto-color decision until point of use, 2011-08-17).
The add-interactive code instead converts immediately to strict boolean
values using want_color(), and then evaluates those. This isn't wrong.
Even though we pass the bool values to diff_use_color(), which expects a
colorbool, the values are compatible. But it is unlike the rest of the
color code, and is questionable from a type-system perspective (but C's
typing between enums, ints, and bools is weak enough that the compiler
does not complain).
Let's switch it to the more usual way of calling want_color() at the
point of use.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:25:26 +0000 (16:25 -0400)]
color: return bool from want_color()
The point of want_color() is to take in a git_colorbool enum value and
collapse it down to a single true/false boolean, letting UNKNOWN fall
back to the color.ui default and checking isatty() for AUTO.
Let's make that more clear in the type system by returning a bool rather
than an integer.
This sadly still does not help us much with compiler warnings for using
the two types interchangeably. But it helps make the intent more clear
to a human reader.
We still retain the idempotency of want_color(), because in C a bool
true/false converts to 1/0 when converted to an integer, which
corresponds to GIT_COLOR_ALWAYS and GIT_COLOR_NEVER. So you can store
the bool in a git_colorbool and get the right result (something a few
pieces of code still do, but which we'll clean up in further patches).
Note that we rely on this same bool/int conversion for
check_auto_color(). We cache its results in a tristate int with "-1" as
"not yet set", but we can assign to it (and return it) with implicit
conversions to/from bool.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 23:13:59 +0000 (19:13 -0400)]
color: use git_colorbool enum type to store colorbools
We traditionally used "int" to store and pass around the values defined
by "enum git_colorbool" (which were originally just #define macros).
Using an int doesn't produce incorrect results, but using the actual
enum makes the intent of the code more clear.
It would be nice if the compiler could catch cases where we used the
enum and an int interchangeably, since it's very easy to accidentally
check the boolean true/false of a colorbool like:
if (branch_use_color)
This is wrong because GIT_COLOR_UNKNOWN and GIT_COLOR_AUTO evaluate to
true in C, even though we may ultimately decide not to use color. But C
is pretty happy to convert between ints and enums (even with various
-Wenum-* warnings). So this sadly doesn't protect us from such mistakes,
but it hopefully does make the code easier to read.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merges contributions made from three different addresses:
- win@wincent.com (old address, initial contributions in 2007–2009)
- greg@hurrell.net (personal address matching full name, so this one is
the "forever" address; contributions made starting in 2018)
- greg.hurrell@datadoghq.com (current work address, used for recent
contributions)
Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:22:26 +0000 (16:22 -0400)]
pretty: use format_commit_context.auto_color as colorbool
When we see "%C(auto)" as a format placeholder, we evaluate the "color"
field of our pretty_print_context to decide whether we want color. The
auto_color field of format_commit_context then stores the boolean result
of want_color(), telling us the yes/no of whether we want color.
But the resulting field is passed to various functions which expect a
git_colorbool, like diff_get_color(), that will then pass it to
want_color() again. It's not wrong to do so, since want_color() is
idempotent. But it makes it harder to reason about the types, since we
sometimes confuse colorbools and strict booleans.
Let's instead store auto_color as the original colorbool itself. We'll
have to make sure it is passed through want_color() when it is
evaluated, but there is only one such spot (right next to where we
assign it!). Every other caller just ends up passing it to get
diff_get_color() either directly or through another helper.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
At first glance this seems wrong. Usually we store the color decision as
a git_colorbool, so the second line would get confused by GIT_COLOR_AUTO
(which is boolean true, but may still mean we do not produce color).
However, the second line is correct because our caller sets color_diff
using want_color(), which collapses the colorbool to a strict true/false
boolean. The first line is _also_ correct because of the idempotence of
want_color(). Even though diff_get_color() will pass our true/false
value through want_color() again, the result will be left untouched.
But let's pass through the colorbool itself, which makes it more
consistent with the rest of the diff code. We'll need to then call
want_color() whenever we treat it as a boolean, but there is only such
spot (the one quoted above).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:20:26 +0000 (16:20 -0400)]
diff: pass o->use_color directly to fill_metainfo()
We pass the use_color parameter of fill_metainfo() as a strict boolean,
using:
want_color(o->use_color) && !pgm
to derive its value. But then inside the function, we pass it to
diff_get_color(), which expects one of the git_colorbool enum values,
and so feeds it to want_color() again.
Even though want_color() produces a strict 0/1 boolean, this doesn't
produce wrong results because want_color() is idempotent. Since
GIT_COLOR_ALWAYS and NEVER are defined as 1 and 0, and because
want_color() passes through those values, evaluating "want_color(foo)"
and "want_color(want_color(foo))" will return the same result.
But as part of a longer strategy to align the types we use for storing
these values, let's pass through the colorbool directly. To handle the
"&&" case here, we'll convert the presence of "pgm" into "NEVER", which
arguably makes the intent of the code more clear anyway.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:19:33 +0000 (16:19 -0400)]
diff: don't use diff_options.use_color as a strict bool
We disable --color-moved if color is not in use at all. This happens in
diff_setup_done(), where we set options->color_moved to 0 if
options->use_color is not true. But a strict boolean check here is not
correct; use_color could be GIT_COLOR_UNKNOWN or GIT_COLOR_AUTO, both of
which evaluate to true, even though we may later decide not to show
colors.
We should be using want_color() to convert that git_colorbool into a
true boolean. As it turns out, this does not produce wrong output. Even
though we go to the trouble to detect the moved lines, ultimately we get
the color values from diff_get_color(), which does check want_color().
And so it returns the empty string for each color, and we "color" the
result with nothing.
So the output is correct, but there is a small but measurable
performance cost to doing the line detection. E.g., in git.git before
and after this patch (there are no colors shown because hyperfine
redirects output to /dev/null):
Benchmark 1: ./git.old log --no-merges -p --color-moved -1000
Time (mean ± σ): 1.019 s ± 0.013 s [User: 0.955 s, System: 0.064 s]
Range (min … max): 1.005 s … 1.045 s 10 runs
Benchmark 2: ./git.new log --no-merges -p --color-moved -1000
Time (mean ± σ): 982.9 ms ± 14.5 ms [User: 925.8 ms, System: 57.1 ms]
Range (min … max): 965.1 ms … 1003.2 ms 10 runs
Summary
./git.new log --no-merges -p --color-moved -1000 ran
1.04 ± 0.02 times faster than ./git.old log --no-merges -p --color-moved -1000
Note that the fix is not quite as simple as just calling want_color()
from diff_setup_done(). There's a subtle timing issue that goes back to daa0c3d971 (color: delay auto-color decision until point of use,
2011-08-17), the commit that adds want_color() in the first place. As
discussed there, we must delay evaluating the colorbool value until all
pager setup is complete.
So instead, we'll leave the "color_moved" field intact in diff_setup_done(),
and modify the point where it is evaluated. Fortunately there is only
one such spot that controls whether we run any of the color-moved code
at all.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:17:19 +0000 (16:17 -0400)]
diff: simplify color_moved check when flushing
In diff_flush_patch_all_file_pairs(), we set o->emitted_symbols if and
only if o->color_moved is true. That causes the lower-level routines to
fill up o->emitted_symbols, which we then analyze in order to do the
actual colorizing.
But in that final step, we do:
if (o->emitted_symbols) {
if (o->color_moved) {
...actual coloring...
}
...clean up of emitted_symbols...
}
The inner "if" will always trigger, since we set emitted_symbols only
when doing color_moved (it is a little confusing that it is set inside
the diff_options struct, but that is for convenience of passing it to
the lower-level routines; we always clear it at the end of flushing,
since 48edf3a02a (diff: clear emitted_symbols flag after use,
2019-01-24)).
Let's simplify the code a bit by just dropping the inner "if" and
running its block unconditionally.
In theory the current code might be useful if another feature besides
color_moved setup and used emitted_symbols, but it would be easy to
refactor later to handle that. And in the meantime, this makes further
work in this area easier.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:16:00 +0000 (16:16 -0400)]
grep: don't treat grep_opt.color as a strict bool
In show_line(), we check to see if colors are desired with just:
if (opt->color)
...we want colors...
But this is incorrect. The color field here is really a git_colorbool,
so it may be "true" for GIT_COLOR_UNKNOWN or GIT_COLOR_AUTO. Either of
those _might_ end up true eventually (once we apply default fallbacks
and check stdout's tty), but they may not. E.g.:
git grep foo | cat
will enter the conditional even though we're not going to show colors.
We should collapse it into a true boolean by calling want_color().
It turns out that this does not produce a user-visible bug. We do some
extra processing to isolate the matched portion of the line in order to
colorize it, but ultimately we pass it to our output_color() helper,
which does correctly check want_color(). So we end up with no colors.
But dropping the extra processing saves a measurable amount of time. For
example, running under hyperfine (which redirects to /dev/null, and thus
does not colorize):
Benchmark 1: ./git.old grep a
Time (mean ± σ): 58.7 ms ± 3.5 ms [User: 580.6 ms, System: 74.3 ms]
Range (min … max): 53.5 ms … 67.1 ms 48 runs
Benchmark 2: ./git.new grep a
Time (mean ± σ): 35.5 ms ± 0.9 ms [User: 276.8 ms, System: 73.8 ms]
Range (min … max): 34.3 ms … 39.3 ms 79 runs
Summary
./git.new grep a ran
1.65 ± 0.11 times faster than ./git.old grep a
That's a fairly extreme benchmark, just because it will come up with a
ton of small matches, but it shows that this really does matter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:14:07 +0000 (16:14 -0400)]
color: return enum from git_config_colorbool()
The git_config_colorbool() function returns an integer which is always
one of the GIT_COLOR_* constants UNKNOWN, NEVER, ALWAYS, or AUTO. We
define these constants with macros, but let's switch to using an enum.
Even though the compiler does not strictly enforce enum/int conversions,
this should make the intent clearer to human readers. And as a bonus,
enum names are typically available to debuggers, making it more pleasant
to step through the code there.
This patch updates the return type of git_config_colorbool(), but holds
off on updating all of the callers. There's some trickiness to some of
them, and in the meantime it's perfectly fine to assign an enum into an
int.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 16 Sep 2025 20:13:28 +0000 (16:13 -0400)]
color: use GIT_COLOR_* instead of numeric constants
Long ago Git's decision to show color for a subsytem was stored in a
tri-state variable: it could be true (1), false (0), or unknown (-1).
But since daa0c3d971 (color: delay auto-color decision until point of
use, 2011-08-17) we want to carry around a new state, "auto", which
bases the decision on the tty-ness of stdout (rather than collapsing
that "auto" state to a true/false immediately).
That commit introduced a set of GIT_COLOR_* defines to represent each
state: UNKNOWN, ALWAYS, NEVER, and AUTO. But it only used the AUTO
value, and left alone code using bare 0/1/-1 values. And of course since
then we've grown many new spots that use those bare values.
Let's switch all of these to use the named constants. That should make
the code a bit easier to read, as it is more obvious that we're
representing a color decision.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 16 Sep 2025 20:36:50 +0000 (13:36 -0700)]
Merge branch 'jk/add-i-color' into jk/color-variable-fixes
* jk/add-i-color:
contrib/diff-highlight: mention interactive.diffFilter
add-interactive: manually fall back color config to color.ui
add-interactive: respect color.diff for diff coloring
stash: pass --no-color to diff plumbing child processes
Transactions are managed via the {begin,end}_odb_transaction() function
in the object-file subsystem and its implementation is specific to the
files object source. Introduce odb_transaction_{begin,commit}() in the
odb subsystem to provide an eventual object source agnostic means to
manage transactions.
Update call sites to instead manage transactions through the odb
subsystem. Also rename {begin,end}_odb_transaction() functions to
object_file_transaction_{begin,commit}() to clarify the object source it
supports.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update the names of several functions and types relocated from the
bulk-checkin subsystem for better clarity. Also drop
finish_tmp_packfile() as a standalone function in favor of embedding it
in flush_packfile_transaction() directly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The bulk-checkin subsystem provides various functions to manage ODB
transactions. Apart from {begin,end}_odb_transaction(), these functions
are only used by the object-file subsystem to manage aspects of a
transaction implementation specific to the files object source.
Relocate all the transaction code in bulk-checkin to object-file. This
simplifies the exposed transaction interface by reducing it to only
{begin,end}_odb_transaction(). Function and type names are adjusted in
the subsequent commit to better fit the new location.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Object database transactions can be explicitly flushed via
flush_odb_transaction() without actually completing the transaction.
This makes the provided transactional interface a bit awkward. Now that
there are no longer any flush_odb_transaction() call sites, drop the
function to simplify the interface and further ensure that a transaction
is only finalized when end_odb_transaction() is invoked.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/update-index: end ODB transaction when --verbose is specified
With 23a3a303 (update-index: use the bulk-checkin infrastructure,
2022-04-04), object database transactions were added to
git-update-index(1) to facilitate writing objects in bulk. With
transactions, newly added objects are instead written to a temporary
object directory and migrated to the primary object database upon
transaction commit.
When the --verbose option is specified, the subsequent set of objects
written are explicitly flushed via flush_odb_transaction() prior to
reporting the update. Flushing the object database transaction migrates
pending objects to the primary object database without marking the
transaction as complete. This is done so objects are immediately visible
to git-update-index(1) callers using the --verbose option and that rely
on parsing verbose output to know when objects are written.
Due to how git-update-index(1) parses arguments, options that come after
a filename are not considered during the object update. Therefore, it
may not be known ahead of time whether the --verbose option is present
and thus object writes are considered transactional by default until a
--verbose option is parsed.
Flushing a transaction after individual object writes negates the
benefit of writing objects to a transaction in the first place.
Furthermore, the mechanism to flush a transaction without actually
committing is rather awkward. Drop the call to flush_odb_transaction()
in favor of ending the transaction altogether when the --verbose flag is
encountered. Subsequent object writes occur outside of a transaction and
are therefore immediately visible which matches the current behavior.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ODB transactions support being nested. Only the outermost
{begin,end}_odb_transaction() start and finish a transaction. This
allows internal object write codepaths to be optimized with ODB
transactions without worrying about whether a transaction is already
active. When {begin,end}_odb_transaction() is invoked during an active
transaction, these operations are essentially treated as no-ops. This
can make the interface a bit awkward to use, as calling
end_odb_transaction() does not guarantee that a transaction is actually
ended. Thus, in situations where a transaction needs to be explicitly
flushed, flush_odb_transaction() must be used.
To remove the need for an explicit transaction flush operation via
flush_odb_transaction() and better clarify transaction semantics, drop
the transaction nesting mechanism in favor of begin_odb_transaction()
returning a NULL transaction value to signal it was a no-op, and
end_odb_transaction() behaving as a no-op when a NULL transaction value
is passed. This is safe for existing callers as the transaction value
wired to end_odb_transaction() already comes from
begin_odb_transaction() and thus continues the same no-op behavior when
a transaction is already pending. With this model, passing a pending
transaction to end_odb_transaction() ensures it is committed at that
point in time.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
With the current implementation of 'git sparse-checkout clean', we
notice that a file that was in a conflicted state does not get cleaned
up because of some internal details around the SKIP_WORKTREE bit.
This test is documenting the current behavior before we update it in the
following change.
Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In my experience, the most-common reason that the sparse index must
expand to a full one is because there is some leftover file in a tracked
directory that is now outside of the sparse-checkout. The new 'git
sparse-checkout clean' command will find and delete these directories,
so point users to it when they hit the sparse index expansion advice.
Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'git sparse-checkout clean' subcommand is focused on directories,
deleting any tracked sparse directories to clean up the worktree and
make the sparse index feature work optimally.
However, this directory-focused approach can leave users wondering why
those directories exist at all. In my experience, these files are left
over due to ignore or exclude patterns, Windows file handles, or
possibly merge conflict resolutions.
Add a new '--verbose' option for users to see all the files that are
being deleted (with '--force') or would be deleted (with '--dry-run').
Based on usage, users may request further context on this list of files for
states such as tracked/untracked, unstaged/staged/conflicted, etc.
Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 15 Sep 2025 15:52:06 +0000 (08:52 -0700)]
Merge branch 'ps/upload-pack-oom-protection'
A broken or malicious "git fetch" can say that it has the same
object for many many times, and the upload-pack serving it can
exhaust memory storing them redundantly, which has been corrected.
Junio C Hamano [Mon, 15 Sep 2025 15:52:06 +0000 (08:52 -0700)]
Merge branch 'ds/midx-write-fixes'
Fixes multiple crashes around midx write-out codepaths.
* ds/midx-write-fixes:
midx-write: simplify error cases
midx-write: reenable signed comparison errors
midx-write: use uint32_t for preferred_pack_idx
midx-write: use cleanup when incremental midx fails
midx-write: put failing response value back
midx-write: only load initialized packs
Junio C Hamano [Mon, 15 Sep 2025 15:52:05 +0000 (08:52 -0700)]
Merge branch 'jt/de-global-bulk-checkin'
The bulk-checkin code used to depend on a file-scope static
singleton variable, which has been updated to pass an instance
throughout the callchain.
* jt/de-global-bulk-checkin:
bulk-checkin: use repository variable from transaction
bulk-checkin: require transaction for index_blob_bulk_checkin()
bulk-checkin: remove global transaction state
bulk-checkin: introduce object database transaction structure
Michael Rappazzo [Sat, 13 Sep 2025 11:31:51 +0000 (07:31 -0400)]
gitk: fix error when remote tracking branch is deleted
When a remote tracking branch is deleted (e.g., via 'git push --delete
origin branch'), the headids array entry for that branch is removed, but
upstreamofref may still reference it. This causes gitk to show an error
and prevents the Tags and Heads view from opening.
Fix by checking that headids($upstreamofref($n)) exists before accessing
it in the refill_reflist function.
Signed-off-by: Michael Rappazzo <rappazzo@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Instead of scanning for the remaining items to see if there are
still commits to be explored in the queue, use khash to remember
which items are still on the queue (an unacceptable alternative is
to reserve one object flag bits).
* rs/describe-with-lazy-queue-and-oidset:
describe: use oidset in finish_depth_computation()
Windows "real-time monitoring" interferes with the execution of
tests and affects negatively in both correctness and performance,
which has been disabled in Gitlab CI.
* ps/gitlab-ci-disable-windows-monitoring:
gitlab-ci: disable realtime monitoring to unbreak Windows jobs
Junio C Hamano [Fri, 12 Sep 2025 17:41:19 +0000 (10:41 -0700)]
Merge branch 'ms/refs-exists'
"git refs exists" that works like "git show-ref --exists" has been
added.
* ms/refs-exists:
t: add test for git refs exists subcommand
t1422: refactor tests to be shareable
t1403: split 'show-ref --exists' tests into a separate file
builtin/refs: add 'exists' subcommand
Junio C Hamano [Fri, 12 Sep 2025 17:41:18 +0000 (10:41 -0700)]
Merge branch 'ps/object-store-midx-dedup-info'
Further code clean-up for multi-pack-index code paths.
* ps/object-store-midx-dedup-info:
midx: compute paths via their source
midx: stop duplicating info redundant with its owning source
midx: write multi-pack indices via their source
midx: load multi-pack indices via their source
midx: drop redundant `struct repository` parameter
odb: simplify calling `link_alt_odb_entry()`
odb: return newly created in-memory sources
odb: consistently use "dir" to refer to alternate's directory
odb: allow `odb_find_source()` to fail
odb: store locality in object database sources
Junio C Hamano [Fri, 12 Sep 2025 17:41:18 +0000 (10:41 -0700)]
Merge branch 'je/doc-add'
Documentation for "git add" has been updated.
* je/doc-add:
doc: rephrase the purpose of the staging area
doc: git-add: simplify discussion of ignored files
doc: git-add: clarify intro & add an example
There is sometimes a need to visit every file within a directory,
recursively. The main example is remove_dir_recursively(), though it has
some extra flags that make it want to iterate over paths in a custom
way. There is also the fill_directory() approach but that involves an
index and a pathspec.
This change adds a new for_each_file_in_dir() method that will be
helpful in the next change.
Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'git sparse-checkout clean' subcommand is somewhat similar to 'git
clean' in that it will delete files that should not be in the worktree.
The big difference is that it focuses on the directories that should not
be in the worktree due to cone-mode sparse-checkout. It also does not
discriminate in the kinds of files and focuses on deleting entire
directories.
However, there are some restrictions that would be good to bring over
from 'git clean', specifically how it refuses to do anything without the
'-f'/'--force' or '-n'/'--dry-run' arguments. The 'clean.requireForce'
config can be set to 'false' to imply '--force'.
Add this behavior to avoid accidental deletion of files that cannot be
recovered from Git.
Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When users change their sparse-checkout definitions to add new
directories and remove old ones, there may be a few reasons why
directories no longer in scope remain (ignored or excluded files still
exist, Windows handles are still open, etc.). When these files still
exist, the sparse index feature notices that a tracked, but sparse,
directory still exists on disk and thus the index expands. This causes a
performance hit _and_ the advice printed isn't very helpful. Using 'git
clean' isn't enough (generally '-dfx' may be needed) but also this may
not be sufficient.
Add a new subcommand to 'git sparse-checkout' that removes these
tracked-but-sparse directories.
The implementation details provide a clear definition of what is happening,
but it is difficult to describe this without including the internal
implementation details. The core operation converts the index to a sparse
index (in memory if not already on disk) and then deletes any directories in
the worktree that correspond with a sparse directory entry in that sparse
index.
In the most common case, this means that a file will be removed if it is
contained within a directory that is both tracked and outside of the
sparse-checkout definition. However, there can be exceptions depending on
the current state of the index:
* If the worktree has a modification to a tracked, sparse file, then that
file's parent directories will be expanded instead of represented as
sparse directories. Siblings of those parent directories may be
considered sparse.
* If the user staged a sparse file with "git add --sparse", then that file
loses the SKIP_WORKTREE bit until the sparse-checkout is reapplied. Until
then, that file's parent directories are not represented as sparse
directory entries and thus will not be removed. Siblings of those parent
directories may be considered sparse. (There may be other reasons why
the SKIP_WORKTREE bit was removed for a file and this impact on the
sparse directories will apply to those as well.)
* If the user has a merge conflict outside of the sparse-checkout
definition, then those conflict entries prevent the parent directories
from being represented as sparse directory entries and thus are not
removed.
* The cases above present reasons why certain _file conditions_ will impact
which _directories_ are considered sparse. The list of tracked
directories that are outside of the sparse-checkout definition but not
represented as a sparse directory further reduces the list of files that
will be removed.
For these complicated reasons, the documentation details a potential list of
files that will be "considered for removal" instead of defining the list
concretely. The special cases can be handled by resolving conflicts,
committing staged changes, and running 'git sparse-checkout reapply' to
update the SKIP_WORKTREE bits as expected by the sparse-checkout definition.
It is important to make clear that this operation will remove ignored and
excluded files which would normally be ignored even by 'git clean -f' unless
the '-x' or '-X' option is provided. This is the most extreme method for
doing this, but it works when the sparse-checkout is in cone mode and is
expected to rescope based on directories, not files.
The current implementation always deletes these sparse directories
without warning. This is unacceptable for a released version, but those
features will be added in changes coming immediately after this one.
Note that this will not remove an untracked directory (or any of its
contents) if its parent is a tracked directory within the sparse-checkout
definition. This is required to prevent removing data created by tools that
perform caching operations for editors or build tools.
Thus, 'git sparse-checkout clean' is both more aggressive and more careful
than 'git clean -fx':
* It is more aggressive because it will remove _tracked_ files within the
sparse directories.
* It is less aggressive because it will leave _untracked_ files that are
not contained in sparse directories.
These special cases will be handled more explicitly in a future change that
expands tests for the 'git sparse-checkout clean' command. We handle some of
the modified, staged, and committed states including some impact on 'git
status' after cleaning.
Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic for the 'git sparse-checkout' builtin uses the_repository all
over the place, despite some use of a repository struct in different
method parameters. Complete this removal of the_repository by using
'repo' when possible.
In one place, there was already a local variable 'r' that was set to
the_repository, so move that to a method parameter.
We cannot remove the USE_THE_REPOSITORY_VARIABLE declaration as we are
still using global constants for the state of the sparse-checkout.
Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ci: don't compile whole project when testing docs with Meson
Our "documentation" CI jobs, unsurprisingly, performs a couple of tests
on our documentation. The job knows to not only test the documentation
generated by our Makefile, but also by Meson.
In the latter case with Meson we end up building the whole project,
including all of the binaries. This is of course quite excessive and a
waste of compute cycles, as we don't care about these binaries at all.
Fix this by using the new "docs" target that we introduced in the
preceding commit.
Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our documentation can be built with either Asciidoc or Asciidoctor as
backend. When Meson is configured to build documentation, then it will
automatically detect which of these tools is available and use them.
It's not obvious to the user though which of these backends is used
unless the user explicitly asks for one backend via `-Ddocs_backend=`.
Improve the status quo by printing the docs backend as part of the
"backends" summary.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
meson: introduce a "docs" alias to compile documentation only
Meson does not currently provide a target to compile documentation,
only. Instead, users needs to compile the whole project, which may be
way more than they really intend to do.
Introduce a new "docs" alias to plug this gap. This alias can be invoked
e.g. with `meson compile docs`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the Git 2.51 release cycle we've refactored the object database layer
to access objects via `struct object_database` directly. To make the
transition a bit easier we have retained some of the old-style functions
in case those were widely used.
Now that Git 2.51 has been released it's time to clean up though and
drop these old wrappers. Do so and adapt the small number of newly added
users to use the new functions instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update clar to fcbed04 (Merge pull request #123 from
pks-gitlab/pks-sandbox-ubsan, 2025-09-10). The most significant changes
since the last version include:
- Fixed platform support for HP-UX.
- Fixes for how clar handles the `-q` flag.
- A couple of leak fixes for reported clar errors.
- A new `cl_invoke()` function that retains line information.
- New infrastructure to create temporary directories.
- Improved printing of error messages so that all lines are now
properly indented.
- Proper selftests for the clar.
Most of these changes are somewhat irrelevant to us, but neither do we
have to adjust to any of these changes, either. What _is_ interesting to
us though is especially the fixed support for HP-UX, and eventually we
may also want to use `cl_invoke()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Colin Stagner [Wed, 10 Sep 2025 03:11:24 +0000 (22:11 -0500)]
contrib/subtree: fix split with squashed subtrees
98ba49ccc2 (subtree: fix split processing with multiple subtrees
present, 2023-12-01) increases the performance of
git subtree split --prefix=subA
by ignoring subtree merges which are outside of `subA/`. It also
introduces a regression. Subtree merges that should be retained
are incorrectly ignored if they:
1. are nested under `subA/`; and
2. are merged with `--squash`.
is erroneously ignored during a split of `subA`. This causes
missing tree files and different commit hashes starting in
git v2.44.0-rc0.
The method:
should_ignore_subtree_split_commit REV
should test only a single commit REV, but the combination of
git log -1 --grep=...
actually searches all *parent* commits until a `--grep` match is
discovered.
Rewrite this method to test only one REV at a time. Extract commit
information with a single `git` call as opposed to three. The
`test` conditions for rejecting a commit remain unchanged.
Unit tests now cover nested subtrees.
Signed-off-by: Colin Stagner <ask+git@howdoi.land> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
doc: fast-import: replace literal block with paragraph
68061e34702 (fast-import: disallow "feature export-marks" by default,
2019-08-29) added the documentation for this option. The second
paragraph is a literal block but it looks like it should just be
a regular paragraph.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
From user feedback on this section: 3 users don't know what "tree-ish"
means and 3 users don't know what "pathspec" means. One user also says
that the section is very confusing and that they don't understand what
the "index" is.
From conversations on Mastodon, several users said that their impression
is that "the index" means the same thing as "HEAD". It would be good to
give those users (and other users who do not know what "index" means) a
hint as to its meaning.
Make this section more accessible to users who don't know what the terms
"pathspec", "tree-ish", and "index" mean by using more familiar language,
adding examples, and using simpler sentence structures.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Julia Evans [Wed, 10 Sep 2025 19:14:28 +0000 (19:14 +0000)]
doc: git-checkout: split up restoring files section
From user feedback: one user mentioned that "When the <tree-ish> (most
often a commit) is not given" is confusing since it starts with a
negative.
Restructuring so that `git checkout main file.txt` and
`git checkout file.txt` are separate items will help us simplify the
sentence structure a lot.
As a bonus, it appears that `-f` actually only applies to one of those
forms, so we can include fewer options, and now the structure of the
DESCRIPTION matches the SYNOPSIS.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
From user feedback: several users say they don't understand the use case
for `--detach`. It's probably not realistic to explain the use case for
detached HEAD state here, but we can improve the situation.
Explain how `git checkout --detach` is different from
`git checkout <branch>` instead of copying over the description from
`git checkout <branch>`, since `git checkout <branch>` will be a
familiar command to many readers.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Julia Evans [Wed, 10 Sep 2025 19:14:26 +0000 (19:14 +0000)]
doc: git-checkout: clarify `-b` and `-B`
From user feedback: several users reported having trouble understanding
the difference between `-b` and `-B` ("I think it's because my brain
expects it to contrast with `-b`, but instead it starts off explaining
how they're the same").
Also, in `-B`, 2 users can't tell what the branch is reset *to*.
Simplify the sentence structure in the explanations of `-b` and `-B` and
add a little extra information (what `<start-point>` is, what the branch
is reset to).
Splitting up `-b` and `-B` into separate items helps simplify the
sentence structure since there's less "In this case...".
Replace the long "the branch is not reset/created unless "git checkout"
is successful..." with just "will fail", since we should generally
assume that Git will fail operations in a clean way and not leave
operations half-finished, and that cases where it does not fail cleanly
are the exceptions that the documentation should flag.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
From user feedback: several users commented that "Local modifications
to the files in the working tree are kept, so that they can be committed
to the <branch>." didn't seem accurate to them, since
`git checkout <branch>` will often fail.
One user also thought that "... and by pointing HEAD at the branch"
was something that _they_ had to do somehow ("How do I point HEAD at
a branch?") rather than a description of what the `git checkout`
operation is doing for them.
Explain when `git checkout <branch>` will fail and clarify that
"pointing HEAD at the branch" is part of what the command does.
6 users commented that the "You could omit <branch>..." section is
extremely confusing. Explain this in a much more direct way.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's no need to use the terms "pathspec" or "tree-ish" in the
ARGUMENT DISAMBIGUATION section, which are terms that (from user
feedback on this page) many users do not understand.
"tree-ish" is actually not accurate here: `git checkout` in this case
takes a commit-ish, not a tree-ish. So we can say "branch or commit"
instead of "tree-ish" which is both more accurate and uses more familiar
terms.
And now that the intro to the man pages mentions that `git checkout` has
"two main modes", it makes sense to refer to this disambiguation section
to understand how Git decides which one to use when there's an overlap
in syntax.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Julia Evans [Wed, 10 Sep 2025 19:14:23 +0000 (19:14 +0000)]
doc: git-checkout: clarify intro sentence
From user feedback: in the first paragraph, 5 users reported not
understanding the terms "pathspec" and 1 user reported not understanding
the term "HEAD". Of the users who said they didn't know what "pathspec"
means, 3 said they couldn't understand what the paragraph was trying to
communicate as a result.
One user also commented that "If no pathspec was given..." makes
`git checkout <branch>` sounds like a special edge case, instead of
being one of the most common ways to use this core Git command.
It looks like the goal of this paragraph is to communicate that `git
checkout` has two different modes: one where you switch branches and one
where you just update your working directory files/index. So say that
directly, and use more familiar language (including examples) to say it.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Wed, 10 Sep 2025 17:16:30 +0000 (19:16 +0200)]
use repo_get_oid_with_flags()
get_oid_with_context() allows specifying flags and reports object
details via a passed-in struct object_context. Some callers just want
to specify flags, but don't need any details back. Convert them to
repo_get_oid_with_flags(), which provides just that and frees them from
dealing with the context structure.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 10 Sep 2025 21:28:23 +0000 (14:28 -0700)]
Merge branch 'master' of https://github.com/j6t/git-gui
* 'master' of https://github.com/j6t/git-gui:
git-gui: sync Makefiles with git.git
git-gui: fix error handling of Revert Changes command
git-gui--askyesno (mingw): use Git for Windows' icon, if available
git-gui--askyesno: allow overriding the window title
git gui: set GIT_ASKPASS=git-gui--askpass if not set yet
git-gui: provide question helper for retry fallback on Windows
git-gui: simplify using nice(1)
git-gui: simplify PATH de-duplication
Junio C Hamano [Wed, 10 Sep 2025 21:27:52 +0000 (14:27 -0700)]
Merge branch 'master' of https://github.com/j6t/gitk
* 'master' of https://github.com/j6t/gitk:
gitk: add README with usage, build, and contribution details
gitk: fix trackpad scrolling for Tcl/Tk 8.7+
gitk: use <Button-3> for ctx menus on macOS with Tcl 8.7+
As the tests are all run in separate repositories, set the branch
name to "master" when creating the repository for the tests where
the result depends on the branch name. In order to make it easier to
change the branch name in the future a helper function is used. This
reduces the number of tests that depend on the default branch name
being "master" and removes the last instance of a test file using
"GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master".
Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the penultimate use of "GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=
master" in our test suite. We have slowly been removing these ever
since we started to switch the default branch name used in tests to
"main".
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove one of the last remaining uses of
"TEST_GIT_DEFAULT_INITIAL_BRANCH= main" in the test suite. We have
been steadily be converting tests from using "master" as the default
branch name since the introduction of TEST_GIT_DEFAULT_INITIAL_BRANCH
in 704fed9ea22 (tests: start moving to a different default main branch
name, 2020-10-23) The changes here are purely mechanical replacing
"master" with "main"
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 1296cbe4b46 (init: document `init.defaultBranch` better,
2020-12-11) "git-init.adoc" has advertised that the default name
of the initial branch may change in the future. The name "main"
is chosen to match the default used by the big Git forge web sites.
The advice printed when init.defaultBranch is not set is updated
to say that the default will change to "main" in Git 3.0. Building
with WITH_BREAKING_CHANGES enabled removes the advice and changes
the default branch name to "main". The code in guess_remote_head()
that looks for "refs/heads/master" is left unchanged as that is only
called when the remote server does not support the symref capability
in the v0 protocol or the symref extension to the ls-refs list in the
v2 protocol. Such an old server is more likely to be using "master"
as the default branch name.
With the exception of the "git-init.adoc" the documentation is left
unchanged. I had hoped to parameterize the name of the default branch
by using an asciidoc attribute. Unfortunately attribute expansion
is inhibited by backticks and we use backticks to mark up ref names
so that idea does not work. As the changes to git-init.adoc show
inserting ifdef's around each instance of the branch name "master"
is cumbersome and makes the documentation sources harder to read.
Apart from "git-init.adoc" there are some other files where "master" is
used as the name of the initial branch rather than as an example of a
branch name such as "user-manual.adoc" and "gitcore-tutorial.adoc". The
name appears a lot in those so updating it with ifdef's is not really
practical. We can update that document in the 3.0 release cycle. The
other documentation where master is used as an example branch name
can be gradually converted over time.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 9 Sep 2025 21:46:00 +0000 (14:46 -0700)]
Merge branch 'jt/de-global-bulk-checkin' into jt/odb-transaction
* jt/de-global-bulk-checkin:
bulk-checkin: use repository variable from transaction
bulk-checkin: require transaction for index_blob_bulk_checkin()
bulk-checkin: remove global transaction state
bulk-checkin: introduce object database transaction structure
Junio C Hamano [Mon, 8 Sep 2025 21:54:35 +0000 (14:54 -0700)]
Merge branch 'tc/last-modified'
A new command "git last-modified" has been added to show the closest
ancestor commit that touched each path.
* tc/last-modified:
last-modified: use Bloom filters when available
t/perf: add last-modified perf script
last-modified: new subcommand to show when files were last modified
Junio C Hamano [Mon, 8 Sep 2025 21:54:34 +0000 (14:54 -0700)]
Merge branch 'ds/ls-files-lazy-unsparse'
"git ls-files <pathspec>..." should not necessarily have to expand
the index fully if a sparsified directory is excluded by the
pathspec; the code is taught to expand the index on demand to avoid
this.
* ds/ls-files-lazy-unsparse:
ls-files: conditionally leave index sparse
Junio C Hamano [Mon, 8 Sep 2025 21:54:34 +0000 (14:54 -0700)]
Merge branch 'am/xdiff-hash-tweak'
Inspired by Ezekiel's recent effort to showcase Rust interface, the
hash function implementation used to hash lines have been updated
to the one used for ELF symbol lookup by Glibc.
When the README for diff-highlight was written, there was no way to
trigger it for the `add -p` interactive patch mode. We've since grown a
feature to support that, but it was documented only on the Git side.
Let's also let people coming the other direction, from diff-highlight,
know that it's an option.
Suggested-by: Isaac Oscar Gariano <IsaacOscar@live.com.au> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Mon, 8 Sep 2025 16:42:39 +0000 (12:42 -0400)]
add-interactive: manually fall back color config to color.ui
Color options like color.interactive and color.diff should fall back to
the value of color.ui if they aren't set. In add-interactive, we check
the specific options (e.g., color.diff) via repo_config_get_value(),
which does not depend on the main command having loaded any color config
via the git_config() callback mechanism.
But then we call want_color() on the result; if our specific config is
unset then that function uses the value of git_use_color_default. That
variable is typically set from color.ui by the git_color_config()
callback, which is called by the main command in its own git_config()
callback function.
This works fine for "add -p", whose add_config() callback calls into
git_color_config(). But it doesn't work for other commands like
"checkout -p", which is otherwise unaware of color at all. People tend
not to notice because the default is "auto", and that's what they'd set
color.ui to as well. But something like:
git -c color.ui=false checkout -p
should disable color, and it doesn't.
This regression goes back to 0527ccb1b5 (add -i: default to the built-in
implementation, 2021-11-30). In the perl version we got the color config
from "git config --get-colorbool", which did the full lookup for us.
The obvious fix is for git-checkout to add a call to git_color_config()
to its own config callback. But we'd have to do so for every command
with this problem, which is error-prone. Let's see if we can fix it more
centrally.
It is tempting to teach want_color() to look up the value of
repo_config_get_value("color.ui") itself. But I think that would have
disastrous consequences. Plumbing commands, especially older ones, avoid
porcelain config like "color.*" by simply not parsing it in their config
callbacks. Looking up the value of color.ui under the hood would
undermine that.
Instead, let's do that lookup in the add-interactive setup code. We're
already demand-loading other color config there, which is probably fine
(even in a plumbing command like "git reset", the interactive mode is
inherently porcelain-ish). That catches all commands that use the
interactive code, whether they were calling git_color_config()
themselves or not.
Reported-by: Isaac Oscar Gariano <isaacoscar@live.com.au> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Mon, 8 Sep 2025 16:42:36 +0000 (12:42 -0400)]
add-interactive: respect color.diff for diff coloring
The old perl git-add--interactive.perl script used the color.diff config
option to decide whether to color diffs (and if not set, it fell back to
the value of color.ui via git-config's --get-colorbool option). When we
switched to the builtin version, this was lost: we respect only
color.ui. So for example:
git -c color.diff=false add -p
would color the diff, even when it should not.
The culprit is this line in add-interactive.c's parse_diff():
if (want_color_fd(1, -1))
That "-1" means "no config has been set", which causes it to fall back
to the color.ui setting. We should instead be passing the value of
color.diff. But the problem is that we never even parse that config
option!
Instead the builtin interactive code parses only the value of
color.interactive, which is used for prompts and other messages. One
could perhaps argue that this should cover interactive diff coloring,
too, but historically it did not. The perl script treated
color.interactive and color.diff separately. So we should grab the
values for both, keeping separate fields in our add_i_state variable,
rather than a single use_color field.
We also load individual color slots (e.g., color.interactive.prompt),
leaving them as the empty string when color is disabled. This happens
via the init_color() helper in add-interactive, which checks that
use_color field. Now that there are two such fields, we need to pass the
appropriate one for each color.
The colors are mostly easy to divide up; color.interactive.* follows
color.interactive, and color.diff.* follows color.diff. But the "reset"
color is tricky. It is used for both types of coloring, but the two can
be configured independently. So we introduce two separate reset colors,
and use each in the appropriate spot.
There are two new tests. The first enables interactive prompt colors but
disables color.diff. We should see a colored prompt but not a colored
diff, showing that we are now respecting color.diff (and not
color.interactive or color.ui).
The second does the opposite. We disable color.interactive but turn on
color.diff with a custom fragment color. When we split a hunk, the
interactive code has to re-color the hunk header, which lets us check
that we correctly loaded the color.diff.frag config based on color.diff,
not color.interactive.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Mon, 8 Sep 2025 16:42:32 +0000 (12:42 -0400)]
stash: pass --no-color to diff plumbing child processes
After a partial stash, we may clear out the working tree by capturing
the output of diff-tree and piping it into git-apply (and likewise we
may use diff-index to restore the index). So we most definitely do not
want color diff output from that diff-tree process. And it normally
would not produce any, since its stdout is not going to a tty, and the
default value of color.ui is "auto".
However, if GIT_PAGER_IN_USE is set in the environment, that overrides
the tty check, and we'll produce a colorized diff that chokes git-apply:
$ echo y | GIT_PAGER_IN_USE=1 git stash -p
[...]
Saved working directory and index state WIP on main: 4f2e2bb foo
error: No valid patches in input (allow with "--allow-empty")
Cannot remove worktree changes
Setting this variable is a relatively silly thing to do, and not
something most users would run into. But we sometimes do it in our tests
to stimulate color. And it is a user-visible bug, so let's fix it rather
than work around it in the tests.
The root issue here is that diff-tree (and other diff plumbing) should
probably not ever produce color by default. It does so not by parsing
color.ui, but because of the baked-in "auto" default from 4c7f1819b3
(make color.ui default to 'auto', 2013-06-10). But changing that is
risky; we've had discussions back and forth on the topic over the years.
E.g.:
So let's accept that as the status quo for now and protect ourselves by
passing --no-color to the child processes. This is the same thing we did
for add-interactive itself in 1c6ffb546b (add--interactive.perl: specify
--no-color explicitly, 2020-09-07).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
promisor-remote: use string_list_split() in mark_remotes_as_accepted()
Previous commits replaced some strbuf_split*() calls with calls to
string_list_split*() in "promisor-remote.c".
For consistency, let's also replace the strbuf_split_str() call in
mark_remotes_as_accepted() with a call to string_list_split(), as we
don't need the splitted strings to be managed by a `struct strbuf`.
Using the lighter-weight `string_list` API is enough for our needs.
While at it let's remove a useless call to `strbuf_strip_suffix()`.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A previous commit allowed a server to pass additional fields through
the "promisor-remote" protocol capability after the "name" and "url"
fields, specifically the "partialCloneFilter" and "token" fields.
Let's make it possible for a client to check if these fields match
what it expects before accepting a promisor remote.
We allow this by introducing a new "promisor.checkFields"
configuration variable. It should contain a comma or space separated
list of fields that will be checked.
By limiting the protocol to specific well-defined fields, we ensure
both server and client have a shared understanding of field
semantics and usage.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
promisor-remote: use string_list_split() in filter_promisor_remote()
A previous commit introduced a new parse_one_advertised_remote()
function that takes a `const char *` argument. This function is called
from filter_promisor_remote() and parses all the fields for one remote.
This means that in filter_promisor_remote() we no longer need to split
the remote information that will be passed to
parse_one_advertised_remote() into an array of relatively heavy and
complex `struct strbuf`.
To use something lighter, let's then replace strbuf_split_str() with
string_list_split() in filter_promisor_remote() to parse the remote
information that is passed to parse_one_advertised_remote().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
promisor-remote: refactor how we parse advertised fields
In a follow up commit we are going to parse more fields, like a filter
and a token, coming from the server when it advertises promisor remotes
using the "promisor-remote" capability.
To prepare for this, let's refactor the code that parses the advertised
fields coming from the server into a new parse_one_advertised_remote()
function that will populate a `struct promisor_info` with the content
of the fields it parsed.
While at it, let's also pass this `struct promisor_info` to the
should_accept_remote() function, instead of passing it the parsed name
and url.
These changes will make it simpler to both parse more fields and access
the content of these parsed fields in follow up commits.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
promisor-remote: use string constants for 'name' and 'url' too
A previous commit started to define `promisor_field_filter` and
`promisor_field_token`, and used them instead of the
"partialCloneFilter" and "token" string literals.
Let's do the same for "name" and "url" to avoid repeating them
several times and for consistency with the other fields.
For skipping "name=" or "url=" in advertisements, let's introduce
a skip_field_name_prefix() helper function to keep parsing clean
and easy to understand.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>