* Re: [PATCH v2 1/1] git-gui: protect rev-parse --show-toplevel call
From: Mark Levedahl @ 2026-04-29 20:14 UTC (permalink / raw)
To: Shroom Moo, git; +Cc: j6t
In-Reply-To: <tencent_AEE968E8E785907BA55A383977C8968ED406@qq.com>
On 4/29/26 1:32 PM, Shroom Moo wrote:
> When starting git-gui from a directory that is a bare repository or
> where the working tree is missing, git-gui previously executed
> 'rev-parse --show-toplevel' without error handling. This caused a
> fatal Tcl error ("this operation must be run in a work tree").
>
> Wrap the call in a catch to prevent the fatal error. The existing
> error paths after this call already handle bare repos and missing
> worktrees appropriately.
>
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
> git-gui/git-gui.sh | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..aee37685e1 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1169,7 +1169,9 @@ if {![file isdirectory $_gitdir]} {
> load_config 0
> apply_config
>
> -set _gitworktree [git rev-parse --show-toplevel]
> +if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
> + set _gitworktree {}
> +}
>
> if {$_prefix ne {}} {
> if {$_gitworktree eq {}} {
Unfortunately, this allows starting git-gui inside the separate gitdir created by
git clone --separate-git-dir=/some/where/else ...
There is no hint where the workdir is, but git recognizes the repository is not bare:
git rev-parse --is-bare-repository ==> false
git rev-parse --is-inside-git-dir ==> true
git rev-parse --is-inside-work-tree ==> false
git rev-parse --show-toplevel ==> fatal: must be run in a work tree
git worktree list ==> absolute path to gitdir (not a worktree)
As git-gui has no protection against modifying what is effectively a bare repository,
allowing git-gui to run in this directory is dangerous, or possibly just very confusing.
git refuses to work in this gitdir: "git status" run in the above gitdir gives: "fatal:
this operation must be run in a work tree."
The simplest safe thing is to catch the error and abort with a more useful message than
currently provided. Or perhaps, check git rev-parse --is-inside-git-dir and abort, and do
so before trying --show-toplevel.
Mark
^ permalink raw reply
* Re: [PATCH] git-gui: handle bare repo or missing worktree
From: Shroom Moo @ 2026-04-29 18:28 UTC (permalink / raw)
To: git; +Cc: Shroom Moo, Johannes Sixt
In-Reply-To: <308229f2-0d20-4f3b-8e6a-c962bd9c3a6e@kdbg.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]
Hi Hannes,
Thanks for taking a close look at the patch. You're right that the
minimal crashfix is just protecting the `rev-parse --show-toplevel`
call. The existing error paths after that line already handle bare
repos and missing worktrees, showing an error dialog before exiting.
That alone would prevent the fatal Tcl error –the original crash is
fixed.
The extra logic is not about incomplete error diagnosis (the other
checks cover that). It's about improving the user experience in the
specific case that prompted this report: a colleague double-clicked
git-gui.exe inside the Git installation directory, where a third-party
program had accidentally left a HEAD file. That caused git to treat
the directory as a bare repository, even though it has no . git folder.
In that context, showing a "Cannot use bare repository" and quitting
was confusing – the user simply started the program without choosing
that path.
So the additional detection and the `choose_repository::pick` call are
a UX improvement, not a correctness requirement. When the program
detects that the current directory is an unusable repository, falling
back to the repository-selection dialog (as we do when no repository
is found) is better than failing with an error. The user can pick a
valid repository and continue without restarting the GUI.
Since a minimal patch is preferred, I have prepared v2 which only
protects `--show-toplevel`. The dialog fallback could be revisited as
a separate enhancement if desired. I've already sent the v2 patch to
the list.
-- Shroom
^ permalink raw reply
* [PATCH v2 1/1] git-gui: protect rev-parse --show-toplevel call
From: Shroom Moo @ 2026-04-29 17:32 UTC (permalink / raw)
To: git; +Cc: j6t, Shroom Moo
In-Reply-To: <308229f2-0d20-4f3b-8e6a-c962bd9c3a6e@kdbg.org>
When starting git-gui from a directory that is a bare repository or
where the working tree is missing, git-gui previously executed
'rev-parse --show-toplevel' without error handling. This caused a
fatal Tcl error ("this operation must be run in a work tree").
Wrap the call in a catch to prevent the fatal error. The existing
error paths after this call already handle bare repos and missing
worktrees appropriately.
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
git-gui/git-gui.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..aee37685e1 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1169,7 +1169,9 @@ if {![file isdirectory $_gitdir]} {
load_config 0
apply_config
-set _gitworktree [git rev-parse --show-toplevel]
+if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
+ set _gitworktree {}
+}
if {$_prefix ne {}} {
if {$_gitworktree eq {}} {
--
2.52.0.windows.1
^ permalink raw reply related
* [PATCH] subtree: allow duplicate cache entries with same value
From: Eric Wendland @ 2026-04-29 14:15 UTC (permalink / raw)
To: git; +Cc: gitster, ask+git, levraiphilippeblain, ps, l.s.r, Eric Wendland
When a subtree is added, removed, and later re-added at the same path,
'git subtree split' fails with:
fatal: cache for <hash> already exists!
This happens because 'find_existing_splits()' scans history and calls
'cache_set()' for each prior split. When the same subtree commit was
added twice at different mainline commits, both map to the same subtree
commit. The second 'cache_set()' call tries to write the same mapping
again, but the old code treated any existing cache entry as a fatal error.
Fix 'cache_set()' to silently succeed when the existing cache entry
already contains the same value we're trying to write.
Add a regression test for this scenario.
Signed-off-by: Eric Wendland <eric@wendland.dev>
Closes: <1d8fa22c-eb13-4cdb-8499-e19f0dea6b42@tionis.dev>
---
contrib/subtree/git-subtree.sh | 5 +++++
contrib/subtree/t/t7900-subtree.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 791fd8260c..f0f2a47d53 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -343,6 +343,11 @@ cache_set () {
test "$oldrev" != "latest_new" &&
test -e "$cachedir/$oldrev"
then
+ read oldcache <"$cachedir/$oldrev"
+ if test "$oldcache" = "$newrev"
+ then
+ return
+ fi
die "fatal: cache for $oldrev already exists!"
fi
echo "$newrev" >"$cachedir/$oldrev"
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 18d2b56448..85fe4362fb 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -528,6 +528,35 @@ do
'
done
+test_expect_success 'split after add, remove, and re-add at same path' '
+ subtree_test_create_repo "$test_count" &&
+ subtree_test_create_repo "$test_count/sub proj" &&
+ test_create_commit "$test_count" main1 &&
+ test_create_commit "$test_count/sub proj" sub1 &&
+ (
+ cd "$test_count" &&
+ git fetch ./"sub proj" HEAD &&
+ git subtree add --prefix="sub dir" FETCH_HEAD
+ ) &&
+ test_create_commit "$test_count" "sub dir"/main-sub1 &&
+ (
+ cd "$test_count" &&
+ git rm -rf "sub dir" &&
+ git commit -m "remove sub dir"
+ ) &&
+ (
+ cd "$test_count" &&
+ git fetch ./"sub proj" HEAD &&
+ git subtree add --prefix="sub dir" FETCH_HEAD
+ ) &&
+ test_create_commit "$test_count" "sub dir"/main-sub2 &&
+ (
+ cd "$test_count" &&
+ git subtree split --prefix="sub dir" --branch=subproj-br &&
+ test "$(git rev-parse --verify subproj-br)"
+ )
+'
+
# Usually,
#
# git subtree merge -P subA --squash f00...
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3 5/5] format-rev: introduce builtin for on-demand pretty formatting
From: Kristoffer Haugsbakk @ 2026-04-29 13:41 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble
In-Reply-To: <V3_format-rev_new_builtin.66f@msgid.xyz>
On Wed, Apr 29, 2026, at 00:25, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>[snip]
CI returned an error.
builtin/name-rev.c:893:25: error: ‘commit’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
893 | get_format_rev(commit, &format_pp, &scratch_buf);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
^ permalink raw reply
* Re: [PATCH 0/6] Handle cloning of objects larger than 4GB on Windows
From: Derrick Stolee @ 2026-04-29 13:35 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git; +Cc: Johannes Schindelin
In-Reply-To: <pull.2102.git.1777393580.gitgitgadget@gmail.com>
On 4/28/26 12:26 PM, Johannes Schindelin via GitGitGadget wrote:
> On Windows, unsigned long is 32-bit even on 64-bit systems. This causes
> multiple problems when Git handles objects larger than 4GB. This patch
> series is a very targeted fix for a very early part of the problem: it
> addresses the most fundamental truncation points that prevent a >4GB object
> from surviving a clone at all.
>
> Specifically, this fixes:
>
> * zlib's uLong wrapping and triggering BUG() assertions in the git_zstream
> wrapper
> * Object sizes being truncated in pack streaming, delta headers, and
> index-pack/unpack-objects
> * pack-objects re-encoding reused pack entries with a truncated size,
> producing corrupt packs on the wire
I'm glad to see this progress in this direction. It's a big step!
> Many other code paths still use unsigned long for object sizes (e.g.,
> cat-file -s, object_info.sizep, the delta machinery) and will need their own
> conversions. This series does not attempt to fix those.
I appreciate the mechanisms used to keep the scope of change minimal. This
differs from the typical "replace all 'unsigned long's with 'size_t'"
proposals in some clever ways.
> Based on work by @LordKiRon in git-for-windows/git#6076.
>
> The last two commits add a test helper that synthesizes a pack with a >4GB
> blob and regression tests that clone it via both the unpack-objects and
> index-pack code paths using file:// transport.
My biggest concern here is about how expensive this test is. I wonder if
we should mark it as expensive for the core project and leave it enabled by
default in git-for-windows/git.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH 6/6] t5608: add regression test for >4GB object clone
From: Derrick Stolee @ 2026-04-29 13:34 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git; +Cc: Johannes Schindelin
In-Reply-To: <a3019888d8465e0f77926a91a20db170fef6989d.1777393580.git.gitgitgadget@gmail.com>
On 4/28/26 12:26 PM, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The shift overflow bug in index-pack and unpack-objects caused incorrect
> object size calculation when the encoded size required more than 32 bits
> of shift. This would result in corrupted or failed unpacking of objects
> larger than 4GB.
>
> Add a test that creates a pack file containing a 4GB+ blob using the
> new 'test-tool synthesize pack --reachable-large' command, then clones
> the repository to verify the fix works correctly.
As mentioned in the previous patch, constructing this large packfile
takes ~4 minutes in CI pipelines. That's quite a lot to handle for
every CI run.
> +test_expect_success SIZE_T_IS_64BIT 'set up repo with >4GB object' '
Your prereq here prevents it from running on 32-bit builds, which is
good. However, I wonder if it would be worth also specifying these
tests as expensive. It's less likely that these layers will be touched
often, so it should be enough to run these on major occasions, such as
testing a release candidate.
(I also think it's appropriate to have these tests _not_ be marked
expensive in their original contribution to git-for-windows/git,
because the pull request build should prove that the tests work. And
maybe git-for-windows/git should keep them for every PR since that's
where the tests matter the most.)
I suppose this also is a question for Junio and our process for
validating releases. Do we have a certain cadence where we run the
expensive tests? What has been our threshold for hiding a test case
behind the expensive label?
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH 4/6] delta, packfile: use size_t for delta header sizes
From: Derrick Stolee @ 2026-04-29 13:28 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git; +Cc: Johannes Schindelin
In-Reply-To: <3274cba862ae42a6813710410274a692ec0f5d29.1777393580.git.gitgitgadget@gmail.com>
On 4/28/26 12:26 PM, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The delta header decoding functions return unsigned long, which
> truncates on Windows for objects larger than 4GB. Introduce size_t
> variants get_delta_hdr_size_sz() and get_size_from_delta_sz() that
> preserve the full 64-bit size, and use them in packed_object_info()
> where the size is needed for streaming decisions.
> + * Size_t variant that doesn't truncate - use for >4GB objects on Windows.
> + */
> +static inline size_t get_delta_hdr_size_sz(const unsigned char **datap,
> + const unsigned char *top)
...
> +static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
> + const unsigned char *top)
> +{
> + size_t size = get_delta_hdr_size_sz(datap, top);
> return cast_size_t_to_ulong(size);
> }
I like this trick to use the 64-bit implementation and only to
down-cast for API compatibility. This allows a more gradual
transition than if we replaced ulongs with size_ts everywhere
at once.
Thanks,
-Stolee
^ permalink raw reply
* Re: [Bug] fetch --deepen truncates history in v2.54.0
From: D. Ben Knoble @ 2026-04-29 13:16 UTC (permalink / raw)
To: Owen Stephens; +Cc: git
In-Reply-To: <CANOh7gEEw+6146NN3JV8EYxQarj0KkyA7r3RZ6v-DxeqQZLrCA@mail.gmail.com>
On Wed, Apr 29, 2026 at 7:27 AM Owen Stephens <owen@owenstephens.co.uk> wrote:
>
> > What did you do before the bug happened? (Steps to reproduce your issue)
>
> Repeatedy called `git fetch --deepen 2` inside a shallow repo that was a
> file:// clone of another repo. Once all commits had been fetched, a subsequent
> `fetch --deepen` appears to "reset" the repo back to being shallow with a depth
> of 2. A reproduction script is included below. This issue appears to have been
> introduced in v2.54.0.
>
> > What did you expect to happen? (Expected behavior)
>
> I expected `git fetch --deepen` in a non-shallow repo with no upstream commits
> to be a no-op.
Here's the relevant part of git-fetch(1):
--depth=<depth>
Limit fetching to the specified number of commits from the tip of
each remote branch history. If fetching to a shallow repository
created by git clone with --depth=<depth> option (see git-clone(1)),
deepen or shorten the history to the specified number of commits.
Tags for the deepened commits are not fetched.
--deepen=<depth>
Similar to --depth, except it specifies the number of commits from
the current shallow boundary instead of from the tip of each remote
branch history.
I can see how one might read this as implying that when fetching in a
non-shallow repository, there's no effect, but I don't think the text
explicitly says that. In fact, the first sentence under "--depth"
(which is of course relevant for "--deepen") is unconditional.
So I'm not sure it should be a no-op.
That said, it is possible the behavior changed between 2.53 and 2.54?
I haven't tried to reproduce or bisect yet.
Best,
D. Ben Knoble
^ permalink raw reply
* Re: [Bug] fetch --deepen truncates history in v2.54.0
From: Owen Stephens @ 2026-04-29 13:14 UTC (permalink / raw)
To: git
In-Reply-To: <CANOh7gEEw+6146NN3JV8EYxQarj0KkyA7r3RZ6v-DxeqQZLrCA@mail.gmail.com>
On Wed, Apr 29, 2026 at 12:27 PM Owen Stephens <owen@owenstephens.co.uk> wrote:
> The key lines in the output are:
> ```
> Shallow repo? true
> 63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
> 864e13c (grafted) Change 3
> remote: Enumerating objects: 10, done.
> remote: Counting objects: 100% (10/10), done.
> remote: Compressing objects: 100% (2/2), done.
> remote: Total 6 (delta 1), reused 0 (delta 0), pack-reused 0 (from 0)
> Unpacking objects: 100% (6/6), 351 bytes | 175.00 KiB/s, done.
>
> Shallow repo? true
> 63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
> 864e13c Change 3
> 3e05d14 Change 2
> 1d9fe14 (grafted) Change 1
> remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
>
> Shallow repo? false
> 63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
> 864e13c Change 3
> 3e05d14 Change 2
> 1d9fe14 Change 1
> remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
>
> Shallow repo? true
> 63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
> 864e13c (grafted) Change 351
> ```
Apologies, I just noticed that I had inadvertently munged the final
line - it should read "864e13c (grafted) Change 3"
Owen.
^ permalink raw reply
* Re: [PATCH v3 6/9] update-ref: handle rejections while adding updates
From: Toon Claes @ 2026-04-29 12:24 UTC (permalink / raw)
To: Karthik Nayak, git; +Cc: Karthik Nayak, ps
In-Reply-To: <20260427-refs-move-to-generic-layer-v3-6-e4638dfb7897@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> @@ -289,22 +300,35 @@ static void parse_cmd_update(struct ref_transaction *transaction,
> if (*next != line_termination)
> die("update %s: extra input: %s", refname, next);
>
> - if (ref_transaction_update(transaction, refname,
> - &new_oid, have_old ? &old_oid : NULL,
> - NULL, NULL,
> - update_flags | create_reflog_flag,
> - msg, &err))
> + tx_err = ref_transaction_update(transaction, refname,
> + &new_oid, have_old ? &old_oid : NULL,
> + NULL, NULL,
> + update_flags | create_reflog_flag,
> + msg, &err);
> +
> + /*
> + * Generic errors are non-recoverable, so we cannot skip the update
> + * or mark it as rejected.
> + */
> + if (tx_err == REF_TRANSACTION_ERROR_GENERIC)
> die("%s", err.buf);
>
> + if (tx_err && opts->allow_update_failures)
> + print_rejected_refs(refname, have_old ? &old_oid : NULL,
> + &new_oid, NULL, NULL, tx_err, err.buf,
> + NULL);
I realize I've made this suggestion, but I think I've made a mistake.
When opts->allow_update_failures is falsey and tx_err is truthy we
should die also. Don't we?
I'm not sure what the nicest way is to write this, but maybe:
if (tx_err) {
if (tx_err == REF_TRANSACTION_ERROR_GENERIC || !opts->allow_update_failures)
die("%s", err.buf);
print_rejected_refs(refname, have_old ? &old_oid : NULL,
&new_oid, NULL, NULL, tx_err, err.buf,
NULL);
}
How did test coverage not find this?
--
Cheers,
Toon
^ permalink raw reply
* [Bug] fetch --deepen truncates history in v2.54.0
From: Owen Stephens @ 2026-04-29 11:27 UTC (permalink / raw)
To: git
> What did you do before the bug happened? (Steps to reproduce your issue)
Repeatedy called `git fetch --deepen 2` inside a shallow repo that was a
file:// clone of another repo. Once all commits had been fetched, a subsequent
`fetch --deepen` appears to "reset" the repo back to being shallow with a depth
of 2. A reproduction script is included below. This issue appears to have been
introduced in v2.54.0.
> What did you expect to happen? (Expected behavior)
I expected `git fetch --deepen` in a non-shallow repo with no upstream commits
to be a no-op.
> What happened instead? (Actual behavior)
`git log` history is truncated to two commits, and repo is considered shallow
by `git rev-parse --is-shallow-repository`.
> What's different between what you expected and what actually happened?
The previously-present commits in `git log` are missing, and the repo is again
considered shallow.
> Anything else you want to add:
Commit 3ef68ff seems relevant.
The following script reproduces the issue in 2.54.0, and does not reproduce the
issue in 2.53.0:
```
mkdir repro.git
cd repro.git
git init
for i in $(seq 1 4); do
echo "$i" >> file.txt
git add file.txt
git commit -m "Change $i"
done
cd ..
git clone --depth 2 "file://$PWD/repro.git" repro_clone.git
cd repro_clone.git
echo "Shallow repo? $(git rev-parse --is-shallow-repository)"
git log --oneline
for i in $(seq 1 3); do
git fetch --deepen 2
echo "Shallow repo? $(git rev-parse --is-shallow-repository)"
git log --oneline
done
```
The key lines in the output are:
```
Shallow repo? true
63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
864e13c (grafted) Change 3
remote: Enumerating objects: 10, done.
remote: Counting objects: 100% (10/10), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 6 (delta 1), reused 0 (delta 0), pack-reused 0 (from 0)
Unpacking objects: 100% (6/6), 351 bytes | 175.00 KiB/s, done.
Shallow repo? true
63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
864e13c Change 3
3e05d14 Change 2
1d9fe14 (grafted) Change 1
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
Shallow repo? false
63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
864e13c Change 3
3e05d14 Change 2
1d9fe14 Change 1
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
Shallow repo? true
63d1ebe (HEAD -> master, origin/master, origin/HEAD) Change 4
864e13c (grafted) Change 351
```
N.b. that 1d9fe14 was present after the second iteration but missing after the
third, along with `--is-shallow-repository` changing from false back to true.
[System Info]
git version:
git version 2.54.0
cpu: arm64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
rust: disabled
feature: fsmonitor--daemon
gettext: enabled
libcurl: 8.7.1
zlib: 1.2.12
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
uname: Darwin 25.4.0 Darwin Kernel Version 25.4.0: Thu Mar 19 19:33:25
PDT 2026; root:xnu-12377.101.15~1/RELEASE_ARM64_T6041 arm64
compiler info: clang: 21.0.0 (clang-2100.0.123.102)
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/zsh
^ permalink raw reply
* [PATCH] checkout: add --autostash option for branch switching
From: Harald Nordgren @ 2026-04-29 11:11 UTC (permalink / raw)
To: phillip.wood123
Cc: chris.torek, git, gitgitgadget, haraldnordgren, peff,
phillip.wood
In-Reply-To: <3170928a-e05e-42cc-b15e-56e0fa479066@gmail.com>
> That all sounds good and the range-diff below looks as I would expect it
> to. I've left some suggestions for possible future work on patch 5 but I
> think this is ready to be merged as-is.
>
> Thanks for working on it
Thank you too!
Harald
^ permalink raw reply
* [PATCH v6] t2000: consolidate second scenario into a single test block
From: Zakariyah Ali @ 2026-04-29 10:36 UTC (permalink / raw)
To: git; +Cc: karthik.188, gitster, Zakariyah Ali
In-Reply-To: <20260407034446.409175-1-zakariyahali100@gmail.com>
The second test scenario in t2000 consists of several fragmented
test_expect_success blocks that handle data setup, tree writes,
execution of git-checkout-index, and final state validation.
Consolidate these nine separate blocks into a single self-contained
test block. This follows the modern Git testing standard where setup,
execution, and validation of a single logical scenario are kept
together.
As a result of this consolidation, the show_files() helper and its
associated test_debug calls are no longer used and have been removed.
This also removes a dependency on the non-portable 'find -ls' command.
Helped-by: Karthik Nayak <karthik.188@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
I'm sorry for replying very late; it was because of some commitments I had to attend to which I've been able to settle.
Changes since v5:
- Consolidate segmented test blocks in the second scenario into a single unit.
- Remove unused show_files debug function and tree3 variable.
- Ensure file setup is inside test_expect_success blocks.
t/t2000-conflict-when-checking-files-out.sh | 65 +++------------------
1 file changed, 8 insertions(+), 57 deletions(-)
diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index af199d8191..7b61370549 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -23,17 +23,6 @@ test_description='git conflicts when checking files out test.'
. ./test-lib.sh
-show_files() {
- # show filesystem files, just [-dl] for type and name
- find path? -ls |
- sed -e 's/^[0-9]* * [0-9]* * \([-bcdl]\)[^ ]* *[0-9]* *[^ ]* *[^ ]* *[0-9]* [A-Z][a-z][a-z] [0-9][0-9] [^ ]* /fs: \1 /'
- # what's in the cache, just mode and name
- git ls-files --stage |
- sed -e 's/^\([0-9]*\) [0-9a-f]* [0-3] /ca: \1 /'
- # what's in the tree, just mode and name.
- git ls-tree -r "$1" |
- sed -e 's/^\([0-9]*\) [^ ]* [0-9a-f]* /tr: \1 /'
-}
test_expect_success 'prepare files path0 and path1/file1' '
date >path0 &&
@@ -83,59 +72,21 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
# path path3 is occupied by a non-directory. With "-f" it should remove
# the symlink path3 and create directory path3 and file path3/file1.
-test_expect_success 'prepare path2/file0 and index' '
+test_expect_success 'checkout-index -f resolves symlink conflict on leading path' '
mkdir path2 &&
date >path2/file0 &&
- git update-index --add path2/file0
-'
-
-test_expect_success 'write tree with path2/file0' '
- tree1=$(git write-tree)
-'
-
-test_debug 'show_files $tree1'
-
-test_expect_success 'prepare path3/file1 and index' '
+ git update-index --add path2/file0 &&
+ tree1=$(git write-tree) &&
mkdir path3 &&
date >path3/file1 &&
- git update-index --add path3/file1
-'
-
-test_expect_success 'write tree with path3/file1' '
- tree2=$(git write-tree)
-'
-
-test_debug 'show_files $tree2'
-
-test_expect_success 'read previously written tree and checkout.' '
+ git update-index --add path3/file1 &&
+ tree2=$(git write-tree) &&
rm -fr path3 &&
git read-tree -m $tree1 &&
- git checkout-index -f -a
-'
-
-test_debug 'show_files $tree1'
-
-test_expect_success 'add a symlink' '
- test_ln_s_add path2 path3
-'
-
-test_expect_success 'write tree with symlink path3' '
- tree3=$(git write-tree)
-'
-
-test_debug 'show_files $tree3'
-
-# Morten says "Got that?" here.
-# Test begins.
-
-test_expect_success 'read previously written tree and checkout.' '
+ git checkout-index -f -a &&
+ test_ln_s_add path2 path3 &&
git read-tree $tree2 &&
- git checkout-index -f -a
-'
-
-test_debug 'show_files $tree2'
-
-test_expect_success 'checking out conflicting path with -f' '
+ git checkout-index -f -a &&
test_path_is_dir_not_symlink path2 &&
test_path_is_dir_not_symlink path3 &&
test_path_is_file_not_symlink path2/file0 &&
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v16 0/5] checkout: 'autostash' for branch switching
From: Phillip Wood @ 2026-04-29 10:02 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <pull.2234.v16.git.git.1777401552.gitgitgadget@gmail.com>
Hi Harald
On 28/04/2026 19:39, Harald Nordgren via GitGitGadget wrote:
> * Updated the git checkout and git switch docs to show the actual output
> when using -m to carry local changes across a branch switch, and changed
> "would carry" to "will carry".
> * Rewrote the merge-conflict example in both docs to match the new, more
> concise message printed on autostash conflicts.
> * Replaced the show_unpack_errors flag and old_commit parameter in
> merge_working_tree()/init_topts() with a plain quiet boolean, so the
> caller decides directly whether to suppress unpack errors.
> * Tightened the 'checkout -m with dirty tree' test by replacing a printf
> with a heredoc.
> * Made the 'checkout -m creates a recoverable stash on conflict' test also
> assert the reflog subject of the new stash entry.
> * Replaced two test_grep calls in the 'checkout -m which would overwrite
> untracked file' test with a single test_cmp, which also catches a
> regression where the "would be overwritten" message could end up printed
> twice.
That all sounds good and the range-diff below looks as I would expect it
to. I've left some suggestions for possible future work on patch 5 but I
think this is ready to be merged as-is.
Thanks for working on it
Phillip
> Harald Nordgren (5):
> stash: add --label-ours, --label-theirs, --label-base for apply
> sequencer: allow create_autostash to run silently
> sequencer: teach autostash apply to take optional conflict marker
> labels
> checkout: rollback lock on early returns in merge_working_tree
> checkout -m: autostash when switching branches
>
> Documentation/git-checkout.adoc | 55 +++++------
> Documentation/git-stash.adoc | 11 ++-
> Documentation/git-switch.adoc | 36 ++++---
> builtin/checkout.c | 166 +++++++++++++++-----------------
> builtin/commit.c | 3 +-
> builtin/merge.c | 15 ++-
> builtin/stash.c | 28 ++++--
> sequencer.c | 69 +++++++++----
> sequencer.h | 7 +-
> t/t3420-rebase-autostash.sh | 16 +--
> t/t3903-stash.sh | 24 +++++
> t/t7201-co.sh | 71 +++++++++++++-
> t/t7600-merge.sh | 3 +-
> xdiff-interface.c | 12 +++
> xdiff-interface.h | 1 +
> xdiff/xmerge.c | 6 +-
> 16 files changed, 343 insertions(+), 180 deletions(-)
>
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2234%2FHaraldNordgren%2Fcheckout_autostash-v16
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2234/HaraldNordgren/checkout_autostash-v16
> Pull-Request: https://github.com/git/git/pull/2234
>
> Range-diff vs v15:
>
> 1: aba8e6a9dc = 1: aba8e6a9dc stash: add --label-ours, --label-theirs, --label-base for apply
> 2: 89e0bfa803 = 2: 89e0bfa803 sequencer: allow create_autostash to run silently
> 3: a428ce7328 = 3: a428ce7328 sequencer: teach autostash apply to take optional conflict marker labels
> 4: f358424085 = 4: f358424085 checkout: rollback lock on early returns in merge_working_tree
> 5: 96b14db827 ! 5: 07d25fda91 checkout -m: autostash when switching branches
> @@ Documentation/git-checkout.adoc: $ git checkout mytopic
>
> -You can give the `-m` flag to the command, which would try a
> -three-way merge:
> -+You can give the `-m` flag to the command, which would carry your local
> ++You can give the `-m` flag to the command, which will carry your local
> +changes to the new branch:
>
> ------------
> $ git checkout -m mytopic
> -Auto-merging frotz
> ++Applied autostash.
> +Switched to branch 'mytopic'
> ++The following paths have local changes:
> ++M frotz
> ------------
>
> -After this three-way merge, the local modifications are _not_
> @@ Documentation/git-checkout.adoc: $ git checkout mytopic
>
> -When a merge conflict happens during switching branches with
> -the `-m` option, you would see something like this:
> -+When the `--merge` (`-m`) option is in effect and the locally
> -+modified files overlap with files that need to be updated by the
> -+branch switch, the changes are stashed and reapplied after the
> -+switch. If this process results in conflicts, a stash entry is saved
> -+and made available in `git stash list`:
> ++When the `--merge` (`-m`) option is given and the local changes
> ++overlap with the changes in the branch we're switching to, the
> ++changes are stashed and reapplied after the switch. If this
> ++process results in conflicts, the stash entry is saved and a
> ++message is printed:
>
> ------------
> $ git checkout -m mytopic
> @@ Documentation/git-checkout.adoc: $ git checkout mytopic
> -ERROR: Merge conflict in frotz
> -fatal: merge program failed
> -------------
> -+Your local changes are stashed, however, applying it to carry
> -+forward your local changes resulted in conflicts:
> -
> +-
> -At this point, `git diff` shows the changes cleanly merged as in
> -the previous example, as well as the changes in the conflicted
> -files. Edit and resolve the conflict and mark it resolved with
> -`git add` as usual:
> -+ - You can try resolving them now. If you resolved them
> -+ successfully, discard the stash entry with "git stash drop".
> -
> -+ - Alternatively you can "git reset --hard" if you do not want
> -+ to deal with them right now, and later "git stash pop" to
> -+ recover your local changes.
> - ------------
> +-
> +-------------
> -$ edit frotz
> -$ git add frotz
> --------------
> -+
> -+You can try resolving the conflicts now. Edit the conflicting files
> -+and mark them resolved with `git add` as usual, then run `git stash
> -+drop` to discard the stash entry. Alternatively, you can clear the
> -+working tree with `git reset --hard` and recover your local changes
> -+later with `git stash pop`.
> ++Your local changes are stashed, however applying them
> ++resulted in conflicts. You can either resolve the conflicts
> ++and then discard the stash with "git stash drop", or, if you
> ++do not want to resolve them now, run "git reset --hard" and
> ++apply the local changes later by running "git stash pop".
> + ------------
>
> CONFIGURATION
> - -------------
>
> ## Documentation/git-switch.adoc ##
> @@ Documentation/git-switch.adoc: variable.
> @@ Documentation/git-switch.adoc: $ git switch mytopic
>
> -You can give the `-m` flag to the command, which would try a three-way
> -merge:
> -+You can give the `-m` flag to the command, which would carry your local
> ++You can give the `-m` flag to the command, which will carry your local
> +changes to the new branch:
>
> ------------
> $ git switch -m mytopic
> -Auto-merging frotz
> ++Applied autostash.
> +Switched to branch 'mytopic'
> ++The following paths have local changes:
> ++M frotz
> ------------
>
> -After this three-way merge, the local modifications are _not_
> @@ builtin/checkout.c: struct checkout_opts {
> char *name; /* The short name used */
> char *path; /* The full name of a real branch */
> @@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)
> + branch->path = strbuf_detach(&buf, NULL);
> + }
>
> - static void init_topts(struct unpack_trees_options *topts, int merge,
> +-static void init_topts(struct unpack_trees_options *topts, int merge,
> ++static void init_topts(struct unpack_trees_options *topts,
> int show_progress, int overwrite_ignore,
> - struct commit *old_commit)
> -+ struct commit *old_commit, bool show_unpack_errors)
> ++ bool quiet)
> {
> memset(topts, 0, sizeof(*topts));
> topts->head_idx = -1;
> @@ builtin/checkout.c: static void init_topts(struct unpack_trees_options *topts, i
> topts->update = 1;
> topts->merge = 1;
> - topts->quiet = merge && old_commit;
> -+ topts->quiet = merge && old_commit && !show_unpack_errors;
> ++ topts->quiet = quiet;
> topts->verbose_update = show_progress;
> topts->fn = twoway_merge;
> topts->preserve_ignored = !overwrite_ignore;
> @@ builtin/checkout.c: static void init_topts(struct unpack_trees_options *topts, i
> static int merge_working_tree(const struct checkout_opts *opts,
> struct branch_info *old_branch_info,
> struct branch_info *new_branch_info,
> -- int *writeout_error)
> -+ int *writeout_error,
> -+ bool show_unpack_errors)
> ++ bool quiet,
> + int *writeout_error)
> {
> int ret;
> - struct lock_file lock_file = LOCK_INIT;
> @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
> + }
>
> /* 2-way merge to the new branch */
> - init_topts(&topts, opts->merge, opts->show_progress,
> +- init_topts(&topts, opts->merge, opts->show_progress,
> - opts->overwrite_ignore, old_branch_info->commit);
> -+ opts->overwrite_ignore, old_branch_info->commit,
> -+ show_unpack_errors);
> ++ init_topts(&topts, opts->show_progress,
> ++ opts->overwrite_ignore, quiet);
> init_checkout_metadata(&topts.meta, new_branch_info->refname,
> new_branch_info->commit ?
> &new_branch_info->commit->object.oid :
> @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
> if (do_merge) {
> - ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> + ret = merge_working_tree(opts, &old_branch_info, new_branch_info,
> -+ &writeout_error, false);
> ++ opts->merge, &writeout_error);
> + if (ret == MERGE_WORKING_TREE_UNPACK_FAILED && opts->merge) {
> + strbuf_addf(&autostash_msg,
> + "autostash while switching to '%s'",
> @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
> + autostash_msg.buf, true);
> + created_autostash = 1;
> + ret = merge_working_tree(opts, &old_branch_info, new_branch_info,
> -+ &writeout_error, true);
> ++ false, &writeout_error);
> + }
> + if (created_autostash) {
> + if (opts->conflict_style >= 0) {
> @@ t/t7201-co.sh: test_expect_success 'checkout -m with dirty tree' '
> test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
>
> - printf "M\t%s\n" one >expect.messages &&
> -+ printf "The following paths have local changes:\nM\t%s\n" one >expect.messages &&
> ++ cat >expect.messages <<-\EOF &&
> ++ The following paths have local changes:
> ++ M one
> ++ EOF
> test_cmp expect.messages messages &&
>
> fill "M one" "A three" "D two" >expect.main &&
> @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
> + test_grep "git stash drop" actual &&
> + test_grep "git stash pop" actual &&
> + test_grep "The following paths have local changes" actual &&
> -+ git show --format=%B --diff-merges=1 refs/stash >actual &&
> ++ git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
> + sed /^index/d actual >actual.trimmed &&
> + cat >expect <<-EOF &&
> ++ autostash while switching to ${SQ}side${SQ}
> + On main: autostash while switching to ${SQ}side${SQ}
> +
> + diff --git a/one b/one
> @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
> + >another-file.t &&
> + fill 1 2 3 4 5 >one &&
> + test_must_fail git checkout -m @{-1} 2>err &&
> -+ test_grep "would be overwritten by checkout" err &&
> -+ test_grep "another-file.t" err
> ++ q_to_tab >expect <<-\EOF &&
> ++ error: The following untracked working tree files would be overwritten by checkout:
> ++ Qanother-file.t
> ++ Please move or remove them before you switch branches.
> ++ Aborting
> ++ Applied autostash.
> ++ EOF
> ++ test_cmp expect err
> +'
> +
> test_expect_success 'switch to another branch while carrying a deletion' '
>
^ permalink raw reply
* Re: [PATCH v16 5/5] checkout -m: autostash when switching branches
From: Phillip Wood @ 2026-04-29 10:02 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <07d25fda91a1bbb3e13005d5462d1879303b15c0.1777401552.git.gitgitgadget@gmail.com>
Hi Harald
On 28/04/2026 19:39, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> if (do_merge) {
> - ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> + ret = merge_working_tree(opts, &old_branch_info, new_branch_info,
> + opts->merge, &writeout_error);
> + if (ret == MERGE_WORKING_TREE_UNPACK_FAILED && opts->merge) {
> + strbuf_addf(&autostash_msg,
> + "autostash while switching to '%s'",
> + new_branch_info->name);
> + create_autostash_ref(the_repository,
> + "CHECKOUT_AUTOSTASH_HEAD",
> + autostash_msg.buf, true);
If there are no local changes then doing this is pointless - it means
unpack_trees() failed for another reason. Having said that the current
code also tries a 3-way merge unconditionally so I think we can happily
leave this for the future as #leftoverbits
> + created_autostash = 1;
> + ret = merge_working_tree(opts, &old_branch_info, new_branch_info,
> + false, &writeout_error);
> + }
> + if (created_autostash) {
> + if (opts->conflict_style >= 0) {
> + struct strbuf cfg = STRBUF_INIT;
> + strbuf_addf(&cfg, "merge.conflictStyle=%s",
> + conflict_style_name(opts->conflict_style));
> + git_config_push_parameter(cfg.buf);
> + strbuf_release(&cfg);
> + }
> + apply_autostash_ref(the_repository,
> + "CHECKOUT_AUTOSTASH_HEAD",
> + new_branch_info->name,
> + "local",
> + stash_label_base,
> + autostash_msg.buf);
> + }
> if (ret) {
> branch_info_release(&old_branch_info);
> - return ret;
> + strbuf_release(&old_commit_shortname);
> + strbuf_release(&autostash_msg);
> + return ret < 0 ? 1 : ret;
> }
> }
If popping the stash created merge conflicts then it would be nice to
print a blank line before the message about which branch we've switched
to so that it is visually separated from the conflicts advice. That
would mean apply_autostash_ref() would have to tell us if there we're
conflicts. Again we can happily leave that for the future as #leftoverbits
Thanks
Phillip
^ permalink raw reply
* [PATCH v2] t5564: use a short path for the SOCKS proxy socket
From: Johannes Schindelin via GitGitGadget @ 2026-04-29 8:22 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2100.git.1777299669889.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The SOCKS proxy test introduced in 0ca365c2ed4 (http: do not ignore
proxy path, 2024-08-02) creates a Unix domain socket in
`$TRASH_DIRECTORY`. When the trash directory path is long (e.g.
when running from a deeply nested worktree), the socket path can
exceed the 108-character limit for `struct sockaddr_un.sun_path` on
Linux, causing the test to fail with "Path length ... is longer
than maximum supported length (108)".
We cannot work around this using the chdir trick our own socket code
employs, because both sides of the connection are outside our control:
the socket is created by socks4-proxy.pl via Perl's IO::Socket::UNIX,
and the client side is libcurl.
Use `mktemp -d` to create a unique temporary directory with a short
path, and place the socket inside it. This avoids collisions between
concurrent test runs (e.g. `--stress`) and tmpdir-race vulnerabilities
that a static `/tmp` path would be susceptible to.
Helped-by: Jeff King <peff@peff.net>
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t5564: use a short path for the SOCKS proxy socket
When trying to run the entire test suite in a slightly deeper path than
usual, I was surprised to see that this test failed due to our old
friend, the 108 character limit of Unix sockets.
Changes since v1:
* Uses mktemp -d now, to handle --stress better (thanks, Jeff!)
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2100%2Fdscho%2Favoid-too-long-unix-socket-path-in-socks-proxy-test-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2100/dscho/avoid-too-long-unix-socket-path-in-socks-proxy-test-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2100
Range-diff vs v1:
1: 16826e612c ! 1: 256b76a999 t5564: use a short path for the SOCKS proxy socket
@@ Commit message
Linux, causing the test to fail with "Path length ... is longer
than maximum supported length (108)".
- Move the socket to `$TMPDIR` (defaulting to `/tmp`) where the path
- is short, following the same approach used in t7528 for the SSH
- agent socket in b7fb2194b96 (t7528: work around ETOOMANY in OpenSSH
- 10.1 and newer, 2025-10-23).
+ We cannot work around this using the chdir trick our own socket code
+ employs, because both sides of the connection are outside our control:
+ the socket is created by socks4-proxy.pl via Perl's IO::Socket::UNIX,
+ and the client side is libcurl.
+ Use `mktemp -d` to create a unique temporary directory with a short
+ path, and place the socket inside it. This avoids collisions between
+ concurrent test runs (e.g. `--stress`) and tmpdir-race vulnerabilities
+ that a static `/tmp` path would be susceptible to.
+
+ Helped-by: Jeff King <peff@peff.net>
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@@ t/t5564-http-proxy.sh: start_socks() {
# proxy string passed to curl.
+# Use a short path for the socket to avoid exceeding the 108-character
+# Unix domain socket limit when the trash directory path is long.
-+SOCKS_SOCK="${TMPDIR:-/tmp}/git-test-socks-%30.sock"
++SOCKS_TMPDIR=$(mktemp -d)
++SOCKS_SOCK="$SOCKS_TMPDIR/%30.sock"
+
test_lazy_prereq SOCKS_PROXY '
test_have_prereq PERL &&
@@ t/t5564-http-proxy.sh: start_socks() {
test_atexit '
test ! -e "$TRASH_DIRECTORY/socks.pid" ||
kill "$(cat "$TRASH_DIRECTORY/socks.pid")"
-+ rm -f "$SOCKS_SOCK"
++ rm -rf "$SOCKS_TMPDIR"
'
# The below tests morally ought to be gated on a prerequisite that Git is
t/t5564-http-proxy.sh | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/t/t5564-http-proxy.sh b/t/t5564-http-proxy.sh
index 3bcbdef409..b4d95b12ca 100755
--- a/t/t5564-http-proxy.sh
+++ b/t/t5564-http-proxy.sh
@@ -50,14 +50,20 @@ start_socks() {
# The %30 tests that the correct amount of percent-encoding is applied to the
# proxy string passed to curl.
+# Use a short path for the socket to avoid exceeding the 108-character
+# Unix domain socket limit when the trash directory path is long.
+SOCKS_TMPDIR=$(mktemp -d)
+SOCKS_SOCK="$SOCKS_TMPDIR/%30.sock"
+
test_lazy_prereq SOCKS_PROXY '
test_have_prereq PERL &&
- start_socks "$TRASH_DIRECTORY/%30.sock"
+ start_socks "$SOCKS_SOCK"
'
test_atexit '
test ! -e "$TRASH_DIRECTORY/socks.pid" ||
kill "$(cat "$TRASH_DIRECTORY/socks.pid")"
+ rm -rf "$SOCKS_TMPDIR"
'
# The below tests morally ought to be gated on a prerequisite that Git is
@@ -70,7 +76,8 @@ old_libcurl_error() {
test_expect_success SOCKS_PROXY 'clone via Unix socket' '
test_when_finished "rm -rf clone" &&
- test_config_global http.proxy "socks4://localhost$PWD/%2530.sock" && {
+ socks_proxy_url="socks4://localhost$(echo "$SOCKS_SOCK" | sed "s/%/%25/g")" &&
+ test_config_global http.proxy "$socks_proxy_url" && {
{
GIT_TRACE_CURL=$PWD/trace \
GIT_TRACE_CURL_COMPONENTS=socks \
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2 00/16] repack: incremental MIDX/bitmap-based repacking
From: Jeff King @ 2026-04-29 8:10 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1776803827.git.me@ttaylorr.com>
On Tue, Apr 21, 2026 at 04:37:08PM -0400, Taylor Blau wrote:
> The main changes since last time are described in [1], which I sent to
> the list last week just before the 2.54 release was tagged. For
> convenience, a copy of the main changes are below:
>
> - Use a strset instead of a string_list for keeping track of the MIDX
> layers to retain when calling `either clear_midx_files_ext()` or
> `clear_incremental_midx_files_ext()`.
>
> - A new patch to rewrite the logic for determining which MIDX layers
> comprise the new chain via keep_hashes to build the array in order.
> The subsequent patch converts that into a strvec, which no longer
> requires direct manipulation.
>
> - The new "--checksum-only" option has been renamed to
> "--no-write-chain-file", and various small implementation tweaks
> (e.g., relying on `is_lock_file_locked()` to determine whether we
> should update the chain file as opposed to reading the flags).
Thanks, I think that addresses my review from the previous round.
I looked over the final few patches. Aside from a small nit about a
possible leak, I didn't notice anything obviously wrong. There's enough
new and tricky concepts that I wouldn't guarantee there's not a subtle
corner case lurking, but I don't think me staring at it any longer is
going to yield any further insight.
-Peff
^ permalink raw reply
* Re: [PATCH v2 14/16] repack: implement incremental MIDX repacking
From: Jeff King @ 2026-04-29 8:10 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Patrick Steinhardt
In-Reply-To: <93e152fb6aa3bbd450d46c6cdd219dfef14d41fc.1776803827.git.me@ttaylorr.com>
On Tue, Apr 21, 2026 at 04:37:54PM -0400, Taylor Blau wrote:
> Unlike the default mode which writes a single flat MIDX, the incremental
> mode constructs a compaction plan that determines which MIDX layers to
> write, compact, or copy, and then executes each step using `git
> multi-pack-index` subcommands with the --checksum-only flag.
This should be --no-write-chain-file, I think.
Ditto here:
> After writing the new layer, the strategy is evaluated among the
> existing MIDX layers in order from oldest to newest. Each step that
> writes a new MIDX layer uses "--checksum-only" to avoid updating the
> multi-pack-index-chain file. After all steps are complete, the new chain
> file is written and then atomically moved into place.
In the code I think it is all good, though:
> + strvec_pushl(&cmd.args, "--incremental", "--no-write-chain-file", NULL);
-Peff
^ permalink raw reply
* Re: [PATCH v2 10/16] repack-midx: extract `repack_fill_midx_stdin_packs()`
From: Jeff King @ 2026-04-29 8:08 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Patrick Steinhardt
In-Reply-To: <44f522ea04df5176ec332b60a38d1eab9b649c30.1776803827.git.me@ttaylorr.com>
On Tue, Apr 21, 2026 at 04:37:42PM -0400, Taylor Blau wrote:
> This simplifies `write_midx_included_packs()` and prepares for a
> subsequent commit where the same helper is called with `cmd->out = -1`
> to capture the MIDX's checksum from the command's standard output,
> which is needed when writing MIDX layers with `--checksum-only`.
This should be --no-write-chain-file now, right? It's not used in the
code here, so it's just a commit message fixup.
-Peff
^ permalink raw reply
* Re: [PATCH v2 14/16] repack: implement incremental MIDX repacking
From: Jeff King @ 2026-04-29 7:51 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Patrick Steinhardt
In-Reply-To: <93e152fb6aa3bbd450d46c6cdd219dfef14d41fc.1776803827.git.me@ttaylorr.com>
On Tue, Apr 21, 2026 at 04:37:54PM -0400, Taylor Blau wrote:
> static int repack_fill_midx_stdin_packs(struct child_process *cmd,
> - struct string_list *include)
> + struct string_list *include,
> + struct string_list *out)
> {
> struct string_list_item *item;
> FILE *in;
> int ret;
>
> cmd->in = -1;
> + if (out)
> + cmd->out = -1;
We are already piping to the sub-command, and now we will read the
output. Looks like we do them sequentially:
> @@ -312,6 +319,17 @@ static int repack_fill_midx_stdin_packs(struct child_process *cmd,
> fprintf(in, "%s\n", item->string);
> fclose(in);
>
> + if (out) {
> + struct strbuf buf = STRBUF_INIT;
> + FILE *outf = xfdopen(cmd->out, "r");
> +
> + while (strbuf_getline(&buf, outf) != EOF)
> + string_list_append(out, buf.buf);
> + strbuf_release(&buf);
> +
> + fclose(outf);
> + }
Is it possible to deadlock here where we block writing to the child, but
the child is blocked trying to write back to us. It's probably quite
unlikely as it implies both pipe buffers are filled up (and we are
counting packs and midx hashes here, neither of which we'd expect to be
too numerous).
Using pipe_command() would solve this, but it also might be impossible
to trigger if the child reads all input before generating any output. I
_think_ that's the case looking at cmd_multi_pack_index_write(). So
we're OK, but you might want to double check.
> +static int midx_compaction_step_exec_compact(struct midx_compaction_step *step,
> + struct repack_write_midx_opts *opts)
> +{
> [...]
> + ret = start_command(&cmd);
> + if (ret)
> + goto out;
OK, if we get an error here we'll jump to "out", but run-command.c will
have cleaned up everything about "cmd" that we need.
But later...
> + out = xfdopen(cmd.out, "r");
> + while (strbuf_getline_lf(&buf, out) != EOF) {
> + if (step->csum) {
> + ret = error(_("unexpected MIDX output: '%s'"), buf.buf);
> + goto out;
> + }
...here we jump to "out" but the command is left running. I guess it
will eventually get SIGPIPE when we fclose(out), but of course we won't
wait() for it and we're probably leaking some memory, too.
> + }
> +
> + ret = finish_command(&cmd);
> +
> +out:
> + if (out)
> + fclose(out);
> + strbuf_release(&buf);
It would be nice if we had an idempotent version of finish_command(),
that would do nothing when the program was already done. But I don't
think we do. We don't even clear cmd->pid when we finish the command, so
you can't even do something like:
out:
if (cmd->pid)
finish_command(&cmd);
So probably that "goto out" in the loop needs to call fclose() and
finish_command() itself.
-Peff
^ permalink raw reply
* Re: [PATCH] git-gui: handle bare repo or missing worktree
From: Johannes Sixt @ 2026-04-29 6:58 UTC (permalink / raw)
To: Shroom Moo; +Cc: git
In-Reply-To: <tencent_3FF7B35FE8AD7236FC9BBB628B45EA65F405@qq.com>
Am 21.04.26 um 18:28 schrieb Shroom Moo:
> When starting git-gui from a directory that Git recognizes as a valid
> repository but the repository is either bare or its working tree is
> missing, git-gui previously attempted to execute 'rev-parse
> --show-toplevel' without error handling. This caused a fatal Tcl error
> ("this operation must be run in a work tree") and prevented the user
> from opening the repository selection dialog.
>
> Improve the repository setup logic:
> - After obtaining the git directory via 'rev-parse --git-dir', check
> whether the repository is bare or if the working tree can be
> successfully located.
> - If the repository is unusable as a working tree, display a warning
> and present the "Create/Clone/Open" repository selection dialog.
I consider it harmful to allow the user to create (or clone) a
repository after Git GUI has already determined the existence of a
repository. If the worktree was not found, we should report an error and
exit after the message is dismissed, and not give the option to create a
repository.
>
> This makes git-gui robust when launched from a bare repository, from
> inside a .git directory without a worktree, or when GIT_DIR points to
> an invalid location. No regressions observed in normal working trees.
>
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
> git-gui/git-gui.sh | 75 +++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 57 insertions(+), 18 deletions(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..2e4bc2f226 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1123,24 +1123,63 @@ unset argv0dir
> ## repository setup
>
> set picked 0
> -if {[catch {
> - set _gitdir $env(GIT_DIR)
> - set _prefix {}
> - }]
> - && [catch {
> - # beware that from the .git dir this sets _gitdir to .
> - # and _prefix to the empty string
> - set _gitdir [git rev-parse --git-dir]
> - set _prefix [git rev-parse --show-prefix]
> - } err]} {
> - load_config 1
> - apply_config
> - choose_repository::pick
> - if {![file isdirectory $_gitdir]} {
> - exit 1
> - }
> - set picked 1
> -}
> +# Save temporarily for restoration later
> +set _startup_cwd [pwd]
> +
> +set need_pick 0
> +if {[catch {set _gitdir $env(GIT_DIR); set _prefix {}}]} {
> + # GIT_DIR not set in environment, attempt auto-detection
> + # beware that from the .git dir this sets _gitdir to .
> + # and _prefix to the empty string
> + if {[catch {set _gitdir [git rev-parse --git-dir]; set _prefix [git rev-parse --show-prefix]} err]} {
> + # Not inside any Git repository, proceed to repository selection UI
> + set need_pick 1
> + } else {
> + # Inside a Git repository, but need to verify its usability
> + cd [file dirname $_gitdir]
> + set bare 0
> + set worktree_missing 0
> + # Check if this is a bare repository (no working tree)
> + if {![catch {set bare [git rev-parse --is-bare-repository]}]} {
> + if {$bare eq "true"} {
> + set bare 1
> + }
> + }
> + # Check if working tree is present and accessible
> + if {[catch {git rev-parse --show-toplevel}]} {
> + set worktree_missing 1
> + }
> +
> + cd $_startup_cwd
> +
> + # For bare repos or missing worktrees, warn and guide to selection
> + if {$bare || $worktree_missing} {
> + set msg [mc "The repository at '%s' cannot be opened:" [file normalize $_gitdir]]
> + if {$bare} {
> + append msg "\n\n" [mc "It is a bare repository (no working tree)."]
> + } else {
> + append msg "\n\n" [mc "The working tree appears to be missing or inaccessible."]
> + }
> + append msg "\n\n" [mc "Please select a valid working repository, or create/clone one."]
> + tk_messageBox -icon warning -type ok -title [mc "Repository Unusable"] -message $msg
> +
> + set need_pick 1
> + }
> + }
> +}
> +
> +if {$need_pick} {
> + load_config 1
> + apply_config
> + choose_repository::pick
> + if {![file isdirectory $_gitdir]} {
> + exit 1
> + }
> + set picked 1
> +}
> +
> +# Clean up working tree checking temporary variables
> +unset -nocomplain _startup_cwd need_pick bare worktree_missing
This adds quite a lot of code with failure cases, but after this point
we already have some quite extensive error diagnosis, too. Except that
the call to git rev-parse --show-toplevel is not protected.
If we do not invoke the Create/Clone/Open dialog after a repository has
already been discovered, would it not be sufficient to just add the
protection around the --show-toplevel call? Or is there something else
missing in the existing error paths?
-- Hannes
^ permalink raw reply
* Re: Git generated tarballs and Debian
From: Jeff King @ 2026-04-29 7:30 UTC (permalink / raw)
To: Theodore Tso; +Cc: brian m. carlson, Simon Richter, git, Ian Jackson
In-Reply-To: <20260428115017.GA71700@macsyma-wired.lan>
On Tue, Apr 28, 2026 at 07:50:17AM -0400, Theodore Tso wrote:
> I know that in the past, using --format=tgz has broken based on
> different compression parameters used by git (and whether it used an
> external or internal compressor). I also know that if $commit is a
> tree-id, this can result in the timestamps being not reproduible. I
> also don't use export-subst.
>
> There is also the difference in the prefix used by github and gitlab,
> but that's arguably not git's fault.
>
> What other gotchas are there? How is this likely to be inconsistent
> in the future? How much work is there to provide that guarantee in
> the future?
The biggest unexpected change I recall was caused by a bug/compatibility
fix. 22f0dcd963 (archive-tar: split long paths more carefully,
2013-01-05) changed how some long paths were represented to be more
compatible between GNU tar and NetBSD. Lots of Homebrew recipes, etc,
were broken when GitHub deployed a version of Git with that commit.
I think there was a more recent one in 2023-ish caused by some
gzip-related changes (but it was after my time and I don't know the
details).
I feel like there was one in the middle, too, but I'm having trouble
digging it up (I think GitHub reverted 22f0dcd963 at the time and
finally reinstated it in 2017 after a warning period, so that might be
what I'm thinking of).
But I'm not sure how often we'd do fixes like that. Not a lot, as the
tar code is pretty stable. But is 82a46af13e (archive-tar: fix pax
extended header length calculation, 2019-08-17), for example, likely to
have changed hashes for some repos? Probably.
So I think if you really want byte-for-byte compatibility of git-archive
you have to cement the behavior, bugs and all, behind some kind of
version flag, and every possible behavior change has to be analyzed for
a potential version bump.
Though breaking some obscure cases once every 5-10 years is maybe not
_so_ bad, and we can live with it. ;)
-Peff
^ permalink raw reply
* Re: [PATCH] parse-options: fix sparse 'plain integer as NULL pointer'
From: Jiamu Sun @ 2026-04-29 5:13 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Ramsay Jones, GIT Mailing-list
In-Reply-To: <d0357af8-ce70-77be-a885-070420d76c0b@gmx.de>
On Sun, Apr 26, 2026 at 02:14:03PM +0200, Johannes Schindelin wrote:
> Indeed. With this patch on top of Ramsay's fixup, it passes the build:
>
> Assisted-by: Claude Opus 4.6
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> parse-options.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 14f3f385eb4..0ba6905fed5 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -648,7 +648,8 @@ static const char *autocorrect_subcommand(const char *cmd,
> struct string_list *cmds)
> {
> struct autocorrect autocorrect = { 0 };
> - unsigned int n = 0, best = 0;
> + unsigned int n = 0;
> + intptr_t best = 0;
> struct string_list_item *cand;
>
> autocorrect_resolve(&autocorrect);
> -- snap --
>
> > > Ramsay Jones
> > >
> > > parse-options.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/parse-options.c b/parse-options.c
> > > index d60e7bd3c9..14f3f385eb 100644
> > > --- a/parse-options.c
> > > +++ b/parse-options.c
> > > @@ -658,7 +658,7 @@ static const char *autocorrect_subcommand(const char *cmd,
> > >
> > > for_each_string_list_item(cand, cmds) {
> > > if (starts_with(cand->string, cmd)) {
> > > - cand->util = 0;
> > > + cand->util = NULL;
> > > } else {
> > > int edit = levenshtein(cmd, cand->string,
> > > 0, 2, 1, 3) + 1;
> > > --
> > > 2.54.0
Will squash these in next re-roll.
Thanks everyone for catching these issues and providing the fixups.
--
Jiamu Sun <39@barroit.sh>
<sunjiamu@outlook.com>
^ permalink raw reply
* [PATCH v3 5/5] format-rev: introduce builtin for on-demand pretty formatting
From: kristofferhaugsbakk @ 2026-04-28 22:25 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ben.knoble
In-Reply-To: <V3_CV_format-rev.66a@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Introduce a new builtin for pretty formatting one revision expression
per line or commit object names found in running text.
Sometimes you want to format commits. Most of the time you’re
walking the graph, e.g. getting a range of commits like
`master..topic`. That’s a job for git-log(1).
But there are times when you want to format commits that you encounter
on demand:
• Full hashes in running text that you might want to pretty-print
• git-last-modified(1) outputs full hashes that you can do the same
with
• git-cherry(1) has `-v` for commit subject, but maybe you want
something else?
But now you can’t use git-log(1), git-show(1), or git-rev-list(1):
• You can’t feed commits piecemeal to these commands, one input
for one output; they block until standard in is closed
• You can’t feed a list of possibly duplicate commits, like the output
of git-last-modified(1); they effectively deduplicate the output
Beyond these two points there’s also the input massage problem: you
cannot feed mixed input (revisions mixed with arbitrary text).
One might hope that git-cat-file(1) can save us. But it doesn’t
support pretty formats.
But there is one command that already both handles revisions as
arguments, revisions on standard input, and even revisions mixed in
with arbitrary text. Namely git-name-rev(1): the command for outputting
symbolic names for commits.
We made some room in `builtin/name-rev.c` two commits ago. Let’s
now add this new git-format-rev(1) command. Taking inspiration from
git-name-rev(1), there are two modes:
• revs: like git-name-rev(1) in argv mode, but one revision per line
on standard in
• text: like git-name-rev(1) with `--annotate-stdin`
***
We need to add this command to the exception list in
`t/t1517-outside-repo.sh` because it uses “EXPERIMENTAL!”
in the usage line.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v3:
• And don’t forget to document --notes this time
https://lore.kernel.org/git/CALnO6CB5WOTp_e7Kv3CrEbQ+3XE-gDxNVHf7qATBEbyKWfxpLg@mail.gmail.com/
Documentation/git-format-rev.adoc | 148 ++++++++++++++++++++++++
Makefile | 1 +
builtin.h | 1 +
builtin/name-rev.c | 186 ++++++++++++++++++++++++++++++
command-list.txt | 1 +
git.c | 1 +
t/t1517-outside-repo.sh | 3 +-
t/t6120-describe.sh | 104 +++++++++++++++++
8 files changed, 444 insertions(+), 1 deletion(-)
create mode 100644 Documentation/git-format-rev.adoc
diff --git a/Documentation/git-format-rev.adoc b/Documentation/git-format-rev.adoc
new file mode 100644
index 00000000000..d960001d750
--- /dev/null
+++ b/Documentation/git-format-rev.adoc
@@ -0,0 +1,148 @@
+git-format-rev(1)
+=================
+
+NAME
+----
+git-format-rev - EXPERIMENTAL: Pretty format revisions on demand
+
+
+SYNOPSIS
+--------
+[synopsis]
+(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--notes=<ref>]
+
+DESCRIPTION
+-----------
+
+Pretty format revisions from standard input.
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
+
+OPTIONS
+-------
+
+`--stdin-mode=<mode>`::
+ How to interpret standard input data:
++
+--
+`revs`:: Each line is interpreted as a commit. Any kind of revision
+ expression can be used (see linkgit:gitrevisions[7]). Annotated
+ tags are peeled (see linkgit:gitglossary[7]).
++
+The argument `rev` is also accepted.
+`text`:: Formats all commit object names found in freeform text. These
+ must the full object names, i.e. abbreviated hexidecimal object
+ names will not be interpreted.
+--
+
+`--format=<pretty>`::
+ Pretty format string.
+
+`--notes=<ref>`::
+`--no-notes`::
+ Custom notes ref. Notes are displayed when using the `%N`
+ atom. See linkgit:git-notes[1].
+
+EXAMPLES
+--------
+
+The command linkgit:git-last-modified[1] shows the commit that each file
+was last modified in.
+
+----
+$ git last-modified -- README.md Makefile
+7798034171030be0909c56377a4e0e10e6d2df93 Makefile
+c50fbb2dd225e7e82abba4380423ae105089f4d7 README.md
+----
+
+We can pipe the result to this command in order to replace the object
+name with the commit author.
+
+----
+$ git last-modified -- README.md Makefile |
+ git format-rev --stdin-mode=text --format=%an
+Junio C Hamano Makefile
+Todd Zullinger README.md
+----
+
+Another example is _formatting commits in commit messages_. Given this commit message:
+
+----
+Fix off-by-one error
+
+Fix off-by-one error introduced in
+e83c5163316f89bfbde7d9ab23ca2e25604af290.
+
+We thought we fixed this in 5569bf9bbedd63a00780fc5c110e0cfab3aa97b9 but
+that only covered 1/3 of the faulty cases.
+----
+
+We can format the commits and use par(1) to reflow the text, say in a
+`commit-msg` hook:
+
+----
+$ git config set hook.reference-commits.event commit-msg
+$ git config set hook.reference-commits.command reference-commits
+$ cat $(which reference-commits)
+#/bin/sh
+
+msg="$1"
+rewritten=$(mktemp)
+git format-rev --stdin-mode=text --format=reference <"$msg" |
+ par >"$rewritten"
+mv "$rewritten" "$msg"
+----
+
+Which will produce something like this:
+
+----
+Fix off-by-one error
+
+Fix off-by-one error introduced in e83c5163316 (Implement better memory
+allocator, 2005-04-07).
+
+We thought we fixed this in 5569bf9bbed (Fix memory allocator,
+2005-06-22) but that only covered 1/3 of the faulty cases.
+----
+
+DISCUSSION
+----------
+
+This command lets you format any number of revisions in any order
+through one command invocation. Consider the
+linkgit:git-last-modified[1] case from the "EXAMPLES" section above:
+
+1. There might be hundreds of files
+2. Commits can be repeated, i.e. two or more files were last modified in
+ the same commit
+
+Two widely-used commands which pretty formats commits are
+linkgit:git-log[1] and linkgit:git-show[1]. It turns out that they are
+not a good fit for the above use case.
+
+- The output of linkgit:git-last-modified[1] would have to be processed
+ in stages since you need to transform the first column separately and
+ then link the author to the filename. But this is surmountable.
+- You can feed each commit to `git show` or `git show --no-walk -1`. But
+ that means that you need to create a process for each line.
+- Let’s say that you want to use one process, not one per line. So you
+ want to feed all the commits to the command. Now you face the problem
+ that you have to feed all the commits to the commands before you get
+ any output (this is also the case for the `--stdin` modes). In other
+ words, you cannot loop through each line, get the author for the
+ commit, and output the author and the filename. You need to feed all
+ the commits, get back all the output, and match the output with the
+ filename.
+- But the next problem is that commands will deduplicate the input and
+ only output one commit one single time only. Thus you cannot make the
+ output order match the input order, since a commit could have been
+ repeated in the original input.
+
+In short, it is straightforward to use these two commands if you use one
+process per line. It is much more work if you just want to use one
+process, but still doable. In contrast, this problem is just another
+shell pipeline with this command.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 15b1ded1a0b..cbaf91fd846 100644
--- a/Makefile
+++ b/Makefile
@@ -895,6 +895,7 @@ BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))
BUILT_INS += git-cherry$X
BUILT_INS += git-cherry-pick$X
BUILT_INS += git-format-patch$X
+BUILT_INS += git-format-rev$X
BUILT_INS += git-fsck-objects$X
BUILT_INS += git-init$X
BUILT_INS += git-maintenance$X
diff --git a/builtin.h b/builtin.h
index 235c51f30e5..63813c90125 100644
--- a/builtin.h
+++ b/builtin.h
@@ -189,6 +189,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix, struct re
int cmd_for_each_ref(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_for_each_repo(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_format_patch(int argc, const char **argv, const char *prefix, struct repository *repo);
+int cmd_format_rev(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_fsck(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_gc(int argc, const char **argv, const char *prefix, struct repository *repo);
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index dc4136f4de3..b60cc766279 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -18,6 +18,9 @@
#include "commit-graph.h"
#include "wildmatch.h"
#include "mem-pool.h"
+#include "pretty.h"
+#include "revision.h"
+#include "notes.h"
/*
* One day. See the 'name a rev shortly after epoch' test in t6120 when
@@ -272,14 +275,26 @@ struct name_ref_data {
struct string_list exclude_filters;
};
+struct pretty_format {
+ struct pretty_print_context ctx;
+ struct userformat_want want;
+};
+
enum command_type {
NAME_REV = 1,
+ FORMAT_REV = 2,
+};
+
+enum stdin_mode {
+ TEXT = 1,
+ REVS = 2,
};
struct command {
enum command_type type;
union {
int name_only;
+ struct pretty_format *pretty_format;
} u;
};
@@ -290,6 +305,13 @@ static void init_name_rev_command(struct command *cmd,
cmd->u.name_only = name_only;
}
+static void init_format_rev_command(struct command *cmd,
+ struct pretty_format *pretty_format)
+{
+ cmd->type = FORMAT_REV;
+ cmd->u.pretty_format = pretty_format;
+}
+
static struct tip_table {
struct tip_table_entry {
struct object_id oid;
@@ -495,6 +517,27 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf)
}
}
+static const char *get_format_rev(const struct commit *c,
+ struct pretty_format *format_ctx,
+ struct strbuf *buf)
+{
+ strbuf_reset(buf);
+
+ if (format_ctx->want.notes) {
+ struct strbuf notebuf = STRBUF_INIT;
+
+ format_display_notes(&c->object.oid, ¬ebuf,
+ get_log_output_encoding(),
+ format_ctx->ctx.fmt == CMIT_FMT_USERFORMAT);
+ format_ctx->ctx.notes_message = strbuf_detach(¬ebuf, NULL);
+ }
+
+ pretty_print_commit(&format_ctx->ctx, c, buf);
+ FREE_AND_NULL(format_ctx->ctx.notes_message);
+
+ return buf->buf;
+}
+
static void show_name(const struct object *obj,
const char *caller_name,
int always, int allow_undefined, int name_only)
@@ -565,6 +608,19 @@ static void name_rev_line(char *p, struct command *cmd)
else
printf("%.*s (%s)", p_len, p_start, name);
break;
+ case FORMAT_REV:
+ if (!oid_ret)
+ o = parse_object(the_repository, &oid);
+ if (o && o->type == OBJ_COMMIT)
+ name = get_format_rev((const struct commit *)o,
+ cmd->u.pretty_format,
+ &buf);
+ *(p + 1) = c;
+ if (name)
+ printf("%.*s%s", p_len - hexsz, p_start, name);
+ else
+ printf("%.*s", p_len, p_start);
+ break;
default:
BUG("uncovered case: %d", cmd->type);
}
@@ -718,3 +774,133 @@ int cmd_name_rev(int argc,
object_array_clear(&revs);
return 0;
}
+
+static enum stdin_mode parse_stdin_mode(const char *stdin_mode)
+{
+ if (!strcmp(stdin_mode, "text"))
+ return TEXT;
+ else if (!strcmp(stdin_mode, "revs") ||
+ !strcmp(stdin_mode, "rev"))
+ return REVS;
+ else
+ die(_("'%s' needs to be either text, revs, or rev"),
+ "--stdin-mode");
+}
+
+static char const *const format_rev_usage[] = {
+ N_("(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--notes=<ref>]"),
+ NULL
+};
+
+int cmd_format_rev(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo UNUSED)
+{
+ const char *format = NULL;
+ enum stdin_mode stdin_mode;
+ const char *stdin_mode_arg = NULL;
+ struct display_notes_opt format_notes_opt;
+ struct rev_info format_rev = REV_INFO_INIT;
+ struct pretty_format format_pp = { 0 };
+ struct string_list notes = STRING_LIST_INIT_NODUP;
+ struct strbuf scratch_buf = STRBUF_INIT;
+ struct command cmd;
+ struct option opts[] = {
+ OPT_STRING(0, "format", &format, N_("format"),
+ N_("pretty format to use")),
+ OPT_STRING(0, "stdin-mode", &stdin_mode_arg, N_("stdin-mode"),
+ N_("how revs are processed")),
+ OPT_STRING_LIST(0, "notes", ¬es, N_("notes"),
+ N_("display notes for pretty format")),
+ OPT_END(),
+ };
+
+ argc = parse_options(argc, argv, prefix, opts, format_rev_usage, 0);
+
+ if (argc > 0) {
+ error(_("too many arguments"));
+ usage_with_options(format_rev_usage, opts);
+ }
+
+ if (!format)
+ die(_("'%s' is required"), "--format");
+ if (!stdin_mode_arg)
+ die(_("'%s' is required"), "--stdin-mode");
+
+ init_display_notes(&format_notes_opt);
+ stdin_mode = parse_stdin_mode(stdin_mode_arg);
+
+ get_commit_format(format, &format_rev);
+ format_pp.ctx.rev = &format_rev;
+ format_pp.ctx.fmt = format_rev.commit_format;
+ format_pp.ctx.abbrev = format_rev.abbrev;
+ format_pp.ctx.date_mode_explicit = format_rev.date_mode_explicit;
+ format_pp.ctx.date_mode = format_rev.date_mode;
+ format_pp.ctx.color = GIT_COLOR_AUTO;
+
+ userformat_find_requirements(format,
+ &format_pp.want);
+ if (format_pp.want.notes) {
+ int ignore_show_notes = 0;
+ struct string_list_item *n;
+
+ for_each_string_list_item(n, ¬es)
+ enable_ref_display_notes(&format_notes_opt,
+ &ignore_show_notes,
+ n->string);
+ load_display_notes(&format_notes_opt);
+ }
+
+ init_format_rev_command(&cmd, &format_pp);
+
+ switch (stdin_mode) {
+ case TEXT:
+ while (strbuf_getline(&scratch_buf, stdin) != EOF) {
+ strbuf_addch(&scratch_buf, '\n');
+ name_rev_line(scratch_buf.buf, &cmd);
+ }
+ break;
+ case REVS:
+ while (strbuf_getline(&scratch_buf, stdin) != EOF) {
+ struct object_id oid;
+ struct object *object;
+ struct object *peeled;
+ struct commit *commit;
+
+ if (repo_get_oid(the_repository, scratch_buf.buf, &oid)) {
+ fprintf(stderr, "Could not get sha1 for %s. Skipping.\n",
+ scratch_buf.buf);
+ continue;
+ }
+
+ object = parse_object(the_repository, &oid);
+ if (!object) {
+ fprintf(stderr, "Could not get object for %s. Skipping.\n",
+ scratch_buf.buf);
+ continue;
+ }
+
+ peeled = deref_tag(the_repository, object, scratch_buf.buf, 0);
+ if (peeled && peeled->type == OBJ_COMMIT)
+ commit = (struct commit *)peeled;
+ if (!commit) {
+ fprintf(stderr, "Could not get commit for %s. Skipping.\n",
+ *argv);
+ continue;
+ }
+
+ get_format_rev(commit, &format_pp, &scratch_buf);
+ printf("%s\n", scratch_buf.buf);
+ strbuf_release(&scratch_buf);
+ }
+ break;
+ default:
+ BUG("uncovered case: %d", stdin_mode);
+ }
+
+ strbuf_release(&scratch_buf);
+ string_list_clear(¬es, 0);
+ release_display_notes(&format_notes_opt);
+ return 0;
+}
diff --git a/command-list.txt b/command-list.txt
index f9005cf4597..df729872dca 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -108,6 +108,7 @@ git-fmt-merge-msg purehelpers
git-for-each-ref plumbinginterrogators
git-for-each-repo plumbinginterrogators
git-format-patch mainporcelain
+git-format-rev plumbinginterrogators
git-fsck ancillaryinterrogators complete
git-gc mainporcelain
git-get-tar-commit-id plumbinginterrogators
diff --git a/git.c b/git.c
index 2b212e6675d..af5b0422b00 100644
--- a/git.c
+++ b/git.c
@@ -578,6 +578,7 @@ static struct cmd_struct commands[] = {
{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
{ "for-each-repo", cmd_for_each_repo, RUN_SETUP_GENTLY },
{ "format-patch", cmd_format_patch, RUN_SETUP },
+ { "format-rev", cmd_format_rev, RUN_SETUP },
{ "fsck", cmd_fsck, RUN_SETUP },
{ "fsck-objects", cmd_fsck, RUN_SETUP },
{ "fsmonitor--daemon", cmd_fsmonitor__daemon, RUN_SETUP },
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index c824c1a25cf..360a9323343 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -114,7 +114,8 @@ do
archimport | citool | credential-netrc | credential-libsecret | \
credential-osxkeychain | cvsexportcommit | cvsimport | cvsserver | \
daemon | \
- difftool--helper | filter-branch | fsck-objects | get-tar-commit-id | \
+ difftool--helper | filter-branch | format-rev | fsck-objects | \
+ get-tar-commit-id | \
gui | gui--askpass | \
http-backend | http-fetch | http-push | init-db | \
merge-octopus | merge-one-file | merge-resolve | mergetool | \
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 62789f76381..725f7d81b6b 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -801,4 +801,108 @@ test_expect_success 'do not be fooled by invalid describe format ' '
test_must_fail git cat-file -t "refs/tags/super-invalid/./../...../ ~^:/?*[////\\\\\\&}/busted.lock-42-g"$(cat out)
'
+test_expect_success 'name-rev --format setup' '
+ mkdir repo-format &&
+ git -C repo-format init &&
+ test_commit -C repo-format first &&
+ test_commit -C repo-format second &&
+ test_commit -C repo-format third &&
+ test_commit -C repo-format fourth &&
+ test_commit -C repo-format fifth &&
+ test_commit -C repo-format sixth &&
+ test_commit -C repo-format seventh &&
+ test_commit -C repo-format eighth
+'
+
+test_expect_success 'format-rev --stdin-mode=revs' '
+ cat >expect <<-\EOF &&
+ eighth
+ seventh
+ fifth
+ EOF
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format=%s >actual <<-\EOF &&
+ HEAD
+ HEAD~
+ HEAD~3
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --stdin-mode=text from rev-list same as log' '
+ git -C repo-format log --format=reference >expect &&
+ test_file_not_empty expect &&
+ git -C repo-format rev-list HEAD >list &&
+ git -C repo-format format-rev --stdin-mode=text \
+ --format=reference <list >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --stdin-mode=text with running text and tree oid' '
+ cmit_oid=$(git -C repo-format rev-parse :/fifth) &&
+ reference=$(git -C repo-format log -n1 --format=reference :/fifth) &&
+ tree=$(git -C repo-format rev-parse HEAD^{tree}) &&
+ cat >expect <<-EOF &&
+ We thought we fixed this in ${reference}.
+ But look at this tree: ${tree}.
+ EOF
+ git -C repo-format format-rev --stdin-mode=text --format=reference \
+ >actual <<-EOF &&
+ We thought we fixed this in ${cmit_oid}.
+ But look at this tree: ${tree}.
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev with %N (note)' '
+ test_when_finished "git -C repo-format notes remove" &&
+ git -C repo-format notes add -m"Make a note" &&
+ printf "Make a note\n\n\n" >expect &&
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format="tformat:%N" \
+ >actual <<-\EOF &&
+ HEAD
+ HEAD~
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --notes<ref> (custom notes ref)' '
+ # One custom notes ref
+ test_when_finished "git -C repo-format notes remove" &&
+ test_when_finished "git -C repo-format notes --ref=word remove" &&
+ git -C repo-format notes add -m"default" &&
+ git -C repo-format notes --ref=word add -m"custom" &&
+ printf "custom\n\n" >expect &&
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format="tformat:%N" \
+ --notes=word \
+ >actual <<-\EOF &&
+ HEAD
+ EOF
+ test_cmp expect actual &&
+ # Glob all
+ printf "default\ncustom\n\n" >expect &&
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format="tformat:%N" \
+ --notes=* >actual <<-\EOF &&
+ HEAD
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --stdin-mode=revs on annotated tag peels to commit' '
+ test_when_finished "git -C repo-format tag -d version" &&
+ git -C repo-format tag -a -m"new version" version &&
+ cat >expect <<-\EOF &&
+ eighth
+ EOF
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format=%s \
+ --notes=* >actual <<-\EOF &&
+ version
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox