Merge branch 'vd/fsck-submodule-url-test'

Tighten URL checks fsck makes in a URL recorded for submodules.

* vd/fsck-submodule-url-test:
  submodule-config.c: strengthen URL fsck check
  t7450: test submodule urls
  test-submodule: remove command line handling for check-name
  submodule-config.h: move check_submodule_url

Merge branch 'kh/maintenance-use-xdg-when-it-should'

When $HOME/.gitignore is missing but XDG config file available, we
should write into the latter, not former.  "git gc" and "git
maintenance" wrote into a wrong "global config" file, which have
been corrected.

* kh/maintenance-use-xdg-when-it-should:
  maintenance: use XDG config if it exists
  config: factor out global config file retrieval
  config: rename global config function
  config: format newlines

Merge branch 'gt/test-commit-o-i-options'

A few tests to "git commit -o <pathspec>" and "git commit -i
<pathspec>" has been added.

* gt/test-commit-o-i-options:
  t7501: add tests for --amend --signoff
  t7501: add tests for --include and --only

Merge branch 'ps/gitlab-ci-macos'

CI for GitLab learned to drive macOS jobs.

* ps/gitlab-ci-macos:
  ci: add macOS jobs to GitLab CI
  ci: make p4 setup on macOS more robust
  ci: handle TEST_OUTPUT_DIRECTORY when printing test failures
  Makefile: detect new Homebrew location for ARM-based Macs
  t7527: decrease likelihood of racing with fsmonitor daemon

Merge branch 'ps/completion-with-reftable-fix'

Completion update to prepare for reftable

* ps/completion-with-reftable-fix:
  completion: treat dangling symrefs as existing pseudorefs
  completion: silence pseudoref existence check
  completion: improve existence check for pseudo-refs
  t9902: verify that completion does not print anything
  completion: discover repo path in `__git_pseudoref_exists ()`

Merge branch 'jt/tests-with-reftable'

Tweak a few tests not to manually modify the reference database
(hence easier to work with other backends like reftable).

* jt/tests-with-reftable:
  t5541: remove lockfile creation
  t1401: remove lockfile creation

Merge branch 'la/strvec-comment-fix'

Comment fix.

* la/strvec-comment-fix:
  strvec: use correct member name in comments

Merge branch 'mj/gitweb-unreadable-config-error'

When given an existing but unreadable file as a configuration file,
gitweb behaved as if the file did not exist at all, but now it
errors out.  This is a change that may break backward compatibility.

* mj/gitweb-unreadable-config-error:
  gitweb: die when a configuration file cannot be read

Merge branch 'ps/worktree-refdb-initialization'

Instead of manually creating refs/ hierarchy on disk upon a
creation of a secondary worktree, which is only usable via the
files backend, use the refs API to populate it.

* ps/worktree-refdb-initialization:
  builtin/worktree: create refdb via ref backend
  worktree: expose interface to look up worktree by name
  builtin/worktree: move setup of commondir file earlier
  refs/files: skip creation of "refs/{heads,tags}" for worktrees
  setup: move creation of "refs/" into the files backend
  refs: prepare `refs_init_db()` for initializing worktree refs

Merge branch 'ps/commit-graph-write-leakfix'

Leakfix.

* ps/commit-graph-write-leakfix:
  commit-graph: fix memory leak when not writing graph

Merge branch 'al/unit-test-ctype'

Move test-ctype helper to the unit-test framework.

* al/unit-test-ctype:
  unit-tests: rewrite t/helper/test-ctype.c as a unit test

Merge branch 'ne/doc-filter-blob-limit-fix'

Docfix.

* ne/doc-filter-blob-limit-fix:
  rev-list-options: fix off-by-one in '--filter=blob:limit=<n>' explainer

Merge branch 'rj/advice-delete-branch-not-fully-merged'

The error message given when "git branch -d branch" fails due to
commits unique to the branch has been split into an error and a new
conditional advice message.

* rj/advice-delete-branch-not-fully-merged:
  branch: make the advice to force-deleting a conditional one
  advice: fix an unexpected leading space
  advice: sort the advice related lists

Merge branch 'es/some-up-to-date-messages-must-stay'

Comment updates to help developers not to attempt to modify
messages from plumbing commands that must stay constant.

It might make sense to reassess the plumbing needs every few years,
but that should be done as a separate effort.

* es/some-up-to-date-messages-must-stay:
  messages: mark some strings with "up-to-date" not to touch

The ninth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ps/p4-use-ref-api'

"git p4" update to prepare for reftable

* ps/p4-use-ref-api:
  git-p4: stop reaching into the refdb

Merge branch 'cp/t4129-pipefix'

Test update.

* cp/t4129-pipefix:
  t4129: prevent loss of exit code due to the use of pipes

Merge branch 'sk/mingw-owner-check-error-message-improvement'

In addition to (rather cryptic) Security Identifiers, show username
and domain in the error message when we barf on mismatch between
the Git directory and the current user on Windows.

* sk/mingw-owner-check-error-message-improvement:
  mingw: give more details about unsafe directory's ownership

Merge branch 'bk/bisect-doc-fix'

Synopsis fix.

* bk/bisect-doc-fix:
  doc: refer to pathspec instead of path
  doc: use singular form of repeatable path arg

Merge branch 'tb/fetch-all-configuration'

"git fetch" learned to pay attention to "fetch.all" configuration
variable, which pretends as if "--all" was passed from the command
line when no remote parameter was given.

* tb/fetch-all-configuration:
  fetch: add new config option fetch.all

Merge branch 'rj/clarify-branch-doc-m'

Doc update.

* rj/clarify-branch-doc-m:
  branch: clarify <oldbranch> term

Merge branch 'ps/gitlab-ci-static-analysis'

GitLab CI update.

* ps/gitlab-ci-static-analysis:
  ci: add job performing static analysis on GitLab CI

Merge branch 'ps/prompt-parse-HEAD-futureproof'

Futureproof command line prompt support (in contrib/).

* ps/prompt-parse-HEAD-futureproof:
  git-prompt: stop manually parsing HEAD with unknown ref formats

submodule-config.c: strengthen URL fsck check

Update the validation of "curl URL" submodule URLs (i.e. those that specify
an "http[s]" or "ftp[s]" protocol) in 'check_submodule_url()' to catch more
invalid URLs. The existing validation using 'credential_from_url_gently()'
parses certain URLs incorrectly, leading to invalid submodule URLs passing
'git fsck' checks. Conversely, 'url_normalize()' - used to validate remote
URLs in 'remote_get()' - correctly identifies the invalid URLs missed by
'credential_from_url_gently()'.

To catch more invalid cases, replace 'credential_from_url_gently()' with
'url_normalize()' followed by a 'url_decode()' and a check for newlines
(mirroring 'check_url_component()' in the 'credential_from_url_gently()'
validation).

Signed-off-by: Victoria Dye <vdye@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7450: test submodule urls

Add tests to 't7450-bad-git-dotfiles.sh' to check the validity of different
submodule URLs. To verify this directly (without setting up test
repositories & submodules), add a 'check-url' subcommand to 'test-tool
submodule' that calls 'check_submodule_url' in the same way that
'check-name' calls 'check_submodule_name'.

Add two tests to separately address cases where the URL check correctly
filters out invalid URLs and cases where the check misses invalid URLs. Mark
the latter ("url check misses invalid cases") with 'test_expect_failure' to
indicate that this is currently broken, which will be fixed in the next step.

Signed-off-by: Victoria Dye <vdye@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

maintenance: use XDG config if it exists

`git maintenance register` registers the repository in the user's global
config. `$XDG_CONFIG_HOME/git/config` is supposed to be used if
`~/.gitconfig` does not exist. However, this command creates a
`~/.gitconfig` file and writes to that one even though the XDG variant
exists.

This used to work correctly until 50a044f1e4 (gc: replace config
subprocesses with API calls, 2022-09-27), when the command started calling
the config API instead of git-config(1).

Also change `unregister` accordingly.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

config: factor out global config file retrieval

Factor out code that retrieves the global config file so that we can use
it in `gc.c` as well.

Use the old name from the previous commit since this function acts
functionally the same as `git_system_config` but for “global”.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

config: rename global config function

Rename this function to a more descriptive name since we want to use the
existing name for a new function.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

config: format newlines

Remove unneeded newlines according to `clang-format`.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

ci: add macOS jobs to GitLab CI

Add a job to GitLab CI which runs tests on macOS, which matches the
equivalent "osx-clang" job that we have for GitHub Workflows. One
significant difference though is that this new job runs on Apple M1
machines and thus uses the "arm64" architecture. As GCC does not yet
support this comparatively new architecture we cannot easily include an
equivalent for the "osx-gcc" job that exists in GitHub Workflows.

Note that one test marked as `test_must_fail` is surprisingly passing:

  t7815-grep-binary.sh                             (Wstat: 0 Tests: 22 Failed: 0)
    TODO passed:   12

This seems to boil down to an unexpected difference in how regcomp(3P)
works when matching NUL bytes. Cross-checking with the respective GitHub
job shows that this is not an issue unique to the GitLab CI job as it
passes in the same way there.

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

ci: make p4 setup on macOS more robust

When setting up Perforce on macOS we put both `p4` and `p4d` into
"$HOME/bin". On GitHub CI this directory is indeed contained in the PATH
environment variable and thus there is no need for additional setup than
to put the binaries there. But GitLab CI does not do this, and thus our
Perforce-based tests would be skipped there even though we download the
binaries.

Refactor the setup code to become more robust by downloading binaries
into a separate directory which we then manually append to our PATH.
This matches what we do on Linux-based jobs.

Note that it may seem like we already did append "$HOME/bin" to PATH
because we're actually removing the lines that adapt PATH. But we only
ever adapted the PATH variable in "ci/install-dependencies.sh", and
didn't adapt it when running "ci/run-build-and-test.sh". Consequently,
the required binaries wouldn't be found during the test run unless the
CI platform already had the "$HOME/bin" in PATH right from the start.

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

ci: handle TEST_OUTPUT_DIRECTORY when printing test failures

The TEST_OUTPUT_DIRECTORY environment variable can be used to instruct
the test suite to write test data and test results into a different
location than into "t/". The "ci/print-test-failures.sh" script does not
know to handle this environment variable though, which means that it
will search for test results in the wrong location if it was set.

Update the script to handle TEST_OUTPUT_DIRECTORY so that we can start
to set it in our CI.

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

Makefile: detect new Homebrew location for ARM-based Macs

With the introduction of the ARM-based Macs the default location for
Homebrew has changed from "/usr/local" to "/opt/homebrew". We only
handle the former location though, which means that unless the user has
manually configured required search paths we won't be able to locate it.

Improve upon this by adding relevant paths to our CFLAGS and LDFLAGS as
well as detecting the location of msgfmt(1).

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

t7527: decrease likelihood of racing with fsmonitor daemon

In t7527, we test that the builtin fsmonitor daemon works well in
various edge cases. One of these tests is frequently failing because
events reported by the fsmonitor--daemon are missing an expected event.
This failure is essentially a race condition: we do not wait for the
daemon to flush out all events before we ask it to quit. Consequently,
it can happen that we miss some expected events.

In other testcases we counteract this race by sending a simple query to
the daemon. Quoting a comment:

  We run a simple query after modifying the filesystem just to introduce
  a bit of a delay so that the trace logging from the daemon has time to
  get flushed to disk.

Now this workaround is not a "proper" fix as we do not wait for all
events to have been synchronized in a deterministic way. But this fix
seems to be sufficient for all the other tests to pass, so it must not
be all that bad.

Convert the failing test to do the same. While the test was previously
failing in about 50% of the test runs, I couldn't reproduce the failure
after the change anymore.

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

test-submodule: remove command line handling for check-name

The 'check-name' subcommand to 'test-tool submodule' is documented as being
able to take a command line argument '<name>'. However, this does not work -
and has never worked - because 'argc > 0' triggers the usage message in
'cmd__submodule_check_name()'. To simplify the helper and avoid future
confusion around proper use of the subcommand, remove any references to
command line arguments for 'check-name' in usage strings and handling in
'check_name()'.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Victoria Dye <vdye@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule-config.h: move check_submodule_url

Move 'check_submodule_url' out of 'fsck.c' and into 'submodule-config.h' as
a public method, similar to 'check_submodule_name'. With the function now
accessible outside of 'fsck', it can be used in a later commit to extend
'test-tool submodule' to check the validity of submodule URLs as it does
with names in the 'check-name' subcommand.

Other than its location, no changes are made to 'check_submodule_url' in
this patch.

Signed-off-by: Victoria Dye <vdye@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7501: add tests for --amend --signoff

Add tests for amending the commit to add Signed-off-by trailer. And
also to check if it does not add another trailer if one already exists.

Currently, there are tests for --signoff separately in t7501, however,
they are not tested with --amend.

Therefore, these tests belong with other similar tests of --amend in
t7501-commit-basic-functionality.

Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7501: add tests for --include and --only

Add tests for --only (-o) and --include (-i). This include testing
with or without staged changes for both -i and -o. Also to test
for committing untracked files with -i, -o and without -i/-o.

Some tests already exist in t7501 for testing --only, however,
it is only tested in combination with --amend and --allow-empty
and on to-be-born branch. The addition of these tests check, when
the pathspec is provided without using -only, that only the files
matching the pathspec get committed. This behavior is same when
we provide --only and it is checked by the tests.
(as --only is the default mode of operation when pathspec is
provided.)

As for --include, there is no prior test for checking if --include
also commits staged changes, thus add test for that. Along with
the tests also document a potential bug, in which, when provided
with -i and a pathspec that does not match any tracked path,
commit does not fail if there are staged changes. And when there
are no staged changes commit fails. However, no error is returned
to stderr in either of the cases. This is described in the TODO
comment before the relevent testcase.

And also add a test for checking incompatibilty when using -o and
-i together.

Thus, these tests belong in t7501 with other similar existing tests,
as described in the case of --only.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ps/gitlab-ci-static-analysis' into ps/gitlab-ci-macos

* ps/gitlab-ci-static-analysis:
  ci: add job performing static analysis on GitLab CI

The eighth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ib/rebase-reschedule-doc'

Doc update.

* ib/rebase-reschedule-doc:
  rebase: clarify --reschedule-failed-exec default

Merge branch 'jk/commit-graph-slab-clear-fix'

