last-modified: change default max-depth to 0

By default git-last-modified(1) doesn't recurse into subtrees. So when
the pathspec contained a path in a subtree, the command would only print
the commit information about the parent tree of the path, like:

    $ git last-modified -- path/file
    aaa0aab1bbb2bcc3ccc4ddd5dde6eee7eff8fff9 path

Change the default behavior to give commit information about the exact
path instead:

    $ git last-modified -- path/file
    aaa0aab1bbb2bcc3ccc4ddd5dde6eee7eff8fff9 path/file

To achieve this, the default max-depth is changed to 0 and recursive is
always enabled.

The handling of option '-r' is modified to disable a max-depth,
resulting in the behavior of this option to remain unchanged.

No existing tests were modified, because there didn't exist any tests
covering the example above. But more tests are added to cover this now.

Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

last-modified: document option '--max-depth'

Option --max-depth is supported by git-last-modified(1), because it was
added to the diff machinery in a1dfa5448d (diff: teach tree-diff a
max-depth parameter, 2025-08-07).

This option is useful for everyday use of the git-last-modified(1)
command, so document it's existence in the man page.

To have it also appear in the help output of `git last-modified -h`,
move the handling of '--max-depth' to parse_options() in
builtin/last-modified.c itself. This prepares for the change in default
behavior in the next commit.

Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

last-modified: document option '-z'

The command git-last-modified(1) already recognizes the option '-z', and
similar to many other commands this will make the output NUL-terminated
instead of using newlines. Although, this option is missing from the
documentation, so add it.

In addition to that, to have '-z' also appear in the help output of `git
last-modified -h`, move the handling of '-z' to parse_options() in
builtin/last-modified.c itself.

Before, the parsing of option '-z' was done by diff_opt_parse(), which
is called by setup_revisions(). That would fill in `struct
diff_options::line_termination`, but that field was not used by the diff
machinery itself. Thus it makes more sense to have the handling of that
option completely in builtin/last-modified.c.

Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

last-modified: clarify in the docs the command takes a pathspec

The documentation mentions git-last-modified(1) takes `<path>...`, but
that argument actually accepts a pathspec. Reword the documentation to
reflect that.

Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jk/remote-tracking-ref-leakfix' into hn/status-compare-with-push

* jk/remote-tracking-ref-leakfix:
  remote: always allocate branch.push_tracking_ref
  remote: fix leak in branch_get_push_1() with invalid "simple" config
  remote: drop const return of tracking_for_push_dest()
  remote: return non-const pointer from error_buf()

replay: drop rev-list formatting options from manual

The rev-list options in our manuals are quite long; git-replay's manual
is no exception. Since replay doesn't use the formatting options at all
(it has its own output format), drop them.

This is the first time we have needed compound tests [1] for if[n]def in
our documentation:

    git grep '^ifn\?def::' Documentation | grep '[,+]'

[1]: https://docs.asciidoctor.org/asciidoc/latest/directives/ifdef-ifndef/

For both ifdef and ifndef, the "," takes on the intuitive meaning:
- ifdef: if any of the listed attributes are set…
- ifndef: unless any of the listed attributes are set

(Use "+" for "all".)

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

gitk: fix highlighted remote prefix of branches with directories

The decoration of a remote ref is colored in two parts: (1) the prefix
that mentions the remove (including "remote/"); and (2) the branch name.
To extract the prefix from the ref name, a regular expression is used.
However, the expression is not restrictive enough: it picks everything
before the last slash character as prefix, so that, for example, the
ref name "remotes/orgin/ml/themes" is split into "remotes/origin/ml" and
"themes".

Tighten the regular expression so that only the name of the remote is
pulled into the prefix, but no part of the branch name. This gives the
desired result in the example: "remotes/origin" and "ml/themes".

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

mailmap: add an entry for Phillip Wood

While all my commits appear under the same address, other addresses
appear in some commit trailers. Map those addresses to the canonical
one.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

remote: always allocate branch.push_tracking_ref

In branch_get_push(), we usually allocate a new string for the @{push}
ref, but will not do so in push.default=upstream mode, where we just
pass back the result of branch_get_upstream() directly.

This led to a hacky memory management scheme in e291c75a95 (remote.c:
add branch_get_push, 2015-05-21): we store the result in the
push_tracking_ref field of a "struct branch", under the assumption that
the branch struct will last until the end of the program. So even though
the struct doesn't know if it has an allocated string or not, it doesn't
matter because we hold on to it either way.

But that assumption was violated by f5ccb535cc (remote: fix leaking
config strings, 2024-08-22), which added a function to free branch
structs. Any struct which is fed to branch_release() is at risk of
leaking its push_tracking_ref member.

I don't think this can actually be triggered in practice. We rarely
actually free the branch structs, and we only fill in the
push_tracking_ref string lazily when it is needed. So triggering the
leak would require a code path that does both, and I couldn't find one.

Still, this is an ugly trap that may eventually spring on us. Since
there is only one code path in branch_get_push() that doesn't allocate,
let's just have it copy the string. And then we know that
push_tracking_ref is always allocated, and we can free it in
branch_release().

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

remote: fix leak in branch_get_push_1() with invalid "simple" config

Most of the code paths in branch_get_push_1() allocate a string for the
@{push} value. We then return the result, which is stored in a "struct
branch", so the value is not leaked.

But there's one path that does leak: when we are in the "simple" push
mode, we have to check that the @{push} value matches what we'd get for
@{upstream}. If it doesn't, we return an error, but forget to free the
@{push} value we computed.

Curiously, the existing tests don't trigger this with LSan, even though
they do exercise the code path. As far as I can tell, it should be
triggered via:

  git -c push.default=simple \
      -c branch.foo.remote=origin \
      -c branch.foo.merge=refs/heads/not-foo \
      rev-parse foo@{push}

which will complain that the upstream ("not-foo") does not match the
push destination ("foo"). We do die() shortly after this, but not until
after returning from branch_get_push_1(), which is where the leak
happens.

So it seems like a false negative in LSan. However, I can trigger it
reliably by printing the @{push} value using for-each-ref. This takes a
little more setup (because we need "foo" to actually exist to iterate
over it with for-each-ref), but we can piggy-back on the existing repo
config in t6300.

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

remote: drop const return of tracking_for_push_dest()

The string returned from tracking_for_push_dest() comes from
apply_refspec(), and thus is always an allocated string (or NULL). We
should return a non-const pointer so that the caller knows that
ownership of the string is being transferred.

This goes back to the function's origin in e291c75a95 (remote.c: add
branch_get_push, 2015-05-21). It never really mattered because our
return is just forwarded through branch_get_push_1(), which returns a
const string as part of an intentionally hacky memory management scheme
(see that commit for details).

As the first step of untangling that hackery, let's drop the extra const
from this helper function (and from the variables that store its
result). There should be no functional change (yet).

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

remote: return non-const pointer from error_buf()

We have an error_buf() helper that functions a bit like our error()
helper, but returns NULL instead of -1. Its return type is "const char
*", but this is overly restrictive. If we use the helper in a function
that returns non-const "char *", the compiler will complain about
the implicit cast from const to non-const.

Meanwhile, the const in the helper is doing nothing useful, as it only
ever returns NULL. Let's drop the const, which will let us use it in
both types of function.

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

ci: skip CVS and P4 tests in leaks job, too

Looking at the CI logs, the p4 and cvs tests account for another 24
minutes of test time and they offer minimal value for quite a
similar reason as the previous step.

Let's introduce and use a mechanism to skip these tests to save
some resources.

Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

