* [PATCH] checkout: add --autostash option for branch switching
From: Harald Nordgren @ 2026-04-28 15:16 UTC (permalink / raw)
To: phillip.wood123
Cc: chris.torek, git, gitgitgadget, haraldnordgren, peff,
phillip.wood
In-Reply-To: <6d60573f-a02d-4aea-b891-6dd52e2d7048@gmail.com>
> This uses the existing label which is sensible, but I wonder if "Stash
> HEAD" would be a better choice as the merge base is always HEAD commit
> that the stash is based on.
>
> We can always change that later
Yeah, seems better to do later.
Harald
^ permalink raw reply
* Re: [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10
From: Derrick Stolee @ 2026-04-28 15:01 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git; +Cc: Johannes Schindelin
In-Reply-To: <pull.2103.git.1777380768.gitgitgadget@gmail.com>
On 4/28/2026 8:52 AM, Johannes Schindelin via GitGitGadget wrote:
> On Windows, maintenance_task_geometric_repack() opens pack index files via
> pack_geometry_init() (which mmap()s the .idx files), then spawns git repack
> as a child process without setting child.odb_to_close. The parent's mmap()s
> prevent the child from deleting old .idx files.
>
> On Windows 10 builds before the POSIX delete semantics change (between Build
> 17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this
> results in Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should
> I try again? during fetch-triggered auto-maintenance with the geometric
> strategy.
>
> The fix adds the missing child.odb_to_close = the_repository->objects line,
> matching all other maintenance tasks.
>
> The first commit introduces a GIT_TEST_LEGACY_DELETE environment variable to
> simulate legacy (pre-POSIX) delete semantics on modern Windows, so the
> regression test can verify the fix even on Windows 11.
>
> This fixes https://github.com/git-for-windows/git/issues/6210.
Thanks for these patches. I reviewed their equivalents in the
git-for-windows/git fork so I'll give my LGTM here, too.
Thanks,
-Stolee
^ permalink raw reply
* [PATCH v4] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Scott Bauersfeld via GitGitGadget @ 2026-04-28 14:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Derrick Stolee, Jeff King, Scott Bauersfeld,
Scott Bauersfeld
In-Reply-To: <pull.2282.v3.git.git.1777317998098.gitgitgadget@gmail.com>
From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
index-pack and unpack-objects both read pack data from stdin through
a 4 KiB static buffer. In index-pack, each fill() flushes consumed
bytes to the pack file via write_or_die(), capping every write(2)
at 4 KiB. unpack-objects uses the same buffer pattern for reads.
On FUSE-backed filesystems every write(2) is a synchronous round
trip through the FUSE protocol (userspace -> kernel -> userspace ->
back), so the 4 KiB buffer turns a clone into many unnecessary tiny
writes with noticeable latency overhead.
Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
DEFAULT_IO_BUFFER_SIZE constant in git-compat-util.h (next to
MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the
hashfile layer in csum-file (which already used 128 KiB but
hardcoded the value).
Pack file writes to a FUSE filesystem with writeback caching
disabled during HTTPS clones of git/git (~293 MB pack):
74,958 -> 4,687 (94% fewer)
Wall-clock time of git clone over HTTPS onto a FUSE passthrough
filesystem with writeback caching disabled, 3 runs per variant:
vscode (~1.26 GB pack): 84.5s -> 75.7s avg (10% faster)
git/git (~306 MB pack): 22.6s -> 20.0s avg (11% faster)
Signed-off-by: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
---
index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
index-pack and unpack-objects read pack data from stdin through a 4 KiB
static buffer. In index-pack, each fill() flushes consumed bytes to the
pack file via write_or_die(), capping every write(2) at 4 KiB.
unpack-objects uses the same buffer pattern for reads.
On FUSE-backed filesystems every write(2) is a synchronous round trip
through the FUSE protocol (userspace → kernel → userspace → back), so
the 4 KiB buffer turns a clone into many unnecessary tiny writes with
noticeable latency overhead.
Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
DEFAULT_IO_BUFFER_SIZE constant in git-compat-util.h (next to
MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the hashfile
layer in csum-file (which already used 128 KiB but hardcoded the value).
Pack file write reduction
=========================
Pack file writes to a FUSE filesystem with writeback caching disabled
during HTTPS clones of git/git (~293 MB pack):
Unpatched avg Patched avg Change 74,958 4,687 −94%
Write counts measured by logging writes in a FUSE passthrough daemon
(libfuse 3.10.5, writeback cache off).
Wall-clock time on FUSE
=======================
Measured wall-clock time of git clone over HTTPS onto a FUSE passthrough
filesystem with writeback caching disabled. 3 runs per variant:
Repo Unpatched avg Patched avg Change microsoft/vscode (~1.26 GB pack)
84.5s 75.7s −10% git/git (~306 MB pack) 22.6s 20.0s −11%
Changes since v3
================
* Replaced strace-based syscall measurements with FUSE daemon write
logging. The earlier strace numbers (72,465 → 24,943, 65% reduction)
were distorted: strace -f ptrace intercepts every syscall in all
traced processes and added enough overhead to distort the
measurements. The FUSE daemon logging captures write sizes without
perturbing the traced processes, showing the true reduction is 94%
(74,958 → 4,687).
* Note: Why 4,687 writes instead of ~2k writes as would be expected
with a 128 KiB buffer size? It appears that fill() is calling xread()
on a pipe and the linux default buffer size for pipes is 64KiB. I
also tested using fcntl(F_SETPIPE_SZ) to increase the pipe's buffer
size to 128KiB, which does indeed reduce total pack file writes to
~2.4K.
Changes since v2
================
* Renamed DEFAULT_PACKFILE_BUFFER_SIZE → DEFAULT_IO_BUFFER_SIZE per
Stolee's feedback. The constant is not packfile-specific, since it is
also used by the hashfile layer.
* Stolee noted that WRITE_BUFFER_SIZE in read-cache.c could be
consolidated. That constant was already removed in f6e2cd0625
("read-cache: delete unused hashing methods", 2021-05-18) when
read-cache.c was converted to use the hashfile API, so there is
nothing left to unify. The rename to DEFAULT_IO_BUFFER_SIZE helps
account for the multiple usages of this constant.
Changes since v1
================
* Introduced shared DEFAULT_PACKFILE_BUFFER_SIZE constant in
git-compat-util.h (next to MAX_IO_SIZE), replacing per-file #define
and the hardcoded value in csum-file.c. Placed here rather than
environment.h since it is an I/O buffer size, not an environment
variable or repo config.
* Added wall-clock timing on a FUSE filesystem.
* Cleaned up the commit description a bit.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2282%2Fsbauersfeld%2Fsb%2Fincrease-index-pack-input-buffer-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2282/sbauersfeld/sb/increase-index-pack-input-buffer-v4
Pull-Request: https://github.com/git/git/pull/2282
Range-diff vs v3:
1: df754ac879 ! 1: 146b1846a5 index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
@@ Commit message
hashfile layer in csum-file (which already used 128 KiB but
hardcoded the value).
- Syscall counts via strace on HTTPS clones of git/git (~296 MB pack,
- 5 runs per variant, isolated builds from the same v2.54.0 source):
+ Pack file writes to a FUSE filesystem with writeback caching
+ disabled during HTTPS clones of git/git (~293 MB pack):
- index-pack pack file writes: 72,465 -> 24,943 avg (65% fewer)
- total write() syscalls: 310,192 -> 259,530 avg (16% fewer)
- writes of exactly 4096 bytes: ~40,077 -> 0
+ 74,958 -> 4,687 (94% fewer)
Wall-clock time of git clone over HTTPS onto a FUSE passthrough
filesystem with writeback caching disabled, 3 runs per variant:
builtin/index-pack.c | 3 +--
builtin/unpack-objects.c | 3 +--
csum-file.c | 2 +-
git-compat-util.h | 6 ++++++
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ca7784dc2c..bb3639641c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -145,8 +145,7 @@ static int check_self_contained_and_connected;
static struct progress *progress;
-/* We always read in 4kB chunks. */
-static unsigned char input_buffer[4096];
+static unsigned char input_buffer[DEFAULT_IO_BUFFER_SIZE];
static unsigned int input_offset, input_len;
static off_t consumed_bytes;
static off_t max_input_size;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index e01cf6e360..af67d1a1d3 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -23,8 +23,7 @@
static int dry_run, quiet, recover, has_errors, strict;
static const char unpack_usage[] = "git unpack-objects [-n] [-q] [-r] [--strict]";
-/* We always read in 4kB chunks. */
-static unsigned char buffer[4096];
+static unsigned char buffer[DEFAULT_IO_BUFFER_SIZE];
static unsigned int offset, len;
static off_t consumed_bytes;
static off_t max_input_size;
diff --git a/csum-file.c b/csum-file.c
index 9558177a11..d7a682c2b6 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -178,7 +178,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
f->algop = unsafe_hash_algo(algop);
f->algop->init_fn(&f->ctx);
- f->buffer_len = opts->buffer_len ? opts->buffer_len : 128 * 1024;
+ f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_IO_BUFFER_SIZE;
f->buffer = xmalloc(f->buffer_len);
f->check_buffer = NULL;
diff --git a/git-compat-util.h b/git-compat-util.h
index ae1bdc90a4..5024814bd4 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -712,6 +712,12 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
# endif
#endif
+/*
+ * Default buffer size for buffered I/O in index-pack, unpack-objects,
+ * and the hashfile layer in csum-file.
+ */
+#define DEFAULT_IO_BUFFER_SIZE (128 * 1024)
+
#ifdef HAVE_ALLOCA_H
# include <alloca.h>
# define xalloca(size) (alloca(size))
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 2/3] http: attempt Negotiate auth in http.emptyAuth=auto mode
From: Matthew John Cheetham @ 2026-04-28 14:38 UTC (permalink / raw)
To: Junio C Hamano, Matthew John Cheetham via GitGitGadget
Cc: git, johannes.schindelin
In-Reply-To: <xmqq7bq63lll.fsf@gitster.g>
On 2026-04-16 17:40, Junio C Hamano wrote:
>> The true and false modes are unchanged: true sends empty credentials
>> on the very first request (before any 401), and false never sends
>> them.
>
> OK. This is a tangent, but "git config --help" on "http.emptyAuth"
> is horrible. It does not say what the allowed values are, so I had
> to first write "There are million other things in the system that
> this patch does not modify, so what's the point of singling out
> these two settings and saying that this patch does not change
> them?", before realizing that 'auto' the patch (and the explanation
> of the "empty-autho auto-detction" above) is about the third
> possiblity of the same variable and take it back.
Agreed - the existing description is pretty opaque about what values it
actually takes. Should I add another patch to this series to spell out
the three values explicitly? How about something like this:
http.emptyAuth::
Attempt authentication without seeking a username or
password. This can be used to attempt GSS-Negotiate
authentication without specifying a username in the URL,
as libcurl normally requires a username for
authentication. Possible values are:
+
--
* `auto` (default) - Send empty credentials only if the server's
401 response advertises an authentication mechanism that
requires them (such as GSS-Negotiate); otherwise fall back to
prompting via the credential helper.
* `true` - Always send empty credentials on the very first
request, before receiving any 401 response from the server.
* `false` - Never send empty credentials. Mechanisms that
require empty credentials, such as GSS-Negotiate, will not
work.
--
Does that read better?
Thanks,
Matthew
^ permalink raw reply
* [PATCH 2/2] maintenance(geometric): do release the `.idx` files before repacking
From: Johannes Schindelin via GitGitGadget @ 2026-04-28 12:52 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2103.git.1777380768.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
As is done for all the other maintenance tasks, let's release the ODB
also before starting the geometric repacking. That way, the `.idx` files
won't be `mmap()`ed when they are to be deleted (which does not work on
Windows because you cannot delete files on that platform as long as they
are kept open by a process).
This regression was introduced by 9bc151850c1c (builtin/maintenance:
introduce "geometric-repack" task, 2025-10-24), but was only noticed
once geometric repacking was made the default in 452b12c2e0fe (builtin/
maintenance: use "geometric" strategy by default, 2026-02-24).
The fix recapitulates my work from df76ee7b77f0 (run-command: offer to
close the object store before running, 2021-09-09) & friends.
To guard against future regressions of this kind, add a check to
`run_and_verify_geometric_pack()` in `t7900` that detects orphaned
`.idx` files left behind after repacking. Contrary to interactive
calls, the `git maintenance` call in that test case would _not_ block on
Windows, asking whether to retry deleting that file, which is the reason
why this bug was not caught earlier.
Furthermore, since the default behavior of `DeleteFileW()` was changed
at some point between Windows 10 Build 17134.1304 and Build 18363.657
to use POSIX semantics (see https://stackoverflow.com/a/60512798),
the added orphaned-`.idx` check would be insufficient to catch this
regression on modern Windows without emulating legacy delete semantics
via `GIT_TEST_LEGACY_DELETE=1`.
This fixes https://github.com/git-for-windows/git/issues/6210.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/gc.c | 1 +
t/t7900-maintenance.sh | 22 +++++++++++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 3a71e314c9..84a66d3240 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1590,6 +1590,7 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
pack_geometry_split(&geometry);
child.git_cmd = 1;
+ child.odb_to_close = the_repository->objects;
strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
if (geometry.split < geometry.pack_nr)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4700beacc1..f497f51b23 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -532,7 +532,16 @@ run_and_verify_geometric_pack () {
# And verify that there are no loose objects anymore.
git count-objects -v >count &&
- test_grep '^count: 0$' count
+ test_grep '^count: 0$' count &&
+
+ # Verify that no orphaned .idx files were left behind. On
+ # Windows, a missing odb_to_close causes the parent to hold
+ # mmap handles on .idx files, silently preventing their
+ # deletion by the child git-repack process.
+ ls .git/objects/pack/pack-*.idx .git/objects/pack/pack-*.pack |
+ sed "s/\.pack$/.idx/" |
+ sort | uniq -u >orphaned-idx &&
+ test_must_be_empty orphaned-idx
}
test_expect_success 'geometric repacking task' '
@@ -580,8 +589,19 @@ test_expect_success 'geometric repacking task' '
# And these two small packs should now be merged via the
# geometric repack. The large packfile should remain intact.
+ cp -R .git/objects .git/objects.save &&
run_and_verify_geometric_pack 2 &&
+ # On Windows, verify the same with legacy delete semantics
+ # that reject deletion of mmap-held .idx files.
+ if test_have_prereq MINGW
+ then
+ rm -rf .git/objects &&
+ mv .git/objects.save .git/objects &&
+ test_env GIT_TEST_LEGACY_DELETE=1 \
+ run_and_verify_geometric_pack 2
+ fi &&
+
# If we now add two more objects and repack twice we should
# then see another all-into-one repack. This time around
# though, as we have unreachable objects, we should also see a
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics
From: Johannes Schindelin via GitGitGadget @ 2026-04-28 12:52 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2103.git.1777380768.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
At some point between Windows 10 Build 17134.1304 and Build 18363.657,
the default behavior of `DeleteFileW()` was changed to use POSIX
semantics (https://stackoverflow.com/a/60512798). Under those semantics,
a file can be deleted even when another process holds an active
`MapViewOfFile` view on it: the directory entry is removed immediately,
but the underlying data persists until the last handle is closed.
On older Windows versions (and Windows 10 builds before that change),
`DeleteFileW()` uses legacy semantics where deletion fails outright if
any process holds a file mapping.
To allow testing code paths that depend on the legacy behavior, introduce
a `GIT_TEST_LEGACY_DELETE` environment variable. When set, `mingw_unlink()`
uses `SetFileInformationByHandle()` with `FileDispositionInfo` (the
non-POSIX variant) instead of `DeleteFileW()`, forcing legacy delete
semantics regardless of the Windows version.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 2023c16db6..04f9aa3922 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -449,20 +449,63 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
return wbuf;
}
+/*
+ * Use SetFileInformationByHandle(FileDispositionInfo) to force legacy
+ * (non-POSIX) delete semantics. On Windows 11, DeleteFileW() uses POSIX
+ * delete semantics internally, allowing deletion even with active
+ * MapViewOfFile views. This helper simulates Windows 10 behavior where
+ * deletion fails if a file mapping exists.
+ *
+ * Returns nonzero on success (like DeleteFileW), 0 on failure.
+ */
+static int legacy_delete_file(const wchar_t *wpathname)
+{
+ FILE_DISPOSITION_INFO fdi = { TRUE };
+ DWORD gle;
+ HANDLE h = CreateFileW(wpathname, DELETE,
+ FILE_SHARE_READ | FILE_SHARE_WRITE |
+ FILE_SHARE_DELETE,
+ NULL, OPEN_EXISTING,
+ FILE_FLAG_OPEN_REPARSE_POINT, NULL);
+ if (h == INVALID_HANDLE_VALUE)
+ return 0;
+
+ if (SetFileInformationByHandle(h, FileDispositionInfo,
+ &fdi, sizeof(fdi))) {
+ CloseHandle(h);
+ return 1;
+ }
+ gle = GetLastError();
+ CloseHandle(h);
+ SetLastError(gle);
+ return 0;
+}
+
+static int try_delete_file(const wchar_t *wpathname, int use_legacy)
+{
+ if (use_legacy)
+ return legacy_delete_file(wpathname);
+ return DeleteFileW(wpathname);
+}
+
int mingw_unlink(const char *pathname, int handle_in_use_error)
{
+ static int use_legacy_delete = -1;
int tries = 0;
wchar_t wpathname[MAX_PATH];
if (xutftowcs_path(wpathname, pathname) < 0)
return -1;
- if (DeleteFileW(wpathname))
+ if (use_legacy_delete < 0)
+ use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
+
+ if (try_delete_file(wpathname, use_legacy_delete))
return 0;
do {
/* read-only files cannot be removed */
_wchmod(wpathname, 0666);
- if (!_wunlink(wpathname))
+ if (try_delete_file(wpathname, use_legacy_delete))
return 0;
if (!is_file_in_use_error(GetLastError()))
break;
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10
From: Johannes Schindelin via GitGitGadget @ 2026-04-28 12:52 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
On Windows, maintenance_task_geometric_repack() opens pack index files via
pack_geometry_init() (which mmap()s the .idx files), then spawns git repack
as a child process without setting child.odb_to_close. The parent's mmap()s
prevent the child from deleting old .idx files.
On Windows 10 builds before the POSIX delete semantics change (between Build
17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this
results in Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should
I try again? during fetch-triggered auto-maintenance with the geometric
strategy.
The fix adds the missing child.odb_to_close = the_repository->objects line,
matching all other maintenance tasks.
The first commit introduces a GIT_TEST_LEGACY_DELETE environment variable to
simulate legacy (pre-POSIX) delete semantics on modern Windows, so the
regression test can verify the fix even on Windows 11.
This fixes https://github.com/git-for-windows/git/issues/6210.
Johannes Schindelin (2):
mingw: optionally use legacy (non-POSIX) delete semantics
maintenance(geometric): do release the `.idx` files before repacking
builtin/gc.c | 1 +
compat/mingw.c | 47 ++++++++++++++++++++++++++++++++++++++++--
t/t7900-maintenance.sh | 22 +++++++++++++++++++-
3 files changed, 67 insertions(+), 3 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2103%2Fdscho%2Favoid-deadlocks-in-geometric-repacking-on-windows-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2103/dscho/avoid-deadlocks-in-geometric-repacking-on-windows-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2103
--
gitgitgadget
^ permalink raw reply
* Re: Git generated tarballs and Debian
From: Theodore Tso @ 2026-04-28 11:50 UTC (permalink / raw)
To: brian m. carlson, Simon Richter, git, Ian Jackson
In-Reply-To: <afCLFJX86yEPKKfk@fruit.crustytoothpaste.net>
On Tue, Apr 28, 2026 at 10:25:24AM +0000, brian m. carlson wrote:
>
> I'll just note that we don't make any guarantees that `git archive`
> produces identical output across versions. Incorrectly making that
> assumption broke kernel.org when we changed the format in the past.
>
> Also, if you use `export-subst`, then it's possible to emit short object
> IDs, which can differ in length depending on how many objects are in the
> repository. It's also possible to use zlib or pigz instead of gzip to
> produce tarballs, in which case the compressed data will also differ.
This is what I've been using to try get reproducible tarballs for
e2fprogs:
git archive --prefix=e2fsprogs-${ver}/ ${commit} | gzip -9n > $fn
,,, where $commit is a signed git tag.
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?
- Ted
P.S. Although I use pristine-tar in Debian because I didn't want to
count on git-archive being reproducible. But it would be lovely if I
could make that guarantee starting on a particular git version.
^ permalink raw reply
* Re: Git generated tarballs and Debian
From: Simon Richter @ 2026-04-28 11:32 UTC (permalink / raw)
To: brian m. carlson, git, Ian Jackson
In-Reply-To: <afCLFJX86yEPKKfk@fruit.crustytoothpaste.net>
Hi,
On 4/28/26 7:25 PM, brian m. carlson wrote:
> I'll just note that we don't make any guarantees that `git archive`
> produces identical output across versions. Incorrectly making that
> assumption broke kernel.org when we changed the format in the past.
Exactly -- that's why I read the tarball and calculate the checksum of
the corresponding tree object, but we have a few cases where we need
extra information that isn't in the archive, and I'm wondering where to
put that extra information: inside the archive itself, or into an extra
file.
> Also, if you use `export-subst`, then it's possible to emit short object
> IDs, which can differ in length depending on how many objects are in the
> repository. It's also possible to use zlib or pigz instead of gzip to
> produce tarballs, in which case the compressed data will also differ.
export-subst breaks verification completely as soon as a blob changes.
Compression isn't an issue, because we're comparing tree checksums.
Simon
^ permalink raw reply
* Re: Git generated tarballs and Debian
From: brian m. carlson @ 2026-04-28 10:25 UTC (permalink / raw)
To: Simon Richter; +Cc: git, Ian Jackson
In-Reply-To: <9030b26d-02ed-4452-b212-a69a4ff21e2d@hogyros.de>
[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]
On 2026-04-28 at 08:40:05, Simon Richter wrote:
> Hi,
>
> in Debian, we're shipping "original" tarballs for each software package, and
> the Debian specific changes in a separate file.
>
> Historically, this users could do a bitwise comparison of the original
> tarball and the one in Debian to verify that these were unchanged.
>
> With git, some authors have stopped releasing official tarballs, so we're
> using git-archive a lot -- but this is reproducible only by accident. GitHub
> also prepares some release tarballs that may or not be bitwise identical to
> what git archive produces.
I'll just note that we don't make any guarantees that `git archive`
produces identical output across versions. Incorrectly making that
assumption broke kernel.org when we changed the format in the past.
Also, if you use `export-subst`, then it's possible to emit short object
IDs, which can differ in length depending on how many objects are in the
repository. It's also possible to use zlib or pigz instead of gzip to
produce tarballs, in which case the compressed data will also differ.
I had intended to create and emit a standard, reproducible format for
`git archive`, but never got around to finishing that. Perhaps I'll try
to pick it up at some point; I expect it will be easier to implement now
that we have Rust support in the tree.
When I was one of the maintainer of Git LFS, we intentionally produced
source tarballs specifically to emit bit-for-bit identical artifacts.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]
^ permalink raw reply
* Re: [PATCH v15 0/5] checkout: 'autostash' for branch switching
From: Phillip Wood @ 2026-04-28 9:35 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <pull.2234.v15.git.git.1777065012.gitgitgadget@gmail.com>
Hi Harald
On 24/04/2026 22:10, Harald Nordgren via GitGitGadget wrote:
> * When the initial merge fails, the caller now knows for sure it was a
> conflict (not some unrelated error) before kicking off the autostash.
>
> * If the second attempt also fails, the user now actually sees why.
> Previously errors like "would be overwritten by checkout" were being
> swallowed.
>
> * The stash is reapplied right after the second attempt, so it won't get
> left behind if something else goes wrong later in the checkout.
>
> * A couple of clean-up steps only run when we actually stashed, so regular
> checkouts aren't paying for work they don't need.
>
> * Added a small header "The following paths have local changes:" before the
> list of changed files, so it doesn't look like part of the conflict
> advice above it.
>
> * Reworded the conflict advice into a single paragraph. Feels less shouty
> and doesn't say "local changes" three times.
>
> * Trimmed and consolidated tests in t7201. A bunch of the old ones couldn't
> actually fail, or were really testing other things; the conflict-path
> tests are now a single test that checks the stash entry directly.
Thanks for the summary, that all looks good. I've left some comments on
Patches 1 and 3 but they can be future work and don't need to hold this
series up. There are a few small issues that need addressing on Patch 5
but hopefully the next iteration will be the last.
Thanks
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 | 58 +++++------
> Documentation/git-stash.adoc | 11 ++-
> Documentation/git-switch.adoc | 33 +++----
> builtin/checkout.c | 165 +++++++++++++++-----------------
> 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 | 61 +++++++++++-
> t/t7600-merge.sh | 3 +-
> xdiff-interface.c | 12 +++
> xdiff-interface.h | 1 +
> xdiff/xmerge.c | 6 +-
> 16 files changed, 335 insertions(+), 177 deletions(-)
>
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2234%2FHaraldNordgren%2Fcheckout_autostash-v15
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2234/HaraldNordgren/checkout_autostash-v15
> Pull-Request: https://github.com/git/git/pull/2234
>
> Range-diff vs v14:
>
> 1: e18c25599a = 1: aba8e6a9dc stash: add --label-ours, --label-theirs, --label-base for apply
> 2: ce29b10264 = 2: 89e0bfa803 sequencer: allow create_autostash to run silently
> 3: 73051d1762 = 3: a428ce7328 sequencer: teach autostash apply to take optional conflict marker labels
> 4: 191058d8e3 = 4: f358424085 checkout: rollback lock on early returns in merge_working_tree
> 5: 86f33df1eb ! 5: 96b14db827 checkout -m: autostash when switching branches
> @@ builtin/checkout.c
> #include "setup.h"
> #include "submodule.h"
> #include "symlinks.h"
> +@@ builtin/checkout.c: struct checkout_opts {
> + .auto_advance = 1, \
> + }
> +
> ++#define MERGE_WORKING_TREE_UNPACK_FAILED (-2)
> ++
> + struct branch_info {
> + 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)
> +
> + static void init_topts(struct unpack_trees_options *topts, int merge,
> + int show_progress, int overwrite_ignore,
> +- struct commit *old_commit)
> ++ struct commit *old_commit, bool show_unpack_errors)
> + {
> + memset(topts, 0, sizeof(*topts));
> + topts->head_idx = -1;
> +@@ builtin/checkout.c: static void init_topts(struct unpack_trees_options *topts, int merge,
> + topts->initial_checkout = is_index_unborn(the_repository->index);
> + topts->update = 1;
> + topts->merge = 1;
> +- topts->quiet = merge && old_commit;
> ++ topts->quiet = merge && old_commit && !show_unpack_errors;
> + 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, int merge,
> + 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)
> + {
> + 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,
> +- opts->overwrite_ignore, old_branch_info->commit);
> ++ opts->overwrite_ignore, old_branch_info->commit,
> ++ show_unpack_errors);
> + init_checkout_metadata(&topts.meta, new_branch_info->refname,
> + new_branch_info->commit ?
> + &new_branch_info->commit->object.oid :
> @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
> ret = unpack_trees(2, trees, &topts);
> clear_unpack_trees_porcelain(&topts);
> @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *op
> - return ret;
> - }
> + rollback_lock_file(&lock_file);
> -+ return ret;
> ++ return MERGE_WORKING_TREE_UNPACK_FAILED;
> }
> }
>
> @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
> do_merge = 0;
> }
>
> -+ if (old_branch_info.name)
> ++ if (old_branch_info.name) {
> + stash_label_base = old_branch_info.name;
> -+ else if (old_branch_info.commit) {
> ++ } else if (old_branch_info.commit) {
> + strbuf_add_unique_abbrev(&old_commit_shortname,
> + &old_branch_info.commit->object.oid,
> + DEFAULT_ABBREV);
> @@ 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);
> -+ if (ret == -1 && opts->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);
> ++ if (ret == MERGE_WORKING_TREE_UNPACK_FAILED && opts->merge) {
> + strbuf_addf(&autostash_msg,
> + "autostash while switching to '%s'",
> + new_branch_info->name);
> @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
> + "CHECKOUT_AUTOSTASH_HEAD",
> + autostash_msg.buf, true);
> + created_autostash = 1;
> -+ 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, true);
> ++ }
> ++ 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) {
> -+ if (created_autostash)
> -+ apply_autostash_ref(the_repository,
> -+ "CHECKOUT_AUTOSTASH_HEAD",
> -+ new_branch_info->name,
> -+ "local",
> -+ stash_label_base,
> -+ autostash_msg.buf);
> branch_info_release(&old_branch_info);
> - return ret;
> + strbuf_release(&old_commit_shortname);
> @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
>
> update_refs_for_switch(opts, &old_branch_info, new_branch_info);
>
> -+ 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);
> ++ if (created_autostash) {
> ++ discard_index(the_repository->index);
> ++ if (repo_read_index(the_repository) < 0)
> ++ die(_("index file corrupt"));
> ++
> ++ if (!opts->quiet && new_branch_info->commit) {
> ++ printf(_("The following paths have local changes:\n"));
> ++ show_local_changes(&new_branch_info->commit->object,
> ++ &opts->diff_options);
> ++ }
> + }
> -+ apply_autostash_ref(the_repository, "CHECKOUT_AUTOSTASH_HEAD",
> -+ new_branch_info->name, "local",
> -+ stash_label_base,
> -+ autostash_msg.buf);
> -+
> -+ discard_index(the_repository->index);
> -+ if (repo_read_index(the_repository) < 0)
> -+ die(_("index file corrupt"));
> -+
> -+ if (created_autostash && !opts->quiet && new_branch_info->commit)
> -+ show_local_changes(&new_branch_info->commit->object,
> -+ &opts->diff_options);
> +
> ret = post_checkout_hook(old_branch_info.commit, new_branch_info->commit, 1);
> branch_info_release(&old_branch_info);
> @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
> ret = error(_("cannot store %s"), stash_oid);
> + else if (attempt_apply)
> + fprintf(stderr,
> -+ _("Your local changes are stashed, however, applying it to carry\n"
> -+ "forward your local changes resulted in conflicts:\n"
> -+ "\n"
> -+ " - You can try resolving them now. If you resolved them\n"
> -+ " successfully, discard the stash entry with \"git stash drop\".\n"
> -+ "\n"
> -+ " - Alternatively you can \"git reset --hard\" if you do not want\n"
> -+ " to deal with them right now, and later \"git stash pop\" to\n"
> -+ " recover your local changes.\n"));
> ++ _("Your local changes are stashed, however applying them\n"
> ++ "resulted in conflicts. You can either resolve the conflicts\n"
> ++ "and then discard the stash with \"git stash drop\", or, if you\n"
> ++ "do not want to resolve them now, run \"git reset --hard\" and\n"
> ++ "apply the local changes later by running \"git stash pop\".\n"));
> else
> fprintf(stderr,
> - _("%s\n"
> @@ t/t3420-rebase-autostash.sh: create_expected_failure_apply () {
> - Applying autostash resulted in conflicts.
> - Your changes are safe in the stash.
> - You can run "git stash pop" or "git stash drop" at any time.
> -+ Your local changes are stashed, however, applying it to carry
> -+ forward your local changes resulted in conflicts:
> -+
> -+ - 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.
> ++ 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".
> EOF
> }
>
> @@ t/t3420-rebase-autostash.sh: create_expected_failure_apply () {
> - Applying autostash resulted in conflicts.
> - Your changes are safe in the stash.
> - You can run "git stash pop" or "git stash drop" at any time.
> -+ Your local changes are stashed, however, applying it to carry
> -+ forward your local changes resulted in conflicts:
> -+
> -+ - 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.
> ++ 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".
> Successfully rebased and updated refs/heads/rebased-feature-branch.
> EOF
> }
>
> ## t/t7201-co.sh ##
> +@@ 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 &&
> + 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_cmp expect two
> '
>
> -+test_expect_success 'checkout --merge --conflict=zdiff3 <branch>' '
> -+ git checkout -f main &&
> -+ git reset --hard &&
> -+ git clean -f &&
> -+
> -+ fill a b X d e >two &&
> -+ git checkout --merge --conflict=zdiff3 simple &&
> -+
> -+ cat <<-EOF >expect &&
> -+ a
> -+ <<<<<<< simple
> -+ c
> -+ ||||||| main
> -+ b
> -+ c
> -+ d
> -+ =======
> -+ b
> -+ X
> -+ d
> -+ >>>>>>> local
> -+ e
> -+ EOF
> -+ test_cmp expect two
> -+'
> -+
> -+test_expect_success 'checkout -m respects merge.conflictStyle config' '
> -+ git checkout -f main &&
> -+ git reset --hard &&
> -+ git clean -f &&
> -+
> -+ test_config merge.conflictStyle diff3 &&
> -+ fill b d >two &&
> -+ git checkout -m simple &&
> -+
> -+ cat <<-EOF >expect &&
> -+ <<<<<<< simple
> -+ a
> -+ c
> -+ e
> -+ ||||||| main
> -+ a
> -+ b
> -+ c
> -+ d
> -+ e
> -+ =======
> -+ b
> -+ d
> -+ >>>>>>> local
> -+ EOF
> -+ test_cmp expect two
> -+'
> -+
> -+test_expect_success 'checkout -m skips stash when no conflict' '
> -+ git checkout -f main &&
> -+ git clean -f &&
> -+
> -+ fill 0 x y z >same &&
> -+ git stash list >stash-before &&
> -+ git checkout -m side >actual 2>&1 &&
> -+ test_grep ! "Created autostash" actual &&
> -+ git stash list >stash-after &&
> -+ test_cmp stash-before stash-after &&
> -+ fill 0 x y z >expect &&
> -+ test_cmp expect same
> -+'
> -+
> -+test_expect_success 'checkout -m skips stash with non-conflicting dirty index' '
> -+ git checkout -f main &&
> -+ git clean -f &&
> -+
> -+ fill 0 x y z >same &&
> -+ git add same &&
> -+ git checkout -m side >actual 2>&1 &&
> -+ test_grep ! "Created autostash" actual &&
> -+ fill 0 x y z >expect &&
> -+ test_cmp expect same
> -+'
> -+
> -+test_expect_success 'checkout -m stashes and applies on conflicting changes' '
> -+ git checkout -f main &&
> -+ git clean -f &&
> -+
> -+ fill 1 2 3 4 5 6 7 >one &&
> -+ git checkout -m side >actual 2>&1 &&
> -+ test_grep ! "Created autostash" actual &&
> -+ test_grep "Applied autostash" actual &&
> -+ fill 1 2 3 4 5 6 7 >expect &&
> -+ test_cmp expect one
> -+'
> -+
> +test_expect_success 'checkout -m with mixed staged and unstaged changes' '
> + git checkout -f main &&
> + git clean -f &&
> @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
> + git add same &&
> + fill 1 2 3 4 5 6 7 >one &&
> + git checkout -m side >actual 2>&1 &&
> -+ test_grep ! "Created autostash" actual &&
> + test_grep "Applied autostash" actual &&
> + fill 0 x y z >expect &&
> + test_cmp expect same &&
> @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
> + test_cmp expect one
> +'
> +
> -+test_expect_success 'checkout -m stashes on truly conflicting changes' '
> ++test_expect_success 'checkout -m creates a recoverable stash on conflict' '
> + git checkout -f main &&
> + git clean -f &&
> +
> @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
> + test_must_fail git checkout side 2>stderr &&
> + test_grep "Your local changes" stderr &&
> + git checkout -m side >actual 2>&1 &&
> -+ test_grep ! "Created autostash" actual &&
> + test_grep "resulted in conflicts" actual &&
> + 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 &&
> ++ sed /^index/d actual >actual.trimmed &&
> ++ cat >expect <<-EOF &&
> ++ On main: autostash while switching to ${SQ}side${SQ}
> ++
> ++ diff --git a/one b/one
> ++ --- a/one
> ++ +++ b/one
> ++ @@ -3,6 +3,3 @@
> ++ 3
> ++ 4
> ++ 5
> ++ -6
> ++ -7
> ++ -8
> ++ EOF
> ++ test_cmp expect actual.trimmed &&
> + git stash drop &&
> + git reset --hard
> +'
> +
> -+test_expect_success 'checkout -m produces usable stash on conflict' '
> -+ git checkout -f main &&
> -+ git clean -f &&
> -+
> ++test_expect_success 'checkout -m which would overwrite untracked file' '
> ++ git checkout -f --detach main &&
> ++ test_commit another-file &&
> ++ git checkout HEAD^ &&
> ++ >another-file.t &&
> + fill 1 2 3 4 5 >one &&
> -+ git checkout -m side >actual 2>&1 &&
> -+ test_grep "recover your local changes" actual &&
> -+ git checkout -f main &&
> -+ git stash pop &&
> -+ fill 1 2 3 4 5 >expect &&
> -+ test_cmp expect one
> -+'
> -+
> -+test_expect_success 'checkout -m autostash message includes target branch' '
> -+ git checkout -f main &&
> -+ git clean -f &&
> -+
> -+ fill 1 2 3 4 5 >one &&
> -+ git checkout -m side >actual 2>&1 &&
> -+ git stash list >stash-list &&
> -+ test_grep "autostash while switching to .side." stash-list &&
> -+ git stash drop &&
> -+ git checkout -f main &&
> -+ git reset --hard
> -+'
> -+
> -+test_expect_success 'checkout -m stashes on staged conflicting changes' '
> -+ git checkout -f main &&
> -+ git clean -f &&
> -+
> -+ fill 1 2 3 4 5 >one &&
> -+ git add one &&
> -+ git checkout -m side >actual 2>&1 &&
> -+ test_grep ! "Created autostash" actual &&
> -+ test_grep "resulted in conflicts" actual &&
> -+ test_grep "git stash drop" actual &&
> -+ git stash drop &&
> -+ git reset --hard
> -+'
> -+
> -+test_expect_success 'checkout -m applies stash cleanly with non-overlapping changes in same file' '
> -+ git checkout -f main &&
> -+ git reset --hard &&
> -+ git clean -f &&
> -+
> -+ git checkout -b nonoverlap_base &&
> -+ fill a b c d >file &&
> -+ git add file &&
> -+ git commit -m "add file" &&
> -+
> -+ git checkout -b nonoverlap_child &&
> -+ fill a b c INSERTED d >file &&
> -+ git commit -a -m "insert line near end of file" &&
> -+
> -+ fill DIRTY a b c INSERTED d >file &&
> -+
> -+ git stash list >stash-before &&
> -+ git checkout -m nonoverlap_base 2>stderr &&
> -+ test_grep "Applied autostash" stderr &&
> -+ test_grep ! "resulted in conflicts" stderr &&
> -+
> -+ git stash list >stash-after &&
> -+ test_cmp stash-before stash-after &&
> -+
> -+ fill DIRTY a b c d >expect &&
> -+ test_cmp expect file &&
> -+
> -+ git checkout -f main &&
> -+ git branch -D nonoverlap_base &&
> -+ git branch -D nonoverlap_child
> -+'
> -+
> -+test_expect_success 'checkout -m -b skips stash with dirty tree' '
> -+ git checkout -f main &&
> -+ git clean -f &&
> -+
> -+ fill 0 x y z >same &&
> -+ git checkout -m -b newbranch >actual 2>&1 &&
> -+ test_grep ! "Created autostash" actual &&
> -+ fill 0 x y z >expect &&
> -+ test_cmp expect same &&
> -+ git checkout main &&
> -+ git branch -D newbranch
> ++ test_must_fail git checkout -m @{-1} 2>err &&
> ++ test_grep "would be overwritten by checkout" err &&
> ++ test_grep "another-file.t" err
> +'
> +
> test_expect_success 'switch to another branch while carrying a deletion' '
> @@ t/t7600-merge.sh: test_expect_success 'merge with conflicted --autostash changes
> test_when_finished "test_might_fail git stash drop" &&
> git merge --autostash c3 2>err &&
> - test_grep "Applying autostash resulted in conflicts." err &&
> -+ test_grep "your local changes resulted in conflicts" err &&
> ++ test_grep "applying them" err &&
> ++ test_grep "resulted in conflicts" err &&
> git show HEAD:file >merge-result &&
> test_cmp result.1-9 merge-result &&
> git stash show -p >actual &&
>
^ permalink raw reply
* Re: [PATCH v15 5/5] checkout -m: autostash when switching branches
From: Phillip Wood @ 2026-04-28 9:35 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <96b14db827e51966aceb6513f7d16ed526274ec2.1777065012.git.gitgitgadget@gmail.com>
Hi Harald
On 24/04/2026 22:10, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> When switching branches with "git checkout -m", the attempted merge
> of local modifications may cause conflicts with the changes made on
> the other branch, which the user may not want to (or may not be able
> to) resolve right now. Because there is no easy way to recover from
> this situation, we discouraged users from using "checkout -m" unless
> they are certain their changes are trivial and within their ability
> to resolve conflicts.
>
> Teach the -m flow to create a temporary stash before switching and
> reapply it after. On success, the stash is silently applied and
> the list of locally modified paths is shown, same as a successful
> "git checkout" without "-m".
>
> If reapplying causes conflicts, the stash is kept and the user is
> told they can resolve and run "git stash drop", or run "git reset
> --hard" and later "git stash pop" to recover their changes.
This is looking good, there are just a few small issues. Hopefully the
next iteration will be the last.
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> Documentation/git-checkout.adoc | 58 ++++++------
> Documentation/git-switch.adoc | 33 +++----
> builtin/checkout.c | 160 ++++++++++++++------------------
> sequencer.c | 14 ++-
> t/t3420-rebase-autostash.sh | 16 ++--
> t/t7201-co.sh | 61 +++++++++++-
> t/t7600-merge.sh | 3 +-
> xdiff-interface.c | 12 +++
> xdiff-interface.h | 1 +
> 9 files changed, 211 insertions(+), 147 deletions(-)
>
> diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc
> index 43ccf47cf6..70dd211ee3 100644
> --- a/Documentation/git-checkout.adoc
> +++ b/Documentation/git-checkout.adoc
> @@ -251,20 +251,19 @@ working tree, by copying them from elsewhere, extracting a tarball, etc.
> are different between the current branch and the branch to
> which you are switching, the command refuses to switch
> branches in order to preserve your modifications in context.
> - However, with this option, a three-way merge between the current
> - branch, your working tree contents, and the new branch
> - is done, and you will be on the new branch.
> -+
> -When a merge conflict happens, the index entries for conflicting
> -paths are left unmerged, and you need to resolve the conflicts
> -and mark the resolved paths with `git add` (or `git rm` if the merge
> -should result in deletion of the path).
> + With this option, the conflicting local changes are
> + automatically stashed before the switch and reapplied
> + afterwards. If the local changes do not overlap with the
> + differences between branches, the switch proceeds without
> + stashing. If reapplying the stash results in conflicts, the
> + entry is saved to the stash list. Resolve the conflicts
> + and run `git stash drop` when done, or clear the working
> + tree (e.g. with `git reset --hard`) before running `git stash
> + pop` later to re-apply your changes.
> +
> When checking out paths from the index, this option lets you recreate
> the conflicted merge in the specified paths. This option cannot be
> used when checking out paths from a tree-ish.
> -+
> -When switching branches with `--merge`, staged changes may be lost.
>
> `--conflict=<style>`::
> The same as `--merge` option above, but changes the way the
> @@ -578,39 +577,44 @@ $ git checkout mytopic
> error: You have local changes to 'frotz'; not switching branches.
> ------------
>
> -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
s/would/will/ or s/would carry/carries/
> +changes to the new branch:
>
> ------------
> $ git checkout -m mytopic
> -Auto-merging frotz
> +Switched to branch 'mytopic'
> ------------
>
> -After this three-way merge, the local modifications are _not_
> +After the switch, the local modifications are reapplied and are _not_
> registered in your index file,
We don't test this but we do test that "git stash apply" does not update
the index so we should be fine.
> so `git diff` would show you what
> changes you made since the tip of the new branch.
>
> === 3. Merge conflict
>
> -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,
It is the changes in the files overlapping that causes the merge
conflict, not the files overlapping
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, a stash entry is saved
s/a/the/
> +and made available in `git stash list`:
I'd drop this line and say instead "a message is printed"
> ------------
> $ git checkout -m mytopic
> -Auto-merging frotz
> -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.
This needs updating to match the new conflict advice.
> ------------
> -$ 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`.
Is this documentation, or program output?
If you've not done so already it would be well worth checking the
generated git-checkout.html and the man page
> CONFIGURATION
> -------------
> diff --git a/Documentation/git-switch.adoc b/Documentation/git-switch.adoc
> index 87707e9265..ee58a4d0fd 100644
> --- a/Documentation/git-switch.adoc
> +++ b/Documentation/git-switch.adoc
> @@ -123,18 +123,19 @@ variable.
>
> `-m`::
> `--merge`::
> - If you have local modifications to one or more files that are
> - different between the current branch and the branch to which
> - you are switching, the command refuses to switch branches in
> - order to preserve your modifications in context. However,
> - with this option, a three-way merge between the current
> - branch, your working tree contents, and the new branch is
> - done, and you will be on the new branch.
> -+
> -When a merge conflict happens, the index entries for conflicting
> -paths are left unmerged, and you need to resolve the conflicts
> -and mark the resolved paths with `git add` (or `git rm` if the merge
> -should result in deletion of the path).
> + If you have local modifications to one or more files that
> + are different between the current branch and the branch to
> + which you are switching, the command normally refuses to
> + switch branches in order to preserve your modifications in
> + context. However, with this option, the conflicting local
> + changes are automatically stashed before the switch and
> + reapplied afterwards. If the local changes do not overlap
> + with the differences between branches, the switch proceeds
> + without stashing. If reapplying the stash results in
> + conflicts, the entry is saved to the stash list. Resolve
> + the conflicts and run `git stash drop` when done, or clear
> + the working tree (e.g. with `git reset --hard`) before
> + running `git stash pop` later to re-apply your changes.
>
> `--conflict=<style>`::
> The same as `--merge` option above, but changes the way the
> @@ -217,15 +218,15 @@ $ git switch mytopic
> error: You have local changes to 'frotz'; not switching branches.
> ------------
>
> -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
> +changes to the new branch:
Same comment as for git-checkout.adoc
> ------------
> $ git switch -m mytopic
> -Auto-merging frotz
> +Switched to branch 'mytopic'
Don't we show the modified files as well now?
> ------------
>
> -After this three-way merge, the local modifications are _not_
> +After the switch, the local modifications are reapplied and are _not_
> registered in your index file, so `git diff` would show you what
> changes you made since the tip of the new branch.
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> @@ -755,7 +757,7 @@ static void setup_branch_path(struct branch_info *branch)
>
> static void init_topts(struct unpack_trees_options *topts, int merge,
> int show_progress, int overwrite_ignore,
> - struct commit *old_commit)
> + struct commit *old_commit, bool show_unpack_errors)
As this function only sets up the flags for unpack_trees() I think we
could call this "quiet" or "show_errors"
> {
> memset(topts, 0, sizeof(*topts));
> topts->head_idx = -1;
> @@ -767,7 +769,7 @@ static void init_topts(struct unpack_trees_options *topts, int merge,
> topts->initial_checkout = is_index_unborn(the_repository->index);
> topts->update = 1;
> topts->merge = 1;
> - topts->quiet = merge && old_commit;
> + topts->quiet = merge && old_commit && !show_unpack_errors;
We've added a function parameter for this option but then we ignore it
unless "merge" and "old_commit" are true which is confusing. The reason
we used to check those was to set "quiet" automatically but we can't do
that now, so why not just use the value the call requested?
> topts->verbose_update = show_progress;
> topts->fn = twoway_merge;
> topts->preserve_ignored = !overwrite_ignore;
> @@ -776,7 +778,8 @@ static void init_topts(struct unpack_trees_options *topts, int merge,
> 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,
This is an "out" parameter, so it would make sense to keep it at the end
of the parameter list.
> + bool show_unpack_errors)
> {
> @@ -853,90 +857,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
> ret = unpack_trees(2, trees, &topts);
> clear_unpack_trees_porcelain(&topts);
> if (ret == -1) {
> [...]
> + rollback_lock_file(&lock_file);
> + return MERGE_WORKING_TREE_UNPACK_FAILED;
We now use a unique return value when unpack_trees() fails - good.
> }
> }
>
> @@ -1181,6 +1103,10 @@ static int switch_branches(const struct checkout_opts *opts,
> struct object_id rev;
> int flag, writeout_error = 0;
> int do_merge = 1;
> + int created_autostash = 0;
> + struct strbuf old_commit_shortname = STRBUF_INIT;
> + struct strbuf autostash_msg = STRBUF_INIT;
> + const char *stash_label_base = NULL;
>
> trace2_cmd_mode("branch");
>
> @@ -1218,11 +1144,49 @@ 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);
> + if (ret == MERGE_WORKING_TREE_UNPACK_FAILED && opts->merge) {
> + strbuf_addf(&autostash_msg,
> + "autostash while switching to '%s'",
In an ideal world we'd create a message that said
On <new branch>: autostash while switching from '<old branch>'
However I as discussed on patch 3, that would require us to be able to
override the branch name when creating a stash as that's where the "On
<branch>: " prefix gets added.
> + new_branch_info->name);
> + create_autostash_ref(the_repository,
> + "CHECKOUT_AUTOSTASH_HEAD",
> + autostash_msg.buf, true);
> + created_autostash = 1;
> + ret = merge_working_tree(opts, &old_branch_info, new_branch_info,
> + &writeout_error, true);
> + }
> + 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);
> + }
We now have a single place where we restore the stashed changes - good
> diff --git a/sequencer.c b/sequencer.c
> index 7c0376d9e4..746f85a442 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4765,15 +4765,19 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
> strvec_push(&store.args, stash_oid);
> if (run_command(&store))
> ret = error(_("cannot store %s"), stash_oid);
> + else if (attempt_apply)
> + fprintf(stderr,
> + _("Your local changes are stashed, however applying them\n"
> + "resulted in conflicts. You can either resolve the conflicts\n"
> + "and then discard the stash with \"git stash drop\", or, if you\n"
> + "do not want to resolve them now, run \"git reset --hard\" and\n"
> + "apply the local changes later by running \"git stash pop\".\n"));
> else
> fprintf(stderr,
> - _("%s\n"
> + _("Autostash exists; creating a new stash entry.\n"
> "Your changes are safe in the stash.\n"
> "You can run \"git stash pop\" or"
> - " \"git stash drop\" at any time.\n"),
> - attempt_apply ?
> - _("Applying autostash resulted in conflicts.") :
> - _("Autostash exists; creating a new stash entry."));
> + " \"git stash drop\" at any time.\n"));
> }
This looks good now
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 9bcf7c0b40..b3293ead8d 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -102,7 +102,7 @@ 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 &&
To create a multi-line file it is clearer to use
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 &&
> @@ -210,6 +210,65 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
> test_cmp expect two
> '
>
> +test_expect_success 'checkout -m with mixed staged and unstaged changes' '
> + git checkout -f main &&
> + git clean -f &&
> +
> + fill 0 x y z >same &&
> + git add same &&
> + fill 1 2 3 4 5 6 7 >one &&
> + git checkout -m side >actual 2>&1 &&
> + test_grep "Applied autostash" actual &&
> + fill 0 x y z >expect &&
> + test_cmp expect same &&
> + fill 1 2 3 4 5 6 7 >expect &&
> + test_cmp expect one
> +'
> +
> +test_expect_success 'checkout -m creates a recoverable stash on conflict' '
> + git checkout -f main &&
> + git clean -f &&
> +
> + fill 1 2 3 4 5 >one &&
> + test_must_fail git checkout side 2>stderr &&
> + test_grep "Your local changes" stderr &&
> + git checkout -m side >actual 2>&1 &&
> + test_grep "resulted in conflicts" actual &&
> + 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 &&
I've realized since I suggested this that we should be checking the
reflog message as well since that's what's shown by "git stash list" so
we need to run
git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual
> + sed /^index/d actual >actual.trimmed &&
> + cat >expect <<-EOF &&
and add
autostash while switching to ${SQ}side${SQ}
> + On main: autostash while switching to ${SQ}side${SQ}
> +
> + diff --git a/one b/one
> + --- a/one
> + +++ b/one
> + @@ -3,6 +3,3 @@
> + 3
> + 4
> + 5
> + -6
> + -7
> + -8
> + EOF
> + test_cmp expect actual.trimmed &&
> + git stash drop &&
> + git reset --hard
> +'
> +
> +test_expect_success 'checkout -m which would overwrite untracked file' '
> + git checkout -f --detach main &&
> + test_commit another-file &&
> + git checkout HEAD^ &&
> + >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
Why the two calls to test_grep, rather than one? Anyway I've realized
since I suggested this test that we also need to check the message only
appears once to prevent a regression where merge_working_tree() calls
unpack_trees() without setting "quiet" the first time it is called. We
can do that by writing an expect file and calling test_cmp(), or by
using "test_line_count = 1 err"
Hopefully the next re-roll with be the final one
Thanks
Phillip
> +'
> +
> test_expect_success 'switch to another branch while carrying a deletion' '
> git checkout -f main &&
> git reset --hard &&
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 9838094b66..f877d9a433 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -914,7 +914,8 @@ test_expect_success 'merge with conflicted --autostash changes' '
> git diff >expect &&
> test_when_finished "test_might_fail git stash drop" &&
> git merge --autostash c3 2>err &&
> - test_grep "Applying autostash resulted in conflicts." err &&
> + test_grep "applying them" err &&
> + test_grep "resulted in conflicts" err &&
> git show HEAD:file >merge-result &&
> test_cmp result.1-9 merge-result &&
> git stash show -p >actual &&
> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index f043330f2a..5ee2b96d0a 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -325,6 +325,18 @@ int parse_conflict_style_name(const char *value)
> return -1;
> }
>
> +const char *conflict_style_name(int style)
> +{
> + switch (style) {
> + case XDL_MERGE_DIFF3:
> + return "diff3";
> + case XDL_MERGE_ZEALOUS_DIFF3:
> + return "zdiff3";
> + default:
> + return "merge";
> + }
> +}
> +
> int git_xmerge_style = -1;
>
> int git_xmerge_config(const char *var, const char *value,
> diff --git a/xdiff-interface.h b/xdiff-interface.h
> index fbc4ceec40..ce54e1c0e0 100644
> --- a/xdiff-interface.h
> +++ b/xdiff-interface.h
> @@ -55,6 +55,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
> void xdiff_clear_find_func(xdemitconf_t *xecfg);
> struct config_context;
> int parse_conflict_style_name(const char *value);
> +const char *conflict_style_name(int style);
> int git_xmerge_config(const char *var, const char *value,
> const struct config_context *ctx, void *cb);
> extern int git_xmerge_style;
^ permalink raw reply
* Re: [PATCH v15 4/5] checkout: rollback lock on early returns in merge_working_tree
From: Phillip Wood @ 2026-04-28 9:33 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <f35842408527a8c69f9936cdb127fb35efe2509c.1777065012.git.gitgitgadget@gmail.com>
Hi Harald
On 24/04/2026 22:10, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> merge_working_tree() acquires the index lock via
> repo_hold_locked_index() but several early return paths exit
> without calling rollback_lock_file(), leaving the lock held.
> While this is currently harmless because the process exits soon
> after, it becomes a problem if the function is ever called more
> than once in the same process.
>
> Add rollback_lock_file() calls to all early return paths.
Thanks for splitting this out, it looks good
Phillip
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> builtin/checkout.c | 29 ++++++++++++++++++++++-------
> 1 file changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e031e61886..c80c62b37b 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -783,8 +783,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
> struct tree *new_tree;
>
> repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
> - if (repo_read_index_preload(the_repository, NULL, 0) < 0)
> + if (repo_read_index_preload(the_repository, NULL, 0) < 0) {
> + rollback_lock_file(&lock_file);
> return error(_("index file corrupt"));
> + }
>
> resolve_undo_clear_index(the_repository->index);
> if (opts->new_orphan_branch && opts->orphan_from_empty_tree) {
> @@ -797,14 +799,18 @@ static int merge_working_tree(const struct checkout_opts *opts,
> } else {
> new_tree = repo_get_commit_tree(the_repository,
> new_branch_info->commit);
> - if (!new_tree)
> + if (!new_tree) {
> + rollback_lock_file(&lock_file);
> return error(_("unable to read tree (%s)"),
> oid_to_hex(&new_branch_info->commit->object.oid));
> + }
> }
> if (opts->discard_changes) {
> ret = reset_tree(new_tree, opts, 1, writeout_error, new_branch_info);
> - if (ret)
> + if (ret) {
> + rollback_lock_file(&lock_file);
> return ret;
> + }
> } else {
> struct tree_desc trees[2];
> struct tree *tree;
> @@ -814,6 +820,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
> refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);
>
> if (unmerged_index(the_repository->index)) {
> + rollback_lock_file(&lock_file);
> error(_("you need to resolve your current index first"));
> return 1;
> }
> @@ -857,15 +864,19 @@ static int merge_working_tree(const struct checkout_opts *opts,
> struct strbuf sb = STRBUF_INIT;
> struct strbuf old_commit_shortname = STRBUF_INIT;
>
> - if (!opts->merge)
> + if (!opts->merge) {
> + rollback_lock_file(&lock_file);
> return 1;
> + }
>
> /*
> * Without old_branch_info->commit, the below is the same as
> * the two-tree unpack we already tried and failed.
> */
> - if (!old_branch_info->commit)
> + if (!old_branch_info->commit) {
> + rollback_lock_file(&lock_file);
> return 1;
> + }
> old_tree = repo_get_commit_tree(the_repository,
> old_branch_info->commit);
>
> @@ -897,8 +908,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
> ret = reset_tree(new_tree,
> opts, 1,
> writeout_error, new_branch_info);
> - if (ret)
> + if (ret) {
> + rollback_lock_file(&lock_file);
> return ret;
> + }
> o.ancestor = old_branch_info->name;
> if (!old_branch_info->name) {
> strbuf_add_unique_abbrev(&old_commit_shortname,
> @@ -920,8 +933,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
> writeout_error, new_branch_info);
> strbuf_release(&o.obuf);
> strbuf_release(&old_commit_shortname);
> - if (ret)
> + if (ret) {
> + rollback_lock_file(&lock_file);
> return ret;
> + }
> }
> }
>
^ permalink raw reply
* Re: [PATCH v15 3/5] sequencer: teach autostash apply to take optional conflict marker labels
From: Phillip Wood @ 2026-04-28 9:33 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <a428ce73281dd288f0c4f44c4f92c03129556b7f.1777065012.git.gitgitgadget@gmail.com>
Hi Harald
On 24/04/2026 22:10, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Add label_ours, label_theirs, label_base, and stash_msg parameters to
> apply_autostash_ref() and the autostash apply machinery so callers can
> pass custom conflict marker labels through to
> "git stash apply --label-ours/--label-theirs/--label-base", as well as
> a custom stash message for "git stash store -m".
I said before that I was confused as to why we need to pass a message
when all we're doing is saving an existing stash and the stash already
has the message we want. As that didn't get any response I've had a more
detailed look. The message here is used in the reflog (which is what
"git stash list" shows) and it appears that the reason for passing a
message when storing the stash is because "git stash store" is not smart
enough to reuse the subject from the stash commit message by default. It
would be nice to improve that rather than having to pass a message here.
"git rebase --autostash" and "git merge --autostash" do not have the
luxury of calling "git stash store" from the same process as "git stash
create" and so to improve the autostash message for those commands we'd
need to generate it twice which seems like an unnecessary complication.
One problem with that approach in the context of this series is that the
stash commit message begins "On <branch we're switching from>: " rather
than "On <branch we're switching to>: " but that means this commit is
just hiding the fact that commit message is wrong when we create the stash.
It may well be that fixing all that turns out to be a lot of work as it
would mean modifying do_create_stash() to allow the branch name to be
overridden and modifying store_stash() to use the commit subject as the
reflog message in which case we should leave that for a future series.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v15 2/5] sequencer: allow create_autostash to run silently
From: Phillip Wood @ 2026-04-28 9:32 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <89e0bfa80326d20d11355ed2e73de0907e5c9e41.1777065012.git.gitgitgadget@gmail.com>
Hi Harald
This looks good
Thanks
Phillip
On 24/04/2026 22:10, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Add a silent parameter to create_autostash_internal and introduce
> create_autostash_ref_silent so that callers can create an autostash
> without printing the "Created autostash" message.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> builtin/merge.c | 6 ++++--
> sequencer.c | 17 +++++++++++------
> sequencer.h | 3 ++-
> 3 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 2cbce56f8d..3ebe190ef1 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1672,7 +1672,8 @@ int cmd_merge(int argc,
> }
>
> if (autostash)
> - create_autostash_ref(the_repository, "MERGE_AUTOSTASH");
> + create_autostash_ref(the_repository, "MERGE_AUTOSTASH",
> + NULL, false);
> if (checkout_fast_forward(the_repository,
> &head_commit->object.oid,
> &commit->object.oid,
> @@ -1764,7 +1765,8 @@ int cmd_merge(int argc,
> die_ff_impossible();
>
> if (autostash)
> - create_autostash_ref(the_repository, "MERGE_AUTOSTASH");
> + create_autostash_ref(the_repository, "MERGE_AUTOSTASH",
> + NULL, false);
>
> /* We are going to make a new commit. */
> git_committer_info(IDENT_STRICT);
> diff --git a/sequencer.c b/sequencer.c
> index b7d8dca47f..ff5258f481 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4657,7 +4657,9 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)
>
> static void create_autostash_internal(struct repository *r,
> const char *path,
> - const char *refname)
> + const char *refname,
> + const char *message,
> + bool silent)
> {
> struct strbuf buf = STRBUF_INIT;
> struct lock_file lock_file = LOCK_INIT;
> @@ -4679,7 +4681,8 @@ static void create_autostash_internal(struct repository *r,
> struct object_id oid;
>
> strvec_pushl(&stash.args,
> - "stash", "create", "autostash", NULL);
> + "stash", "create",
> + message ? message : "autostash", NULL);
> stash.git_cmd = 1;
> stash.no_stdin = 1;
> strbuf_reset(&buf);
> @@ -4702,7 +4705,8 @@ static void create_autostash_internal(struct repository *r,
> &oid, null_oid(the_hash_algo), 0, UPDATE_REFS_DIE_ON_ERR);
> }
>
> - printf(_("Created autostash: %s\n"), buf.buf);
> + if (!silent)
> + printf(_("Created autostash: %s\n"), buf.buf);
> if (reset_head(r, &ropts) < 0)
> die(_("could not reset --hard"));
> discard_index(r->index);
> @@ -4714,12 +4718,13 @@ static void create_autostash_internal(struct repository *r,
>
> void create_autostash(struct repository *r, const char *path)
> {
> - create_autostash_internal(r, path, NULL);
> + create_autostash_internal(r, path, NULL, NULL, false);
> }
>
> -void create_autostash_ref(struct repository *r, const char *refname)
> +void create_autostash_ref(struct repository *r, const char *refname,
> + const char *message, bool silent)
> {
> - create_autostash_internal(r, NULL, refname);
> + create_autostash_internal(r, NULL, refname, message, silent);
> }
>
> static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply)
> diff --git a/sequencer.h b/sequencer.h
> index a6fa670c7c..02d2d9db06 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -229,7 +229,8 @@ void commit_post_rewrite(struct repository *r,
> const struct object_id *new_head);
>
> void create_autostash(struct repository *r, const char *path);
> -void create_autostash_ref(struct repository *r, const char *refname);
> +void create_autostash_ref(struct repository *r, const char *refname,
> + const char *message, bool silent);
> int save_autostash(const char *path);
> int save_autostash_ref(struct repository *r, const char *refname);
> int apply_autostash(const char *path);
^ permalink raw reply
* Re: [PATCH v15 1/5] stash: add --label-ours, --label-theirs, --label-base for apply
From: Phillip Wood @ 2026-04-28 9:32 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Chris Torek, Jeff King, Harald Nordgren
In-Reply-To: <aba8e6a9dc7e1620a46888685bb081f878cc6932.1777065012.git.gitgitgadget@gmail.com>
Hi Harald
On 24/04/2026 22:10, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Allow callers of "git stash apply" to pass custom labels for conflict
> markers instead of the default "Updated upstream" and "Stashed changes".
> Document the new options and add a test.
This all looks good, just one small comment below
> - o.branch1 = "Updated upstream";
> - o.branch2 = "Stashed changes";
> - o.ancestor = "Stash base";
> + o.branch1 = label_ours ? label_ours : "Updated upstream";
> + o.branch2 = label_theirs ? label_theirs : "Stashed changes";
> + o.ancestor = label_base ? label_base : "Stash base";
This uses the existing label which is sensible, but I wonder if "Stash
HEAD" would be a better choice as the merge base is always HEAD commit
that the stash is based on.
We can always change that later
Thanks
Phillip
>
> if (oideq(&info->b_tree, &c_tree))
> o.branch1 = "Version stash was based on";
> @@ -723,11 +725,18 @@ static int apply_stash(int argc, const char **argv, const char *prefix,
> int ret = -1;
> int quiet = 0;
> int index = use_index;
> + const char *label_ours = NULL, *label_theirs = NULL, *label_base = NULL;
> struct stash_info info = STASH_INFO_INIT;
> struct option options[] = {
> OPT__QUIET(&quiet, N_("be quiet, only report errors")),
> OPT_BOOL(0, "index", &index,
> N_("attempt to recreate the index")),
> + OPT_STRING(0, "label-ours", &label_ours, N_("label"),
> + N_("label for the upstream side in conflict markers")),
> + OPT_STRING(0, "label-theirs", &label_theirs, N_("label"),
> + N_("label for the stashed side in conflict markers")),
> + OPT_STRING(0, "label-base", &label_base, N_("label"),
> + N_("label for the base in diff3 conflict markers")),
> OPT_END()
> };
>
> @@ -737,7 +746,8 @@ static int apply_stash(int argc, const char **argv, const char *prefix,
> if (get_stash_info(&info, argc, argv))
> goto cleanup;
>
> - ret = do_apply_stash(prefix, &info, index, quiet);
> + ret = do_apply_stash(prefix, &info, index, quiet,
> + label_ours, label_theirs, label_base);
> cleanup:
> free_stash_info(&info);
> return ret;
> @@ -836,7 +846,8 @@ static int pop_stash(int argc, const char **argv, const char *prefix,
> if (get_stash_info_assert(&info, argc, argv))
> goto cleanup;
>
> - if ((ret = do_apply_stash(prefix, &info, index, quiet)))
> + if ((ret = do_apply_stash(prefix, &info, index, quiet,
> + NULL, NULL, NULL)))
> printf_ln(_("The stash entry is kept in case "
> "you need it again."));
> else
> @@ -877,7 +888,8 @@ static int branch_stash(int argc, const char **argv, const char *prefix,
> strvec_push(&cp.args, oid_to_hex(&info.b_commit));
> ret = run_command(&cp);
> if (!ret)
> - ret = do_apply_stash(prefix, &info, 1, 0);
> + ret = do_apply_stash(prefix, &info, 1, 0,
> + NULL, NULL, NULL);
> if (!ret && info.is_stash_ref)
> ret = do_drop_stash(&info, 0);
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 70879941c2..bdaad22e1f 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -56,6 +56,7 @@ setup_stash() {
> git add other-file &&
> test_tick &&
> git commit -m initial &&
> + git tag initial &&
> echo 2 >file &&
> git add file &&
> echo 3 >file &&
> @@ -1790,4 +1791,27 @@ test_expect_success 'stash.index=false overridden by --index' '
> test_cmp expect file
> '
>
> +test_expect_success 'apply with custom conflict labels' '
> + git reset --hard initial &&
> + test_commit label-base conflict-file base-content &&
> + echo stashed >conflict-file &&
> + git stash push -m "stashed" &&
> + test_commit label-upstream conflict-file upstream-content &&
> + test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
> + test_grep "^<<<<<<< UP" conflict-file &&
> + test_grep "^||||||| Stash base" conflict-file &&
> + test_grep "^>>>>>>> STASH" conflict-file
> +'
> +
> +test_expect_success 'apply with empty conflict labels' '
> + git reset --hard initial &&
> + test_commit empty-label-base conflict-file base-content &&
> + echo stashed >conflict-file &&
> + git stash push -m "stashed" &&
> + test_commit empty-label-upstream conflict-file upstream-content &&
> + test_must_fail git stash apply --label-ours= --label-theirs= &&
> + test_grep "^<<<<<<<$" conflict-file &&
> + test_grep "^>>>>>>>$" conflict-file
> +'
> +
> test_done
> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> index 29dad98c49..659ad4ec97 100644
> --- a/xdiff/xmerge.c
> +++ b/xdiff/xmerge.c
> @@ -199,9 +199,9 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
> int size, int i, int style,
> xdmerge_t *m, char *dest, int marker_size)
> {
> - int marker1_size = (name1 ? strlen(name1) + 1 : 0);
> - int marker2_size = (name2 ? strlen(name2) + 1 : 0);
> - int marker3_size = (name3 ? strlen(name3) + 1 : 0);
> + int marker1_size = (name1 && *name1 ? strlen(name1) + 1 : 0);
> + int marker2_size = (name2 && *name2 ? strlen(name2) + 1 : 0);
> + int marker3_size = (name3 && *name3 ? strlen(name3) + 1 : 0);
> int needs_cr = is_cr_needed(xe1, xe2, m);
>
> if (marker_size <= 0)
^ permalink raw reply
* [PATCH v5] checkout: extend --track with a "fetch" mode to refresh start-point
From: Harald Nordgren via GitGitGadget @ 2026-04-28 9:03 UTC (permalink / raw)
To: git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2281.v4.git.git.1777228346809.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
A common workflow is:
git fetch origin
git checkout -b new_branch --track origin/some-branch
The first command exists so the second sees an up-to-date view of the
remote. If it is forgotten, origin/some-branch points at a stale
commit and the new local branch is created from the wrong start
point. This only matters when the user is setting up tracking and
expects the new branch to start at the freshest tip; for a one-off
checkout of an arbitrary commit there is no reason to "freshen" the
start-point.
Tie the new behavior to --track for that reason: extend its argument
to take a comma-separated list, where "fetch" can be combined with the
existing "direct" (default) and "inherit" modes. Examples:
git checkout --track=fetch -b new_branch origin/some-branch
git checkout --track=fetch,inherit -b new_branch some_local_branch
git switch --track=fetch -c new_branch origin/some-branch
When "fetch" is requested and <start-point> is in <remote>/<branch>
form, run "git fetch <remote> <branch>" before resolving the ref. This
narrows the fetch to the requested branch so that other
remote-tracking branches are left untouched -- many tools rely on the
stability of remote-tracking refs between explicit fetches. If
<start-point> is a bare remote name like "origin" (which resolves to
that remote's default branch), "git fetch <remote>" is run instead,
since the target branch is not known up front. Abort the checkout if
the fetch fails.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
checkout: add --fetch to fetch remote before resolving start-point
* Folded --fetch into --track. The standalone --fetch/--no-fetch flags
are gone; the same behavior is now requested via --track=fetch
(combinable as --track=fetch,inherit).
* Removed the checkout.fetch config. Since fetching is tied to --track,
there's no separate config knob anymore.
* Docs reworked accordingly. --track's syntax is now
(direct|inherit|fetch)[,...] in both git-checkout and git-switch man
pages, with the fetch behavior described under it. The old --fetch
and checkout.fetch sections are deleted.
* New parser callback in checkout.c. A small parse_opt_checkout_track
splits the comma-separated argument with string_list_split and sets
opts->track and opts->fetch together.
* Tests updated and trimmed. All test invocations switched from --fetch
to --track=fetch. Dropped the checkout.fetch=true and --no-fetch
override tests (those features no longer exist). Added a
--track=fetch,inherit test, a --track=bogus error test, and stronger
config-assertion checks on the basic test. Two redundant tests
(fetch,direct and order-insensitivity) were removed.
* Completion list cleaned up. --fetch removed from the expected git
checkout option list.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2281%2FHaraldNordgren%2Fcheckout-fetch-start-point-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2281/HaraldNordgren/checkout-fetch-start-point-v5
Pull-Request: https://github.com/git/git/pull/2281
Range-diff vs v4:
1: 150ccbb621 ! 1: 8ebc2f94b9 checkout: add --fetch to fetch remote before resolving start-point
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- checkout: add --fetch to fetch remote before resolving start-point
+ checkout: extend --track with a "fetch" mode to refresh start-point
A common workflow is:
git fetch origin
- git checkout -b new_branch origin/some-branch
+ git checkout -b new_branch --track origin/some-branch
- The first command exists purely so the second sees an up-to-date view
- of the remote. If it is forgotten, origin/some-branch points at a stale
- commit and the new local branch is created from the wrong start point.
+ The first command exists so the second sees an up-to-date view of the
+ remote. If it is forgotten, origin/some-branch points at a stale
+ commit and the new local branch is created from the wrong start
+ point. This only matters when the user is setting up tracking and
+ expects the new branch to start at the freshest tip; for a one-off
+ checkout of an arbitrary commit there is no reason to "freshen" the
+ start-point.
- Teach checkout (and switch) a --fetch flag that folds the two steps
- into one:
+ Tie the new behavior to --track for that reason: extend its argument
+ to take a comma-separated list, where "fetch" can be combined with the
+ existing "direct" (default) and "inherit" modes. Examples:
- git checkout --fetch -b new_branch origin/some-branch
+ git checkout --track=fetch -b new_branch origin/some-branch
+ git checkout --track=fetch,inherit -b new_branch some_local_branch
+ git switch --track=fetch -c new_branch origin/some-branch
- When --fetch is given and <start-point> is in <remote>/<branch> form,
- run "git fetch <remote> <branch>" before resolving the ref. This
+ When "fetch" is requested and <start-point> is in <remote>/<branch>
+ form, run "git fetch <remote> <branch>" before resolving the ref. This
narrows the fetch to the requested branch so that other
remote-tracking branches are left untouched -- many tools rely on the
stability of remote-tracking refs between explicit fetches. If
@@ Commit message
since the target branch is not known up front. Abort the checkout if
the fetch fails.
- Also add a checkout.fetch config to enable this by default.
-
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
- ## Documentation/config/checkout.adoc ##
-@@ Documentation/config/checkout.adoc: commands or functionality in the future.
- option in `git checkout` and `git switch`. See
- linkgit:git-switch[1] and linkgit:git-checkout[1].
-
-+`checkout.fetch`::
-+ Provides the default value for the `--fetch` or `--no-fetch`
-+ option in `git checkout` and `git switch`. See
-+ linkgit:git-switch[1] and linkgit:git-checkout[1].
-+
- `checkout.workers`::
- The number of parallel workers to use when updating the working tree.
- The default is one, i.e. sequential execution. If set to a value less
-
## Documentation/git-checkout.adoc ##
-@@ Documentation/git-checkout.adoc: linkgit:git-config[1].
- The default behavior can be set via the `checkout.guess` configuration
- variable.
+@@ Documentation/git-checkout.adoc: of it").
+ resets _<branch>_ to the start point instead of failing.
-+`--fetch`::
-+`--no-fetch`::
-+ If _<start-point>_ refers to a remote-tracking branch, fetch
-+ from that remote before resolving it. When _<start-point>_ is
-+ in _<remote>/<branch>_ form, only that branch is updated; when
-+ it is a bare remote name (e.g. `origin`), the whole remote is
-+ fetched. If the fetch fails, the checkout is aborted.
+ `-t`::
+-`--track[=(direct|inherit)]`::
++`--track[=(direct|inherit|fetch)[,...]]`::
+ When creating a new branch, set up "upstream" configuration. See
+ `--track` in linkgit:git-branch[1] for details. As a convenience,
+ --track without -b implies branch creation.
+ +
++The argument is a comma-separated list. `direct` (the default) and
++`inherit` select the tracking mode. Adding `fetch` requests that the
++remote be fetched before _<start-point>_ is resolved, so the new branch
++starts from a fresh tip: when _<start-point>_ is in
++_<remote>/<branch>_ form, only that branch is updated; when it is a
++bare remote name (e.g. `origin`), the whole remote is fetched. If the
++fetch fails, the checkout is aborted.
++
-+The default behavior can be set via the `checkout.fetch` configuration
-+variable.
-+
- `-l`::
- Create the new branch's reflog; see linkgit:git-branch[1] for
- details.
+ If no `-b` option is given, the name of the new branch will be
+ derived from the remote-tracking branch, by looking at the local part of
+ the refspec configured for the corresponding remote, and then stripping
## Documentation/git-switch.adoc ##
-@@ Documentation/git-switch.adoc: ambiguous but exists on the 'origin' remote. See also
- The default behavior can be set via the `checkout.guess` configuration
- variable.
+@@ Documentation/git-switch.adoc: should result in deletion of the path).
+ attached to a terminal, regardless of `--quiet`.
-+`--fetch`::
-+`--no-fetch`::
-+ If _<start-point>_ refers to a remote-tracking branch, fetch
-+ from that remote before resolving it. When _<start-point>_ is
-+ in _<remote>/<branch>_ form, only that branch is updated; when
-+ it is a bare remote name (e.g. `origin`), the whole remote is
-+ fetched. If the fetch fails, the switch is aborted.
+ `-t`::
+-`--track[ (direct|inherit)]`::
++`--track[=(direct|inherit|fetch)[,...]]`::
+ When creating a new branch, set up "upstream" configuration.
+ `-c` is implied. See `--track` in linkgit:git-branch[1] for
+ details.
+ +
++The argument is a comma-separated list. `direct` (the default) and
++`inherit` select the tracking mode. Adding `fetch` requests that the
++remote be fetched before _<start-point>_ is resolved, so the new branch
++starts from a fresh tip: when _<start-point>_ is in
++_<remote>/<branch>_ form, only that branch is updated; when it is a
++bare remote name (e.g. `origin`), the whole remote is fetched. If the
++fetch fails, the switch is aborted.
++
-+The default behavior can be set via the `checkout.fetch` configuration
-+variable.
-+
- `-f`::
- `--force`::
- An alias for `--discard-changes`.
+ If no `-c` option is given, the name of the new branch will be derived
+ from the remote-tracking branch, by looking at the local part of the
+ refspec configured for the corresponding remote, and then stripping
## builtin/checkout.c ##
@@
@@ builtin/checkout.c: struct branch_info {
+ if (run_command(&cmd))
+ die(_("failed to fetch start-point '%s'"), arg);
+}
++
++static int parse_opt_checkout_track(const struct option *opt,
++ const char *arg, int unset)
++{
++ struct checkout_opts *opts = opt->value;
++ struct string_list tokens = STRING_LIST_INIT_DUP;
++ struct string_list_item *item;
++ int ret = 0;
++
++ if (unset) {
++ opts->track = BRANCH_TRACK_NEVER;
++ opts->fetch = 0;
++ return 0;
++ }
++
++ opts->track = BRANCH_TRACK_EXPLICIT;
++ if (!arg)
++ return 0;
++
++ string_list_split(&tokens, arg, ",", -1);
++ for_each_string_list_item(item, &tokens) {
++ if (!strcmp(item->string, "fetch")) {
++ opts->fetch = 1;
++ } else if (!strcmp(item->string, "direct")) {
++ opts->track = BRANCH_TRACK_EXPLICIT;
++ } else if (!strcmp(item->string, "inherit")) {
++ opts->track = BRANCH_TRACK_INHERIT;
++ } else {
++ ret = error(_("option `%s' expects \"%s\", \"%s\", "
++ "or \"%s\""),
++ "--track", "direct", "inherit", "fetch");
++ break;
++ }
++ }
++
++ string_list_clear(&tokens, 0);
++ return ret;
++}
+
static void branch_info_release(struct branch_info *info)
{
@@ builtin/checkout.c: static int git_checkout_config(const char *var, const char *
opts->dwim_new_local_branch = git_config_bool(var, value);
return 0;
}
-+ if (!strcmp(var, "checkout.fetch")) {
-+ opts->fetch = git_config_bool(var, value);
-+ return 0;
-+ }
-
+-
if (starts_with(var, "submodule."))
return git_default_submodule_config(var, value, NULL);
+
+@@ builtin/checkout.c: static struct option *add_common_switch_branch_options(
+ {
+ struct option options[] = {
+ OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
+- OPT_CALLBACK_F('t', "track", &opts->track, "(direct|inherit)",
++ OPT_CALLBACK_F('t', "track", opts, "(direct|inherit|fetch)[,...]",
+ N_("set branch tracking configuration"),
+ PARSE_OPT_OPTARG,
+- parse_opt_tracking_mode),
++ parse_opt_checkout_track),
+ OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
+ PARSE_OPT_NOCOMPLETE),
+ OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unborn branch")),
@@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const char *prefix,
opts->dwim_new_local_branch &&
opts->track == BRANCH_TRACK_UNSPECIFIED &&
@@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const
argv += n;
argc -= n;
} else if (!opts->accept_ref && opts->from_treeish) {
-@@ builtin/checkout.c: int cmd_checkout(int argc,
- OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
- OPT_BOOL(0, "auto-advance", &opts.auto_advance,
- N_("auto advance to the next file when selecting hunks interactively")),
-+ OPT_BOOL(0, "fetch", &opts.fetch,
-+ N_("fetch from the remote first if <start-point> is a remote-tracking branch")),
- OPT_END()
- };
-
-@@ builtin/checkout.c: int cmd_switch(int argc,
- N_("second guess 'git switch <no-such-branch>'")),
- OPT_BOOL(0, "discard-changes", &opts.discard_changes,
- N_("throw away local modifications")),
-+ OPT_BOOL(0, "fetch", &opts.fetch,
-+ N_("fetch from the remote first if <start-point> is a remote-tracking branch")),
- OPT_END()
- };
-
## t/t7201-co.sh ##
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
test_cmp_config "" --default "" branch.main2.merge
'
-+test_expect_success 'setup upstream for --fetch tests' '
++test_expect_success 'setup upstream for --track=fetch tests' '
+ git checkout main &&
+ git init fetch_upstream &&
+ test_commit -C fetch_upstream u_main &&
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_commit -C fetch_upstream u_new
+'
+
-+test_expect_success 'checkout --fetch -b picks up branch created upstream after clone' '
++test_expect_success 'checkout --track=fetch -b picks up branch created upstream after clone' '
+ git checkout main &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_new &&
-+ git checkout --fetch -b local_new fetch_upstream/fetch_new &&
-+ test_cmp_rev refs/remotes/fetch_upstream/fetch_new HEAD
++ git checkout --track=fetch -b local_new fetch_upstream/fetch_new &&
++ test_cmp_rev refs/remotes/fetch_upstream/fetch_new HEAD &&
++ test_cmp_config fetch_upstream branch.local_new.remote &&
++ test_cmp_config refs/heads/fetch_new branch.local_new.merge
+'
+
-+test_expect_success 'checkout --fetch <remote>/<branch> leaves other tracking branches untouched' '
++test_expect_success 'checkout --track=fetch <remote>/<branch> leaves other tracking branches untouched' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_target &&
+ test_commit -C fetch_upstream u_target_pre &&
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_commit -C fetch_upstream u_target_post &&
+ git -C fetch_upstream checkout fetch_other &&
+ test_commit -C fetch_upstream u_other_post &&
-+ git checkout --fetch -b local_target fetch_upstream/fetch_target &&
++ git checkout --track=fetch -b local_target fetch_upstream/fetch_target &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_target HEAD &&
+ test "$(git rev-parse refs/remotes/fetch_upstream/fetch_other)" = "$other_before"
+'
+
-+test_expect_success 'checkout --fetch with bare remote name fetches the remote' '
++test_expect_success 'checkout --track=fetch with bare remote name fetches the remote' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_new2 &&
+ test_commit -C fetch_upstream u_new2 &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_new2 &&
-+ git checkout --fetch -b local_from_remote fetch_upstream &&
++ git checkout --track=fetch -b local_from_remote fetch_upstream &&
+ git rev-parse --verify refs/remotes/fetch_upstream/fetch_new2
+'
+
-+test_expect_success 'checkout --fetch aborts and does not create branch on fetch failure' '
++test_expect_success 'checkout --track=fetch aborts and does not create branch on fetch failure' '
+ git checkout main &&
+ test_might_fail git branch -D bogus &&
-+ test_must_fail git checkout --fetch -b bogus fetch_upstream/does_not_exist &&
++ test_must_fail git checkout --track=fetch -b bogus fetch_upstream/does_not_exist &&
+ test_must_fail git rev-parse --verify refs/heads/bogus
+'
+
-+test_expect_success 'checkout.fetch=true enables fetching without --fetch' '
++test_expect_success 'checkout --track=fetch,inherit fetches and inherits' '
+ git checkout main &&
-+ git -C fetch_upstream checkout -b fetch_cfg &&
-+ test_commit -C fetch_upstream u_cfg &&
-+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_cfg &&
-+ git -c checkout.fetch=true checkout -b local_cfg fetch_upstream/fetch_cfg &&
-+ test_cmp_rev refs/remotes/fetch_upstream/fetch_cfg HEAD
-+'
-+
-+test_expect_success '--no-fetch overrides checkout.fetch=true' '
++ git -C fetch_upstream checkout -b fetch_inherit &&
++ test_commit -C fetch_upstream u_inherit &&
++ git fetch fetch_upstream fetch_inherit &&
++ git checkout -b base_inherit fetch_upstream/fetch_inherit &&
++ test_commit -C fetch_upstream u_inherit2 &&
+ git checkout main &&
-+ git -C fetch_upstream checkout -b fetch_nofetch &&
-+ test_commit -C fetch_upstream u_nofetch &&
-+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_nofetch &&
-+ test_must_fail git -c checkout.fetch=true checkout --no-fetch \
-+ -b local_nofetch fetch_upstream/fetch_nofetch &&
-+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_nofetch &&
-+ test_must_fail git rev-parse --verify refs/heads/local_nofetch
++ git checkout --track=fetch,inherit -b local_inherit base_inherit &&
++ test_cmp_rev refs/remotes/fetch_upstream/fetch_inherit HEAD &&
++ test_cmp_config fetch_upstream branch.local_inherit.remote &&
++ test_cmp_config refs/heads/fetch_inherit branch.local_inherit.merge
+'
+
-+test_expect_success '--no-fetch overrides earlier --fetch on command line' '
++test_expect_success 'checkout --track=bogus reports an error' '
+ git checkout main &&
-+ git -C fetch_upstream checkout -b fetch_override &&
-+ test_commit -C fetch_upstream u_override &&
-+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_override &&
-+ test_must_fail git checkout --fetch --no-fetch \
-+ -b local_override fetch_upstream/fetch_override &&
-+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_override &&
-+ test_must_fail git rev-parse --verify refs/heads/local_override
++ test_must_fail git checkout --track=bogus -b bogus_branch fetch_upstream/fetch_new 2>err &&
++ test_grep "expects" err
+'
+
-+test_expect_success 'switch --fetch -c picks up branch created upstream after clone' '
++test_expect_success 'switch --track=fetch -c picks up branch created upstream after clone' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_switch &&
+ test_commit -C fetch_upstream u_switch &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_switch &&
-+ git switch --fetch -c local_switch fetch_upstream/fetch_switch &&
++ git switch --track=fetch -c local_switch fetch_upstream/fetch_switch &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_switch HEAD
+'
+
test_done
-
- ## t/t9902-completion.sh ##
-@@ t/t9902-completion.sh: test_expect_success 'double dash "git checkout"' '
- --ignore-other-worktrees Z
- --recurse-submodules Z
- --auto-advance Z
-+ --fetch Z
- --progress Z
- --guess Z
- --no-guess Z
Documentation/git-checkout.adoc | 10 +++-
Documentation/git-switch.adoc | 10 +++-
builtin/checkout.c | 85 +++++++++++++++++++++++++++++++--
t/t7201-co.sh | 81 +++++++++++++++++++++++++++++++
4 files changed, 179 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc
index 43ccf47cf6..3b8292612d 100644
--- a/Documentation/git-checkout.adoc
+++ b/Documentation/git-checkout.adoc
@@ -158,11 +158,19 @@ of it").
resets _<branch>_ to the start point instead of failing.
`-t`::
-`--track[=(direct|inherit)]`::
+`--track[=(direct|inherit|fetch)[,...]]`::
When creating a new branch, set up "upstream" configuration. See
`--track` in linkgit:git-branch[1] for details. As a convenience,
--track without -b implies branch creation.
+
+The argument is a comma-separated list. `direct` (the default) and
+`inherit` select the tracking mode. Adding `fetch` requests that the
+remote be fetched before _<start-point>_ is resolved, so the new branch
+starts from a fresh tip: when _<start-point>_ is in
+_<remote>/<branch>_ form, only that branch is updated; when it is a
+bare remote name (e.g. `origin`), the whole remote is fetched. If the
+fetch fails, the checkout is aborted.
++
If no `-b` option is given, the name of the new branch will be
derived from the remote-tracking branch, by looking at the local part of
the refspec configured for the corresponding remote, and then stripping
diff --git a/Documentation/git-switch.adoc b/Documentation/git-switch.adoc
index 87707e9265..35a03e8a52 100644
--- a/Documentation/git-switch.adoc
+++ b/Documentation/git-switch.adoc
@@ -154,11 +154,19 @@ should result in deletion of the path).
attached to a terminal, regardless of `--quiet`.
`-t`::
-`--track[ (direct|inherit)]`::
+`--track[=(direct|inherit|fetch)[,...]]`::
When creating a new branch, set up "upstream" configuration.
`-c` is implied. See `--track` in linkgit:git-branch[1] for
details.
+
+The argument is a comma-separated list. `direct` (the default) and
+`inherit` select the tracking mode. Adding `fetch` requests that the
+remote be fetched before _<start-point>_ is resolved, so the new branch
+starts from a fresh tip: when _<start-point>_ is in
+_<remote>/<branch>_ form, only that branch is updated; when it is a
+bare remote name (e.g. `origin`), the whole remote is fetched. If the
+fetch fails, the switch is aborted.
++
If no `-c` option is given, the name of the new branch will be derived
from the remote-tracking branch, by looking at the local part of the
refspec configured for the corresponding remote, and then stripping
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e031e61886..de4d7c00c7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -30,7 +30,9 @@
#include "repo-settings.h"
#include "resolve-undo.h"
#include "revision.h"
+#include "run-command.h"
#include "setup.h"
+#include "strvec.h"
#include "submodule.h"
#include "symlinks.h"
#include "trace2.h"
@@ -61,6 +63,7 @@ struct checkout_opts {
int count_checkout_paths;
int overlay_mode;
int dwim_new_local_branch;
+ int fetch;
int discard_changes;
int accept_ref;
int accept_pathspec;
@@ -112,6 +115,74 @@ struct branch_info {
char *checkout;
};
+static void fetch_remote_for_start_point(const char *arg)
+{
+ const char *slash;
+ char *remote_name;
+ struct remote *remote;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+
+ if (!arg || !*arg)
+ return;
+
+ slash = strchr(arg, '/');
+ if (slash == arg)
+ return;
+ remote_name = slash ? xstrndup(arg, slash - arg) : xstrdup(arg);
+
+ remote = remote_get(remote_name);
+ if (!remote || !remote_is_configured(remote, 1)) {
+ free(remote_name);
+ return;
+ }
+
+ strvec_pushl(&cmd.args, "fetch", remote_name, NULL);
+ if (slash && slash[1])
+ strvec_push(&cmd.args, slash + 1);
+ cmd.git_cmd = 1;
+ free(remote_name);
+ if (run_command(&cmd))
+ die(_("failed to fetch start-point '%s'"), arg);
+}
+
+static int parse_opt_checkout_track(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct checkout_opts *opts = opt->value;
+ struct string_list tokens = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+ int ret = 0;
+
+ if (unset) {
+ opts->track = BRANCH_TRACK_NEVER;
+ opts->fetch = 0;
+ return 0;
+ }
+
+ opts->track = BRANCH_TRACK_EXPLICIT;
+ if (!arg)
+ return 0;
+
+ string_list_split(&tokens, arg, ",", -1);
+ for_each_string_list_item(item, &tokens) {
+ if (!strcmp(item->string, "fetch")) {
+ opts->fetch = 1;
+ } else if (!strcmp(item->string, "direct")) {
+ opts->track = BRANCH_TRACK_EXPLICIT;
+ } else if (!strcmp(item->string, "inherit")) {
+ opts->track = BRANCH_TRACK_INHERIT;
+ } else {
+ ret = error(_("option `%s' expects \"%s\", \"%s\", "
+ "or \"%s\""),
+ "--track", "direct", "inherit", "fetch");
+ break;
+ }
+ }
+
+ string_list_clear(&tokens, 0);
+ return ret;
+}
+
static void branch_info_release(struct branch_info *info)
{
free(info->name);
@@ -1237,7 +1308,6 @@ static int git_checkout_config(const char *var, const char *value,
opts->dwim_new_local_branch = git_config_bool(var, value);
return 0;
}
-
if (starts_with(var, "submodule."))
return git_default_submodule_config(var, value, NULL);
@@ -1734,10 +1804,10 @@ static struct option *add_common_switch_branch_options(
{
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_CALLBACK_F('t', "track", &opts->track, "(direct|inherit)",
+ OPT_CALLBACK_F('t', "track", opts, "(direct|inherit|fetch)[,...]",
N_("set branch tracking configuration"),
PARSE_OPT_OPTARG,
- parse_opt_tracking_mode),
+ parse_opt_checkout_track),
OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
PARSE_OPT_NOCOMPLETE),
OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unborn branch")),
@@ -1942,8 +2012,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
opts->dwim_new_local_branch &&
opts->track == BRANCH_TRACK_UNSPECIFIED &&
!opts->new_branch;
- int n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
- &new_branch_info, opts, &rev);
+ int n;
+
+ if (opts->fetch)
+ fetch_remote_for_start_point(argv[0]);
+
+ n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
+ &new_branch_info, opts, &rev);
argv += n;
argc -= n;
} else if (!opts->accept_ref && opts->from_treeish) {
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 9bcf7c0b40..39236dca12 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -801,4 +801,85 @@ test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
test_cmp_config "" --default "" branch.main2.merge
'
+test_expect_success 'setup upstream for --track=fetch tests' '
+ git checkout main &&
+ git init fetch_upstream &&
+ test_commit -C fetch_upstream u_main &&
+ git remote add fetch_upstream fetch_upstream &&
+ git fetch fetch_upstream &&
+ git -C fetch_upstream checkout -b fetch_new &&
+ test_commit -C fetch_upstream u_new
+'
+
+test_expect_success 'checkout --track=fetch -b picks up branch created upstream after clone' '
+ git checkout main &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_new &&
+ git checkout --track=fetch -b local_new fetch_upstream/fetch_new &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_new HEAD &&
+ test_cmp_config fetch_upstream branch.local_new.remote &&
+ test_cmp_config refs/heads/fetch_new branch.local_new.merge
+'
+
+test_expect_success 'checkout --track=fetch <remote>/<branch> leaves other tracking branches untouched' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_target &&
+ test_commit -C fetch_upstream u_target_pre &&
+ git -C fetch_upstream checkout -b fetch_other &&
+ test_commit -C fetch_upstream u_other_pre &&
+ git fetch fetch_upstream &&
+ other_before=$(git rev-parse refs/remotes/fetch_upstream/fetch_other) &&
+ git -C fetch_upstream checkout fetch_target &&
+ test_commit -C fetch_upstream u_target_post &&
+ git -C fetch_upstream checkout fetch_other &&
+ test_commit -C fetch_upstream u_other_post &&
+ git checkout --track=fetch -b local_target fetch_upstream/fetch_target &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_target HEAD &&
+ test "$(git rev-parse refs/remotes/fetch_upstream/fetch_other)" = "$other_before"
+'
+
+test_expect_success 'checkout --track=fetch with bare remote name fetches the remote' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_new2 &&
+ test_commit -C fetch_upstream u_new2 &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_new2 &&
+ git checkout --track=fetch -b local_from_remote fetch_upstream &&
+ git rev-parse --verify refs/remotes/fetch_upstream/fetch_new2
+'
+
+test_expect_success 'checkout --track=fetch aborts and does not create branch on fetch failure' '
+ git checkout main &&
+ test_might_fail git branch -D bogus &&
+ test_must_fail git checkout --track=fetch -b bogus fetch_upstream/does_not_exist &&
+ test_must_fail git rev-parse --verify refs/heads/bogus
+'
+
+test_expect_success 'checkout --track=fetch,inherit fetches and inherits' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_inherit &&
+ test_commit -C fetch_upstream u_inherit &&
+ git fetch fetch_upstream fetch_inherit &&
+ git checkout -b base_inherit fetch_upstream/fetch_inherit &&
+ test_commit -C fetch_upstream u_inherit2 &&
+ git checkout main &&
+ git checkout --track=fetch,inherit -b local_inherit base_inherit &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_inherit HEAD &&
+ test_cmp_config fetch_upstream branch.local_inherit.remote &&
+ test_cmp_config refs/heads/fetch_inherit branch.local_inherit.merge
+'
+
+test_expect_success 'checkout --track=bogus reports an error' '
+ git checkout main &&
+ test_must_fail git checkout --track=bogus -b bogus_branch fetch_upstream/fetch_new 2>err &&
+ test_grep "expects" err
+'
+
+test_expect_success 'switch --track=fetch -c picks up branch created upstream after clone' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_switch &&
+ test_commit -C fetch_upstream u_switch &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_switch &&
+ git switch --track=fetch -c local_switch fetch_upstream/fetch_switch &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_switch HEAD
+'
+
test_done
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* Git generated tarballs and Debian
From: Simon Richter @ 2026-04-28 8:40 UTC (permalink / raw)
To: git; +Cc: Ian Jackson
Hi,
in Debian, we're shipping "original" tarballs for each software package,
and the Debian specific changes in a separate file.
Historically, this users could do a bitwise comparison of the original
tarball and the one in Debian to verify that these were unchanged.
With git, some authors have stopped releasing official tarballs, so
we're using git-archive a lot -- but this is reproducible only by
accident. GitHub also prepares some release tarballs that may or not be
bitwise identical to what git archive produces.
I've written a small tool that generates the tree checksum for a given
tarball (running inside a SECCOMP environment, not writing anything to
disk), that already goes a long way to make tarballs verifiable: one can
check whether that ID is the same as the one mentioned in a commit (and
the comment inside a git-archive generated tarball is helpful in finding
which commit).
The downsides of that are:
1. that you still need a copy of the commit to verify it, as it's not
included in the tarball.
We could add an ancillary file that contains the commit object (its
checksum being reproducible, and containing the tree checksum) and
possibly a signed tag object as well, so that is solvable inside Debian.
Another option would be to extend the git-archive format to include them
as a (longer) comment in the global pax header.
2. that it doesn't work for submodules
What we do currently is generate multiple archives with different
prefixes, and concatenate them using tar. That loses all the pax global
headers though, so commit information is lost. In addition, putting the
actual contents into a subdirectory instead of a commit reference means
that generating the tree object from the tarball contents means the
checksum does not match.
What we could do is generate multiple archives, and keep them separate,
but the Debian toolchain can only unpack additional archives into a
direct subdirectory of the main archive (e.g. "orig.tar.gz" gets
unpacked to "foo-1.0", then "orig-addon.tar.gz" gets unpacked into
"foo-1.0/addon"). We can fix _that_ with symlinks, but it gets more and
more hacky.
One thing we could do inside git here is add a method to create archives
that include submodules (that gets rid of the concatenation), but in
order for this to be easily verifiable, I still need to know where
submodules are and what their commit objects are (so I know the commit
checksum and can verify the tree checksum).
The goal is to extend what I can already do inside the Linux kernel:
$ git rev-parse HEAD
94dfcc4a99b0cece77e73dc3011284050f95da89
$ git rev-parse HEAD^{tree}
2d14d43ce9f062160262f4e4f162f5ff0ed91a5e
$ git archive --format=tar HEAD | git-treeof
Commit-Hint: 94dfcc4a99b0cece77e73dc3011284050f95da89
Tree-SHA1: 2d14d43ce9f062160262f4e4f162f5ff0ed91a5e
so the "Commit-Hint" can become a stronger statement "I have seen a
commit object with this checksum that actually refers to the correct
tree", and to allow this to work for repositories with submodules.
Does it make sense to extend git here to allow this, or should I try to
solve this entirely within Debian?
Simon
^ permalink raw reply
* Re: [PATCH] checkout: add --fetch to fetch remote before resolving start-point
From: Harald Nordgren @ 2026-04-28 8:44 UTC (permalink / raw)
To: gitster
Cc: ben.knoble, git, gitgitgadget, haraldnordgren,
kristofferhaugsbakk, marcnarc, ramsay
In-Reply-To: <xmqqwlxrzwid.fsf@gitster.g>
I hope you had a good leave and is back with renewed energy! ☀️
> Git is among projects that encourage forking only from a well-known
> point in history (like the latest released version), not at a random
> "tip of the day" commit from the upstream.
Are you talking about the Git project that we are working on right now, or
talking about how people use Git "in the wild"?
Because how people use Git in the wild can be a bit different, and merge
conflicts arguably the worst part of collaborating with a team using Git.
In my early days as a professional coder, snubbed my toe countless times on
forgetting to pull in the latest changes, before starting to work on
something.
I respect that things work differently in a neatly ordered project like Git
itself, where you do a great work of organizing, but all other projects are
not like that. My advice to a junior developer is to pull in the latest
changes when starting and to rebase obsessively to prevent a large merge
conflict down the road.
> So instead of introducing a totally new option that can only be used
> only when "--track" is given, it might make more sense to introduce
> this as a variant of "--track", perhaps "--track=fetch,[in]direct"
> or something like that.
> I may have already pointed this out (but I do not remember), but
> this option would not make any sense when --track is not in effect,
> so instead of adding a brand new option, making it an extension to
> the existing --track option might make it slightly more palatable.
Fair enough. You did point it out and I will give that a try!
Harald
^ permalink raw reply
* [PATCH 3/3] line-log: allow non-patch diff formats with -L
From: Michael Montalbo via GitGitGadget @ 2026-04-28 4:05 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2094.git.1777349126.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
Now that -L flows through log_tree_diff_flush() and diff_flush(),
metadata-only diff formats work because they only read filepair
fields (status, mode, path, oid) already set on the pre-computed
pairs.
Expand the allowlist in setup_revisions() to also accept --raw,
--name-only, --name-status, and --summary. Diff stat formats
(--stat, --numstat, --shortstat, --dirstat) remain blocked because
they call compute_diffstat() on full blob content and would show
whole-file statistics rather than range-scoped ones.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/line-range-options.adoc | 10 +++---
revision.c | 7 ++--
t/t4211-line-log.sh | 47 +++++++++++++++++++++++++--
3 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc
index ecb2c79fb9..72f639b5e7 100644
--- a/Documentation/line-range-options.adoc
+++ b/Documentation/line-range-options.adoc
@@ -8,12 +8,14 @@
give zero or one positive revision arguments, and
_<start>_ and _<end>_ (or _<funcname>_) must exist in the starting revision.
You can specify this option more than once. Implies `--patch`.
- Patch output can be suppressed using `--no-patch`, but other diff formats
- (namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
- `--name-only`, `--name-status`, `--check`) are not currently implemented.
+ Patch output can be suppressed using `--no-patch`.
+ Non-patch diff formats `--raw`, `--name-only`, `--name-status`,
+ and `--summary` are supported. Diff stat formats
+ (`--stat`, `--numstat`, `--shortstat`, `--dirstat`) are not
+ currently implemented.
+
Patch formatting options such as `--word-diff`, `--color-moved`,
`--no-prefix`, and whitespace options (`-w`, `-b`) are supported,
-as are pickaxe options (`-S`, `-G`).
+as are pickaxe options (`-S`, `-G`) and `--diff-filter`.
+
include::line-range-format.adoc[]
diff --git a/revision.c b/revision.c
index a1c795de96..b41ec4016b 100644
--- a/revision.c
+++ b/revision.c
@@ -3179,8 +3179,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
if (revs->line_level_traverse &&
- (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
- die(_("-L does not yet support diff formats besides -p and -s"));
+ (revs->diffopt.output_format &
+ ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT |
+ DIFF_FORMAT_RAW | DIFF_FORMAT_NAME |
+ DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY)))
+ die(_("-L does not yet support the requested diff format"));
if (revs->line_level_traverse && revs->full_diff)
die(_("-L is not compatible with --full-diff"));
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 1d566ea9bd..63cf8e5d9f 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -155,8 +155,45 @@ test_expect_success '-p shows the default patch output' '
test_cmp expect actual
'
-test_expect_success '--raw is forbidden' '
- test_must_fail git log -L1,24:b.c --raw
+test_expect_success '--raw shows mode, oid, status and path' '
+ git log -L1,24:b.c --raw --format= >actual &&
+ grep "^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual &&
+ ! grep "^diff --git" actual &&
+ ! grep "^@@" actual
+'
+
+test_expect_success '--name-only shows path' '
+ git log -L1,24:b.c --name-only --format= >actual &&
+ grep "^b.c$" actual &&
+ ! grep "^diff --git" actual &&
+ ! grep "^@@" actual
+'
+
+test_expect_success '--name-status shows status and path' '
+ git log -L1,24:b.c --name-status --format= >actual &&
+ grep "^M b.c$" actual &&
+ ! grep "^diff --git" actual &&
+ ! grep "^@@" actual
+'
+
+test_expect_success '--stat is not yet supported with -L' '
+ test_must_fail git log -L1,24:b.c --stat 2>err &&
+ test_grep "does not yet support" err
+'
+
+test_expect_success '--numstat is not yet supported with -L' '
+ test_must_fail git log -L1,24:b.c --numstat 2>err &&
+ test_grep "does not yet support" err
+'
+
+test_expect_success '--shortstat is not yet supported with -L' '
+ test_must_fail git log -L1,24:b.c --shortstat 2>err &&
+ test_grep "does not yet support" err
+'
+
+test_expect_success '--dirstat is not yet supported with -L' '
+ test_must_fail git log -L1,24:b.c --dirstat 2>err &&
+ test_grep "does not yet support" err
'
test_expect_success 'setup for checking fancy rename following' '
@@ -737,4 +774,10 @@ test_expect_success '-L --oneline has no extra blank line before diff' '
sed -n 2p actual | grep "^diff --git"
'
+test_expect_success '--summary shows new file on root commit' '
+ git checkout parent-oids &&
+ git log -L:func2:file.c --summary --format= >actual &&
+ grep "create mode 100644 file.c" actual
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/3] line-log: integrate -L output with the standard log-tree pipeline
From: Michael Montalbo via GitGitGadget @ 2026-04-28 4:05 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2094.git.1777349126.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
`git log -L` has bypassed log_tree_diff() and log_tree_diff_flush()
since the feature was introduced, short-circuiting from
log_tree_commit() directly into line_log_print(). This skips the
no_free save/restore (noted in a NEEDSWORK comment added by
f8781bfda3), the always_show_header fallback, show_diff_of_diff(),
and diff_free() cleanup.
Restructure so that -L flows through log_tree_diff() ->
log_tree_diff_flush(), the same path used by the normal
single-parent and merge diff codepaths:
- Rename line_log_print() to line_log_queue_pairs() and strip it
down to just queuing pre-computed filepairs. The show_log(),
separator, diffcore_std(), and diff_flush() calls are removed
since log_tree_diff_flush() handles all of those.
- In log_tree_diff(), call line_log_queue_pairs() then
log_tree_diff_flush(), mirroring the diff_tree_oid() + flush
pattern used by the single-parent and merge codepaths.
- Remove the early return in log_tree_commit() that bypassed
no_free save/restore, always_show_header, and diff_free().
Because show_log() is now deferred until after diffcore_std() inside
log_tree_diff_flush(), pickaxe (-S, -G, --find-object) and
--diff-filter now properly suppress commits when all pairs are
filtered out.
The blank-line separator between commit header and diff changes
slightly: the old code printed one unconditionally, while
log_tree_diff_flush() only emits one for verbose headers. This
matches the rest of log output.
Also reject --full-diff, which is meaningless with -L: the filepairs
are pre-computed during the history walk and scoped to tracked paths,
so there is no tree diff to widen.
Update tests accordingly.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
line-log.c | 30 ++++-------
line-log.h | 2 +-
log-tree.c | 9 ++--
revision.c | 2 +
t/t4211-line-log.sh | 52 ++++++++++++++-----
t/t4211/sha1/expect.parallel-change-f-to-main | 1 -
.../sha256/expect.parallel-change-f-to-main | 1 -
7 files changed, 56 insertions(+), 41 deletions(-)
diff --git a/line-log.c b/line-log.c
index 858a899cd2..7ee55b05cc 100644
--- a/line-log.c
+++ b/line-log.c
@@ -13,7 +13,6 @@
#include "revision.h"
#include "xdiff-interface.h"
#include "strbuf.h"
-#include "log-tree.h"
#include "line-log.h"
#include "setup.h"
#include "strvec.h"
@@ -1004,29 +1003,18 @@ static int process_all_files(struct line_log_data **range_out,
return changed;
}
-int line_log_print(struct rev_info *rev, struct commit *commit)
+void line_log_queue_pairs(struct rev_info *rev, struct commit *commit)
{
- show_log(rev);
- if (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT)) {
- struct line_log_data *range = lookup_line_range(rev, commit);
- struct line_log_data *r;
- const char *prefix = diff_line_prefix(&rev->diffopt);
-
- fprintf(rev->diffopt.file, "%s\n", prefix);
-
- for (r = range; r; r = r->next) {
- if (r->pair) {
- struct diff_filepair *p =
- diff_filepair_dup(r->pair);
- p->line_ranges = &r->ranges;
- diff_q(&diff_queued_diff, p);
- }
- }
+ struct line_log_data *range = lookup_line_range(rev, commit);
+ struct line_log_data *r;
- diffcore_std(&rev->diffopt);
- diff_flush(&rev->diffopt);
+ for (r = range; r; r = r->next) {
+ if (r->pair) {
+ struct diff_filepair *p = diff_filepair_dup(r->pair);
+ p->line_ranges = &r->ranges;
+ diff_q(&diff_queued_diff, p);
+ }
}
- return 1;
}
static int bloom_filter_check(struct rev_info *rev,
diff --git a/line-log.h b/line-log.h
index 04a6ea64d3..99e1755ce3 100644
--- a/line-log.h
+++ b/line-log.h
@@ -46,7 +46,7 @@ int line_log_filter(struct rev_info *rev);
int line_log_process_ranges_arbitrary_commit(struct rev_info *rev,
struct commit *commit);
-int line_log_print(struct rev_info *rev, struct commit *commit);
+void line_log_queue_pairs(struct rev_info *rev, struct commit *commit);
void line_log_free(struct rev_info *rev);
diff --git a/log-tree.c b/log-tree.c
index 7e048701d0..1ead481891 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1105,6 +1105,11 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
if (!all_need_diff && !opt->merges_need_diff)
return 0;
+ if (opt->line_level_traverse) {
+ line_log_queue_pairs(opt, commit);
+ return log_tree_diff_flush(opt);
+ }
+
parse_commit_or_die(commit);
oid = get_commit_tree_oid(commit);
@@ -1179,10 +1184,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
opt->loginfo = &log;
opt->diffopt.no_free = 1;
- /* NEEDSWORK: no restoring of no_free? Why? */
- if (opt->line_level_traverse)
- return line_log_print(opt, commit);
-
if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
shown = log_tree_diff(opt, commit, &log);
diff --git a/revision.c b/revision.c
index 4a8e24bc38..a1c795de96 100644
--- a/revision.c
+++ b/revision.c
@@ -3181,6 +3181,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
if (revs->line_level_traverse &&
(revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
die(_("-L does not yet support diff formats besides -p and -s"));
+ if (revs->line_level_traverse && revs->full_diff)
+ die(_("-L is not compatible with --full-diff"));
if (revs->expand_tabs_in_log < 0)
revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index aaf197d2ed..1d566ea9bd 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -368,7 +368,6 @@ test_expect_success '-L diff output includes index and new file mode' '
test_expect_success '-L with --word-diff' '
cat >expect <<-\EOF &&
-
diff --git a/file.c b/file.c
--- a/file.c
+++ b/file.c
@@ -377,7 +376,6 @@ test_expect_success '-L with --word-diff' '
{
return [-F2;-]{+F2 + 2;+}
}
-
diff --git a/file.c b/file.c
new file mode 100644
--- /dev/null
@@ -433,7 +431,6 @@ test_expect_success 'show line-log with graph' '
null_blob=$(test_oid zero | cut -c1-7) &&
qz_to_tab_space >expect <<-EOF &&
* $head_oid Modify func2() in file.c
- |Z
| diff --git a/file.c b/file.c
| index $head_blob_old..$head_blob_new 100644
| --- a/file.c
@@ -445,7 +442,6 @@ test_expect_success 'show line-log with graph' '
| + return F2 + 2;
| }
* $root_oid Add func1() and func2() in file.c
- ZZ
diff --git a/file.c b/file.c
new file mode 100644
index $null_blob..$root_blob
@@ -494,23 +490,17 @@ test_expect_success '-L --find-object does not crash with merge and rename' '
--find-object=$(git rev-parse HEAD:file) >actual
'
-# Commit-level filtering with pickaxe does not yet work for -L.
-# show_log() prints the commit header before diffcore_std() runs
-# pickaxe, so commits cannot be suppressed even when no diff pairs
-# survive filtering. Fixing this would require deferring show_log()
-# until after diffcore_std(), which is a larger restructuring of the
-# log-tree output pipeline.
-test_expect_failure '-L -G should filter commits by pattern' '
+test_expect_success '-L -G should filter commits by pattern' '
git log --format="%s" --no-patch -L 1,1:file -G "nomatch" >actual &&
test_must_be_empty actual
'
-test_expect_failure '-L -S should filter commits by pattern' '
+test_expect_success '-L -S should filter commits by pattern' '
git log --format="%s" --no-patch -L 1,1:file -S "nomatch" >actual &&
test_must_be_empty actual
'
-test_expect_failure '-L --find-object should filter commits by object' '
+test_expect_success '-L --find-object should filter commits by object' '
git log --format="%s" --no-patch -L 1,1:file \
--find-object=$ZERO_OID >actual &&
test_must_be_empty actual
@@ -711,4 +701,40 @@ test_expect_success '-L with -G filters to diff-text matches' '
grep "F2 + 2" actual
'
+test_expect_success '-L with --diff-filter=M excludes root commit' '
+ git checkout parent-oids &&
+ git log -L:func2:file.c --diff-filter=M --format=%s --no-patch >actual &&
+ # Root commit is an Add (A), not a Modify (M), so it should
+ # be excluded; only the modification commit remains.
+ echo "Modify func2() in file.c" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '-L with --diff-filter=A shows only root commit' '
+ git checkout parent-oids &&
+ git log -L:func2:file.c --diff-filter=A --format=%s --no-patch >actual &&
+ echo "Add func1() and func2() in file.c" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '-L with -S suppresses non-matching commits' '
+ git checkout parent-oids &&
+ git log -L:func2:file.c -S "F2 + 2" --format=%s --no-patch >actual &&
+ # Only the commit that changes the count of "F2 + 2" should appear.
+ echo "Modify func2() in file.c" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--full-diff is not supported with -L' '
+ test_must_fail git log -L1,24:b.c --full-diff 2>err &&
+ test_grep "not compatible with --full-diff" err
+'
+
+test_expect_success '-L --oneline has no extra blank line before diff' '
+ git checkout parent-oids &&
+ git log --oneline -L:func2:file.c -1 >actual &&
+ # Oneline header on line 1, diff starts immediately on line 2
+ sed -n 2p actual | grep "^diff --git"
+'
+
test_done
diff --git a/t/t4211/sha1/expect.parallel-change-f-to-main b/t/t4211/sha1/expect.parallel-change-f-to-main
index 65a8cc673a..6d7a201036 100644
--- a/t/t4211/sha1/expect.parallel-change-f-to-main
+++ b/t/t4211/sha1/expect.parallel-change-f-to-main
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200
Merge across the rename
-
commit 6ce3c4ff690136099bb17e1a8766b75764726ea7
Author: Thomas Rast <trast@student.ethz.ch>
Date: Thu Feb 28 10:49:50 2013 +0100
diff --git a/t/t4211/sha256/expect.parallel-change-f-to-main b/t/t4211/sha256/expect.parallel-change-f-to-main
index 3178989253..c93e03bef4 100644
--- a/t/t4211/sha256/expect.parallel-change-f-to-main
+++ b/t/t4211/sha256/expect.parallel-change-f-to-main
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200
Merge across the rename
-
commit 4f7a58195a92c400e28a2354328587f1ff14fb77f5cf894536f17ccbc72931b9
Author: Thomas Rast <trast@student.ethz.ch>
Date: Thu Feb 28 10:49:50 2013 +0100
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/3] revision: move -L setup before output_format-to-diff derivation
From: Michael Montalbo via GitGitGadget @ 2026-04-28 4:05 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2094.git.1777349126.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
The line_level_traverse block sets a default DIFF_FORMAT_PATCH when
no output format has been explicitly requested. This default must
be visible to the "Did the user ask for any diff output?" check
that derives revs->diff from revs->diffopt.output_format.
Currently the -L block runs after that derivation, so revs->diff
stays 0 when no explicit format is given. This does not matter yet
because log_tree_commit() short-circuits into line_log_print()
before consulting revs->diff, but the next commit will route -L
through the normal log_tree_diff() path, which checks revs->diff.
Move the block above the derivation so the default DIFF_FORMAT_PATCH
is in place when revs->diff is computed. No behavior change on its
own.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
revision.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/revision.c b/revision.c
index 599b3a66c3..4a8e24bc38 100644
--- a/revision.c
+++ b/revision.c
@@ -3112,6 +3112,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
object_context_release(&oc);
}
+ if (revs->line_level_traverse) {
+ if (want_ancestry(revs))
+ revs->limited = 1;
+ revs->topo_order = 1;
+ if (!revs->diffopt.output_format)
+ revs->diffopt.output_format = DIFF_FORMAT_PATCH;
+ }
+
/* Did the user ask for any diff output? Run the diff! */
if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)
revs->diff = 1;
@@ -3125,14 +3133,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
if (revs->diffopt.objfind)
revs->simplify_history = 0;
- if (revs->line_level_traverse) {
- if (want_ancestry(revs))
- revs->limited = 1;
- revs->topo_order = 1;
- if (!revs->diffopt.output_format)
- revs->diffopt.output_format = DIFF_FORMAT_PATCH;
- }
-
if (revs->topo_order && !generation_numbers_enabled(the_repository))
revs->limited = 1;
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/3] line-log: integrate -L with the standard log output pipeline
From: Michael Montalbo via GitGitGadget @ 2026-04-28 4:05 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo
Since its introduction, git log -L has short-circuited from
log_tree_commit() into its own output function, bypassing log_tree_diff()
and log_tree_diff_flush(). This skips no_free save/restore,
always_show_header, diff_free() cleanup, and means that pickaxe (-S, -G,
--find-object) and --diff-filter cannot suppress commits whose pairs are all
filtered out, because show_log() runs before diffcore_std().
This series restructures the flow so that -L goes through the same
log_tree_diff() -> log_tree_diff_flush() path as normal single-parent and
merge diffs, then uses that to enable several non-patch diff formats.
Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in setup_revisions(). The -L block sets a default
DIFF_FORMAT_PATCH when no format is requested; move it before the derivation
of revs->diff from output_format so the default is visible to that check. No
behavior change on its own.
Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename line_log_print() to line_log_queue_pairs(), stripping it down to only
queue pre-computed filepairs. log_tree_diff_flush() handles show_log(),
diffcore_std(), and diff_flush(). This fixes pickaxe and --diff-filter
suppression, and aligns the commit/diff separator with the rest of log
output. Also rejects --full-diff, which is meaningless when filepairs are
pre-computed.
Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and
--summary. These only read filepair metadata already set by the line-log
machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat)
remain blocked because they call compute_diffstat() on full blob content and
would show whole-file statistics rather than range-scoped ones.
Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -L
Documentation/line-range-options.adoc | 10 +-
line-log.c | 30 ++----
line-log.h | 2 +-
log-tree.c | 9 +-
revision.c | 25 +++--
t/t4211-line-log.sh | 99 ++++++++++++++++---
t/t4211/sha1/expect.parallel-change-f-to-main | 1 -
.../sha256/expect.parallel-change-f-to-main | 1 -
8 files changed, 120 insertions(+), 57 deletions(-)
base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2094
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH] t5564: use a short path for the SOCKS proxy socket
From: Jeff King @ 2026-04-28 2:33 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.2100.git.1777299669889.gitgitgadget@gmail.com>
On Mon, Apr 27, 2026 at 02:21:09PM +0000, Johannes Schindelin via GitGitGadget wrote:
> 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)".
OK. We try to work around this with a chdir in our own socket code, but
I guess we're not using it here:
1. The socket is created by our socks4-proxy.pl script, which just
feeds it to perl's IO::Socket::UNIX. And it looks like it
recognizes the long path and complains. We could fix that, but...
2. The reading side is implemented by libcurl, not by us. And it seems
to similarly detect and complain. We _could_ work around that with
a chdir, but it would be quite nasty, as we'd have to do it before
every curl call. So that's probably off the table.
And so we are stuck with either using a relative path, or a known-small
one.
> 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).
OK, there we went with the known-small solution, since openssh made it
easy to do so. I think that is OK here, but...
> # 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_SOCK="${TMPDIR:-/tmp}/git-test-socks-%30.sock"
This is a static path in /tmp, so:
1. Multiple instances of the test suite will stomp on each other
(e.g., a --stress run).
2. It creates a tmpdir-race vulnerability if an attacker links that
path to something precious writeable by the user running the tests.
I think it would be sufficient to use mktemp to get a unique name. We
don't want a file, of course, so we perhaps need "mktemp -d" to get a
temp directory, and then we can use whatever short name we like inside
it.
> 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 -f "$SOCKS_SOCK"
> '
And the rest of your path can remain as-is, since SOCKS_SOCK will have
the unique name in it.
-Peff
^ permalink raw reply
* Re: [PATCH] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Jeff King @ 2026-04-28 2:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Derrick Stolee, Scott Bauersfeld via GitGitGadget, git,
Scott Bauersfeld
In-Reply-To: <xmqqecjz26wr.fsf@gitster.g>
On Tue, Apr 28, 2026 at 10:46:44AM +0900, Junio C Hamano wrote:
> The application may have produced only 2kB before it issues a
> "flush". Whether the buffer size is 4kB or 128kB, such a flush will
> only write out 2kB, and the larger buffer size does not help at all.
> But if the application has produced 90kB before it issues a "flush",
> the larger buffer size would give us a great improvement. With 4kB
> buffer, before such an application level "flush", we would have seen
> 22 = floor(90/4) calls of write(2) to flush the buffer, plus a 2kB
> write(2). With 128kB buffer, we would see a single 90kB write(2).
>
> So the apparently lower improvement than I naively have expected may
> be attributable to the fact that many application level "flush" was
> not large enough to benefit from 128kB buffer? How much of the
> total number of bytes written came in large batches, vs tiny ones?
The input to index-pack in a fetch is going to be the demuxing of the
sideband via git-fetch. So it's probably flushing 64k or less each time
(because that's the max size of a packet), and unless index-pack is
going much slower than the input, that maximizes how much it will read.
Depending on the source, though, it may be possible to go faster than
index-pack (which has to at least update the pack checksum for every
byte, and may even zlib inflate and hash the object itself if it's a
non-delta). In which case the sideband demuxer would start filling the
pipe and index-pack may get larger reads.
We could actually reduce the number of syscalls further if index-pack
did the demuxing itself, and we just handed it the descriptor. That
probably doesn't help all that much in this case, though, if the problem
is not raw reads/writes on pipes, but rather ones that go to the slow
FUSE filesystem. And as long as those pipe reads/writes are "wide"
(allowing the eventual filesystem writes to also be wide), then the
exact number may not be as important.
But the demuxing may also explain why the total number of writes did not
decrease as much as you expected, since those ones will probably not be
reduced by the patch in question. So the improvement is a percentage of
only a smaller portion of the total (but not necessarily half, because
they may have been larger writes in the first place).
-Peff
^ permalink raw reply
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