Clearing in-core repository (happens during e.g., "git fetch
--recurse-submodules" with commit graph enabled) made in-core
commit object in an inconsistent state by discarding the necessary
data from commit-graph too early, which has been corrected.

* jk/commit-graph-slab-clear-fix:
  commit-graph: retain commit slab when closing NULL commit_graph

Merge branch 'jk/index-pack-lsan-false-positive-fix'

Fix false positive reported by leak sanitizer.

* jk/index-pack-lsan-false-positive-fix:
  index-pack: spawn threads atomically

Merge branch 'cp/sideband-array-index-comment-fix'

In-code comment fix.

* cp/sideband-array-index-comment-fix:
  sideband.c: remove redundant 'NEEDSWORK' tag

Merge branch 'ps/refstorage-extension'

Introduce a new extension "refstorage" so that we can mark a
repository that uses a non-default ref backend, like reftable.

* ps/refstorage-extension:
  t9500: write "extensions.refstorage" into config
  builtin/clone: introduce `--ref-format=` value flag
  builtin/init: introduce `--ref-format=` value flag
  builtin/rev-parse: introduce `--show-ref-format` flag
  t: introduce GIT_TEST_DEFAULT_REF_FORMAT envvar
  setup: introduce GIT_DEFAULT_REF_FORMAT envvar
  setup: introduce "extensions.refStorage" extension
  setup: set repository's formats on init
  setup: start tracking ref storage format
  refs: refactor logic to look up storage backends
  worktree: skip reading HEAD when repairing worktrees
  t: introduce DEFAULT_REPO_FORMAT prereq

Merge branch 'ps/reftable-fixes-and-optims'

More fixes and optimizations to the reftable backend.

* ps/reftable-fixes-and-optims:
  reftable/merged: transfer ownership of records when iterating
  reftable/merged: really reuse buffers to compute record keys
  reftable/record: store "val2" hashes as static arrays
  reftable/record: store "val1" hashes as static arrays
  reftable/record: constify some parts of the interface
  reftable/writer: fix index corruption when writing multiple indices
  reftable/stack: do not auto-compact twice in `reftable_stack_add()`
  reftable/stack: do not overwrite errors when compacting

completion: treat dangling symrefs as existing pseudorefs

The `__git_pseudoref_exists ()` helper function back to git-rev-parse(1)
in case the reftable backend is in use. This is not in the same spirit
as the simple existence check that the "files" backend does though,
because there we only check for the pseudo-ref to exist with `test -f`.
With git-rev-parse(1) we not only check for existence, but also verify
that the pseudo-ref resolves to an object, which may not be the case
when the pseudo-ref points to an unborn branch.

Fix this issue by using `git show-ref --exists` instead. Note that we do
not have to silence stdout anymore as git-show-ref(1) will not print
anything.

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

completion: silence pseudoref existence check

In 44dbb3bf29 (completion: support pseudoref existence checks for
reftables, 2023-12-19), we have extended the Bash completion script to
support future ref backends better by using git-rev-parse(1) to check
for pseudo-ref existence. This conversion has introduced a bug, because
even though we pass `--quiet` to git-rev-parse(1) it would still output
the resolved object ID of the ref in question if it exists.

Fix this by redirecting its stdout to `/dev/null` and add a test that
catches this behaviour. Note that the test passes even without the fix
for the "files" backend because we parse pseudo refs via the filesystem
directly in that case. But the test will fail with the "reftable"
backend.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

completion: improve existence check for pseudo-refs

Improve the existence check along the following lines:

  - Stop stripping the "ref :" prefix and compare to the expected value
    directly. This allows us to drop a now-unused variable that was
    previously leaking into the user's shell.

  - Mark the "head" variable as local so that we don't leak its value
    into the user's shell.

  - Stop manually handling the `-C $__git_repo_path` option, which the
    `__git ()` wrapper aleady does for us.

  - In simlar spirit, stop redirecting stderr, which is also handled by
    the wrapper already.

Suggested-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t9902: verify that completion does not print anything

The Bash completion script must not print anything to either stdout or
stderr. Instead, it is only expected to populate certain variables.
Tighten our `test_completion ()` test helper to verify this requirement.

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

completion: discover repo path in `__git_pseudoref_exists ()`

The helper function `__git_pseudoref_exists ()` expects that the repo
path has already been discovered by its callers, which makes for a
rather fragile calling convention. Refactor the function to discover the
repo path itself to make it more self-contained, which also removes the
need to discover the path in some of its callers.

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

rev-list-options: fix off-by-one in '--filter=blob:limit=<n>' explainer

'--filter=blob:limit=<n>' was introduced in 25ec7bcac0 (list-objects:
filter objects in traverse_commit_list, 2017-11-21) and later expanded
to bitmaps in 84243da129 (pack-bitmap: implement BLOB_LIMIT filtering,
2020-02-14)

The logic that was introduced in these commits (and that still persists
to this day) omits blobs larger than _or equal_ to n bytes or units.

However, the documentation (Documentation/rev-list-options.txt) states:

>The form '--filter=blob:limit=<n>[kmg]' omits blobs larger than n
bytes or units. n may be zero.

Moreover, the t6113-rev-list-bitmap-filters.sh tests for exactly this
logic, so it seems it is the documentation that needs fixing, not the
code.

This changes the explanation to be similar to
Documentation/git-clone.txt, which is correct.

Signed-off-by: Nikolay Edigaryev <edigaryev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

unit-tests: rewrite t/helper/test-ctype.c as a unit test

In the recent codebase update (8bf6fbd00d (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.

This commit migrates the unit tests for C character classification
functions (isdigit(), isspace(), etc) from the legacy approach
using the test-tool command `test-tool ctype` in t/helper/test-ctype.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_*()).

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Helped-by: René Scharfe <l.s.r@web.de>
Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Achu Luma <ach.lumap@gmail.com>
Acked-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: fix memory leak when not writing graph

When `write_commit_graph()` bails out writing a split commit-graph early
then it may happen that we have already gathered the set of existing
commit-graph file names without yet determining the new merged set of
files. This can result in a memory leak though because we only clear the
preimage of files when we have collected the postimage.

Fix this issue by dropping the condition altogether so that we always
try to free both preimage and postimage filenames. As the context
structure is zero-initialized this simplification is safe to do.

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

The seventh batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'cp/git-flush-is-an-env-bool'

Unlike other environment variables that took the usual
true/false/yes/no as well as 0/1, GIT_FLUSH only understood 0/1,
which has been corrected.

* cp/git-flush-is-an-env-bool:
  write-or-die: make GIT_FLUSH a Boolean environment variable

Merge branch 'ms/rebase-insnformat-doc-fix'

Docfix.

* ms/rebase-insnformat-doc-fix:
  Documentation: fix statement about rebase.instructionFormat

Merge branch 'jx/sideband-chomp-newline-fix'

Sideband demultiplexer fixes.

* jx/sideband-chomp-newline-fix:
  pkt-line: do not chomp newlines for sideband messages
  pkt-line: memorize sideband fragment in reader
  test-pkt-line: add option parser for unpack-sideband

Merge branch 'tb/multi-pack-verbatim-reuse'

Streaming spans of packfile data used to be done only from a
single, primary, pack in a repository with multiple packfiles.  It
has been extended to allow reuse from other packfiles, too.

* tb/multi-pack-verbatim-reuse: (26 commits)
  t/perf: add performance tests for multi-pack reuse
  pack-bitmap: enable reuse from all bitmapped packs
  pack-objects: allow setting `pack.allowPackReuse` to "single"
  t/test-lib-functions.sh: implement `test_trace2_data` helper
  pack-objects: add tracing for various packfile metrics
  pack-bitmap: prepare to mark objects from multiple packs for reuse
  pack-revindex: implement `midx_pair_to_pack_pos()`
  pack-revindex: factor out `midx_key_to_pack_pos()` helper
  midx: implement `midx_preferred_pack()`
  git-compat-util.h: implement checked size_t to uint32_t conversion
  pack-objects: include number of packs reused in output
  pack-objects: prepare `write_reused_pack_verbatim()` for multi-pack reuse
  pack-objects: prepare `write_reused_pack()` for multi-pack reuse
  pack-objects: pass `bitmapped_pack`'s to pack-reuse functions
  pack-objects: keep track of `pack_start` for each reuse pack
  pack-objects: parameterize pack-reuse routines over a single pack
  pack-bitmap: return multiple packs via `reuse_partial_packfile_from_bitmap()`
  pack-bitmap: simplify `reuse_partial_packfile_from_bitmap()` signature
  ewah: implement `bitmap_is_empty()`
  pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functions
  ...

Merge branch 'jk/t1006-cat-file-objectsize-disk'

Test update.

* jk/t1006-cat-file-objectsize-disk:
  t1006: prefer shell loop to awk for packed object sizes
  t1006: add tests for %(objectsize:disk)

Merge branch 'jw/builtin-objectmode-attr'

The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.

* jw/builtin-objectmode-attr:
  attr: add builtin objectmode values support

Merge branch 'js/contributor-docs-updates'

Doc update.

* js/contributor-docs-updates:
  SubmittingPatches: hyphenate non-ASCII
  SubmittingPatches: clarify GitHub artifact format
  SubmittingPatches: clarify GitHub visual
  SubmittingPatches: provide tag naming advice
  SubmittingPatches: update extra tags list
  SubmittingPatches: discourage new trailers
  SubmittingPatches: drop ref to "What's in git.git"
  CodingGuidelines: write punctuation marks
  CodingGuidelines: move period inside parentheses

strvec: use correct member name in comments

In d70a9eb611 (strvec: rename struct fields, 2020-07-28), we renamed the
"argv" member to "v". In the same patch we also did the following rename
in strvec.c:

    -void strvec_pushv(struct strvec *array, const char **argv)
    +void strvec_pushv(struct strvec *array, const char **items)

and it appears that this s/argv/items operation was erroneously applied
to strvec.h.

Rename "items" to "v".

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

messages: mark some strings with "up-to-date" not to touch

The treewide clean-up of "up-to-date" strings done in 7560f547
(treewide: correct several "up-to-date" to "up to date", 2017-08-23)
deliberately left some out, but unlike the lines that were changed
by the commit, the lines that were deliberately left untouched by
the commit is impossible to ask "git blame" to link back to the
commit that did not touch them.

Let's do the second best thing, leave a short comment near them
explaining why those strings should not be modified or localized.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
[es: make in-code comment more developer-friendly]
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t5541: remove lockfile creation

To create error conditions, some tests set up reference locks by
directly creating its lockfile. While this works for the files reference
backend, this approach is incompatible with the reftable backend.
Refactor the test to create a d/f conflict via git-update-ref(1) instead
so that the test is reference backend agnostic.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t1401: remove lockfile creation

To create error conditions, some tests set up reference locks by
directly creating its lockfile. While this works for the files reference
backend, this approach is incompatible with the reftable backend.
Refactor the test to create a d/f conflict via git-symbolic-ref(1)
instead so that the test is reference backend agnostic.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

branch: make the advice to force-deleting a conditional one

The error message we show when the user tries to delete a not fully
merged branch describes the error and gives a hint to the user:

error: the branch 'foo' is not fully merged.
If you are sure you want to delete it, run 'git branch -D foo'.

Let's move the hint part so that it is displayed using the advice
machinery:

error: the branch 'foo' is not fully merged
hint: If you are sure you want to delete it, run 'git branch -D foo'
hint: Disable this message with "git config advice.forceDeleteBranch false"

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

advice: fix an unexpected leading space

This space was introduced, presumably unintentionally, in b3b18d1621
(advice: revamp advise API, 2020-03-02)

I notice this space due to confuse diff outputs while doing some
changes to enum advice_type.

As a reference, a recent change we have to that enum is:

    $ git show 35f0383

    ...
    diff --git a/advice.h b/advice.h
    index 0f584163f5..2affbe1426 100644
    --- a/advice.h
    +++ b/advice.h
    @@ -49,6 +49,7 @@ struct string_list;
    ADVICE_UPDATE_SPARSE_PATH,
    ADVICE_WAITING_FOR_EDITOR,
    ADVICE_SKIPPED_CHERRY_PICKS,
    +       ADVICE_WORKTREE_ADD_ORPHAN,
     };

Note the hunk header, instead of a much more expected:

    @@ -49,6 +49,7 @@ enum advice_type

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

advice: sort the advice related lists

Let's keep the advice related lists sorted to make them more
digestible.

A multi-line comment has also been changed; that produces the unexpected
'insertion != deletion' in this supposedly 'only sort lines' commit.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-p4: stop reaching into the refdb

The git-p4 tool creates a bunch of temporary branches that share a
common prefix "refs/git-p4-tmp/". These branches get cleaned up via
git-update-ref(1) after the import has finished. Once done, we try to
manually remove the now supposedly-empty ".git/refs/git-p4-tmp/"
directory.

This last step can fail in case there still are any temporary branches
around that we failed to delete because `os.rmdir()` refuses to delete a
non-empty directory. It can thus be seen as kind of a sanity check to
verify that we really did delete all temporary branches. Another failure
mode though is when the directory didn't exist in the first place, which
can be the case when using an alternate ref backend like the upcoming
"reftable" backend.

Convert the code to instead use git-for-each-ref(1) to verify that there
are no more temporary branches around. This works alright with alternate
ref backends while retaining the sanity check that we really did prune
all temporary branches.

This is a modification in behaviour for the "files" backend because the
empty directory does not get deleted anymore. But arguably we should not
care about such implementation details of the ref backend anyway, and
this should not cause any user-visible change in behaviour.

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

gitweb: die when a configuration file cannot be read

Fix a possibility of a permission to access error go unnoticed.

Perl uses two different variables to manage errors from a "do
$filename" construct. One is $@, which is set in this case when do
is unable to compile the file. The other is $!, which is set in case
do cannot read the file.  The current code only checks "$@", which
means a configuration file passed to GitWeb that is not readable by
the server process does not cause it to "die".

Make sure we also check and act on "$!" to fix this.

Signed-off-by: Marcelo Roberto Jimenez <marcelo.jimenez@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: refer to pathspec instead of path

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: use singular form of repeatable path arg

This is more correct because the <path>... doc syntax already indicates
that the arg is "array-type".  It's how other tools do it.  Finally, the
later document text mentions 'path' arguments, while it doesn't mention
'paths'.

Signed-off-by: Britton Leo Kerin <britton.kergin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t4129: prevent loss of exit code due to the use of pipes

Piping the output of git commands like git-ls-files to another
command (grep in this case) hides the exit code returned by
these commands. Prevent this by storing the output of git-ls-files
to a temporary file and then "grep-ping" from that file. Replace
grep with test_grep as the latter is more verbose when it fails.

Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mingw: give more details about unsafe directory's ownership

Add domain/username in error message, if owner sid of repository and
user sid are not equal on windows systems.

Old error message:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'S-1-5-21-571067702-4104414259-3379520149-500'
but the current user is:
'S-1-5-21-571067702-4104414259-3379520149-1001'
To add an exception for this directory, call:

git config --global --add safe.directory C:/Users/test/source/repos/git
'''

New error message:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
        DESKTOP-L78JVA6/Administrator (S-1-5-21-571067702-4104414259-3379520149-500)
but the current user is:
        DESKTOP-L78JVA6/test (S-1-5-21-571067702-4104414259-3379520149-1001)
To add an exception for this directory, call:

        git config --global --add safe.directory C:/Users/test/source/repos/git
'''

Signed-off-by: Sören Krecker <soekkle@freenet.de>
Acked-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The sixth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'rs/mem-pool-improvements'

MemPool allocator fixes.

* rs/mem-pool-improvements:
  mem-pool: simplify alignment calculation
  mem-pool: fix big allocations

Merge branch 'rs/fast-import-simplify-mempool-allocation'

Code simplification.

* rs/fast-import-simplify-mempool-allocation:
  fast-import: use mem_pool_calloc()

Merge branch 'en/sparse-checkout-eoo'

"git sparse-checkout (add|set) --[no-]cone --end-of-options" did
not handle "--end-of-options" correctly after a recent update.

* en/sparse-checkout-eoo:
  sparse-checkout: be consistent with end of options markers

Merge branch 'jc/sparse-checkout-set-default-fix'

"git sparse-checkout set" added default patterns even when the
patterns are being fed from the standard input, which has been
corrected.

* jc/sparse-checkout-set-default-fix:
  sparse-checkout: use default patterns for 'set' only !stdin

Merge branch 'en/header-cleanup'

Remove unused header "#include".

* en/header-cleanup:
  treewide: remove unnecessary includes in source files
  treewide: add direct includes currently only pulled in transitively
  trace2/tr2_tls.h: remove unnecessary include
  submodule-config.h: remove unnecessary include
  pkt-line.h: remove unnecessary include
  line-log.h: remove unnecessary include
  http.h: remove unnecessary include
  fsmonitor--daemon.h: remove unnecessary includes
  blame.h: remove unnecessary includes
  archive.h: remove unnecessary include
  treewide: remove unnecessary includes in source files
  treewide: remove unnecessary includes from header files

Merge branch 'ml/doc-merge-updates'

Doc update.

* ml/doc-merge-updates:
  Documentation/git-merge.txt: use backticks for command wrapping
  Documentation/git-merge.txt: fix reference to synopsis

Merge branch 'jc/archive-list-with-extra-args'

"git archive --list extra garbage" silently ignored excess command
line parameters, which has been corrected.

* jc/archive-list-with-extra-args:
  archive: "--list" does not take further options

fetch: add new config option fetch.all

Introduce a boolean configuration option fetch.all which allows to
fetch all available remotes by default. The config option can be
overridden by explicitly specifying a remote or by using --no-all.
The behavior for --all is unchanged and calling git-fetch with --all
and a remote will still result in an error.

Additionally, describe the configuration variable in the config
documentation and implement new tests to cover the expected behavior.
Also add --no-all to the command-line documentation of git-fetch.

Signed-off-by: Tamino Bauknecht <dev@tb6.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/worktree: create refdb via ref backend

When creating a worktree we create the worktree's ref database manually
by first writing a "HEAD" file so that the directory is recognized as a
Git repository by other commands, and then running git-update-ref(1) or
git-symbolic-ref(1) to write the actual value. But while this is fine
for the files backend, this logic simply assumes too much about how the
ref backend works and will leave behind an invalid ref database once any
other ref backend lands.

Refactor the code to instead use `refs_init_db()` to initialize the ref
database so that git-worktree(1) itself does not need to know about how
to initialize it. This will allow future ref backends to customize how
the per-worktree ref database is set up. Furthermore, as we now already
have a worktree ref store around, we can also avoid spawning external
commands to write the HEAD reference and instead use the refs API to do
so.

Note that we do not have an equivalent to passing the `--quiet` flag to
git-symbolic-ref(1) as we did before. This flag does not have an effect
anyway though, as git-symbolic-ref(1) only honors it when reading a
symref, but never when writing one.

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

worktree: expose interface to look up worktree by name

Our worktree interfaces do not provide a way to look up a worktree by
its name. Expose `get_linked_worktree()` to allow for this usecase. As
callers are responsible for freeing this worktree, introduce a new
function `free_worktree()` that does so.

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

builtin/worktree: move setup of commondir file earlier

Shuffle around how we create supporting worktree files so that we first
ensure that the worktree has all link files ("gitdir", "commondir")
before we try to initialize the ref database by writing "HEAD". This
will be required by a subsequent commit where we start to initialize the
ref database via `refs_init_db()`, which will require an initialized
`struct worktree *`.

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

refs/files: skip creation of "refs/{heads,tags}" for worktrees

The files ref backend will create both "refs/heads" and "refs/tags" in
the Git directory. While this logic makes sense for normal repositories,
it does not for worktrees because those refs are "common" refs that
would always be contained in the main repository's ref database.

Introduce a new flag telling the backend that it is expected to create a
per-worktree ref database and skip creation of these dirs in the files
backend when the flag is set. No other backends (currently) need
worktree-specific logic, so this is the only required change to start
creating per-worktree ref databases via `refs_init_db()`.

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

setup: move creation of "refs/" into the files backend

When creating the ref database we unconditionally create the "refs/"
directory in "setup.c". This is a mandatory prerequisite for all Git
repositories regardless of the ref backend in use, because Git will be
unable to detect the directory as a repository if "refs/" doesn't exist.

We are about to add another new caller that will want to create a ref
database when creating worktrees. We would require the same logic to
create the "refs/" directory even though the caller really should not
care about such low-level details. Ideally, the ref database should be
fully initialized after calling `refs_init_db()`.

Move the code to create the directory into the files backend itself to
make it so. This means that future ref backends will also need to have
equivalent logic around to ensure that the directory exists, but it
seems a lot more sensible to have it this way round than to require
callers to create the directory themselves.

An alternative to this would be to create "refs/" in `refs_init_db()`
directly. This feels conceptually unclean though as the creation of the
refdb is now cluttered across different callsites. Furthermore, both the
"files" and the upcoming "reftable" backend write backend-specific data
into the "refs/" directory anyway, so splitting up this logic would only
make it harder to reason about.

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

refs: prepare `refs_init_db()` for initializing worktree refs

The purpose of `refs_init_db()` is to initialize the on-disk files of a
new ref database. The function is quite inflexible right now though, as
callers can neither specify the `struct ref_store` nor can they pass any
flags.

Refactor the interface to accept both of these. This will be required so
that we can start initializing per-worktree ref databases via the ref
backend instead of open-coding the initialization in "worktree.c".

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

Merge branch 'ps/refstorage-extension' into ps/worktree-refdb-initialization

* ps/refstorage-extension:
  t9500: write "extensions.refstorage" into config
  builtin/clone: introduce `--ref-format=` value flag
  builtin/init: introduce `--ref-format=` value flag
  builtin/rev-parse: introduce `--show-ref-format` flag
  t: introduce GIT_TEST_DEFAULT_REF_FORMAT envvar
  setup: introduce GIT_DEFAULT_REF_FORMAT envvar
  setup: introduce "extensions.refStorage" extension
  setup: set repository's formats on init
  setup: start tracking ref storage format
  refs: refactor logic to look up storage backends
  worktree: skip reading HEAD when repairing worktrees
  t: introduce DEFAULT_REPO_FORMAT prereq
  builtin/clone: create the refdb with the correct object format
  builtin/clone: skip reading HEAD when retrieving remote
  builtin/clone: set up sparse checkout later
  builtin/clone: fix bundle URIs with mismatching object formats
  remote-curl: rediscover repository when fetching refs
  setup: allow skipping creation of the refdb
  setup: extract function to create the refdb

ci: add job performing static analysis on GitLab CI

Our GitHub Workflows definitions have a static analysis job that
runs the following tasks:

  - Coccinelle to check for suggested refactorings.

  - `make hdr-check` to check for missing includes or forward
    declarations in our header files.

  - `make check-pot` to check our translations for issues.

  - `./ci/check-directional-formatting.bash` to check whether our
    sources contain any Unicode directional formatting code points.

Add an equivalent job to our GitLab CI definitions.

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

git-prompt: stop manually parsing HEAD with unknown ref formats

We're manually parsing the HEAD reference in git-prompt to figure out
whether it is a symbolic or direct reference. This makes it intimately
tied to the on-disk format we use to store references and will stop
working once we gain additional reference backends in the Git project.

Ideally, we would refactor the code to exclusively use plumbing tools to
read refs such that we do not have to care about the on-disk format at
all. Unfortunately though, spawning processes can be quite expensive on
some systems like Windows. As the Git prompt logic may be executed quite
frequently we try very hard to spawn as few processes as possible. This
refactoring is thus out of question for now.

Instead, condition the logic on the repository's ref format: if the repo
uses the the "files" backend we can continue to use the old logic and
read the respective files from disk directly. If it's anything else,
then we use git-symbolic-ref(1) to read the value of HEAD.

This change makes the Git prompt compatible with the upcoming "reftable"
format.

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

Merge branch 'ps/refstorage-extension' into ps/prompt-parse-HEAD-futureproof

* ps/refstorage-extension:
  t9500: write "extensions.refstorage" into config
  builtin/clone: introduce `--ref-format=` value flag
  builtin/init: introduce `--ref-format=` value flag
  builtin/rev-parse: introduce `--show-ref-format` flag
  t: introduce GIT_TEST_DEFAULT_REF_FORMAT envvar
  setup: introduce GIT_DEFAULT_REF_FORMAT envvar
  setup: introduce "extensions.refStorage" extension
  setup: set repository's formats on init
  setup: start tracking ref storage format
  refs: refactor logic to look up storage backends
  worktree: skip reading HEAD when repairing worktrees
  t: introduce DEFAULT_REPO_FORMAT prereq

branch: clarify <oldbranch> term

Since 52d59cc645 (branch: add a --copy (-c) option to go with --move
(-m), 2017-06-18) <oldbranch> is used in more operations than just -m.

Let's also clarify what we do if <oldbranch> is omitted.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

rebase: clarify --reschedule-failed-exec default

Documentation should mention the default behavior.

It is better to explain the persistent nature of the
--reschedule-failed-exec flag from the user standpoint, rather than from
the implementation standpoint.

Signed-off-by: Illia Bobyr <illia.bobyr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

index-pack: spawn threads atomically

The t5309 script triggers a racy false positive with SANITIZE=leak on a
multi-core system. Running with "--stress --run=6" usually fails within
10 seconds or so for me, complaining with something like:

    + git index-pack --fix-thin --stdin
    fatal: REF_DELTA at offset 46 already resolved (duplicate base 01d7713666f4de822776c7622c10f1b07de280dc?)

    =================================================================
    ==3904583==ERROR: LeakSanitizer: detected memory leaks

    Direct leak of 32 byte(s) in 1 object(s) allocated from:
        #0 0x7fa790d01986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98
        #1 0x7fa790add769 in __pthread_getattr_np nptl/pthread_getattr_np.c:180
        #2 0x7fa790d117c5 in __sanitizer::GetThreadStackTopAndBottom(bool, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:150
        #3 0x7fa790d11957 in __sanitizer::GetThreadStackAndTls(bool, unsigned long*, unsigned long*, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:598
        #4 0x7fa790d03fe8 in __lsan::ThreadStart(unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../src/libsanitizer/lsan/lsan_posix.cpp:51
        #5 0x7fa790d013fd in __lsan_thread_start_func ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:440
        #6 0x7fa790adc3eb in start_thread nptl/pthread_create.c:444
        #7 0x7fa790b5ca5b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

    SUMMARY: LeakSanitizer: 32 byte(s) leaked in 1 allocation(s).
    Aborted

What happens is this:

  0. We construct a bogus pack with a duplicate object in it and trigger
     index-pack.

  1. We spawn a bunch of worker threads to resolve deltas (on my system
     it is 16 threads).

  2. One of the threads sees the duplicate object and bails by calling
     exit(), taking down all of the threads. This is expected and is the
     point of the test.

  3. At the time exit() is called, we may still be spawning threads from
     the main process via pthread_create(). LSan hooks thread creation
     to update its book-keeping; it has to know where each thread's
     stack is (so it can find entry points for reachable memory). So it
     calls pthread_getattr_np() to get information about the new thread.
     That may allocate memory that must be freed with a matching call to
     pthread_attr_destroy(). Probably LSan does that immediately, but
     if you're unlucky enough, the exit() will happen while it's between
     those two calls, and the allocated pthread_attr_t appears as a
     leak.

This isn't a real leak. It's not even in our code, but rather in the
LSan instrumentation code. So we could just ignore it. But the false
positive can cause people to waste time tracking it down.

It's possibly something that LSan could protect against (e.g., cover the
getattr/destroy pair with a mutex, and then in the final post-exit()
check for leaks try to take the same mutex). But I don't know enough
about LSan to say if that's a reasonable approach or not (or if my
analysis is even completely correct).

In the meantime, it's pretty easy to avoid the race by making creation
of the worker threads "atomic". That is, we'll spawn all of them before
letting any of them start to work. That's easy to do because we already
have a work_lock() mutex for handing out that work. If the main process
takes it, then all of the threads will immediately block until we've
finished spawning and released it.

This shouldn't make any practical difference for non-LSan runs. The
thread spawning is quick, and could happen before any worker thread gets
scheduled anyway.

Probably other spots that use threads are subject to the same issues.
But since we have to manually insert locking (and since this really is
kind of a hack), let's not bother with them unless somebody experiences
a similar racy false-positive in practice.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: retain commit slab when closing NULL commit_graph

This fixes a regression introduced in ac6d45d11f (commit-graph: move
slab-clearing to close_commit_graph(), 2023-10-03), in which running:

  git -c fetch.writeCommitGraph=true fetch --recurse-submodules

multiple times in a freshly cloned repository causes a segfault. What
happens in the second (and subsequent) runs is this:

  1. We make a "struct commit" for any ref tips which we're storing
     (even if we already have them, they still go into FETCH_HEAD).

     Because the first run will have created a commit graph, we'll find
     those commits in the graph.

     The commit struct is therefore created with a NULL "maybe_tree"
     entry, because we can load its oid from the graph later. But to do
     that we need to remember that we got the commit from the graph,
     which is recorded in a global commit_graph_data_slab object.

  2. Because we're using --recurse-submodules, we'll try to fetch each
     of the possible submodules. That implies creating a separate
     "struct repository" in-process for each submodule, which will
     require a later call to repo_clear().

     The call to repo_clear() calls raw_object_store_clear(), which in
     turn calls close_object_store(), which in turn calls
     close_commit_graph(). And the latter frees the commit graph data
     slab.

  3. Later, when trying to write out a new commit graph, we'll ask for
     their tree oid via get_commit_tree_oid(), which will see that the
     object is parsed but with a NULL maybe_tree field. We'd then
     usually pull it from the graph file, but because the slab was
     cleared, we don't realize that we can do so! We end up returning
     NULL and segfaulting.

     (It seems questionable that we'd write a graph entry for such a
     commit anyway, since we know we already have one. I didn't
     double-check, but that may simply be another side effect of having
     cleared the slab).