ci(*-leaks): skip the git-svn tests to save time

I noticed recently that the leak-checking jobs still take a lot of time,
and upon analysis, the git-svn tests contribute significantly to this.

Analyzing a recent CI run, I saw that the Git test suite contains
1,017 tests, running for approximately 5¼ hours total. Of these, 65
git-svn-related tests (~6% of test count) took 42.24 minutes combined,
accounting for ~13.% of the total runtime. This implies that the git-svn
tests are roughly twice as expernsive compared to the other tests.

However, testing git-svn in the leak-checking jobs provides minimal
value: git-svn is implemented as a Perl script, and leak checking only
handles C code. While git-svn does call into Git's built-in commands
that are implemented in C, these are standard Git operations that are
already thoroughly exercised elsewhere in the test suite. Therefore,
running the git-svn tests in the leak-checking jobs only adds to the
overall run time with little value in return.

Given that the leak-checking jobs are particularly time-intensive and
these 42+ minutes of SVN tests per job provide no additional leak
detection value, skip them in the *-leaks jobs to reduce CI runtime.

Assisted-by: Claude Sonnet 4.5
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t1005: modernize "! test -f" to "test_path_is_missing"

Replace instances of "! test -f <file>" with "test_path_is_missing <file>".
This macro provides better diagnostics when the test fails (it prints
"Path exists:" instead of silently failing).

Signed-off-by: Tian Yuchen <a3205153416@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

help: report on whether or not gettext is enabled

When users report that Git has no localized output, we need to check not
only their locale settings, but also whether Git was built with GETTEXT
support in the first place.

Expose this information via the existing build info output by adding a
"gettext: enabled" line to `git version --build-options` (and therefore
also to `git bugreport`) when `NO_GETTEXT` is not defined at build time.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t4073: add test for diffstat paths length when containing UTF-8 chars

Add test checking the length of filepaths containing UTF-8 chars when
generating a diffstat with various `name-width`s.

Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
[jc: fixed up t/meson.build to spell the name of the new test file correctly]
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t0610-reftable-basics: mitigate a flaky test on cygwin

Test #29 ('ref transaction: corrupted tables cause failure') started to
fail intermittently for me (from v2.52.0-rc0) when running the testsuite
with '-j8'. (Also, having moved to a new laptop and windows 11, rather
than windows 10). If the test is run by hand, or without any parallelism,
then it passes without issue.

When the test fails (e.g. 1 out of 32 parallel runs) the cause is due to
a permission error while corrupting a table file:

  ./test-lib.sh: line 1010: .git/reftable/0x000000000001-0x000000000002-d89bb8ee.ref: Permission denied

This corruption is done in a shell loop, directly after a 'test_commit',
which uses an ': >"$f"' expression to truncate the file. Adding a sleep
of one second after the 'test_commit' and before the shell loop fixes
the test (it is not clear why). Replacing the redirection shell expression
with a 'test-tool truncate "$f" 0' invocation also provides a fix, which
could simply be another way to change the timing sufficiently to win the
race.

During a debug session, I tried looking at the strace output for the
shell redirection:

  $ rm /tmp/hello; echo hello >/tmp/hello; ls -l /tmp/hello
  -rw-r--r-- 1 ramsay None 6 Nov 10 17:25 /tmp/hello
  $

  $ strace -o zzz bash -c ': >/tmp/hello'
  $

Similarly, for the test-tool solution:

  $ strace -o xxx ./t/helper/test-tool truncate /tmp/hello 0
  $

When comparing the output, the differences seemed to be what you would
expect and, if anything, the shell redirect probably would have taken
longer than the test-tool solution (many fcntl() calls to dup the stdout
to the <fd>).  The call to the win32 api NtCreateFile() was identical,
apart from the first (FileHandle) parameter, of course.

In order to fix this flaky test on cygwin, despite not knowing why it
works, replace the shell redirection with the above 'test-tool truncate'
invocation.

Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t9700/test.pl: fix path type expectation on cygwin

Commit 4ec7ac101b ("t9700: accommodate for Windows paths", 2025-12-17)
changed the type of the absolute path to the git directory from unix to
win32 for both GfW and cygwin. This fixed the test for GfW but causes
new failures on cygwin, since the test expectation is that it uses unix
paths on cygwin. In order to not break cygwin, disable the new code by
removing the "or $^O eq 'cygwin'" sub-expression from the conditional
part of the fix.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge a handful more topics after -rc0

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

Merge branch 'ml/doc-blame-markup'

Doc mark-up update.

* ml/doc-blame-markup:
  doc: git-blame: convert to new doc format
  doc: blame-options: convert to new doc format

Merge branch 'kh/doc-patch-id'

"git patch-id" documentation updates.

* kh/doc-patch-id:
  doc: patch-id: --verbatim locks in --stable
  doc: patch-id: spell out the git-diff-tree(1) form
  doc: patch-id: use definite article for the result
  patch-id: use “patch ID” throughout
  doc: patch-id: capitalize Git version
  doc: patch-id: don’t use semicolon between bullet points

Merge branch 'bc/doc-stash-import-export'

Update a FAQ entry on synching two separate repositories using the
"git stash export/import" recently introduced.

* bc/doc-stash-import-export:
  gitfaq: document using stash import/export to sync working tree

Merge branch 'kj/t7101-modernize'

Test update.

* kj/t7101-modernize:
  t7101: modernize test path checks

Merge branch 'ds/builtin-doc-update'

Update in-code comment doc to match the current API.

* ds/builtin-doc-update:
  builtin.h: update documentation

Merge branch 'ac/t1420-use-more-direct-check'

Test update.

* ac/t1420-use-more-direct-check:
  t1420: modernize the lost-found test

Merge branch 'jk/cat-file-avoid-bitmap-when-unneeded'

Fix for a performance regression in "git cat-file".

* jk/cat-file-avoid-bitmap-when-unneeded:
  cat-file: only use bitmaps when filtering

Merge branch 'jk/t-perf-fixes'

Perf-test fixes.

* jk/t-perf-fixes:
  t/perf/run: preserve GIT_PERF_* from environment
  t/perf/perf-lib: fix assignment of TEST_OUTPUT_DIRECTORY

diff: improve scaling of filenames in diffstat to handle UTF-8 chars

The `show_stats()` function tries to scale the filenames in the diffstat to
ensure they don't exceed the given `name-width`. It does so by calculating
the "display width" of the characters to be dropped, but then advances the
filename pointer by that number of bytes.

However, the "display width" of a character is not always equal to its byte
count. The result is that sometimes, when displaying UTF-8 characters,
filenames exceed the given `name-width`, and frequently the bytes of the
UTF-8 characters are truncated.

The following is an example of the issue, where the 2 files are "HelloHi" and
"Hello你好", and `name-width=6`:

    ...oHi | 0
    ...<BD><A0>好 | 0

Make the filename pointer move by the actual number of bytes of the
characters to drop from the filename, rather than their display width, using
the `utf8_width()` function.

Force `len` to not be less than 0 (this happens if the given `name-width` is
2 or less), otherwise an infinite loop is entered.

Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

cocci: remove obsolete the_repository rules

035c7de9e9e (cocci: apply the "revision.h" part of
"the_repository.pending", 2023-03-28) removed the last of the repo-less
functions and macros mentioned in the_repository.cocci at the time.  No
stragglers appeared since then.  Remove the applied rules now that they
have outlived their usefulness.

Also add a reminder to eventually remove the just added rules for
tree.h.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Revert "Merge branch 'ar/run-command-hook'"

This reverts commit f406b8955295d01089ba2baf35eceadff2d11cae,
reversing changes made to 1627809eeff75e6ec936fc609e7be46d5eb2fa9e.

It seems to have caused a few regressions, two of the three known
ones we have proposed solutions for.  Let's give ourselves a bit
more room to maneuver during the pre-release freeze period and
restart once the 2.53 ships.

Git 2.53-rc0

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

Merge branch 'ps/clar-integers'

Import newer version of "clar", unit testing framework.

* ps/clar-integers:
  gitattributes: disable blank-at-eof errors for clar test expectations
  t/unit-tests: demonstrate use of integer comparison assertions
  t/unit-tests: update clar to 39f11fe

Merge branch 'kh/replay-invalid-onto-advance'

Improve the error message when a bad argument is given to the
`--onto` option of "git replay".  Test coverage of "git replay" has
been improved.

* kh/replay-invalid-onto-advance:
  t3650: add more regression tests for failure conditions
  replay: die if we cannot parse object
  replay: improve code comment and die message
  replay: die descriptively when invalid commit-ish is given
  replay: find *onto only after testing for ref name
  replay: remove dead code and rearrange

Merge branch 'ps/odb-misc-fixes'

Miscellaneous fixes on object database layer.

* ps/odb-misc-fixes:
  odb: properly close sources before freeing them
  builtin/gc: fix condition for whether to write commit graphs

Merge branch 'pt/t7800-difftool-test-racefix'

Test fixup.

* pt/t7800-difftool-test-racefix:
  t7800: fix racy "difftool --dir-diff syncs worktree" test

Documentation/config: fix replacement for --get-urlmatch

The documentation claims that --get-urlmatch is replaced by

  git config get --all --show-names --url=<URL> <name>

However, --url cannot be combined with --all, and this command
fails in practice.

Update the replacement to use only --url, which matches the
actual behavior of --get-urlmatch.

Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ps/packfile-store-in-odb-source' into ps/odb-for-each-object

* ps/packfile-store-in-odb-source:
  packfile: move MIDX into packfile store
  packfile: refactor `find_pack_entry()` to work on the packfile store
  packfile: inline `find_kept_pack_entry()`
  packfile: only prepare owning store in `packfile_store_prepare()`
  packfile: only prepare owning store in `packfile_store_get_packs()`
  packfile: move packfile store into object source
  packfile: refactor misleading code when unusing pack windows
  packfile: refactor kept-pack cache to work with packfile stores
  packfile: pass source to `prepare_pack()`
  packfile: create store via its owning source
  odb: properly close sources before freeing them
  builtin/gc: fix condition for whether to write commit graphs

Merge branch 'ps/read-object-info-improvements' into ps/odb-for-each-object

* ps/read-object-info-improvements:
  packfile: drop repository parameter from `packed_object_info()`
  packfile: skip unpacking object header for disk size requests
  packfile: disentangle return value of `packed_object_info()`
  packfile: always populate pack-specific info when reading object info
  packfile: extend `is_delta` field to allow for "unknown" state
  packfile: always declare object info to be OI_PACKED
  object-file: always set OI_LOOSE when reading object info

commit: rename `free_commit_list()` to conform to coding guidelines

Our coding guidelines say that:

  Functions that operate on `struct S` are named `S_<verb>()` and should
  generally receive a pointer to `struct S` as first parameter.

While most of the functions related to `struct commit_list` already
follow that naming schema, `free_commit_list()` doesn't.

Rename the function to address this and adjust all of its callers. Add a
compatibility wrapper for the old function name to ease the transition
and avoid any semantic conflicts with in-flight patch series. This
wrapper will be removed once Git 2.53 has been released.

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

commit: rename `reverse_commit_list()` to conform to coding guidelines

Our coding guidelines say that:

  Functions that operate on `struct S` are named `S_<verb>()` and should
  generally receive a pointer to `struct S` as first parameter.

While most of the functions related to `struct commit_list` already
follow that naming schema, `reverse_commit_list()` doesn't.

Rename the function to address this and adjust all of its callers. Add a
compatibility wrapper for the old function name to ease the transition
and avoid any semantic conflicts with in-flight patch series. This
wrapper will be removed once Git 2.53 has been released.

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

commit: rename `copy_commit_list()` to conform to coding guidelines

Our coding guidelines say that:

  Functions that operate on `struct S` are named `S_<verb>()` and should
  generally receive a pointer to `struct S` as first parameter.

While most of the functions related to `struct commit_list` already
follow that naming schema, `copy_commit_list()` doesn't.

Rename the function to address this and adjust all of its callers. Add a
compatibility wrapper for the old function name to ease the transition
and avoid any semantic conflicts with in-flight patch series. This
wrapper will be removed once Git 2.53 has been released.

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

remote-curl: use auth for probe_rpc() requests too

If a large request requires post_rpc() to call probe_rpc(), the latter
does not use the authorization credentials used for other requests. If
this fails with an HTTP 401 error and http_auth.multistage isn't set,
then the whole request just fails.

For example, using git-credential-msal [1], the following attempt to clone a
large repository fails partway through because the initial request to download
the commit history and promisor packs succeeds, but the
subsequent request to download the blobs needed to construct the working
tree fails with a 401 error and the checkout fails.

(lines removed for brevity)

  git clone --filter=blob:none https://secure-server.example/repo
  11:03:26.855369 git.c:502               trace: built-in: git clone --filter=blob:none https://secure-server.example/repo
  Cloning into 'sw'...
  warning: templates not found in /home/aaron/share/git-core/templates
  11:03:26.857169 run-command.c:673       trace: run_command: git remote-https origin https://secure-server.example/repo
  11:03:27.012104 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
  11:03:27.049243 http.c:849              <= Recv header: HTTP/1.1 401 Unauthorized
  11:03:27.049270 http.c:849              <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
  11:03:27.053786 run-command.c:673       trace: run_command: 'git credential-msal get'
  11:03:27.952830 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
  11:03:27.952849 http.c:849              => Send header: Authorization: Bearer <redacted>
  11:03:27.995419 http.c:849              <= Recv header: HTTP/1.1 200 OK
  11:03:28.230039 http.c:890              == Info: Reusing existing https: connection with host secure-server.example
  11:03:28.230208 http.c:849              => Send header: POST repo/git-upload-pack HTTP/1.1
  11:03:28.230216 http.c:849              => Send header: Content-Type: application/x-git-upload-pack-request
  11:03:28.230221 http.c:849              => Send header: Authorization: Bearer <redacted>
  11:03:28.269085 http.c:849              <= Recv header: HTTP/1.1 200 OK
  11:03:28.684163 http.c:890              == Info: Reusing existing https: connection with host secure-server.example
  11:03:28.684379 http.c:849              => Send header: POST repo/git-upload-pack HTTP/1.1
  11:03:28.684391 http.c:849              => Send header: Accept: application/x-git-upload-pack-result
  11:03:28.684393 http.c:849              => Send header: Authorization: Bearer <redacted>
  11:03:28.869546 run-command.c:673       trace: run_command: git index-pack --stdin --fix-thin '--keep=fetch-pack 43856 on dgx-spark' --promisor
  11:06:39.861237 run-command.c:673       trace: run_command: git -c fetch.negotiationAlgorithm=noop fetch origin --no-tags --no-write-fetch-head --recurse-submodules=no --filter=blob:none --stdin
  11:06:39.865981 run-command.c:673       trace: run_command: git remote-https origin https://secure-server.example/repo
  11:06:39.868039 run-command.c:673       trace: run_command: git-remote-https origin https://secure-server.example/repo
  11:07:30.412575 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
  11:07:30.456285 http.c:849              <= Recv header: HTTP/1.1 401 Unauthorized
  11:07:30.456318 http.c:849              <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
  11:07:30.456439 run-command.c:673       trace: run_command: 'git credential-cache get'
  11:07:30.461266 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
  11:07:30.461282 http.c:849              => Send header: Authorization: Bearer <redacted>
  11:07:30.501628 http.c:849              <= Recv header: HTTP/1.1 200 OK
  11:07:34.725262 http.c:849              => Send header: POST repo/git-upload-pack HTTP/1.1
  11:07:34.725279 http.c:849              => Send header: Content-Type: application/x-git-upload-pack-request
  11:07:34.761407 http.c:849              <= Recv header: HTTP/1.1 401 Unauthorized
  11:07:34.761443 http.c:890              == Info: Bearer authentication problem, ignoring.
  11:07:34.761453 http.c:849              <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
  11:07:34.761509 http.c:890              == Info: The requested URL returned error: 401
  11:07:34.761530 http.c:890              == Info: closing connection #0
  11:07:34.761913 run-command.c:673       trace: run_command: 'git credential-cache erase'
  11:07:34.761927 run-command.c:765       trace: start_command: /bin/sh -c 'git credential-cache erase' 'git credential-cache erase'
  11:07:34.768069 git.c:502               trace: built-in: git credential-cache erase
  11:07:34.768690 run-command.c:673       trace: run_command: 'git credential-msal erase'
  11:07:34.768713 run-command.c:765       trace: start_command: /bin/sh -c 'git credential-msal erase' 'git credential-msal erase'
  11:07:34.772742 git.c:808               trace: exec: git-credential-msal erase
  11:07:34.772783 run-command.c:673       trace: run_command: git-credential-msal erase
  11:07:34.772819 run-command.c:765       trace: start_command: /usr/bin/git-credential-msal erase
  error: RPC failed; HTTP 401 curl 22 The requested URL returned error: 401
  fatal: unable to write request to remote: Broken pipe
  fatal: could not fetch c4fff0229c9be06ecf576356a4d39a8a755b8d81 from promisor remote
  warning: Clone succeeded, but checkout failed.
  You can inspect what was checked out with 'git status'
  and retry with 'git restore --source=HEAD :/'

