The computation of column width made by "git diff --stat" was
confused when pathnames contain non-ASCII characters.
* lp/diff-stat-utf8-display-width-fix:
t4073: add test for diffstat paths length when containing UTF-8 chars
diff: improve scaling of filenames in diffstat to handle UTF-8 chars
Use the hook API to replace ad-hoc invocation of hook scripts via
the run_command() API.
* ar/run-command-hook-take-2:
hook: make ungroup opt-out instead of opt-in
hook: allow hooks to disable stdout_to_stderr
hook: check for NULL pointer before deref
receive-pack: convert receive hooks to hook API
receive-pack: convert update hooks to new API
hooks: allow callers to capture output
run-command: allow capturing of collated output
hook: allow overriding the ungroup option
reference-transaction: use hook API instead of run-command
transport: convert pre-push to hook API
hook: convert 'post-rewrite' hook in sequencer.c to hook API
hook: provide stdin via callback
run-command: add stdin callback for parallelization
run-command: add first helper for pp child states
Rename three functions around the commit_list data structure.
* ps/commit-list-functions-renamed:
commit: rename `free_commit_list()` to conform to coding guidelines
commit: rename `reverse_commit_list()` to conform to coding guidelines
commit: rename `copy_commit_list()` to conform to coding guidelines
A handful of code paths that started using batched ref update API
(after Git 2.51 or so) lost detailed error output, which have been
corrected.
* kn/ref-batch-output-error-reporting-fix:
fetch: delay user information post committing of transaction
receive-pack: utilize rejected ref error details
fetch: utilize rejected ref error details
update-ref: utilize rejected error details if available
refs: add rejection detail to the callback function
refs: skip to next ref when current ref is rejected
refs: drop unnecessary header includes
"git fetch --deepen" that tries to go beyond merged branch used to
get confused where the updated shallow points are, which has been
corrected.
Comments?
* sp/shallow-deepen-relative-fix:
shallow: handling fetch relative-deepen
shallow: free local object_array allocations
"git add -p" and friends notes what the current status of the hunk
being shown is.
* aa/add-p-previous-decisions:
add -p: show user's hunk decision when selecting hunks
Additional tests were introduced to see the interaction with netrc
auth with auth failure on the http transport.
Comments?
* ag/http-netrc-tests:
t5550: add netrc tests for http 401/403
"git receive-pack", when namespace is involved, segfaulted when a
symbolic ref cross the namespace boundary.
Comments?
* tt/receive-pack-oo-namespace-symref-fix:
receive-pack: fix crash on out-of-namespace symref
HTTP transport failed to authenticate in some code pahts, which has
been corrected.
* ap/http-probe-rpc-use-auth:
remote-curl: use auth for probe_rpc() requests too
Avoid local submodule repository directory paths overlapping with
each other by encoding submodule names before using them as path
components.
* ar/submodule-gitdir-tweak:
submodule: detect conflicts with existing gitdir configs
submodule: hash the submodule name for the gitdir path
submodule: fix case-folding gitdir filesystem collisions
submodule--helper: fix filesystem collisions by encoding gitdir paths
builtin/credential-store: move is_rfc3986_unreserved to url.[ch]
submodule--helper: add gitdir migration command
submodule: allow runtime enabling extensions.submodulePathConfig
submodule: introduce extensions.submodulePathConfig
builtin/submodule--helper: add gitdir command
submodule: always validate gitdirs inside submodule_name_to_gitdir
submodule--helper: use submodule_name_to_gitdir in add_submodule
Giving "git last-modified" a tree (not a commit-ish) died an
uncontrolled death, which has been corrected.
* tc/last-modified-not-a-tree:
last-modified: verify revision argument is a commit-ish
last-modified: remove double error message
last-modified: fix memory leak when more than one revision is given
last-modified: rewrite error message when more than one revision given
Remove implicit reliance on the_repository global in the APIs
around tree objects and make it explicit which repository to work
in.
* rs/tree-wo-the-repository:
cocci: remove obsolete the_repository rules
cocci: convert parse_tree functions to repo_ variants
tree: stop using the_repository
tree: use repo_parse_tree()
path-walk: use repo_parse_tree_gently()
pack-bitmap-write: use repo_parse_tree()
delta-islands: use repo_parse_tree()
bloom: use repo_parse_tree()
add-interactive: use repo_parse_tree_indirect()
tree: add repo_parse_tree*()
environment: move access to core.maxTreeDepth into repo settings
The logic that avoids reusing MIDX files with a wrong checksum was
broken, which has been corrected.
* tb/midx-write-corrupt-checksum-fix:
midx-write.c: assume checksum-invalid MIDXs require an update
t/t5319-multi-pack-index.sh: drop early 'test_done'
"git repack --geometric" did not work with promisor packs, which
has been corrected.
* ps/geometric-repacking-with-promisor-remotes:
builtin/repack: handle promisor packs with geometric repacking
repack-promisor: extract function to remove redundant packs
repack-promisor: extract function to finalize repacking
repack-geometry: extract function to compute repacking split
builtin/pack-objects: exclude promisor objects with "--stdin-packs"
The object-info API has been cleaned up.
* 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
Update code paths that check data integrity around refs subsystem.
* ps/ref-consistency-checks:
builtin/fsck: drop `fsck_head_link()`
builtin/fsck: move generic HEAD check into `refs_fsck()`
builtin/fsck: move generic object ID checks into `refs_fsck()`
refs/reftable: introduce generic checks for refs
refs/reftable: fix consistency checks with worktrees
refs/reftable: extract function to retrieve backend for worktree
refs/reftable: adapt includes to become consistent
refs/files: introduce function to perform normal ref checks
refs/files: extract generic symref target checks
fsck: drop unused fields from `struct fsck_ref_report`
refs/files: perform consistency checks for root refs
refs/files: improve error handling when verifying symrefs
refs/files: extract function to check single ref
refs/files: remove useless indirection
refs/files: remove `refs_check_dir` parameter
refs/files: move fsck functions into global scope
refs/files: simplify iterating through root refs
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>
When a shallowed repository gets deepened beyond the beginning of a
merged branch, we may end up with some shallows that are hidden behind
the reachable shallow commits. Added test 'fetching deepen beyond
merged branch' exposes that behaviour.
An example showing the problem based on added test:
0. Whole initial git repo to be cloned from
Graph:
* 033585d (HEAD -> main) Merge branch 'branch'
|\
| * 984f8b1 (branch) five
| * ecb578a four
|/
* 0cb5d20 three
* 2b4e70d two
* 61ba98b one
1. Initial shallow clone --depth=3 (all good)
Shallows:
2b4e70da2a10e1d3231a0ae2df396024735601f1
ecb578a3cf37198d122ae5df7efed9abaca17144
Graph:
* 033585d (HEAD -> main) Merge branch 'branch'
|\
| * 984f8b1 five
| * ecb578a (grafted) four
* 0cb5d20 three
* 2b4e70d (grafted) two
2. Deepen shallow clone with fetch --deepen=1 (NOT OK)
Shallows:
0cb5d204f4ef96ed241feb0f2088c9f4794ba758
61ba98be443fd51c542eb66585a1f6d7e15fcdae
Graph:
* 033585d (HEAD -> main) Merge branch 'branch'
|\
| * 984f8b1 five
| * ecb578a four
|/
* 0cb5d20 (grafted) three
---
Note that second shallow commit 61ba98be443fd51c542eb66585a1f6d7e15fcdae
is not reachable.
On the other hand, it seems that equivalent absolute depth driven
fetches result in all the correct shallows. That led to this proposal,
which unifies absolute and relative deepening in a way that the same
get_shallow_commits() call is used in both cases. The difference is
only that depth is adapted for relative deepening by measuring
equivalent depth of current local shallow commits in the current remote
repo. Thus a new function get_shallows_depth() has been added and the
function get_reachable_list() became redundant / removed.
Same example showing the corrected second step:
2. Deepen shallow clone with fetch --deepen=1 (all good)
Shallow:
61ba98be443fd51c542eb66585a1f6d7e15fcdae
Graph:
* 033585d (HEAD -> main) Merge branch 'branch'
|\
| * 984f8b1 five
| * ecb578a four
|/
* 0cb5d20 three
* 2b4e70d two
* 61ba98b (grafted) one
The get_shallows_depth() function also shares the logic of the
get_shallow_commits() function, but it focuses on counting depth of
each existing shallow commit. The minimum result is stored as
'data->deepen_relative', which is set not to be zero for relative
deepening anyway. That way we can always sum 'data->deepen_relative'
and 'depth' values, because 'data->deepen_relative' is always 0 in
absolute deepening.
To avoid duplicating logic between get_shallows_depth() and
get_shallow_commits(), get_shallow_commits() was modified so that
it is used by get_shallows_depth().
Signed-off-by: Samo Pogačnik <samo_pogacnik@t-2.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In Git 2.50 and earlier, we would display failure codes and error
message as part of the status display:
$ git fetch . v1.0.0:refs/heads/foo
error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'
From .
! [new tag] v1.0.0 -> foo (unable to update local ref)
With the addition of batched updates, this information is no longer
shown to the user:
$ git fetch . v1.0.0:refs/heads/foo
From .
* [new tag] v1.0.0 -> foo
error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'
Since reference updates are batched and processed together at the end,
information around the outcome is not available during individual
reference parsing.
To overcome this, collate and delay the output to the end. Introduce
`ref_update_display_info` which will hold individual update's
information and also whether the update failed or succeeded. This
finally allows us to iterate over all such updates and print them to the
user. While this brings back the functionality, it does change the order
of the output. Modify the tests to reflect this.
Using an strmap does add some overhead to 'git-fetch(1)', but from
benchmarking this seems to be not too bad:
Benchmark 1: fetch: many refs (refformat = files, refcount = 1000, revision = master)
Time (mean ± σ): 51.9 ms ± 2.5 ms [User: 15.6 ms, System: 36.9 ms]
Range (min … max): 47.4 ms … 58.3 ms 41 runs
Benchmark 2: fetch: many refs (refformat = files, refcount = 1000, revision = HEAD)
Time (mean ± σ): 53.0 ms ± 1.8 ms [User: 17.6 ms, System: 36.0 ms]
Range (min … max): 49.4 ms … 57.6 ms 40 runs
Summary
fetch: many refs (refformat = files, refcount = 1000, revision = master) ran
1.02 ± 0.06 times faster than fetch: many refs (refformat = files, refcount = 1000, revision = HEAD)
Another approach would be to move the status printing logic to be
handled post the transaction being committed. That however would require
adding an iterator to the ref transaction that tracks both the outcome
(success/failure) and the original refspec information for each update,
which is more involved infrastructure work compared to the strmap
approach here.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19),
git-receive-pack(1) switched to using batched reference updates. This also
introduced a regression wherein instead of providing detailed error
messages for failed referenced updates, the users were provided generic
error messages based on the error type.
Now that the updates also contain detailed error message, propagate
those to the client via 'rp_error'. The detailed error messages can be
very verbose, for e.g. in the files backend, when trying to write a
non-commit object to a branch, you would see:
! [remote rejected] 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d ->
branch (cannot update ref 'refs/heads/branch': trying to write
non-commit object 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d to branch
'refs/heads/branch')
Here the refname is repeated multiple times due to how error messages
are propagated and filled over the code stack. This potentially can be
cleaned up in a future commit.
Reported-by: Elijah Newren <newren@gmail.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 0e358de64a (fetch: use batched reference updates, 2025-05-19),
git-fetch(1) switched to using batched reference updates. This also
introduced a regression wherein instead of providing detailed error
messages for failed referenced updates, the users were provided generic
error messages based on the error type.
Similar to the previous commit, switch to using detailed error messages
if present for failed reference updates to fix this regression.
Reported-by: Elijah Newren <newren@gmail.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When git-update-ref(1) received the '--update-ref' flag, the error
details generated in the refs namespace wasn't propagated with failed
updates. Instead only an error code pertaining to the type of rejection
was noted.
This missed detailed error message which the user can act upon. The
previous commits added the required code to propagate these detailed
error messages from the refs namespace. Now that additional details are
available, let's output this additional details to stderr. This allows
users to have additional information over the already present machine
parsable output.
While we're here, improve the existing tests for the machine parsable
output by checking for the entire output string and not just the
rejection reason.
Reported-by: Elijah Newren <newren@gmail.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
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>
Passing a tree OID to git-last-modified(1) would trigger BUG behavior.
git last-modified HEAD^{tree}
BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified
Fix this error by verifying the parsed revision is a commit-ish.
Reported-by: Gusted <gusted@codeberg.org>
Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When more than one revision is passed to the git-last-modified(1)
command, this error message was printed:
error: last-modified can only operate on one tree at a time
Calling these a "tree" is technically not correct. git-last-modified(1)
expects revisions that peel to a commit.
Rephrase the error message to:
error: last-modified can only operate on one revision at a time
Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 857f047e40 (hook: allow overriding the ungroup option, 2025-12-26),
I accidentally made the ungroup option opt-in instead of opt-out and
despite my best efforts to set it for all API users, I missed a case
which requires it to be set: the pre-push hook which regressed.
The only thing I needed in that commit was a way to change the default,
to convert the remaining receive-pack hooks which require ungroup == 0
for sideband output, so it doesn't matter if it's on or off by default.
Bring back the original behavior by setting it for all hooks in the
struct run_hooks_opt initializer, which nicely allows changing the
default value only where needed, in receive-pack.c.
While at it add a few hook tests which exercise receive-pack sideband
output since they are the only ungroup=0 exceptions and there are no
other tests exercising this functionality.
Fixes: 857f047e40f7 ("hook: allow overriding the ungroup option")
Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The last batch of hooks converted to the hook.[ch] API introduced
a regression because pick_next_hook() always sets stdout_to_stderr
for its child processes.
Pre-push is the only hook API user which requires stdout_to_stderr
to be 0, so it can be argued that pre-push needs fixing, however
this will likely break many pre-push hooks, so it's better to allow
it to be 0, i.e. to match the previous behavior.
To prevent such regressions in the future, extend the hook tests to
verify hooks write to the expected stdout vs stderr streams and
maintain backward compatibility with the hooks output assumptions.
The tests are independent of the actual hook implementations: I've
tested they work the same before and after the hook.[ch] conversion
and will continue to work after we eventually introduce parallel
hook execution and config-based hooks.
Fixes: 3e2836a742d8 ("transport: convert pre-push to hook API")
Reported-by: Chris Darroch <chrisd@apache.org>
Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some callers, for example server-side hooks which wish to relay hook
output to clients across a transport, want to capture what would
normally print to stderr and do something else with it. Allow that via a
callback.
By calling the callback regardless of whether there's output available,
we allow clients to send e.g. a keepalive if necessary.
Because we expose a strbuf, not a fd or FILE*, there's no need to create
a temporary pipe or similar - we can just skip the print to stderr and
instead hand it to the caller.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a user of the run_processes_parallel() API wants to pipe a large
amount of information to the stdin of each parallel command, that
data could exceed the pipe buffer of the process's stdin and can be
too big to store in-memory via strbuf & friends or to slurp to a file.
Generally this is solved by repeatedly writing to child_process.in
between calls to start_command() and finish_command(). For a specific
pre-existing example of this, see transport.c:run_pre_push_hook().
This adds a generic callback API to run_processes_parallel() to do
exactly that in a unified manner, similar to the existing callback APIs,
which can then be used by hooks.h to convert the remaining hooks to the
new, simpler parallel interface.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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.
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
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
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
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>
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>
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>
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>
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>
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>
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>
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>