The bug is in step (2) above. We should not be clearing the slab when
cleaning up the submodule repository structs. Prior to ac6d45d11f, we
did not do so because it was done inside a helper function that returned
early when it saw NULL. So the behavior change from that commit is that
we'll now _always_ clear the slab via repo_clear(), even if the
repository being closed did not have a commit graph (and thus would have
a NULL commit_graph struct).

The most immediate fix is to add in a NULL check in close_commit_graph(),
making it a true noop when passed in an object_store with a NULL
commit_graph (it's OK to just return early, since the rest of its code
is already a noop when passed NULL). That restores the pre-ac6d45d11f
behavior. And that's what this patch does, along with a test that
exercises it (we already have a test that uses submodules along with
fetch.writeCommitGraph, but the bug only triggers when there is a
subsequent fetch and when that fetch uses --recurse-submodules).

So that fixes the regression in the least-risky way possible.

I do think there's some fragility here that we might want to follow up
on. We have a global commit_graph_data_slab that contains graph
positions, and our global commit structs depend on the that slab
remaining valid. But close_commit_graph() is just about closing _one_
object store's graph. So it's dangerous to call that function and clear
the slab without also throwing away any "struct commit" we might have
parsed that depends on it.

Which at first glance seems like a bug we could already trigger. In the
situation described here, there is no commit graph in the submodule
repository, so our commit graph is NULL (in fact, in our test script
there is no submodule repo at all, so we immediately return from
repo_init() and call repo_clear() only to free up memory). But what
would happen if there was one? Wouldn't we see a non-NULL commit_graph
entry, and then clear the global slab anyway?

The answer is "no", but for very bizarre reasons. Remember that
repo_clear() calls raw_object_store_clear(), which then calls
close_object_store() and thus close_commit_graph(). But before it does
so, raw_object_store_clear() does something else: it frees the commit
graph and sets it to NULL! So by this code path we'll _never_ see a
non-NULL commit_graph struct, and thus never clear the slab.

So it happens to work out. But it still seems questionable to me that we
would clear a global slab (which might still be in use) when closing the
commit graph. This clearing comes from 957ba814bf (commit-graph: when
closing the graph, also release the slab, 2021-09-08), and was fixing a
case where we really did need it to be closed (and in that case we
presumably call close_object_store() more directly).

So I suspect there may still be a bug waiting to happen there, as any
object loaded before the call to close_object_store() may be stranded
with a bogus maybe_tree entry (and thus looking at it after the call
might cause an error). But I'm not sure how to trigger it, nor what the
fix should look like (you probably would need to "unparse" any objects
pulled from the graph). And so this patch punts on that for now in favor
of fixing the recent regression in the most direct way, which should not
have any other fallouts.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

write-or-die: make GIT_FLUSH a Boolean environment variable

Among Git's environment variables, the ones marked as "Boolean"
accept values in a way similar to Boolean configuration variables,
i.e. values like 'yes', 'on', 'true' and positive numbers are
taken as "on" and values like 'no', 'off', 'false' are taken as
"off".

GIT_FLUSH can be used to force Git to use non-buffered I/O when
writing to stdout. It can only accept two values, '1' which causes
Git to flush more often and '0' which makes all output buffered.
Make GIT_FLUSH accept more values besides '0' and '1' by turning it
into a Boolean environment variable, modifying the required logic.
Update the related documentation.

Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Documentation: fix statement about rebase.instructionFormat

Since commit 62db5247 (rebase -i: generate the script via
rebase--helper, 2017-07-14), the short hash is given in
rebase-todo. Specifying rebase.instructionFormat does not alter this
behavior, contrary to what the documentation implies.

Signed-off-by: Maarten van der Schrieck <maarten@thingsconnected.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>