In this case, the HTTP_REAUTH retry logic is not used because the
credential helper didn't set the 'continue' flag, so
http_auth.multistage is false and handle_curl_result() fails with
HTTP_NOAUTH instead.

Fix the immediate problem by including the authorization headers in the
probe_rpc() request as well.

Add a test for this scenario:

 1. Create a repository with two thousand refs.
 2. Clone that into the web root used by t5563-simple-http-auth.sh.
 3. Configure http.postBuffer to be very small in order to trigger the
    probe_rpc() path that fails.
 4. Clone using a valid Bearer token.

[1] https://github.com/Binary-Eater/git-credential-msal

Tested-by: Lucas De Marchi <demarchi@kernel.org>
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/repack: handle promisor packs with geometric repacking

When performing a fetch with an object filter, we mark the resulting
packfile as a promisor pack. An object part of such a pack may miss any
of its referenced objects, and Git knows to handle this case by fetching
any such missing objects from the promisor remote.

The "promisor" property needs to be retained going forward. So every
time we pack a promisor object, the resulting pack must be marked as a
promisor pack. git-repack(1) does this already: when a repository has a
promisor remote, it knows to pass "--exclude-promisor-objects" to the
git-pack-objects(1) child process. Promisor packs are written separately
when doing an all-into-one repack via `repack_promisor_objects()`.

But we don't support promisor objects when doing a geometric repack yet.
Promisor packs do not get any special treatment there, as we simply
merge promisor and non-promisor packs. The resulting pack is not even
marked as a promisor pack, which essentially corrupts the repository.

This corruption couldn't happen in the real world though: we pass both
"--exclude-promisor-objects" and "--stdin-packs" to git-pack-objects(1)
if a repository has a promisor remote, but as those options are mutually
exclusive we always end up dying. And while we made those flags
compatible with one another in a preceding commit, we still end up dying
in case git-pack-objects(1) is asked to repack a promisor pack.

There's multiple ways to fix this:

  - We can exclude promisor packs from the geometric progression
    altogether. This would have the consequence that we never repack
    promisor packs at all. But in a partial clone it is quite likely
    that the user generates a bunch of promisor packs over time, as
    every backfill fetch would create another one. So this doesn't
    really feel like a sensible option.

  - We can adapt git-pack-objects(1) to support repacking promisor packs
    and include them in the normal geometric progression. But this would
    mean that the set of promisor objects expands over time as the packs
    are merged with normal packs.

  - We can use a separate geometric progression to repack promisor
    packs.

The first two options both have significant downsides, so they aren't
really feasible. But the third option fixes both of these downsides: we
make sure that promisor packs get merged, and at the same time we never
expand the set of promisor objects beyond the set of objects that are
already marked as promisor objects.

Implement this strategy so that geometric repacking works in partial
clones.

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

repack-promisor: extract function to remove redundant packs

We're about to add a second caller that wants to remove redundant packs
after a geometric repack. Split out the function which does this to
prepare for that.

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

repack-promisor: extract function to finalize repacking

We're about to add a second caller that wants to finalize repacking of
promisor objects. Split out the function which does this to prepare for
that.

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

repack-geometry: extract function to compute repacking split

We're about to add a second caller that wants to compute the repacking
split for a set of packfiles. Split out the function that computes this
split to prepare for that.

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

builtin/pack-objects: exclude promisor objects with "--stdin-packs"

It is currently not possible to combine "--exclude-promisor-objects"
with "--stdin-packs" because both flags want to set up a revision walk
to enumerate the objects to pack. In a subsequent commit though we want
to extend geometric repacks to support promisor objects, and for that we
need to handle the combination of both flags.

There are two cases we have to think about here:

  - "--stdin-packs" asks us to pack exactly the objects part of the
    specified packfiles. It is somewhat questionable what to do in the
    case where the user asks us to exclude promisor objects, but at the
    same time explicitly passes a promisor pack to us. For now, we
    simply abort the request as it is self-contradicting. As we have
    also been dying before this commit there is no regression here.

  - "--stdin-packs=follow" does the same as the first flag, but it also
    asks us to include all objects transitively reachable from any
    object in the packs we are about to repack. This is done by doing
    the revision walk mentioned further up. Luckily, fixing this case is
    trivial: we only need to modify the revision walk to also set the
    `exclude_promisor_objects` field.

Note that we do not support the "--exclude-promisor-objects-best-effort"
flag for now as we don't need it to support geometric repacking with
promisor objects.

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

gitk: use config settings for head/tag colors

The drawtags procedure currently uses headfgcolor for all label text,
ignoring the tagfgcolor setting.

The call to create the outline polygon for (non-tag) heads currently
has the color for headoutlinecolor hardcoded to black.

This patch maintains the variables for the non-tag refs so that heads
are colored differently from non-head (non-tag) refs.

The outline and fill colors for the non-head refs remain hardcoded to
the prior values, black & #ddddff.

Signed-off-by: Shannon Barber <sgbarber@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>

replay: drop commits that become empty

If the changes in a commit being replayed are already in the branch
that the commits are being replayed onto, then "git replay" creates an
empty commit. This is confusing because the commit message no longer
matches the contents of the commit. Drop the commit instead. Commits
that start off empty are not dropped. This matches the behavior of
"git rebase --reapply-cherry-pick --empty=drop" and "git cherry-pick
--empty-drop".

