[pull] master from git:master#185
Merged
pull[bot] merged 30 commits intoturkdevops:masterfrom Mar 28, 2026
Merged
Conversation
"git apply" has an option -p that takes an integer as its argument. Unfortunately the function apply_option_parse_p() in charge of parsing this argument uses atoi() to convert from string to integer, which allows a non-digit after the number (e.g. "1q") to be silently ignored. As a consequence, an argument that does not begin with a digit silently becomes a zero. Despite this command working fine when a non-positive argument is passed, it might be useful for the end user to know that their input contains non-digits that might've been unintended. Replace atoi() with strtol_i() to catch malformed inputs. Signed-off-by: Mirko Faina <mroik@delayed.space> Signed-off-by: Junio C Hamano <gitster@pobox.com>
See e.g. 0ae23ab (doc: convert git worktree to synopsis style, 2025-10-05) for the markup rules for this style. There aren’t many subtleties to the transformation of this doc since it doesn’t use any advanced constructs. The only thing is that "`:`{nbsp}" is used instead of `': '` to refer to effective inline-verbatim with a space (␠).[1] I also use (_) for emphasis although (') gives the same result. Also prefer linking to Git commands instead of saying e.g. `git format-patch`. But for this command we can type out git-interpret- trailers(1) to avoid a self-reference. Also replace camel case `<keyAlias>` with kebab case `<key-alias>`. And while doing that make sure to replace `trailer.*` with `trailer.<key-alias>`. † 1: Similar to "`tag:`{nbsp}" in `Documentation/pretty-formats.adoc` Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some negated options are missing according to `git interpret-trailers -h`. Also normalize to the “stuck form” (see gitcli(7)) like what was done in 806337c (doc: notes: use stuck form throughout, 2025-05-27).[1] Also normalize the order of the regular and negated options according to the current convention.[2] Also note that `--no-trailer` will reset the list. † 1: See also https://lore.kernel.org/git/6f7d027e-088a-4d66-92af-b8d1c32d730c@app.fastmail.com/ † 2: https://lore.kernel.org/git/xmqqcyct1mtq.fsf@gitster.g/ Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert this part of the configuration documentation to synopsis style so that all of git-interpret-trailers(1) is consistent. See the commit message from two commits ago. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use `<key-alias>` instead of `*` in order to be consistent with the documentation. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "reference-transaction" hook is invoked multiple times during a ref transaction. Each invocation corresponds to a different phase: - The "prepared" phase indicates that references have been locked. - The "committed" phase indicates that all updates have been written to disk. - The "aborted" phase indicates that the transaction has been aborted and that all changes have been rolled back. This hook can be used to learn about the updates that Git wants to perform. For example, forges use it to coordinate reference updates across multiple nodes. However, the phases are insufficient for some specific use cases. The earliest observable phase in the "reference-transaction" hook is "prepared", at which point Git has already taken exclusive locks on every affected reference. This makes it suitable for last-chance validation, but not for serialization. So by the time a hook sees the "prepared" phase, it has no way to defer locking, and thus it cannot rearrange multiple concurrent ref transactions relative to one another. Introduce a new "preparing" phase that runs before the "prepared" phase, that is before Git acquires any reference lock on disk. This gives callers a well-defined window to perform validation, enable higher-level ordering of concurrent transactions, or reject the transaction entirely, all without interfering with the locking state. This change is strictly speaking not backwards compatible. Existing hook scripts that do not know how to handle unknown phases may treat 'preparing' as an error and return non-zero. But the hook is considered to expose internal implementation details of how Git works, and as such we have been a bit more lenient with changing its exact semantics, like for example in a8ae923 (refs: support symrefs in 'reference-transaction' hook, 2024-05-07). An alternative would be to introduce a "reference-transaction-v2" hook that knows about the new phase. This feels like a rather heavy-weight option though, and was thus discarded. Helped-by: Patrick Steinhardt <ps@pks.im> Helped-by: Justin Tobler <jltobler@gmail.com> Helped-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A prio_queue with a NULL compare function acts as a stack -- the last element in is the first one out (LIFO). Use an actual commit_stack instead where possible, as it documents the behavior better, provides type safety and saves some memory because prio_queue stores an additional tie-breaking counter per element. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `--object-id` option was added in commit e1068f0 (merge-file: add an option to process object IDs, 2023-11-01) together with a call to setup_git_directory() to avoid crashing when run outside a repository. However, the call to setup_git_directory() is redundant when run inside a repository, as merge-file runs with RUN_SETUP_GENTLY, so the repository has already been set up. The redundant call is harmless when linked worktrees are not used, but in a linked worktree, the repo_set_gitdir() function ends up being called twice. Calling repo_set_gitdir() used to be silently accepted, but commit 2816b74 (odb: handle changing a repository's commondir, 2025-11-19) changed this to a BUG in repository.c with the error message: "cannot reinitialize an already-initialized object directory". Guard the redundant call to setup_git_directory() behind a repo pointer check, to ensure that we continue to give the correct "not a git repo" error whilst avoiding the BUG when running in a linked worktree. Signed-off-by: Mathias Rav <m@git.strova.dk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Gabriel “gabldotink” <gabl@gabl.ink> Signed-off-by: Junio C Hamano <gitster@pobox.com>
According to its readme, the "contrib/" directory's main intent is to
collect stuff that is not an official part of Git, either because it is
too specialized or because it is still considered experimental. The
reality tells a bit of a different story though: while it _does_ contain
such things, it also contains other things:
- Our credential helpers, which are being distributed by many
packagers nowadays and which can be considered "stable".
- A bunch of tooling that relates to our build and test
infrastructure.
Especially the second category is somewhat of a sore spot. You really
wouldn't expect build-related tooling to be considered an optional part
of Git. Quite the opposite.
Create a new top-level "tools/" directory to fix this discrepancy. This
directory will contain all kind of tools that are related to our build
infrastructure and that Git developers are likely to use day to day.
For now, this directory doesn't contain anything yet except for a
readme and a Meson skeleton. This will change in subsequent commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The Coccinelle tool is an ingrained part of our build infrastructure. It is executed by our CI to detect antipatterns and is used to detect misuses of certain interfaces. It's presence in "contrib/" is thus rather misleading. Promote the configuration into the new "tools/" directory. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "coverage-diff.sh" script can be used to get information about test coverage fro the Git codebase. It is thus rather specific to our build and test infrastructure and part of the developer-facing tooling. The fact that this script is part of "contrib/" is thus rather misleading and a historic wart. Promote the tool into the new "tools/" directory. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "update-unicode.sh" script is used to update the unicode data compiled into Git whenever a new version of the Unicode standard has been released. As such, it is a natural part of our developer-facing tooling, and its presence in "contrib/" is misleading. Promote the script into the new "tools/" directory. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have a bunch of scripts used by our different build systems that are all located in the top-level directory. Now that we have introduced the new "tools/" directory though we have a better home for them. Move the scripts into the "tools/" directory. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "git-compat-util.h" header is supposed to be the first header included by every code compilation unit. As such, a subsequent commit will start to precompile this header to speed up compilation of Git. This will cause an issue though with the way that we have set up the "-Wsign-compare" warnings. It is expected that any compilation unit that fails with that compiler warning sets `DISABLE_SIGN_COMPARE_WARNINGS` before including "git-compat-util.h". If so, we'll disable the warning right away via a compiler pragma. But with precompiled headers we do not know ahead of time whether the code unit wants to disable those warnings, and thus we'll have to precompile the header without defining `DISABLE_SIGN_COMPARE_WARNINGS`. But as the pragma statement is wrapped by our include guards, the second include of that file will not have the desired effect of disabling the warnings anymore. We could fix this issue by declaring a new macro that compilation units are expected to invoke after having included the file. In retrospect, that would have been the better way to handle this as it allows for more flexibility: we could for example toggle the warning for specific code blocks, only. But changing this now would require a bunch of changes, and the churn feels excessive for what we gain. Instead, prepare for the precompiled headers by moving the code outside of the include guards. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the next commit we're about to introduce a precompiled header for "git-compat-util.h". The consequence of this change is that we'll implicitly include that header for every compilation unit that uses the precompiled headers. This is okay for our "normal" library sources and our builtins. But some of our compatibility sources do not include the header on purpose, and doing so would cause compilation errors. Prepare for this change by splitting out compatibility sources into their static library. Like this, we can selectively enable precompiled headers for the library sources. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Every compilation unit in Git is expected to include "git-compat-util.h"
first, either directly or indirectly via "builtin.h". This header papers
over differences between platforms so that we can expect the typical
POSIX functions to exist. Furthermore, it provides functionality that we
end up using everywhere.
This header is thus quite heavy as a consequence. Preprocessing it as a
standalone unit via `clang -E git-compat-util.h` yields over 23,000
lines of code overall. Naturally, it takes quite some time to compile
all of this.
Luckily, this is exactly the kind of use case that precompiled headers
aim to solve: instead of recompiling it every single time, we compile it
once and then link the result into the executable. If include guards are
set up properly it means that the file won't need to be reprocessed.
Set up such a precompiled header for "git-compat-util.h" and wire it up
via Meson. This causes Meson to implicitly include the precompiled
header in all compilation units. With GCC and Clang for example this is
done via the "-include" statement [1].
This leads to a significant speedup when performing full builds:
Benchmark 1: ninja (rev = HEAD~)
Time (mean ± σ): 14.467 s ± 0.126 s [User: 248.133 s, System: 31.298 s]
Range (min … max): 14.195 s … 14.633 s 10 runs
Benchmark 2: ninja (rev = HEAD)
Time (mean ± σ): 10.307 s ± 0.111 s [User: 173.290 s, System: 23.998 s]
Range (min … max): 10.030 s … 10.433 s 10 runs
Summary
ninja (rev = HEAD) ran
1.40 ± 0.02 times faster than ninja (rev = HEAD~)
[1]: https://gcc.gnu.org/onlinedocs/gcc/Precompiled-Headers.html
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Back when b4833a2c (rerere: Fix use of an empty strbuf.buf, 2007-09-26) was written, a freshly initialized empty strbuf had NULL in its .buf member, with .len set to 0. The code this patch touches in rerere.c was written to _fix_ the original code that assumed that the .buf member is always pointing at a NUL-terminated string, even for an empty string, which did not hold back then. That changed in b315c5c (strbuf change: be sure ->buf is never ever NULL., 2007-09-27), and it has again become safe to assume that .buf is never NULL, and .buf[0] has '\0' for an empty string (i.e., a strbuf with its .len member set to 0). A funny thing is, this piece of code has been moved around from builtin-rerere.c to rerere.c and also adjusted for updates to the hash function API over the years, but nobody bothered to question if this special casing for an empty strbuf was still necessary: b4833a2 (rerere: Fix use of an empty strbuf.buf, 2007-09-26) 5b2fd95 (rerere: Separate libgit and builtin functions, 2008-07-09) 9126f00 (fix openssl headers conflicting with custom SHA1 implementations, 2008-10-01) c0f16f8 (rerere: factor out handle_conflict function, 2018-08-05) 0d7c419 (rerere: convert to use the_hash_algo, 2018-10-15) 0578f1e (global: adapt callers to use generic hash context helpers, 2025-01-31) Finally get rid of the special casing that was unnecessary for the last 19 years. Signed-off-by: Junio C Hamano <gitster@pobox.com>
We recently noticed one old code from 19 years ago protecting against an ancient strbuf convention that the .buf member can be NULL for an empty strbuf. As that is no longer the case in the modern codebase, let's catch such a construct. Signed-off-by: Junio C Hamano <gitster@pobox.com>
We pair lines for highlighting based on their position in the hunk. So we should never see two identical lines paired, like: -one -two +one +something else which would pair -one/+one, because that implies that the diff could easily be shrunk by turning line "one" into context. But there is (at least) one exception: removing a newline at the end of a file will produce a diff like: -foo +foo \No newline at end of file And we will pair those two lines. As a result, we end up marking the whole line, including the newline, as the shared prefix. And there's an empty suffix. The most obvious bug here is that when we try to print the highlighted lines, we remove the trailing newline from the suffix, but do not bother with the prefix (under the assumption that there had to be a difference _somewhere_ in the line, and thus the prefix would not eat all the way up to the newline). And so you get an extra line like: -foo +foo \No newline at end of file This is obviously ugly, but also causes interactive.diffFilter to (rightly) complain that the input and output do not match their lines 1-to-1. This could easily be fixed by chomping the prefix, too, but I think the problem is deeper. For one, I suspect some of the other logic gets confused by forming an array with zero-indexed element "3" in a 3-element array. But more importantly, we try not to highlight whole lines, as there's nothing interesting to show there. So let's catch this early in is_pair_interesting() and bail to our usual passthrough strategy. Reported-by: Scott Baker <scott@perturb.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The handling of the incomplete lines at the end by "git diff-highlight" has been fixed. * jk/diff-highlight-identical-pairs: contrib/diff-highlight: do not highlight identical pairs
Tweak the build infrastructure by moving tools around. * ps/build-tweaks: meson: precompile "git-compat-util.h" meson: compile compatibility sources separately git-compat-util.h: move warning infra to prepare for PCHs builds: move build scripts into "tools/" contrib: move "update-unicode.sh" script into "tools/" contrib: move "coverage-diff.sh" script into "tools/" contrib: move "coccinelle/" directory into "tools/" Introduce new "tools/" directory
Uses of prio_queue as a LIFO stack of commits have been written with commit_stack. * rs/prio-queue-to-commit-stack: use commit_stack instead of prio_queue in LIFO mode
merge-file --object-id used to trigger a BUG when run in a linked worktree, which has been fixed. * mr/merge-file-object-id-worktree-fix: merge-file: fix BUG when --object-id is used in a worktree
Doc typofix. * gi/doc-boolean-config-typofix: doc: add missing space on git-config page
"git apply -p<n>" parses <n> more carefully now. * mf/apply-p-no-atoi: apply.c: fix -p argument parsing
The reference-transaction hook was taught to be triggered before taking locks on references in the "preparing" phase. * ej/ref-transaction-hook-preparing: refs: add 'preparing' phase to the reference-transaction hook
Doc updates. * kh/doc-interpret-trailers-1: interpret-trailers: use placeholder instead of * doc: config: convert trailers section to synopsis style doc: interpret-trailers: normalize and fill out options doc: interpret-trailers: convert to synopsis style
Code clean-up overdue by 19 years. * jc/rerere-modern-strbuf-handling: cocci: strbuf.buf is never NULL rerere: update to modern representation of empty strbufs
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )