]> git.ipfire.org Git - thirdparty/git.git/log
thirdparty/git.git
23 months agochainlint.pl: latch CPU count directly reported by /proc/cpuinfo
Eric Sunshine [Mon, 20 May 2024 19:01:31 +0000 (15:01 -0400)] 
chainlint.pl: latch CPU count directly reported by /proc/cpuinfo

On Linux, ncores() computes the number of CPUs by counting the
"processor" or "CPU" lines emitted by /proc/cpuinfo. However, on some
platforms, /proc/cpuinfo does not enumerate the CPUs at all, but
instead merely mentions the total number of CPUs. In such cases, pluck
the CPU count directly from the /proc/cpuinfo line which reports the
number of active CPUs. (In particular, check for "cpus active: NN" and
"ncpus active: NN" since both variants have been seen in the
wild[1,2].)

[1]: https://lore.kernel.org/git/503a99f3511559722a3eeef15d31027dfe617fa1.camel@physik.fu-berlin.de/
[2]: https://lore.kernel.org/git/7acbd5c6c68bd7ba020e2d1cc457a8954fd6edf4.camel@physik.fu-berlin.de/

Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agochainlint.pl: fix incorrect CPU count on Linux SPARC
John Paul Adrian Glaubitz [Mon, 20 May 2024 19:01:30 +0000 (15:01 -0400)] 
chainlint.pl: fix incorrect CPU count on Linux SPARC

On SPARC systems running Linux, individual processors are denoted with
"CPUnn:" in /proc/cpuinfo instead of the usual "processor : NN". As a
result, the regexp in ncores() matches 0 times. Address this shortcoming
by extending the regexp to also match lines with "CPUnn:".

Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
[es: simplified regexp; tweaked commit message]
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoRevert "Add a helper function to compare file contents"
Johannes Schindelin [Mon, 20 May 2024 20:22:05 +0000 (20:22 +0000)] 
Revert "Add a helper function to compare file contents"

Now that during a `git clone`, the hooks' contents are no longer
compared to the templates' files', the caller for which the
`do_files_match()` function was introduced is gone, and therefore this
function can be retired, too.

This reverts commit 584de0b4c23 (Add a helper function to compare file
contents, 2024-03-30).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoclone: drop the protections where hooks aren't run
Johannes Schindelin [Mon, 20 May 2024 20:22:04 +0000 (20:22 +0000)] 
clone: drop the protections where hooks aren't run

As part of the security bug-fix releases v2.39.4, ..., v2.45.1, I
introduced logic to safeguard `git clone` from running hooks that were
installed _during_ the clone operation.

The rationale was that Git's CVE-2024-32002, CVE-2021-21300,
CVE-2019-1354, CVE-2019-1353, CVE-2019-1352, and CVE-2019-1349 should
have been low-severity vulnerabilities but were elevated to
critical/high severity by the attack vector that allows a weakness where
files inside `.git/` can be inadvertently written during a `git clone`
to escalate to a Remote Code Execution attack by virtue of installing a
malicious `post-checkout` hook that Git will then run at the end of the
operation without giving the user a chance to see what code is executed.

Unfortunately, Git LFS uses a similar strategy to install its own
`post-checkout` hook during a `git clone`; In fact, Git LFS is
installing four separate hooks while running the `smudge` filter.

While this pattern is probably in want of being improved by introducing
better support in Git for Git LFS and other tools wishing to register
hooks to be run at various stages of Git's commands, let's undo the
clone protections to unbreak Git LFS-enabled clones.

This reverts commit 8db1e8743c0 (clone: prevent hooks from running
during a clone, 2024-03-28).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agotests: verify that `clone -c core.hooksPath=/dev/null` works again
Johannes Schindelin [Mon, 20 May 2024 20:22:03 +0000 (20:22 +0000)] 
tests: verify that `clone -c core.hooksPath=/dev/null` works again

As part of the protections added in Git v2.45.1 and friends,
repository-local `core.hooksPath` settings are no longer allowed, as a
defense-in-depth mechanism to prevent future Git vulnerabilities to
raise to critical level if those vulnerabilities inadvertently allow the
repository-local config to be written.

What the added protection did not anticipate is that such a
repository-local `core.hooksPath` can not only be used to point to
maliciously-placed scripts in the current worktree, but also to
_prevent_ hooks from being called altogether.

We just reverted the `core.hooksPath` protections, based on the Git
maintainer's recommendation in
https://lore.kernel.org/git/xmqq4jaxvm8z.fsf@gitster.g/ to address this
concern as well as related ones. Let's make sure that we won't regress
while trying to protect the clone operation further.

Reported-by: Brooke Kuhlmann <brooke@alchemists.io>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoRevert "core.hooksPath: add some protection while cloning"
Johannes Schindelin [Mon, 20 May 2024 20:22:02 +0000 (20:22 +0000)] 
Revert "core.hooksPath: add some protection while cloning"

This defense-in-depth was intended to protect the clone operation
against future escalations where bugs in `git clone` would allow
attackers to write arbitrary files in the `.git/` directory would allow
for Remote Code Execution attacks via maliciously-placed hooks.

However, it turns out that the `core.hooksPath` protection has
unintentional side effects so severe that they do not justify the
benefit of the protections. For example, it has been reported in
https://lore.kernel.org/git/FAFA34CB-9732-4A0A-87FB-BDB272E6AEE8@alchemists.io/
that the following invocation, which is intended to make `git clone`
safer, is itself broken by that protective measure:

git clone --config core.hooksPath=/dev/null <url>

Since it turns out that the benefit does not justify the cost, let's revert
20f3588efc6 (core.hooksPath: add some protection while cloning,
2024-03-30).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoinit: use the correct path of the templates directory again
Johannes Schindelin [Mon, 20 May 2024 20:22:01 +0000 (20:22 +0000)] 
init: use the correct path of the templates directory again

In df93e407f06 (init: refactor the template directory discovery into its
own function, 2024-03-29), I refactored the way the templates directory
is discovered.

The refactoring was faithful, but missed a reference in the `Makefile`
where the `DEFAULT_GIT_TEMPLATE_DIR` constant is defined. As a
consequence, Git v2.45.1 and friends will always use the hard-coded path
`/usr/share/git-core/templates`.

Let's fix that by defining the `DEFAULT_GIT_TEMPLATE_DIR` when building
`setup.o`, where that constant is actually used.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agohook: plug a new memory leak
Johannes Schindelin [Mon, 20 May 2024 20:22:00 +0000 (20:22 +0000)] 
hook: plug a new memory leak

In 8db1e8743c0 (clone: prevent hooks from running during a clone,
2024-03-28), I introduced an inadvertent memory leak that was
unfortunately not caught before v2.45.1 was released. Here is a fix.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoci: stop installing "gcc-13" for osx-gcc
Jeff King [Thu, 9 May 2024 16:25:44 +0000 (12:25 -0400)] 
ci: stop installing "gcc-13" for osx-gcc

Our osx-gcc job explicitly asks to install gcc-13. But since the GitHub
runner image already comes with gcc-13 installed, this is mostly doing
nothing (or in some cases it may install an incremental update over the
runner image). But worse, it recently started causing errors like:

    ==> Fetching gcc@13
    ==> Downloading https://ghcr.io/v2/homebrew/core/gcc/13/blobs/sha256:fb2403d97e2ce67eb441b54557cfb61980830f3ba26d4c5a1fe5ecd0c9730d1a
    ==> Pouring gcc@13--13.2.0.ventura.bottle.tar.gz
    Error: The `brew link` step did not complete successfully
    The formula built, but is not symlinked into /usr/local
    Could not symlink bin/c++-13
    Target /usr/local/bin/c++-13
    is a symlink belonging to gcc. You can unlink it:
      brew unlink gcc

which cause the whole CI job to bail.

I didn't track down the root cause, but I suspect it may be related to
homebrew recently switching the "gcc" default to gcc-14. And it may even
be fixed when a new runner image is released. But if we don't need to
run brew at all, it's one less thing for us to worry about.

[jc: cherry-picked from v2.45.0-3-g7df2405b38]

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoci: avoid bare "gcc" for osx-gcc job
Jeff King [Thu, 9 May 2024 16:24:15 +0000 (12:24 -0400)] 
ci: avoid bare "gcc" for osx-gcc job

On macOS, a bare "gcc" (without a version) will invoke a wrapper for
clang, not actual gcc. Even when gcc is installed via homebrew, that
only provides version-specific links in /usr/local/bin (like "gcc-13"),
and never a version-agnostic "gcc" wrapper.

As far as I can tell, this has been the case for a long time, and this
osx-gcc job has largely been doing nothing. We can point it at "gcc-13",
which will pick up the homebrew-installed version.

The fix here is specific to the github workflow file, as the gitlab one
does not have a matching job.

It's a little unfortunate that we cannot just ask for the latest version
of gcc which homebrew provides, but as far as I can tell there is no
easy alias (you'd have to find the highest number gcc-* in
/usr/local/bin yourself).

[jc: cherry-picked from v2.45.0-2-g11c7001e3d]

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoci: drop mention of BREW_INSTALL_PACKAGES variable
Jeff King [Thu, 9 May 2024 16:23:05 +0000 (12:23 -0400)] 
ci: drop mention of BREW_INSTALL_PACKAGES variable

The last user of this variable went away in 4a6e4b9602 (CI: remove
Travis CI support, 2021-11-23), so it's doing nothing except making it
more confusing to find out which packages _are_ installed.

[jc: cherry-picked from v2.45.0-1-g9d4453e8d6]

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agosend-email: avoid creating more than one Term::ReadLine object
Jeff King [Tue, 8 Aug 2023 18:15:31 +0000 (14:15 -0400)] 
send-email: avoid creating more than one Term::ReadLine object

Every time git-send-email calls its ask() function to prompt the user,
we call term(), which instantiates a new Term::ReadLine object. But in
v1.46 of Term::ReadLine::Gnu (which provides the Term::ReadLine
interface on some platforms), its constructor refuses to create a second
instance[1]. So on systems with that version of the module, most
git-send-email instances will fail (as we usually prompt for both "to"
and "in-reply-to" unless the user provided them on the command line).

We can fix this by keeping a single instance variable and returning it
for each call to term(). In perl 5.10 and up, we could do that with a
"state" variable. But since we only require 5.008, we'll do it the
old-fashioned way, with a lexical "my" in its own scope.

Note that the tests in t9001 detect this problem as-is, since the
failure mode is for the program to die. But let's also beef up the
"Prompting works" test to check that it correctly handles multiple
inputs (if we had chosen to keep our FakeTerm hack in the previous
commit, then the failure mode would be incorrectly ignoring prompts
after the first).

[1] For discussion of why multiple instances are forbidden, see:
    https://github.com/hirooih/perl-trg/issues/16

[jc: cherry-picked from v2.42.0-rc2~6^2]

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agosend-email: drop FakeTerm hack
Jeff King [Tue, 8 Aug 2023 18:14:36 +0000 (14:14 -0400)] 
send-email: drop FakeTerm hack

Back in 280242d1cc (send-email: do not barf when Term::ReadLine does not
like your terminal, 2006-07-02), we added a fallback for when
Term::ReadLine's constructor failed: we'd have a FakeTerm object
instead, which would then die if anybody actually tried to call
readline() on it. Since we instantiated the $term variable at program
startup, we needed this workaround to let the program run in modes when
we did not prompt the user.

But later, in f4dc9432fd (send-email: lazily load modules for a big
speedup, 2021-05-28), we started loading Term::ReadLine lazily only when
ask() is called. So at that point we know we're trying to prompt the
user, and we can just die if ReadLine instantiation fails, rather than
making this fake object to lazily delay showing the error.

This should be OK even if there is no tty (e.g., we're in a cron job),
because Term::ReadLine will return a stub object in that case whose "IN"
and "OUT" functions return undef. And since 5906f54e47 (send-email:
don't attempt to prompt if tty is closed, 2009-03-31), we check for that
case and skip prompting.

And we can be sure that FakeTerm was not kicking in for such a
situation, because it has actually been broken since that commit! It
does not define "IN" or "OUT" methods, so perl would barf with an error.
If FakeTerm was in use, we were neither honoring what 5906f54e47 tried
to do, nor producing the readable message that 280242d1cc intended.

So we're better off just dropping FakeTerm entirely, and letting the
error reported by constructing Term::ReadLine through.

[jc: cherry-picked from v2.42.0-rc2~6^2~1]

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoapply: fix uninitialized hash function
Junio C Hamano [Mon, 20 May 2024 23:14:34 +0000 (16:14 -0700)] 
apply: fix uninitialized hash function

"git apply" can work outside a repository as a better "GNU patch",
but when it does so, it still assumed that it can access
the_hash_algo, which is no longer true in the new world order.

Make sure we explicitly fall back to SHA-1 algorithm for backward
compatibility.

It is of dubious value to make this configurable to other hash
algorithms, as the code does not use the_hash_algo for hashing
purposes when working outside a repository (which is how
the_hash_algo is left to NULL)---it is only used to learn the max
length of the hash when parsing the object names on the "index"
line, but failing to parse the "index" line is not a hard failure,
and the program does not support operations like applying binary
patches and --3way fallback that requires object access outside a
repository.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/hash-object: fix uninitialized hash function
Patrick Steinhardt [Mon, 20 May 2024 23:14:33 +0000 (16:14 -0700)] 
builtin/hash-object: fix uninitialized hash function

The git-hash-object(1) command allows users to hash an object even
without a repository. Starting with c8aed5e8da (repository: stop setting
SHA1 as the default object hash, 2024-05-07), this will make us hit an
uninitialized hash function, which subsequently leads to a segfault.

Fix this by falling back to SHA-1 explicitly when running outside of
a Git repository. Users can use GIT_DEFAULT_HASH environment to
specify what hash algorithm they want, so arguably this code should
not be needed.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/patch-id: fix uninitialized hash function
Patrick Steinhardt [Mon, 20 May 2024 23:14:32 +0000 (16:14 -0700)] 
builtin/patch-id: fix uninitialized hash function

In c8aed5e8da (repository: stop setting SHA1 as the default object hash,
2024-05-07), we have adapted `initialize_repository()` to no longer set
up a default hash function. As this function is also used to set up
`the_repository`, the consequence is that `the_hash_algo` will now by
default be a `NULL` pointer unless the hash algorithm was configured
properly. This is done as a mechanism to detect cases where we may be
using the wrong hash function by accident.

This change now causes git-patch-id(1) to segfault when it's run outside
of a repository. As this command can read diffs from stdin, it does not
necessarily need a repository, but then relies on `the_hash_algo` to
compute the patch ID itself.

It is somewhat dubious that git-patch-id(1) relies on `the_hash_algo` in
the first place. Quoting its manpage:

    A "patch ID" is nothing but a sum of SHA-1 of the file diffs
    associated with a patch, with line numbers ignored. As such, it’s
    "reasonably stable", but at the same time also reasonably unique,
    i.e., two patches that have the same "patch ID" are almost
    guaranteed to be the same thing.

We explicitly document patch IDs to be using SHA-1. Furthermore, patch
IDs are supposed to be stable for most of the part. But even with the
same input, the patch IDs will now be different depending on the repo's
configured object hash.

Work around the issue by setting up SHA-1 when there was no startup
repository for now. This is arguably not the correct fix, but for now we
rather want to focus on getting the segfault fixed.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot1517: test commands that are designed to be run outside repository
Junio C Hamano [Mon, 20 May 2024 23:14:31 +0000 (16:14 -0700)] 
t1517: test commands that are designed to be run outside repository

A few commands, like "git apply" and "git patch-id", have been
broken with a recent change to stop setting the default hash
algorithm to SHA-1.  Test them and fix them in later commits.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agosetup: add an escape hatch for "no more default hash algorithm" change
Junio C Hamano [Mon, 20 May 2024 23:14:30 +0000 (16:14 -0700)] 
setup: add an escape hatch for "no more default hash algorithm" change

Partially revert c8aed5e8 (repository: stop setting SHA1 as the
default object hash, 2024-05-07), to keep end-user systems still
broken when we have gap in our test coverage but yet give them an
escape hatch to set the GIT_TEST_DEFAULT_HASH_ALGO environment
variable to "sha1" in order to revert to the previous behaviour, in
case we haven't done a thorough job in fixing the fallout from
c8aed5e8.  After we build confidence, we should remove the escape
hatch support, but we are not there yet after only fixing three
commands (hash-object, apply, and patch-id) in this series.

Due to the way the end-user facing GIT_DEFAULT_HASH environment
variable is used in our test suite, we unfortunately cannot reuse it
for this purpose.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot/: port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c
Ghanshyam Thakkar [Sun, 19 May 2024 20:44:42 +0000 (02:14 +0530)] 
t/: port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c

In the recent codebase update (8bf6fbd (Merge branch
'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
merged, providing a standardized approach for testing C code. Prior to
this update, some unit tests relied on the test helper mechanism,
lacking a dedicated unit testing framework. It's more natural to perform
these unit tests using the new unit test framework.

Let's migrate the unit tests for strcmp-offset functionality from the
legacy approach using the test-tool command `test-tool strcmp-offset` in
helper/test-strcmp-offset.c to the new unit testing framework
(t/unit-tests/test-lib.h).

The migration involves refactoring the tests to utilize the testing
macros provided by the framework (TEST() and check_*()).

Helped-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Co-authored-by: Achu Luma <ach.lumap@gmail.com>
Signed-off-by: Achu Luma <ach.lumap@gmail.com>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agochainlint.pl: make CPU count computation more robust
Eric Sunshine [Mon, 20 May 2024 19:01:29 +0000 (15:01 -0400)] 
chainlint.pl: make CPU count computation more robust

There have been reports[1,2] of chainlint.pl failing to produce output
when output is expected. In fact, the underlying problem is more severe:
in these cases, it isn't doing any work at all, thus not checking Git
tests for semantic problems. In the reported cases, the problem was
tracked down to ncores() returning 0 for the CPU count, which resulted
in chainlint.pl not performing any work (since it thought it had no
cores on which to process).

In the reported cases, the reason for the failure was that the regular
expression counting the number of processors reported by /proc/cpuinfo
failed to find any matches, hence it counted 0 processors. Although
fixing each case as it is reported allows chaining.pl to work correctly
on that architecture, it does nothing to improve the overall robustness
of the core count computation which may still return 0 on some yet
untested architecture.

Address this shortcoming by ensuring that ncores() returns a sensible
fallback value in all cases.

[1]: https://lore.kernel.org/git/pull.1385.git.git.1669148861635.gitgitgadget@gmail.com/
[2]: https://lore.kernel.org/git/8baa12f8d044265f1ddeabd64209e7ac0d3700ae.camel@physik.fu-berlin.de/

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoThe sixth batch
Junio C Hamano [Mon, 20 May 2024 17:48:30 +0000 (10:48 -0700)] 
The sixth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoMerge branch 'jc/compat-regex-calloc-fix'
Junio C Hamano [Mon, 20 May 2024 18:20:04 +0000 (11:20 -0700)] 
Merge branch 'jc/compat-regex-calloc-fix'

Windows CI running in GitHub Actions started complaining about the
order of arguments given to calloc(); the imported regex code uses
the wrong order almost consistently, which has been corrected.

* jc/compat-regex-calloc-fix:
  compat/regex: fix argument order to calloc(3)

23 months agoMerge branch 'kn/ref-transaction-symref'
Junio C Hamano [Mon, 20 May 2024 18:20:04 +0000 (11:20 -0700)] 
Merge branch 'kn/ref-transaction-symref'

Updates to symbolic refs can now be made as a part of ref
transaction.

* kn/ref-transaction-symref:
  refs: remove `create_symref` and associated dead code
  refs: rename `refs_create_symref()` to `refs_update_symref()`
  refs: use transaction in `refs_create_symref()`
  refs: add support for transactional symref updates
  refs: move `original_update_refname` to 'refs.c'
  refs: support symrefs in 'reference-transaction' hook
  files-backend: extract out `create_symref_lock()`
  refs: accept symref values in `ref_transaction_update()`

23 months agodoc: describe the project's decision-making process
Josh Steadmon [Fri, 17 May 2024 20:35:44 +0000 (13:35 -0700)] 
doc: describe the project's decision-making process

The Git project currently operates according to an informal
consensus-building process, which is currently described in the
SubmittingPatches document. However, that focuses on small/medium-scale
patch series. For larger-scale decisions, the process is not as well
described. Document what to expect so that we have something concrete to
help inform newcomers to the project.

This document explicitly does not aim to impose a formal process to
decision-making, nor to change pre-existing norms. Its only aim is to
describe how the project currently operates today.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoscalar: make enlistment delete to work on all POSIX platforms
Marcel Telka [Fri, 17 May 2024 14:42:59 +0000 (16:42 +0200)] 
scalar: make enlistment delete to work on all POSIX platforms

The ability to remove the current working directory is not guaranteed by
POSIX so it is better to go out of the directory we want to delete on
all platforms unconditionally.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot/t9001-send-email.sh: sed - remove the i flag for s
Marcel Telka [Fri, 17 May 2024 16:57:46 +0000 (18:57 +0200)] 
t/t9001-send-email.sh: sed - remove the i flag for s

The 'i' flag for the 's' command of sed is not specified by POSIX so
it is not portable.  Replace its usage by different and portable
syntax.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot/t9118-git-svn-funky-branch-names.sh: sed needs semicolon
Marcel Telka [Fri, 17 May 2024 15:39:28 +0000 (17:39 +0200)] 
t/t9118-git-svn-funky-branch-names.sh: sed needs semicolon

POSIX specifies that all editing commands between braces shall be
terminated by a <newline> or <semicolon>.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot/t1700-split-index.sh: mv -v is not portable
Marcel Telka [Fri, 17 May 2024 15:27:41 +0000 (17:27 +0200)] 
t/t1700-split-index.sh: mv -v is not portable

The -v option for mv is not specified by POSIX.  The illumos
implementation of mv does not support -v.  Since we do not need the
verbose mv output we just drop -v for mv.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot/t4202-log.sh: fix misspelled variable
Marcel Telka [Fri, 17 May 2024 13:40:00 +0000 (15:40 +0200)] 
t/t4202-log.sh: fix misspelled variable

The GPGSSH_GOOD_SIGNATURE_TRUSTED variable was spelled as
GOOD_SIGNATURE_TRUSTED and so the grep was used the null RE that
matches everything.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot/t0600-reffiles-backend.sh: rm -v is not portable
Marcel Telka [Fri, 17 May 2024 13:19:00 +0000 (15:19 +0200)] 
t/t0600-reffiles-backend.sh: rm -v is not portable

The -v option for rm is not specified by POSIX.  The illumos
implementation of rm does not support -v.  Since we do not need the
verbose rm output we just drop -v for rm.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot/t9902-completion.sh: backslashes in echo
Marcel Telka [Fri, 17 May 2024 14:08:45 +0000 (16:08 +0200)] 
t/t9902-completion.sh: backslashes in echo

The usage of backslashes in echo is not portable.  Since some tests
tries to output strings containing '\b' it is safer to use printf
here.  The usage of printf instead of echo is also preferred by POSIX.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoSwitch grep from non-portable BRE to portable ERE
Marcel Telka [Fri, 17 May 2024 19:01:49 +0000 (21:01 +0200)] 
Switch grep from non-portable BRE to portable ERE

This makes the grep usage fully POSIX compliant.  The ability to
enable ERE features in BRE using backslash is a GNU extension.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agodiff: document what --name-only shows
Junio C Hamano [Fri, 17 May 2024 17:14:46 +0000 (10:14 -0700)] 
diff: document what --name-only shows

The "--name-only" option is about showing the name of each file in
the post-image tree that got changed and nothing else (like "was it
created?").  Unlike the "--name-status" option that tells how the
change happened (e.g., renamed with similarity), it does not give
anything else, like the name of the corresponding file in the old
tree.

For example, if you start from a clean checkout that has a file
whose name is COPYING, here is what you would see:

    $ git mv COPYING RENAMING
    $ git diff -M --name-only HEAD
    RENAMING
    $ git diff -M --name-status HEAD
    R100 COPYING RENAMING

Lack of the description of this fact has confused readers in the
past.  Even back when dda2d79a ([PATCH] Clean up diff option
descriptions., 2005-07-13) documented "--name-only", "git diff"
already supported the renames, so in a sense, from day one, this
should have been documented more clearly but it wasn't.

Belatedly clarify it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoSubmittingPatches: advertise git-manpages-l10n project a bit
Junio C Hamano [Thu, 9 May 2024 17:32:09 +0000 (10:32 -0700)] 
SubmittingPatches: advertise git-manpages-l10n project a bit

The project takes our AsciiDoc sources of documentation and actively
maintains the translations to various languages.

Let's give them enhanced visibility to help those who want to
volunteer find them.

Acked-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs/packed: remove references to `the_hash_algo`
Patrick Steinhardt [Fri, 17 May 2024 08:19:29 +0000 (10:19 +0200)] 
refs/packed: remove references to `the_hash_algo`

Remove references to `the_hash_algo` in favor of the hash algo specified
by the repository associated with the packed ref store.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs/files: remove references to `the_hash_algo`
Patrick Steinhardt [Fri, 17 May 2024 08:19:24 +0000 (10:19 +0200)] 
refs/files: remove references to `the_hash_algo`

Remove references to `the_hash_algo` in favor of the hash algo specified
by the repository associated with the files ref store.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs/files: use correct repository
Patrick Steinhardt [Fri, 17 May 2024 08:19:19 +0000 (10:19 +0200)] 
refs/files: use correct repository

There are several places in the "files" backend where we use
`the_repository` instead of the repository associated with the ref store
itself. Adapt those to use the correct repository.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: remove `dwim_log()`
Patrick Steinhardt [Fri, 17 May 2024 08:19:14 +0000 (10:19 +0200)] 
refs: remove `dwim_log()`

Remove `dwim_log()` in favor of `repo_dwim_log()` so that we can get rid
of one more dependency on `the_repository`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: drop `git_default_branch_name()`
Patrick Steinhardt [Fri, 17 May 2024 08:19:09 +0000 (10:19 +0200)] 
refs: drop `git_default_branch_name()`

The `git_default_branch_name()` function is a thin wrapper around
`repo_default_branch_name()` with two differences:

  - We implicitly rely on `the_repository`.

  - We cache the default branch name.

None of the callsites of `git_default_branch_name()` are hot code paths
though, so the caching of the branch name is not really required.

Refactor the callsites to use `repo_default_branch_name()` instead and
drop `git_default_branch_name()`, thus getting rid of one more case
where we rely on `the_repository`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: pass repo when peeling objects
Patrick Steinhardt [Fri, 17 May 2024 08:19:04 +0000 (10:19 +0200)] 
refs: pass repo when peeling objects

Both `peel_object()` and `peel_iterated_oid()` implicitly rely on
`the_repository` to look up objects. Despite the fact that we want to
get rid of `the_repository`, it also leads to some restrictions in our
ref iterators when trying to retrieve the peeled value for a repository
other than `the_repository`.

Refactor these functions such that both take a repository as argument
and remove the now-unnecessary restrictions.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: move object peeling into "object.c"
Patrick Steinhardt [Fri, 17 May 2024 08:18:59 +0000 (10:18 +0200)] 
refs: move object peeling into "object.c"

Peeling an object has nothing to do with refs, but we still have the
code in "refs.c". Move it over into "object.c", which is a more natural
place to put it.

Ideally, we'd also move `peel_iterated_oid()` over into "object.c". But
this function is tied to the refs interfaces because it uses a global
ref iterator variable to optimize peeling when the iterator already has
the peeled object ID readily available.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: pass ref store when detecting dangling symrefs
Patrick Steinhardt [Fri, 17 May 2024 08:18:53 +0000 (10:18 +0200)] 
refs: pass ref store when detecting dangling symrefs

Both `warn_dangling_symref()` and `warn_dangling_symrefs()` derive the
ref store via `the_repository`. Adapt them to instead take in the ref
store as a parameter. While at it, rename the functions to have a `ref_`
prefix to align them with other functions that take a ref store.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: convert iteration over replace refs to accept ref store
Patrick Steinhardt [Fri, 17 May 2024 08:18:49 +0000 (10:18 +0200)] 
refs: convert iteration over replace refs to accept ref store

The function `for_each_replace_ref()` is a bit of an oddball across the
refs interfaces as it accepts a pointer to the repository instead of a
pointer to the ref store. The only reason for us to accept a repository
is so that we can eventually pass it back to the callback function that
the caller has provided. This is somewhat arbitrary though, as callers
that need the repository can instead make it accessible via the callback
payload.

Refactor the function to instead accept the ref store and adjust callers
accordingly. This allows us to get rid of some of the boilerplate that
we had to carry to pass along the repository and brings us in line with
the other functions that iterate through refs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: retrieve worktree ref stores via associated repository
Patrick Steinhardt [Fri, 17 May 2024 08:18:44 +0000 (10:18 +0200)] 
refs: retrieve worktree ref stores via associated repository

Similar as with the preceding commit, the worktree ref stores are always
looked up via `the_repository`. Also, again, those ref stores are stored
in a global map.

Refactor the code so that worktrees have a pointer to their repository.
Like this, we can move the global map into `struct repository` and stop
using `the_repository`. With this change, we can now in theory look up
worktree ref stores for repositories other than `the_repository`. In
practice, the worktree code will need further changes to look up
arbitrary worktrees.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: refactor `resolve_gitlink_ref()` to accept a repository
Patrick Steinhardt [Fri, 17 May 2024 08:18:39 +0000 (10:18 +0200)] 
refs: refactor `resolve_gitlink_ref()` to accept a repository

In `resolve_gitlink_ref()` we implicitly rely on `the_repository` to
look up the submodule ref store. Now that we can look up submodule ref
stores for arbitrary repositories we can improve this function to
instead accept a repository as parameter for which we want to resolve
the gitlink.

Do so and adjust callers accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: pass repo when retrieving submodule ref store
Patrick Steinhardt [Fri, 17 May 2024 08:18:34 +0000 (10:18 +0200)] 
refs: pass repo when retrieving submodule ref store

Looking up submodule ref stores has two deficiencies:

  - The initialized subrepo will be attributed to `the_repository`.

  - The submodule ref store will be tracked in a global map.

This makes it impossible to have submodule ref stores for a repository
other than `the_repository`.

Modify the function to accept the parent repository as parameter and
move the global map into `struct repository`. Like this it becomes
possible to look up submodule ref stores for arbitrary repositories.

Note that this also adds a new reference to `the_repository` in
`resolve_gitlink_ref()`, which is part of the refs interfaces. This will
get adjusted in the next patch.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: track ref stores via strmap
Patrick Steinhardt [Fri, 17 May 2024 08:18:28 +0000 (10:18 +0200)] 
refs: track ref stores via strmap

The refs code has two global maps that track the submodule and worktree
ref stores. Even though both of these maps track values by strings, we
still use a `struct hashmap` instead of a `struct strmap`. This has the
benefit of saving us an allocation because we can combine key and value
in a single struct. But it does introduce significant complexity that is
completely unneeded.

Refactor the code to use `struct strmap`s instead to reduce complexity.
It's unlikely that this will have any real-world impact on performance
given that most repositories likely won't have all that many ref stores.
Furthermore, this refactoring allows us to de-globalize those maps and
move them into `struct repository` in a subsequent commit more easily.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: implement releasing ref storages
Patrick Steinhardt [Fri, 17 May 2024 08:18:24 +0000 (10:18 +0200)] 
refs: implement releasing ref storages

Ref storages are typically only initialized once for `the_repository`
and then never released. Until now we got away with that without causing
memory leaks because `the_repository` stays reachable, and because the
ref backend is reachable via `the_repository` its memory basically never
leaks.

This is about to change though because of the upcoming migration logic,
which will create a secondary ref storage. In that case, we will either
have to release the old or new ref storage to avoid leaks.

Implement a new `release` callback and expose it via a new
`ref_storage_release()` function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: rename `init_db` callback to avoid confusion
Patrick Steinhardt [Fri, 17 May 2024 08:18:19 +0000 (10:18 +0200)] 
refs: rename `init_db` callback to avoid confusion

Reference backends have two callbacks `init` and `init_db`. The
similarity of these two callbacks has repeatedly confused me whenever I
was looking at them, where I always had to look up which of them does
what.

Rename the `init_db` callback to `create_on_disk`, which should
hopefully be clearer.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: adjust names for `init` and `init_db` callbacks
Patrick Steinhardt [Fri, 17 May 2024 08:18:14 +0000 (10:18 +0200)] 
refs: adjust names for `init` and `init_db` callbacks

The names of the functions that implement the `init` and `init_db`
callbacks in the "files" and "packed" backends do not match the names of
the callbacks, which is inconsistent. Rename them so that they match,
which makes it easier to discover their respective implementations.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoSubmittingPatches: add section for iterating patches
Karthik Nayak [Fri, 17 May 2024 12:27:24 +0000 (14:27 +0200)] 
SubmittingPatches: add section for iterating patches

Add a section to explain how to work around other in-flight patches and
how to navigate conflicts which arise as a series is being iterated.
This provides the necessary steps that users can follow to reduce
friction with other ongoing topics and also provides guidelines on how
the users can also communicate this to the list efficiently.

Co-authored-by: Junio C Hamano <gitster@pobox.com>
Suggested-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>
23 months agoMerge branch 'jc/patch-flow-updates' into kn/patch-iteration-doc
Junio C Hamano [Fri, 17 May 2024 17:31:38 +0000 (10:31 -0700)] 
Merge branch 'jc/patch-flow-updates' into kn/patch-iteration-doc

* jc/patch-flow-updates:
  SubmittingPatches: extend the "flow" section
  SubmittingPatches: move the patch-flow section earlier

23 months agocompletion: adapt git-config(1) to complete subcommands
Patrick Steinhardt [Fri, 17 May 2024 06:13:36 +0000 (08:13 +0200)] 
completion: adapt git-config(1) to complete subcommands

With fe3ccc7aab (Merge branch 'ps/config-subcommands', 2024-05-15),
git-config(1) has gained support for subcommands. These subcommands live
next to the old, action-based mode, so that both the old and new way
continue to work.

The manpage for this command has been updated to prominently show the
subcommands, and the action-based modes are marked as deprecated. Update
Bash completion scripts accordingly to advertise subcommands instead of
actions.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agot0017: clarify dubious test set-up
Junio C Hamano [Wed, 15 May 2024 19:32:42 +0000 (12:32 -0700)] 
t0017: clarify dubious test set-up

1ff750b1 (tests: make GIT_TEST_GETTEXT_POISON a boolean, 2019-06-21)
added this test, in which "test-tool -C" is fed a name of a
directory that does not exist, and expects that it dies because of a
failure to read the configuration file(s), because the configuration
setting is screwed up to contain mutual inclusion loop, before it
notices that the directory to chdir into does not exist and dies.

It is of dubious value to etch the current order of events, i.e.,
the configuration needs to be read that early (for initializing
trace2 subsystem) before we even notice the lack of the directory
and have a chance to fail, into stone.  Indeed, if you completely
compile out trace2 subsystem so that it does not even attempt to
read the configuration that early, we would die with a different
error message (i.e. "unable to chdir to 'cycle'") and this test will
fail.

At least give a bogus argument to "test-tool -C" a name that is
clearly bogus to make sure we can more easily see what is going on
with plenty of comments.

We may want to remove this test altogether, instead, though.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoThe fifth batch
Junio C Hamano [Thu, 16 May 2024 17:11:24 +0000 (10:11 -0700)] 
The fifth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoMerge branch 'ps/refs-without-the-repository'
Junio C Hamano [Thu, 16 May 2024 17:10:13 +0000 (10:10 -0700)] 
Merge branch 'ps/refs-without-the-repository'

The refs API lost functions that implicitly assumes to work on the
primary ref_store by forcing the callers to pass a ref_store as an
argument.

* ps/refs-without-the-repository:
  refs: remove functions without ref store
  cocci: apply rules to rewrite callers of "refs" interfaces
  cocci: introduce rules to transform "refs" to pass ref store
  refs: add `exclude_patterns` parameter to `for_each_fullref_in()`
  refs: introduce missing functions that accept a `struct ref_store`

23 months agoMerge branch 'jl/git-no-advice'
Junio C Hamano [Thu, 16 May 2024 17:10:13 +0000 (10:10 -0700)] 
Merge branch 'jl/git-no-advice'

A new global "--no-advice" option can be used to disable all advice
messages, which is meant to be used only in scripts.

* jl/git-no-advice:
  t0018: two small fixes
  advice: add --no-advice global option
  doc: add spacing around paginate options
  doc: clean up usage documentation for --no-* opts

23 months agoMerge branch 'rs/external-diff-with-exit-code'
Junio C Hamano [Thu, 16 May 2024 17:09:23 +0000 (10:09 -0700)] 
Merge branch 'rs/external-diff-with-exit-code'

* rs/external-diff-with-exit-code:
  Revert "diff: fix --exit-code with external diff"

23 months agoRevert "diff: fix --exit-code with external diff"
Junio C Hamano [Thu, 16 May 2024 17:08:35 +0000 (10:08 -0700)] 
Revert "diff: fix --exit-code with external diff"

This reverts commit 11be65cfa43416219e85384a3a80d672b65b76ba, per
original author's request to come up with a better strategy.

23 months agoMerge branch 'ps/refs-without-the-repository' into ps/refs-without-the-repository...
Junio C Hamano [Thu, 16 May 2024 16:48:46 +0000 (09:48 -0700)] 
Merge branch 'ps/refs-without-the-repository' into ps/refs-without-the-repository-updates

* ps/refs-without-the-repository:
  refs: remove functions without ref store
  cocci: apply rules to rewrite callers of "refs" interfaces
  cocci: introduce rules to transform "refs" to pass ref store
  refs: add `exclude_patterns` parameter to `for_each_fullref_in()`
  refs: introduce missing functions that accept a `struct ref_store`

23 months agot/t0211-trace2-perf.sh: fix typo patern -> pattern
Marcel Telka [Thu, 16 May 2024 07:45:06 +0000 (09:45 +0200)] 
t/t0211-trace2-perf.sh: fix typo patern -> pattern

The bug went unnoticed because grep with null RE matches everything.

Signed-off-by: Marcel Telka <marcel@telka.sk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoosxkeychain: state to skip unnecessary store operations
Koji Nakamaru [Wed, 15 May 2024 19:21:07 +0000 (19:21 +0000)] 
osxkeychain: state to skip unnecessary store operations

git passes a credential that has been used successfully to the helpers
to record. If a credential is already stored,
"git-credential-osxkeychain store" just records the credential returned
by "git-credential-osxkeychain get", and unnecessary (sometimes
problematic) SecItemAdd() and/or SecItemUpdate() are performed.

We can skip such unnecessary operations by marking a credential returned
by "git-credential-osxkeychain get". This marking can be done by
utilizing the "state[]" feature:

- The "get" command sets the field "state[]=osxkeychain:seen=1".

- The "store" command skips its actual operation if the field
  "state[]=osxkeychain:seen=1" exists.

Introduce a new state "state[]=osxkeychain:seen=1".

Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Koji Nakamaru <koji.nakamaru@gree.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoosxkeychain: exclusive lock to serialize execution of operations
Koji Nakamaru [Wed, 15 May 2024 19:21:06 +0000 (19:21 +0000)] 
osxkeychain: exclusive lock to serialize execution of operations

git passes a credential that has been used successfully to the helpers
to record. If "git-credential-osxkeychain store" commands run in
parallel (with fetch.parallel configuration and/or by running multiple
git commands simultaneously), some of them may exit with the error
"failed to store: -25299". This is because SecItemUpdate() in
add_internet_password() may return errSecDuplicateItem (-25299) in this
situation. Apple's documentation [1] also states as below:

  In macOS, some of the functions of this API block while waiting for
  input from the user (for example, when the user is asked to unlock a
  keychain or give permission to change trust settings). In general, it
  is safe to use this API in threads other than your main thread, but
  avoid calling the functions from multiple operations, work queues, or
  threads concurrently. Instead, serialize function calls or confine
  them to a single thread.

The error has not been noticed before, because the former implementation
ignored the error.

Introduce an exclusive lock to serialize execution of operations.

[1] https://developer.apple.com/documentation/security/certificate_key_and_trust_services/working_with_concurrency

Signed-off-by: Koji Nakamaru <koji.nakamaru@gree.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoThe fourth batch
Junio C Hamano [Wed, 15 May 2024 16:07:20 +0000 (09:07 -0700)] 
The fourth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoMerge branch 'ds/scalar-reconfigure-all-fix'
Junio C Hamano [Wed, 15 May 2024 16:52:55 +0000 (09:52 -0700)] 
Merge branch 'ds/scalar-reconfigure-all-fix'

Scalar fix.

* ds/scalar-reconfigure-all-fix:
  scalar: avoid segfault in reconfigure --all

23 months agoMerge branch 'vd/doc-merge-tree-x-option'
Junio C Hamano [Wed, 15 May 2024 16:52:54 +0000 (09:52 -0700)] 
Merge branch 'vd/doc-merge-tree-x-option'

Doc update.

* vd/doc-merge-tree-x-option:
  Documentation/git-merge-tree.txt: document -X

23 months agoMerge branch 'rs/external-diff-with-exit-code'
Junio C Hamano [Wed, 15 May 2024 16:52:54 +0000 (09:52 -0700)] 
Merge branch 'rs/external-diff-with-exit-code'

The "--exit-code" option of "git diff" command learned to work with
the "--ext-diff" option.

* rs/external-diff-with-exit-code:
  diff: fix --exit-code with external diff
  diff: report unmerged paths as changes in run_diff_cmd()

23 months agoMerge branch 'jt/port-ci-whitespace-check-to-gitlab'
Junio C Hamano [Wed, 15 May 2024 16:52:54 +0000 (09:52 -0700)] 
Merge branch 'jt/port-ci-whitespace-check-to-gitlab'

The "whitespace check" task that was enabled for GitHub Actions CI
has been ported to GitLab CI.

* jt/port-ci-whitespace-check-to-gitlab:
  gitlab-ci: add whitespace error check
  ci: make the whitespace report optional
  ci: separate whitespace check script
  github-ci: fix link to whitespace error
  ci: pre-collapse GitLab CI sections

23 months agoMerge branch 'ow/refspec-glossary-update'
Junio C Hamano [Wed, 15 May 2024 16:52:53 +0000 (09:52 -0700)] 
Merge branch 'ow/refspec-glossary-update'

Doc update.

* ow/refspec-glossary-update:
  Documentation: Mention that refspecs are explained elsewhere

23 months agoMerge branch 'jp/tag-trailer'
Junio C Hamano [Wed, 15 May 2024 16:52:53 +0000 (09:52 -0700)] 
Merge branch 'jp/tag-trailer'

"git tag" learned the "--trailer" option to futz with the trailers
in the same way as "git commit" does.

* jp/tag-trailer:
  builtin/tag: add --trailer option
  builtin/commit: refactor --trailer logic
  builtin/commit: use ARGV macro to collect trailers

23 months agoMerge branch 'ps/config-subcommands'
Junio C Hamano [Wed, 15 May 2024 16:52:52 +0000 (09:52 -0700)] 
Merge branch 'ps/config-subcommands'

The operation mode options (like "--get") the "git config" command
uses have been deprecated and replaced with subcommands (like "git
config get").

* ps/config-subcommands:
  builtin/config: display subcommand help
  builtin/config: introduce "edit" subcommand
  builtin/config: introduce "remove-section" subcommand
  builtin/config: introduce "rename-section" subcommand
  builtin/config: introduce "unset" subcommand
  builtin/config: introduce "set" subcommand
  builtin/config: introduce "get" subcommand
  builtin/config: introduce "list" subcommand
  builtin/config: pull out function to handle `--null`
  builtin/config: pull out function to handle config location
  builtin/config: use `OPT_CMDMODE()` to specify modes
  builtin/config: move "fixed-value" option to correct group
  builtin/config: move option array around
  config: clarify memory ownership when preparing comment strings

23 months agoMerge branch 'js/unit-test-suite-runner'
Junio C Hamano [Wed, 15 May 2024 16:52:52 +0000 (09:52 -0700)] 
Merge branch 'js/unit-test-suite-runner'

The "test-tool" has been taught to run testsuite tests in parallel,
bypassing the need to use the "prove" tool.

* js/unit-test-suite-runner:
  cmake: let `test-tool` run the unit tests, too
  ci: use test-tool as unit test runner on Windows
  t/Makefile: run unit tests alongside shell tests
  unit tests: add rule for running with test-tool
  test-tool run-command testsuite: support unit tests
  test-tool run-command testsuite: remove hardcoded filter
  test-tool run-command testsuite: get shell from env
  t0080: turn t-basic unit test into a helper

23 months agorefs: refuse to write pseudorefs
Patrick Steinhardt [Wed, 15 May 2024 06:51:10 +0000 (08:51 +0200)] 
refs: refuse to write pseudorefs

Pseudorefs are not stored in the ref database as by definition, they
carry additional metadata that essentially makes them not a ref. As
such, writing pseudorefs via the ref backend does not make any sense
whatsoever as the ref backend wouldn't know how exactly to store the
data.

Restrict writing pseudorefs via the ref backend.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoref-filter: properly distinuish pseudo and root refs
Patrick Steinhardt [Wed, 15 May 2024 06:51:05 +0000 (08:51 +0200)] 
ref-filter: properly distinuish pseudo and root refs

The ref-filter interfaces currently define root refs as either a
detached HEAD or a pseudo ref. Pseudo refs aren't root refs though, so
let's properly distinguish those ref types.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: pseudorefs are no refs
Patrick Steinhardt [Wed, 15 May 2024 06:51:01 +0000 (08:51 +0200)] 
refs: pseudorefs are no refs

The `is_root_ref()` function will happily clarify a pseudoref as a root
ref, even though pseudorefs are no refs. Next to being wrong, it also
leads to inconsistent behaviour across ref backends: while the "files"
backend accidentally knows to parse those pseudorefs and thus yields
them to the caller, the "reftable" backend won't ever see the pseudoref
at all because they are never stored in the "reftable" backend.

Fix this issue by filtering out pseudorefs in `is_root_ref()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: classify HEAD as a root ref
Patrick Steinhardt [Wed, 15 May 2024 06:50:56 +0000 (08:50 +0200)] 
refs: classify HEAD as a root ref

Root refs are those refs that live in the root of the ref hierarchy.
Our old and venerable "HEAD" reference falls into this category, but we
don't yet classify it as such in `is_root_ref()`.

Adapt the function to also treat "HEAD" as a root ref. This change is
safe to do for all current callers:

  - `ref_kind_from_refname()` already handles "HEAD" explicitly before
    calling `is_root_ref()`.

  - The "files" and "reftable" backends explicitly call both
    `is_root_ref()` and `is_headref()` together.

This also aligns behaviour or `is_root_ref()` and `is_headref()` such
that we stop checking for ref existence. This changes semantics for our
backends:

  - In the reftable backend we already know that the ref must exist
    because `is_headref()` is called as part of the ref iterator. The
    existence check is thus redundant, and the change is safe to do.

  - In the files backend we use it when populating root refs, where we
    would skip adding the "HEAD" file if it was not possible to resolve
    it. The new behaviour is to instead mark "HEAD" as broken, which
    will cause us to emit warnings in various places.

As there are no callers of `is_headref()` left afer the refactoring, we
can absorb it completely into `is_root_ref()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: do not check ref existence in `is_root_ref()`
Patrick Steinhardt [Wed, 15 May 2024 06:50:51 +0000 (08:50 +0200)] 
refs: do not check ref existence in `is_root_ref()`

Before this patch series, root refs except for "HEAD" and our special
refs were classified as pseudorefs. Furthermore, our terminology
clarified that pseudorefs must not be symbolic refs. This restriction
is enforced in `is_root_ref()`, which explicitly checks that a supposed
root ref resolves to an object ID without recursing.

This has been extremely confusing right from the start because (in old
terminology) a ref name may sometimes be a pseudoref and sometimes not
depending on whether it is a symbolic or regular ref. This behaviour
does not seem reasonable at all and I very much doubt that it results in
anything sane.

Last but not least, the current behaviour can actually lead to a
segfault when calling `is_root_ref()` with a reference that either does
not exist or that is a symbolic ref because we never initialized `oid`,
but then read it via `is_null_oid()`.

We have now changed terminology to clarify that pseudorefs are really
only "MERGE_HEAD" and "FETCH_HEAD", whereas all the other refs that live
in the root of the ref hierarchy are just plain refs. Thus, we do not
need to check whether the ref is symbolic or not. In fact, we can now
avoid looking up the ref completely as the name is sufficient for us to
figure out whether something would be a root ref or not.

This change of course changes semantics for our callers. As there are
only three of them we can assess each of them individually:

  - "ref-filter.c:ref_kind_from_refname()" uses it to classify refs.
    It's clear that the intent is to classify based on the ref name,
    only.

  - "refs/reftable_backend.c:reftable_ref_iterator_advance()" uses it to
    filter root refs. Again, using existence checks is pointless here as
    the iterator has just surfaced the ref, so we know it does exist.

  - "refs/files_backend.c:add_pseudoref_and_head_entries()" uses it to
    determine whether it should add a ref to the root directory of its
    iterator. This had the effect that we skipped over any files that
    are either a symbolic ref, or which are not a ref at all.

    The new behaviour is to include symbolic refs know, which aligns us
    with the adapted terminology. Furthermore, files which look like
    root refs but aren't are now mark those as "broken". As broken refs
    are not surfaced by our tooling, this should not lead to a change in
    user-visible behaviour, but may cause us to emit warnings. This
    feels like the right thing to do as we would otherwise just silently
    ignore corrupted root refs completely.

So in all cases the existence check was either superfluous, not in line
with the adapted terminology or masked potential issues. This commit
thus changes the behaviour as proposed and drops the existence check
altogether.

Add a test that verifies that this does not change user-visible
behaviour. Namely, we still don't want to show broken refs to the user
by default in git-for-each-ref(1). What this does allow though is for
internal callers to surface dangling root refs when they pass in the
`DO_FOR_EACH_INCLUDE_BROKEN` flag.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: rename `is_special_ref()` to `is_pseudo_ref()`
Patrick Steinhardt [Wed, 15 May 2024 06:50:47 +0000 (08:50 +0200)] 
refs: rename `is_special_ref()` to `is_pseudo_ref()`

Rename `is_special_ref()` to `is_pseudo_ref()` to adapt to the newly
defined terminology in our gitglossary(7). Note that in the preceding
commit we have just renamed `is_pseudoref()` to `is_root_ref()`, where
there may be confusion for in-flight patch series that add new calls to
`is_pseudoref()`. In order to intentionally break such patch series we
have thus picked `is_pseudo_ref()` instead of `is_pseudoref()` as the
new name.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agorefs: rename `is_pseudoref()` to `is_root_ref()`
Patrick Steinhardt [Wed, 15 May 2024 06:50:42 +0000 (08:50 +0200)] 
refs: rename `is_pseudoref()` to `is_root_ref()`

Rename `is_pseudoref()` to `is_root_ref()` to adapt to the newly defined
terminology in our gitglossary(7).

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoDocumentation/glossary: define root refs as refs
Patrick Steinhardt [Wed, 15 May 2024 06:50:37 +0000 (08:50 +0200)] 
Documentation/glossary: define root refs as refs

Except for the pseudorefs MERGE_HEAD and FETCH_HEAD, all refs that live
in the root of the ref hierarchy behave the exact same as normal refs.
They can be symbolic refs or direct refs and can be read, iterated over
and written via normal tooling. All of these refs are stored in the ref
backends, which further demonstrates that they are just normal refs.

Extend the definition of "ref" to also cover such root refs. The only
additional restriction for root refs is that they must conform to a
specific naming schema.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoDocumentation/glossary: clarify limitations of pseudorefs
Patrick Steinhardt [Wed, 15 May 2024 06:50:33 +0000 (08:50 +0200)] 
Documentation/glossary: clarify limitations of pseudorefs

Clarify limitations that pseudorefs have:

  - They can be read via git-rev-parse(1) and similar tools.

  - They are not surfaced when iterating through refs, like when using
    git-for-each-ref(1). They are not refs, so iterating through refs
    should not surface them.

  - They cannot be written via git-update-ref(1) and related commands.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoDocumentation/glossary: redefine pseudorefs as special refs
Patrick Steinhardt [Wed, 15 May 2024 06:50:28 +0000 (08:50 +0200)] 
Documentation/glossary: redefine pseudorefs as special refs

Nowadays, Git knows about three different kinds of refs. As defined in
gitglossary(7):

  - Regular refs that start with "refs/", like "refs/heads/main".

  - Pseudorefs, which live in the root directory. These must have
    all-caps names and must be a file that start with an object hash.
    Consequently, symbolic refs are not pseudorefs because they do not
    start with an object hash.

  - Special refs, of which we only have "FETCH_HEAD" and "MERGE_HEAD".

This state is extremely confusing, and I would claim that most folks
don't fully understand what is what here. The current definitions also
have several problems:

  - Where does "HEAD" fit in? It's not a pseudoref because it can be
    a symbolic ref. It's not a regular ref because it does not start
    with "refs/". And it's not a special ref, either.

  - There is a strong overlap between pseudorefs and special refs. The
    pseudoref section for example mentions "MERGE_HEAD", even though it
    is a special ref. Is it thus both a pseudoref and a special ref?

  - Why do we even need to distinguish refs that live in the root from
    other refs when they behave just like a regular ref anyway?

In other words, the current state is quite a mess and leads to wild
inconsistencies without much of a good reason.

The original reason why pseudorefs were introduced is that there are
some refs that sometimes behave like a ref, even though they aren't a
ref. And we really only have two of these nowadays, namely "MERGE_HEAD"
and "FETCH_HEAD". Those files are never written via the ref backends,
but are instead written by git-fetch(1), git-pull(1) and git-merge(1).
They contain additional metadata that highlights where a ref has been
fetched from or the list of commits that have been merged.

This original intent in fact matches the definition of special refs that
we have recently introduced in 8df4c5d205 (Documentation: add "special
refs" to the glossary, 2024-01-19). Due to the introduction of the new
reftable backend we were forced to distinguish those refs more clearly
such that we don't ever try to read or write them via the reftable
backend. In the same series, we also addressed all the other cases where
we used to write those special refs via the filesystem directly, thus
circumventing the ref backend, to instead write them via the backends.
Consequently, there are no other refs left anymore which are special.

Let's address this mess and return the pseudoref terminology back to its
original intent: a ref that sometimes behave like a ref, but which isn't
really a ref because it gets written to the filesystem directly. Or in
other words, let's redefine pseudorefs to match the current definition
of special refs. As special refs and pseudorefs are now the same per
definition, we can drop the "special refs" term again. It's not exposed
to our users and thus they wouldn't ever encounter that term anyway.

Refs that live in the root of the ref hierarchy but which are not
pseudorefs will be further defined in a subsequent commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: pass data between callbacks via local variables
Patrick Steinhardt [Wed, 15 May 2024 06:43:12 +0000 (08:43 +0200)] 
builtin/config: pass data between callbacks via local variables

We use several global variables to pass data between callers and
callbacks in `get_color()` and `get_colorbool()`. Convert those to use
callback data structures instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: convert flags to a local variable
Patrick Steinhardt [Wed, 15 May 2024 06:43:07 +0000 (08:43 +0200)] 
builtin/config: convert flags to a local variable

Both the `do_all` and `use_key_regexp` bits essentially act like flags
to `get_value()`. Let's convert them to actual flags so that we can get
rid of the last two remaining global variables that track options.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: track "fixed value" option via flags only
Patrick Steinhardt [Wed, 15 May 2024 06:43:02 +0000 (08:43 +0200)] 
builtin/config: track "fixed value" option via flags only

We track the "fixed value" option via two separate bits: once via the
global variable `fixed_value`, and once via the CONFIG_FLAGS_FIXED_VALUE
bit in `flags`. This is confusing and may easily lead to issues when one
is not aware that this is tracked via two separate mechanisms.

Refactor the code to use the flag exclusively. We already pass it to all
the required callsites anyway, except for `collect_config()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: convert `key` to a local variable
Patrick Steinhardt [Wed, 15 May 2024 06:42:58 +0000 (08:42 +0200)] 
builtin/config: convert `key` to a local variable

The `key` variable is used by the `get_value()` function for two
purposes:

  - It is used to store the result of `git_config_parse_key()`, which is
    then passed on to `collect_config()`.

  - It is used as a store to convert the provided key to an
    all-lowercase key when `use_key_regexp` is set.

Neither of these cases warrant a global variable at all. In the former
case we can pass the key via `struct collect_config_data`. And in the
latter case we really only want to have it as a temporary local variable
such that we can free associated memory.

Refactor the code accordingly to reduce our reliance on global state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: convert `key_regexp` to a local variable
Patrick Steinhardt [Wed, 15 May 2024 06:42:53 +0000 (08:42 +0200)] 
builtin/config: convert `key_regexp` to a local variable

The `key_regexp` variable is used by the `format_config()` callback when
`use_key_regexp` is set. It is only ever set up by its only caller,
`collect_config()` and can thus easily be moved into the
`collect_config_data` structure.

Do so to remove our reliance on global state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: convert `regexp` to a local variable
Patrick Steinhardt [Wed, 15 May 2024 06:42:49 +0000 (08:42 +0200)] 
builtin/config: convert `regexp` to a local variable

The `regexp` variable is used by the `format_config()` callback when
`CONFIG_FLAGS_FIXED_VALUE` is not set. It is only ever set up by its
only caller, `collect_config()` and can thus easily be moved into the
`collect_config_data` structure.

Do so to remove our reliance on global state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: convert `value_pattern` to a local variable
Patrick Steinhardt [Wed, 15 May 2024 06:42:44 +0000 (08:42 +0200)] 
builtin/config: convert `value_pattern` to a local variable

The `value_pattern` variable is used by the `format_config()` callback
when `CONFIG_FLAGS_FIXED_VALUE` is used. It is only ever set up by its
only caller, `collect_config()` and can thus easily be moved into the
`collect_config_data` structure.

Do so to remove our reliance on global state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: convert `do_not_match` to a local variable
Patrick Steinhardt [Wed, 15 May 2024 06:42:39 +0000 (08:42 +0200)] 
builtin/config: convert `do_not_match` to a local variable

The `do_not_match` variable is used by the `format_config()` callback as
an indicator whether or not the passed regular expression is negated. It
is only ever set up by its only caller, `collect_config()` and can thus
easily be moved into the `collect_config_data` structure.

Do so to remove our reliance on global state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: move `respect_includes_opt` into location options
Patrick Steinhardt [Wed, 15 May 2024 06:42:35 +0000 (08:42 +0200)] 
builtin/config: move `respect_includes_opt` into location options

The variable tracking whether or not we want to honor includes is
tracked via a global variable. Move it into the location options
instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: move default value into display options
Patrick Steinhardt [Wed, 15 May 2024 06:42:30 +0000 (08:42 +0200)] 
builtin/config: move default value into display options

The default value is tracked via a global variable. Move it into the
display options instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: move type options into display options
Patrick Steinhardt [Wed, 15 May 2024 06:42:25 +0000 (08:42 +0200)] 
builtin/config: move type options into display options

The type options are tracked via a global variable. Move it into the
display options instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: move display options into local variables
Patrick Steinhardt [Wed, 15 May 2024 06:42:21 +0000 (08:42 +0200)] 
builtin/config: move display options into local variables

The display options are tracked via a set of global variables. Move
them into a self-contained structure so that we can easily parse all
relevant options and hand them over to the various functions that
require them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: move location options into local variables
Patrick Steinhardt [Wed, 15 May 2024 06:42:16 +0000 (08:42 +0200)] 
builtin/config: move location options into local variables

The location options are tracked via a set of global variables. Move
them into a self-contained structure so that we can easily parse all
relevant options and hand them over to the various functions that
require them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: refactor functions to have common exit paths
Patrick Steinhardt [Wed, 15 May 2024 06:42:11 +0000 (08:42 +0200)] 
builtin/config: refactor functions to have common exit paths

Refactor functions to have a single exit path. This will make it easier
in subsequent commits to add common cleanup code.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agoconfig: make the config source const
Patrick Steinhardt [Wed, 15 May 2024 06:42:06 +0000 (08:42 +0200)] 
config: make the config source const

The `struct git_config_source` passed to `config_with_options()` is
never modified. Let's mark it as `const` to clarify.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: check for writeability after source is set up
Patrick Steinhardt [Wed, 15 May 2024 06:42:02 +0000 (08:42 +0200)] 
builtin/config: check for writeability after source is set up

The `check_write()` function verifies that we do not try to write to a
config source that cannot be written to, like for example stdin. But
while the new subcommands do call this function, they do so before
calling `handle_config_location()`. Consequently, we only end up
checking the default config location for writeability, not the location
that was actually specified by the caller of git-config(1).

Fix this by calling `check_write()` after `handle_config_location()`. We
will further clarify the relationship between those two functions in a
subsequent commit where we remove the global state that both implicitly
rely on.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: move actions into `cmd_config_actions()`
Patrick Steinhardt [Wed, 15 May 2024 06:41:57 +0000 (08:41 +0200)] 
builtin/config: move actions into `cmd_config_actions()`

We only use actions in the legacy mode. Convert them to an enum and move
them into `cmd_config_actions()` to clearly demonstrate that they are
not used anywhere else.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
23 months agobuiltin/config: move legacy options into `cmd_config()`
Patrick Steinhardt [Wed, 15 May 2024 06:41:52 +0000 (08:41 +0200)] 
builtin/config: move legacy options into `cmd_config()`

Move the legacy options as well some of the variables it references into
`cmd_config_action()`. This reduces our reliance on global state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>