If a branch points to a commit that is dropped it will be updated
to point to the last commit that was not dropped. This can be seen
in the new test where "topic1" is updated to point to the rebased
"C" as "F" is dropped because it is already upstream. While this is
a breaking change, "git replay" is marked as experimental to allow
improvements like this that change the behavior.

Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ps/history' into pw/replay-drop-empty

* ps/history: (186 commits)
  builtin/history: implement "reword" subcommand
  builtin: add new "history" command
  wt-status: provide function to expose status for trees
  replay: support updating detached HEAD
  replay: support empty commit ranges
  replay: small set of cleanups
  builtin/replay: move core logic into "libgit.a"
  builtin/replay: extract core logic to replay revisions
  The 15th batch
  t3650: add more regression tests for failure conditions
  replay: die if we cannot parse object
  replay: improve code comment and die message
  replay: die descriptively when invalid commit-ish is given
  replay: find *onto only after testing for ref name
  replay: remove dead code and rearrange
  The 14th batch
  The 13th batch
  config: use git_parse_int() in git_config_get_expiry_in_days()
  receive-pack: convert receive hooks to hook API
  receive-pack: convert update hooks to new API
  ...

builtin/history: implement "reword" subcommand

Implement a new "reword" subcommand for git-history(1). This subcommand
is similar to the user performing an interactive rebase with a single
commit changed to use the "reword" instruction.

The "reword" subcommand is built on top of the replay subsystem
instead of the sequencer. This leads to some major differences compared
to git-rebase(1):

  - We do not check out the commit that is to be reworded and instead
    perform the operation in-memory. This has the obvious benefit of
    being significantly faster compared to git-rebase(1), but even more
    importantly it allows the user to rewrite history even if there are
    local changes in the working tree or in the index.

  - We do not execute any hooks, even though we leave some room for
    changing this in the future.

  - By default, all local branches that contain the commit will be
    rewritten. This especially helps with workflows that use stacked
    branches.

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

builtin: add new "history" command

When rewriting history via git-rebase(1) there are a few very common use
cases:

  - The ordering of two commits should be reversed.

  - A commit should be split up into two commits.

  - A commit should be dropped from the history completely.

  - Multiple commits should be squashed into one.

  - Editing an existing commit that is not the tip of the current
    branch.

While these operations are all doable, it often feels needlessly kludgey
to do so by doing an interactive rebase, using the editor to say what
one wants, and then perform the actions. Also, some operations like
splitting up a commit into two are way more involved than that and
require a whole series of commands.

Rebases also do not update dependent branches. The use of stacked
branches has grown quite common with competing version control systems
like Jujutsu though, so it clearly is a need that users have. While
rebases _can_ serve this use case if one always works on the latest
stacked branch, it is somewhat awkward and very easy to get wrong.

Add a new "history" command to plug these gaps. This command will have
several different subcommands to imperatively rewrite history for common
use cases like the above.

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

wt-status: provide function to expose status for trees

The "wt-status" subsystem is responsible for printing status information
around the current state of the working tree. This most importantly
includes information around whether the working tree or the index have
any changes.

We're about to introduce a new command where the changes in neither of
them are actually relevant to us. Instead, what we want is to format the
changes between two different trees. While it is a little bit of a
stretch to add this as functionality to _working tree_ status, it
doesn't make any sense to open-code this functionality, either.

Implement a new function `wt_status_collect_changes_trees()` that diffs
two trees and formats the status accordingly. This function is not yet
used, but will be in a subsequent commit.

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

replay: support updating detached HEAD

In a subsequent commit we're about to introduce a new git-history(1)
command, which will by default work on all local branches and HEAD. This
is already well-supported by the replay machinery for most of the part:
updating branches is one of its prime use cases, and the HEAD ref is
also updated in case it points to any of the branches.

However, what's not supported yet is to update HEAD in case it is not a
symbolic ref. We determine the refs that need to be updated by iterating
through the decorations of the original commit, but we only update those
refs that are `DECORATION_REF_LOCAL`, which covers local branches.

Address this gap by also handling `DECORATION_REF_HEAD`. Note though
that this needs to only happen in case we're working on a detached HEAD.
If HEAD is pointing to a branch, then we'd already update that branch
via `DECORATION_REF_LOCAL`.

Refactor the loop that iterates through the decorations a bit to make
the individual conditions easier to understand.

Based-on-patch-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

replay: support empty commit ranges

In a subsequent commit we're about to introduce a new user of the replay
subsystem. With that new user, the range of commits that we'll want to
replay will be identified implicitly via a single commit, and will
include all descendants of that commit to any branch. If that commit has
no descendants (because it's the tip of some branch), then the range of
revisions that we're asked to replay becomes empty. This case does not
make sense with git-replay(1), but with the new command it will.

This case is not currently supported by `replay_revisions()` though
because we zero-initialize `struct merge_result`. This includes its
`.clean` member, which indicates whether the merge ran into a conflict
or not. But given that we don't have any revision to replay, we won't
ever perform any merge at all, and consequently that member will never
be set to `1`. We thus later think that there's been a merge conflict
and return an error from `replay_commits()`.

Address this issue by initializing the `.clean` member to `1`.

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

replay: small set of cleanups

Perform a small set of cleanups so that the "replay" logic compiles with
"-Wsign-compare" and doesn't use `the_repository` anymore. Note that
there are still some implicit dependencies on `the_repository`, e.g.
because we use `get_commit_output_encoding()`.

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

builtin/replay: move core logic into "libgit.a"

Move the core logic used to replay commits into "libgit.a" so that it
can be easily reused by other commands. It will be used in a subsequent
commit where we're about to introduce a new git-history(1) command.

Note that with this change we have no sign-comparison warnings anymore,
and neither do we depend on `the_repository`.

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

builtin/replay: extract core logic to replay revisions

We're about to move the core logic used to replay revisions onto a new
base into the "libgit.a" library. Prepare for this by pulling out the
logic into a new function `replay_revisions()` that:

  1. Takes a set of revisions to replay and some options that tell it how
     it ought to replay the revisions.

  2. Replays the commits.

  3. Records any reference updates that would be caused by replaying the
     commits in a structure that is owned by the caller.

The logic itself will be moved into a separate file in the next commit.
This change is not expected to cause user-visible change in behaviour.

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

midx-write.c: assume checksum-invalid MIDXs require an update

In 6ce9d558ced (midx-write: skip rewriting MIDX with `--stdin-packs`
unless needed, 2025-12-10), the MIDX machinery learned how to optimize
out unnecessary writes with "--stdin-packs".

In order to do this, it compares the contents of the in-progress write
against a MIDX loaded directly from the object store. We load a separate
MIDX (as opposed to checking our update relative to "ctx.m") because the
MIDX code does not reuse an existing MIDX with --stdin-packs, and always
leaves "ctx.m" as NULL. See commit 0c5a62f14bc (midx-write.c: do not
read existing MIDX with `packs_to_include`, 2024-06-11) for details on
why.

If "ctx.m" is non-NULL, however, it is guaranteed to be checksum-valid,
since we only assign "ctx.m" when "midx_checksum_valid()" returns true.
Since the same guard does not exist for the MIDX we pass to
"midx_needs_update()", we may ignore on-disk corruption when determining
whether or not we can optimize out the write.

Add a similar guard within "midx_needs_update()" to prevent such an
issue.

A more robust fix would involve revising 0c5a62f14bc and teaching the
MIDX generation code how to reuse an existing MIDX even when invoked
with "--stdin-packs", such that we could avoid side-loading the MIDX
directly from the object store in order to call "midx_needs_update()".
For now, pursue the minimal fix.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t/t5319-multi-pack-index.sh: drop early 'test_done'

In 6ce9d558ced (midx-write: skip rewriting MIDX with `--stdin-packs`
unless needed, 2025-12-10), an extra 'test_done' was added, causing the
test script to finish before having run all of its tests.

Dropping this extraneous 'test_done' exposes a bug from commit
6ce9d558ced that causes a subsequent test to fail. Mark that test with a
'test_expect_failure' for now, and the subsequent commit will explain
and fix the bug.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ps/repack-avoid-noop-midx-rewrite' into tb/midx-write-corrupt-checksum-fix

* ps/repack-avoid-noop-midx-rewrite:
  midx-write: skip rewriting MIDX with `--stdin-packs` unless needed
  midx-write: extract function to test whether MIDX needs updating
  midx: fix `BUG()` when getting preferred pack without a reverse index

doc: MyFirstContribution: fix missing dependencies and clarify build steps

Fix issues in the MyFirstContribution guide that can lead to confusion or
test failures when following the documented steps.

* Add missing header includes in code examples (environment.h and strbuf.h).

* Correct manpage synopsis formatting to prevent failing documentation tests.

* Specify the use of parallel test execution with -j$(nproc), noting that it
runs tests using all available CPUs and may be adjusted.

These updates improve documentation accuracy and make the first-time contributor
journey smoother.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

utf8.c: enable workaround for iconv under macOS 14/15

The previous commit introduced a workaround in utf8.c to deal
with broken iconv implementations.

It is enabled when a MacOS version is used that has a buggy
iconv library and there is no external library provided
(and linked against) from neither MacPorts nor Homebrew nor Fink.
For Homebrew, MacPorts and Fink we check if libiconv exist.
Introduce 2 new macros: HAS_GOOD_LIBICONV and NEEDS_GOOD_LIBICONV.

For Homebrew HAS_GOOD_LIBICONV is set when the libiconv directory
exist.
MacPorts can be installed with or without libiconv, so check if
libiconv.dylib exists (which is a softlink)

Fink compiles and installs libiconv by default.
Note that a fresh installation of Fink now defaults to /opt/sw.
Older versions used /sw as default, so leave the check and setting
of BASIC_CFLAGS and BASIC_LDFLAGS as is.
For the new default check for the existance of /opt/sw as well.
Add a check for /opt/sw/lib/libiconv.dylib which sets HAS_GOOD_LIBICONV

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

utf8.c: prepare workaround for iconv under macOS 14/15

MacOS14 (Sonoma) has started to ship an iconv library with bugs.
The same bugs exists even in MacOS 15 (Sequoia)

A bug report running the Git test suite says:

three tests of t3900 fail on macOS 26.1 for me:

  not ok 17 - ISO-2022-JP should be shown in UTF-8 now
  not ok 25 - ISO-2022-JP should be shown in UTF-8 now
  not ok 38 - commit --fixup into ISO-2022-JP from UTF-8

Here's the verbose output of the first one:

=================
expecting success of 3900.17 'ISO-2022-JP should be shown in UTF-8 now':
                compare_with ISO-2022-JP "$TEST_DIRECTORY"/t3900/2-UTF-8.txt

 --- /Users/x/src/git/t/t3900/2-UTF-8.txt 2024-10-01 19:43:24.605230684 +0000
 +++ current     2025-12-08 21:52:45.786161909 +0000
@@ -1,5 +1,5 @@
 はれひほふ

 しているのが、いるので。
 -濱浜ほれぷりぽれまびぐりろへ。
 +濱浜ほれぷりぽれまび$0$j$m$X!#
not ok 17 - ISO-2022-JP should be shown in UTF-8 now
1..17
=================

compare_with runs git show to display a commit message, which in this
case here was encoded using ISO-2022-JP and is supposed to be reencoded
to UTF-8, but git show only does that half-way -- the "$0$j$m$X!#" part
is from the original ISO-2022-JP representation.

That botched conversion is done by utf8.c::reencode_string_iconv().  It
calls iconv(3) to do the actual work, initially with an output buffer of
the same size as the input.  If the output needs more space the function
enlarges the buffer and calls iconv(3) again.

iconv(3) won't tell us how much space it needs, but it will report what
part it already managed to convert, so we can increase the buffer and
continue from there.  ISO-2022-JP has escape codes for switching between
character sets, so it's a stateful encoding.  I guess the iconv(3) on my
machine forgets the state at the end of part one and then messes up part
two.

[end of citation]

Working around the buggy iconv shipped with the OS can be done in
two  ways:
a) Link Git against a different version of iconv
b) Improve the handling when iconv needs a larger output buffer

a) is already done by default when either Fink [1] or MacPorts [2]
   or Homebrew [3] is installed.
b) is implemented here, in case that no fixed iconv is available:
   When the output buffer is too short, increase it (as before)
   and start from scratch (this is new).

This workound needs to be enabled with
'#define ICONV_RESTART_RESET'
and a makefile knob will be added in the next commit

Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
[1] https://www.finkproject.org/
[2] https://www.macports.org/
[3] https://brew.sh/

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t5403: use test_cmp for post-checkout argument checks

Update check_post_checkout and the post-checkout hook implementation to
use test_cmp instead of individual test commands. This provides better
error messages when tests fail, making it easier to debug which specific
argument (old ref, new ref, or flag) was incorrect.

The hook now outputs in key=value format which test_cmp can display
clearly when there's a mismatch.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t5403: introduce check_post_checkout helper function

The test file repeatedly uses the same four-line pattern to validate
post-checkout hook arguments: read the args file, then test each of
the three values individually.

Introduce a check_post_checkout helper function that encapsulates this
pattern. This patch does not change test behavior; it prepares the
code for improvement in the next step.

Additionally, the 'post-checkout hook is triggered by clone' test is
improved to validate the hook arguments (old ref, new ref, and flag)
rather than just checking that the hook file was created.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: detect conflicts with existing gitdir configs

Credit goes to Emily and Josh for testing and noticing a corner-case
which caused conflicts with existing gitdir configs to silently pass
validation, then fail later in add_submodule() with a cryptic error:

fatal: A git directory for 'nested%2fsub' is found locally with remote(s):
  origin /.../trash directory.t7425-submodule-gitdir-path-extension/sub

This change ensures the validation step checks existing gitdirs for
conflicts. We only have to do this for submodules having gitdirs,
because those without submodule.%s.gitdir need to be migrated and
will throw an error earlier in the submodule codepath.

Quoting Josh:
 My testing setup has been as follows:
 * Using our locally-built Git with our downstream patch of [1] included:
   * create a repo "sub"
   * create a repo "super"
   * In "super":
     * mkdir nested
     * git submodule add ../sub nested/sub
     * Verify that the submodule's gitdir is .git/modules/nested%2fsub
 * Using a build of git from upstream `next` plus this series:
   * git config set --global extensions.submodulepathconfig true
   * git clone --recurse-submodules super super2
   * create a repo "nested%2fsub"
   * In "super2":
     * git submodule add ../nested%2fsub

At this point I'd expect the collision detection / encoding to take
effect, but instead I get the error listed above.
End quote

Suggested-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: hash the submodule name for the gitdir path

If none of the previous plain-text / encoding / derivation steps work
and case 2.4 is reached, then try a hash of the submodule name to see
if that can be a valid gitdir before giving up and throwing an error.

This is a "last resort" type of measure to avoid conflicts since it
loses the human readability of the gitdir path. This logic will be
reached in rare cases, as can be seen in the test we added.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: fix case-folding gitdir filesystem collisions

Add a new check when extension.submodulePathConfig is enabled, to
detect and prevent case-folding filesystem colisions. When this
new check is triggered, a stricter casefolding aware URI encoding
is used to percent-encode uppercase characters.

By using this check/retry mechanism the uppercase encoding is
only applied when necessary, so case-sensitive filesystems are
not affected.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule--helper: fix filesystem collisions by encoding gitdir paths

Fix nested filesystem collisions by url-encoding gitdir paths stored
in submodule.%s.gitdir, when extensions.submodulePathConfig is enabled.

Credit goes to Junio and Patrick for coming up with this design: the
encoding is only applied when necessary, to newly added submodules.

Existing modules don't need the encoding because git already errors
out when detecting nested gitdirs before this patch.

This commit adds the basic url-encoding and some tests. Next commits
extend the encode -> validate -> retry loop to fix more conflicts.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/credential-store: move is_rfc3986_unreserved to url.[ch]

is_rfc3986_unreserved() was moved to credential-store.c and was made
static by f89854362c (credential-store: move related functions to
credential-store file, 2023-06-06) under a correct assumption, at the
time, that it was the only place using it.

However now we need it to apply URL-encoding to submodule names when
constructing gitdir paths, to avoid conflicts, so bring it back as a
public function exposed via url.h, instead of the old helper path
(strbuf), which has nothing to do with 3986 encoding/decoding anymore.

This function will be used in subsequent commits which do the encoding.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule--helper: add gitdir migration command

Manually running
"git config submodule.<name>.gitdir .git/modules/<name>"
for each submodule can be impractical, so add a migration command to
submodule--helper to automatically create configs for all submodules
as required by extensions.submodulePathConfig.

The command calls create_default_gitdir_config() which validates the
gitdir paths before adding the configs.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: allow runtime enabling extensions.submodulePathConfig

Add a new config `init.defaultSubmodulePathConfig` which allows
enabling `extensions.submodulePathConfig` for new submodules by
default (those created via git init or clone).

Important: setting init.defaultSubmodulePathConfig = true does
not globally enable `extensions.submodulePathConfig`. Existing
repositories will still have the extension disabled and will
require migration (for example via git submodule--helper command
added in the next commit).

Suggested-by: Patrick Steinhardt <ps@pks.im>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: introduce extensions.submodulePathConfig

The idea of this extension is to abstract away the submodule gitdir
path implementation: everyone is expected to use the config and not
worry about how the path is computed internally, either in git or
other implementations.

With this extension enabled, the submodule.<name>.gitdir repo config
becomes the single source of truth for all submodule gitdir paths.

The submodule.<name>.gitdir config is added automatically for all new
submodules when this extension is enabled.

Git will throw an error if the extension is enabled and a config is
missing, advising users how to migrate. Migration is manual for now.

E.g. to add a missing config entry for an existing "foo" module:
git config submodule.foo.gitdir .git/modules/foo

Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/submodule--helper: add gitdir command

This exposes the gitdir name computed by submodule_name_to_gitdir()
internally, to make it easier for users and tests to interact with it.

Next commit will add a gitdir configuration, so this helper can also be
used to easily query that config or validate any gitdir path the user
sets (submodule_name_to_git_dir now runs the validation logic, since
our previous commit).

Based-on-patch-by: Brandon Williams <bwilliams.eng@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: always validate gitdirs inside submodule_name_to_gitdir

Move the ad-hoc validation checks sprinkled across the source tree,
after calling submodule_name_to_gitdir() into the function proper,
which now always validates the gitdir before returning it.

This simplifies the API and helps to:
1. Avoid redundant validation calls after submodule_name_to_gitdir().
2. Avoid the risk of callers forgetting to validate.
3. Ensure gitdir paths provided by users via configs are always valid
   (config gitdir paths are added in a subsequent commit).

The validation function can still be called as many times as needed
outside submodule_name_to_gitdir(), for example we keep two calls
which are still required, to avoid parallel clone races by re-running
the validation in builtin/submodule-helper.c.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule--helper: use submodule_name_to_gitdir in add_submodule

While testing submodule gitdir path encoding, I noticed submodule--helper
is still using a hardcoded modules gitdir path leading to test failures.

Call the submodule_name_to_gitdir() helper instead, which was invented
exactly for this purpose and is already used by all the other locations
which work on gitdirs.

Also narrow the scope of the submod_gitdir_path variable which is not
used anymore in the updated "else" branch.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/fsck: drop `fsck_head_link()`

The function `fsck_head_link()` was historically used to perform a
couple of consistency checks for refs. (Almost) all of these checks have
now been moved into the refs subsystem. There's only a single check
remaining that verifies whether `refs_resolve_ref_unsafe()` returns a
`NULL` pointer. This may happen in a couple of cases:

  - When `refs_is_safe()` declares the ref to be unsafe. We already have
    checks for this as we verify refnames with `check_refname_format()`.

  - When the ref doesn't exist. A repository without "HEAD" is
    completely broken though, and we would notice this error ahead of
    time already.

  - In case the caller passes `RESOLVE_REF_READING` and the ref is a
    symref that doesn't resolve. We don't pass this flag though.

As such, this check doesn't cover anything anymore that isn't already
covered by `refs_fsck()`. Drop it, which also allows us to inline the
call to `refs_resolve_ref_unsafe()`.

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

builtin/fsck: move generic HEAD check into `refs_fsck()`

Move the check that detects "HEAD" refs that do not point at a branch
into `refs_fsck()`. This follows the same motivation as the preceding
commit.

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

builtin/fsck: move generic object ID checks into `refs_fsck()`

While most of the logic that verifies the consistency of refs is
driven by `refs_fsck()`, we still have a small handful of checks in
`fsck_head_link()`. These checks don't use the git-fsck(1) reporting
infrastructure, and as such it's impossible to for example disable
some of those checks.

One such check detects refs that point to the all-zeroes object ID.
Extract this check into the generic `refs_fsck_ref()` function that is
used by both the "files" and "reftable" backends.

Note that this will cause us to not return an error code from
`fsck_head_link()` anymore in case this error was detected. This is fine
though: the only caller of this function does not check the error code
anyway. To demonstrate this, adapt the function to drop its return value
altogether. The function will be removed in a subsequent commit anyway.

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

refs/reftable: introduce generic checks for refs

In a preceding commit we have extracted generic checks for both direct
and symbolic refs that apply for all backends. Wire up those checks for
the "reftable" backend.

Note that this is done by iterating through all refs manually with the
low-level reftable ref iterator. We explicitly don't want to use the
higher-level iterator that is exposed to users of the reftable backend
as that iterator may swallow for example broken refs.

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

refs/reftable: fix consistency checks with worktrees

The ref consistency checks are driven via `cmd_refs_verify()`. That
function loops through all worktrees (including the main worktree) and
then checks the ref store for each of them individually. It follows that
the backend is expected to only verify refs that belong to the specified
worktree.

While the "files" backend handles this correctly, the "reftable" backend
doesn't. In fact, it completely ignores the passed worktree and instead
verifies refs of _all_ worktrees. The consequence is that we'll end up
every ref store N times, where N is the number of worktrees.

Or rather, that would be the case if we actually iterated through the
worktree reftable stacks correctly. But we use `strmap_for_each_entry()`
to iterate through the stacks, but the map is in fact not even properly
populated. So instead of checking stacks N^2 times, we actually only end
up checking the reftable stack of the main worktree.

Fix this bug by only verifying the stack of the passed-in worktree and
constructing the backends via `backend_for_worktree()`.

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

refs/reftable: extract function to retrieve backend for worktree

Pull out the logic to retrieve a backend for a given worktree. This
function will be used in a subsequent commit.

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

refs/reftable: adapt includes to become consistent

Adapt the includes to be sorted and to use include paths that are
relative to the "refs/" directory.

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

refs/files: introduce function to perform normal ref checks

In a subsequent commit we'll introduce new generic checks for direct
refs. These checks will be independent of the actual backend.

Introduce a new function `refs_fsck_ref()` that will be used for this
purpose. At the current point in time it's still empty, but it will get
populated in a subsequent commit.

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

refs/files: extract generic symref target checks

The consistency checks for the "files" backend contain a couple of
verifications for symrefs that verify generic properties of the target
reference. These properties need to hold for every backend, no matter
whether it's using the "files" or "reftable" backend.

Reimplementing these checks for every single backend doesn't really make
sense. Extract it into a generic `refs_fsck_symref()` function that can
be used by other backends, as well. The "reftable" backend will be wired
up in a subsequent commit.

While at it, improve the consistency checks so that we don't complain
about refs pointing to a non-ref target in case the target refname
format does not verify. Otherwise it's very likely that we'll generate
both error messages, which feels somewhat redundant in this case.

Note that the function has a couple of `UNUSED` parameters. These will
become referenced in a subsequent commit.

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

fsck: drop unused fields from `struct fsck_ref_report`

The `struct fsck_ref_report` has a couple fields that are intended to
improve the error reporting for broken ref reports by showing which
object ID or target reference the ref points to. These fields are never
set though and are thus essentially unused.

Remove them.

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

refs/files: perform consistency checks for root refs

While the "files" backend already knows to perform consistency checks
for the "refs/" hierarchy, it doesn't verify any of its root refs. Plug
this omission.

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

refs/files: improve error handling when verifying symrefs

The error handling when verifying symbolic refs is a bit on the wild
side:

  - `fsck_report_ref()` can be told to ignore specific errors. If an
    error has been ignored and a previous check raised an unignored
    error, then assigning `ret = fsck_report_ref()` will cause us to
    swallow the previous error.

  - When the target reference is not valid we bail out early without
    checking for other errors.

Fix both of these issues by consistently or'ing the return value and not
bailing out early.

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

refs/files: extract function to check single ref

When checking the consistency of references we create a directory
iterator and then verify each single reference in a loop. The logic to
perform the actual checks is embedded into that loop, which makes it
hard to reuse. But In a subsequent commit we're about to introduce a
second path that wants to verify references.

Prepare for this by extracting the logic to check a single reference
into a standalone function.

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

refs/files: remove useless indirection

The function `files_fsck_refs()` only has a single callsite and forwards
all of its arguments as-is, so it's basically a useless indirection.
Inline the function call.

While at it, also remove the bitwise or that we have for return values.
We don't really want to or them at all, but rather just want to return
an error in case either of the functions has failed.

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

refs/files: remove `refs_check_dir` parameter

The parameter `refs_check_dir` determines which directory we want to
check references for. But as we always want to check the complete
refs hierarchy, this parameter is always set to "refs".

Drop the parameter and hardcode it.

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

refs/files: move fsck functions into global scope

When performing consistency checks we pass the functions that perform
the verification down the calling stack. This is somewhat unnecessary
though, as the set of functions doesn't ever change.

Simplify the code by moving the array into global scope and remove the
parameter.

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

refs/files: simplify iterating through root refs

When iterating through root refs we first need to determine the
directory in which the refs live. This is done by retrieving the root of
the loose refs via `refs->loose->root->name`, and putting it through
`files_ref_path()` to derive the final path.

This is somewhat redundant though: the root name of the loose files
cache is always going to be the empty string. As such, we always end up
passing that empty string to `files_ref_path()` as the ref hierarchy we
want to start. And this actually makes sense: `files_ref_path()` already
computes the location of the root directory, so of course we need to
pass the empty string for the ref hierarchy itself. So going via the
loose ref cache to figure out that the root of a ref hierarchy is empty
is only causing confusion.

But next to the added confusion, it can also lead to a segfault. The
loose ref cache is populated lazily, so it may not always be set. It
seems to be sheer luck that this is a condition we do not currently hit.
The right thing to do would be to call `get_loose_ref_cache()`, which
knows to populate the cache if required.

Simplify the code and fix the potential segfault by simply removing the
indirection via the loose ref cache completely.

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

packfile: drop repository parameter from `packed_object_info()`

The function `packed_object_info()` takes a packfile and offset and
returns the object info for the corresponding object. Despite these two
parameters though it also takes a repository pointer. This is redundant
information though, as `struct packed_git` already has a repository
pointer that is always populated.

Drop the redundant parameter.

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

packfile: skip unpacking object header for disk size requests

While most of the object info requests for a packed object require us to
unpack its headers, reading its disk size doesn't. We still unpack the
object header in that case though, which is unnecessary work.

Skip reading the header if only the disk size is requested. This leads
to a small speedup when reading disk size, only. The following benchmark
was done in the Git repository:

    Benchmark 1: ./git rev-list --disk-usage HEAD (rev = HEAD~)
      Time (mean ± σ):     105.2 ms ±   0.6 ms    [User: 91.4 ms, System: 13.3 ms]
      Range (min … max):   103.7 ms … 106.0 ms    27 runs

    Benchmark 2: ./git rev-list --disk-usage HEAD (rev = HEAD)
      Time (mean ± σ):      96.7 ms ±   0.4 ms    [User: 86.2 ms, System: 10.0 ms]
      Range (min … max):    96.2 ms …  98.1 ms    30 runs

    Summary
      ./git rev-list --disk-usage HEAD (rev = HEAD) ran
        1.09 ± 0.01 times faster than ./git rev-list --disk-usage HEAD (rev = HEAD~)

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

packfile: disentangle return value of `packed_object_info()`

The `packed_object_info()` function returns the type of the packed
object. While we use an `enum object_type` to store the return value,
this type is not to be confused with the actual object type. It _may_
contain the object type, but it may just as well encode that the given
packed object is stored as a delta.

We have removed the only caller that relied on this returned object type
in the preceding commit, so let's simplify semantics and return either 0
on success or a negative error code otherwise.

This unblocks a small optimization where we can skip reading the object
type altogether.

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

packfile: always populate pack-specific info when reading object info

When reading object information via `packed_object_info()` we may not
populate the object info's packfile-specific fields. This leads to
inconsistent object info depending on whether the info was populated via
`packfile_store_read_object_info()` or `packed_object_info()`.

Fix this inconsistency so that we can always assume the pack info to be
populated when reading object info from a pack.

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

packfile: extend `is_delta` field to allow for "unknown" state

The `struct object_info::u::packed::is_delta` field determines whether
or not a specific object is stored as a delta. It only stores whether or
not the object is stored as delta, so it is treated as a boolean value.

This boolean is insufficient though: when reading a packed object via
`packfile_store_read_object_info()` we know to skip parsing the actual
object when the user didn't request any object-specific data. In that
case we won't read the object itself, but will only look up its position
in the packfile. Consequently, we do not know whether it is a delta or
not.

This isn't really an issue right now, as the check for an empty request
is broken. But a subsequent commit will fix it, and once we do we will
have the need to also represent an "unknown" delta state.

Prepare for this change by introducing a new enum that encodes the
object type. We don't use the "unknown" state just yet, but will start
to do so in a subsequent commit.

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