Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3 0/6] t: add greplint.pl and convert grep to test_grep
From: Junio C Hamano @ 2026-07-05  1:38 UTC (permalink / raw)
  To: Michael Montalbo via GitGitGadget
  Cc: git, D. Ben Knoble, Eric Sunshine, SZEDER Gábor,
	Michael Montalbo
In-Reply-To: <pull.2135.v3.git.1783054466.gitgitgadget@gmail.com>

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Changes since v2:
>
>  * t3420-rebase-autostash: dropped the change to the '! grep dirty file3'
>    line under 'rebase --quit'. As SZEDER pointed out, file3 never exists in
>    the conflicted state, so that grep was passing only because it could not
>    open the file. SZEDER's fix (sg/t3420-do-not-grep-in-missing-file, now in
>    'next') replaces the line with 'test_path_is_missing file3', which is the
>    right check; this series simply leaves that line to his fix.
>
>  * Audited the remaining '# lint-ok' annotations for the same "grep a file
>    that never exists with correctly running Git" gotcha, as Junio suggested.
>    The rule the audit applies: 'grep' becomes 'test_grep' only where its
>    exit code is the assertion; grep that produces data (a filter) or that
>    reads a file whose presence is conditional stays a plain 'grep', because
>    test_grep BUGs on a missing file.
>    
>    * t5537 (.git/shallow): the file is still present after the repack (the
>      client stays shallow), so the assertion is converted to 'test_grep !'
>      like any other; the "may not exist" note was wrong.
>    
>    * t1400 (.git/packed-refs): the file exists only with the files backend.
>      Guarded the packed-refs check with a REFFILES prerequisite; the
>      backend-agnostic 'git show-ref' check that follows still runs under
>      every backend.
>    
>    * t7450 (squatting-clone/d/a/git~2): kept as '! grep' with an improved '#
>      lint-ok'. 'git~2' is the NTFS 8.3 short name of a planted '..git' decoy
>      and only exists when 8.3 short-name generation is enabled. Verified on
>      a Windows VM: with 8.3 disabled (the modern default) the short name is
>      absent, the '! grep' correctly tolerates it, and a plain test_grep
>      would BUG. So this one deliberately stays a missing-file-tolerant grep.
>    
>    * t5326 and t5702 remain annotated: these are genuine data filters (grep
>      produces data that is redirected/captured, not an assertion).

Great.

>      ++   test_grep requires <file> to exist and will BUG otherwise.
>      ++   When a file's presence is conditional (a backend-specific
>      ++   file, or a path that only exists on some platforms, such as
>      ++   an NTFS 8.3 short name), keep a plain guarded 'grep' instead.

It is not quite clear if I can follow this instruction myself,
without knowing what a "plain guarded 'grep'" is, unfortunately.  Is
it different from bog-standard grep?

>      @@ t/t1400-update-ref.sh: test_expect_success "move $m (by HEAD)" '
>        	test_when_finished "git update-ref -d $m" &&
>        	git update-ref -d HEAD $B &&
>       -	! grep "$m" .git/packed-refs &&
>      -+	! grep "$m" .git/packed-refs && # lint-ok: file may not exist (reftable)
>      ++	if test_have_prereq REFFILES
>      ++	then
>      ++		test_grep ! "$m" .git/packed-refs
>      ++	fi &&

The intent is shown very well in this version (admittedly, the
lint-ok comment is readable but only by humans and LLMs).  Here, we
expect .git/packed-refs only while REFFILES prerequiste is active.

Thanks.

^ permalink raw reply

* Re: [PATCH v2 0/6] odb: refactor source-specific information in object info
From: Junio C Hamano @ 2026-07-05  1:28 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Justin Tobler
In-Reply-To: <20260702-b4-pks-odb-drop-whence-v2-0-b0af7468ad95@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> Changes in v2:
>   - Rename `struct object_info_source` to `odb_source_info` and the
>     `sourcep` pointer to `source_infop`. This follows a suggestion made
>     by Justin, as the current naming is too easy to confuse with the
>     actual source.
>   - Link to v1: https://patch.msgid.link/20260624-b4-pks-odb-drop-whence-v1-0-8d1877b790ac@pks.im

Thanks.  Queued.

^ permalink raw reply

* CARGO trouble appeared from 2.54.0 to 2.55.0
From: Kurt Mielke @ 2026-07-05  0:21 UTC (permalink / raw)
  To: git

Hi


I had a clean alma10 and wanted the newest git BUT building fails:

make

....

     CC version.o
     AR libgit.a
     CARGO target/release/libgitcore.a
/bin/sh: line 1: cargo: command not found
make: *** [Makefile:3021: target/release/libgitcore.a] Error 127

I got 2.54.0 and it compiled just fine

I found out CARGO is rust related

Changing make command to

make NO_RUST=1

And I got my 2.55.0 version, but allow me to suggest it again is the default

II have no caommands in path beginning with the letters 'rus'

Thank you in advance

Kirt Mielke


^ permalink raw reply

* [PATCH v2] precompose_utf8: use a flex array for d_name
From: Ihar Hrachyshka @ 2026-07-04 23:37 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <20260703023554.36577-1-ihar.hrachyshka@gmail.com>

On macOS, git status may abort while reading a directory entry
whose UTF-8 name grows past NAME_MAX bytes:

  __chk_fail_overflow
  __strlcpy_chk
  precompose_utf8_readdir
  read_directory_recursive
  wt_status_collect
  cmd_status

The precompose wrapper already reallocates dirent_prec_psx for
long names, but d_name is declared as char[NAME_MAX + 1]. A
fortified libc can still see that declared object size and reject a
larger strlcpy bound, even though the allocation was grown.

Make d_name a FLEX_ARRAY and size allocations from offsetof(). That
matches the actual object layout with the dynamic allocation, so the
fortified copy sees a destination whose size can grow with max_name_len.

Add a regression test that creates an over-NAME_MAX non-ASCII basename
and runs status with core.precomposeunicode enabled.

Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
---

Changes in v2:
- Drop perl from the regression test and use printf/tr instead.
- Use the minimal 256-byte filename that reproduces the crash.

 compat/precompose_utf8.c     | 12 ++++++++----
 compat/precompose_utf8.h     |  9 +++++----
 t/t3910-mac-os-precompose.sh | 16 ++++++++++++++++
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 1711794..8077f62 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -19,6 +19,11 @@ typedef char *iconv_ibp;
 static const char *repo_encoding = "UTF-8";
 static const char *path_encoding = "UTF-8-MAC";
 
+static size_t dirent_prec_psx_size(size_t max_name_len)
+{
+	return st_add(offsetof(dirent_prec_psx, d_name), max_name_len);
+}
+
 static size_t has_non_ascii(const char *s, size_t maxlen, size_t *strlen_c)
 {
 	const uint8_t *ptr = (const uint8_t *)s;
@@ -114,8 +119,8 @@ const char *precompose_argv_prefix(int argc, const char **argv, const char *pref
 PREC_DIR *precompose_utf8_opendir(const char *dirname)
 {
 	PREC_DIR *prec_dir = xmalloc(sizeof(PREC_DIR));
-	prec_dir->dirent_nfc = xmalloc(sizeof(dirent_prec_psx));
-	prec_dir->dirent_nfc->max_name_len = sizeof(prec_dir->dirent_nfc->d_name);
+	prec_dir->dirent_nfc = xmalloc(dirent_prec_psx_size(NAME_MAX + 1));
+	prec_dir->dirent_nfc->max_name_len = NAME_MAX + 1;
 
 	prec_dir->dirp = opendir(dirname);
 	if (!prec_dir->dirp) {
@@ -145,8 +150,7 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir)
 		int ret_errno = errno;
 
 		if (new_maxlen > prec_dir->dirent_nfc->max_name_len) {
-			size_t new_len = sizeof(dirent_prec_psx) + new_maxlen -
-				sizeof(prec_dir->dirent_nfc->d_name);
+			size_t new_len = dirent_prec_psx_size(new_maxlen);
 
 			prec_dir->dirent_nfc = xrealloc(prec_dir->dirent_nfc, new_len);
 			prec_dir->dirent_nfc->max_name_len = new_maxlen;
diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index fea06cf..c7c3cc2 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -14,11 +14,12 @@ typedef struct dirent_prec_psx {
 
 	/*
 	 * See http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/dirent.h.html
-	 * NAME_MAX + 1 should be enough, but some systems have
-	 * NAME_MAX=255 and strlen(d_name) may return 508 or 510
-	 * Solution: allocate more when needed, see precompose_utf8_readdir()
+	 * Start with room for NAME_MAX + 1 bytes, but keep d_name as a
+	 * flexible array. Some systems have NAME_MAX=255 while strlen(d_name)
+	 * from readdir() may return 508 or 510 bytes. Grow the allocation as
+	 * needed in precompose_utf8_readdir().
 	 */
-	char   d_name[NAME_MAX+1];
+	char   d_name[FLEX_ARRAY];
 } dirent_prec_psx;
 
 
diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh
index 6d5918c..ea75fb4 100755
--- a/t/t3910-mac-os-precompose.sh
+++ b/t/t3910-mac-os-precompose.sh
@@ -207,6 +207,22 @@ test_expect_success "Add long precomposed filename" '
 	git commit -m "Long filename"
 '
 
+test_expect_success "status with long non-ASCII filename" '
+	test_when_finished "rm -rf long-utf8-status" &&
+	git init long-utf8-status &&
+	(
+		cd long-utf8-status &&
+		test "$(git config --bool core.precomposeunicode)" = true &&
+		long_utf8_name=$(
+			printf "%253s\342\200\224" "" |
+			tr " " a
+		) &&
+		test "$(printf "%s" "$long_utf8_name" | wc -c | tr -d " ")" = 256 &&
+		printf "content\n" >"$long_utf8_name" &&
+		git status --porcelain=v1 >actual
+	)
+'
+
 test_expect_failure 'handle existing decomposed filenames' '
 	echo content >"verbatim.$Adiarnfd" &&
 	git -c core.precomposeunicode=false add "verbatim.$Adiarnfd" &&
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH 0/2] small leak fix in format-patch
From: Karthik Nayak @ 2026-07-04 21:13 UTC (permalink / raw)
  To: Jeff King, git; +Cc: Patrick Steinhardt, Kaartic Sivaraam
In-Reply-To: <20260630063944.GA3733670@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 341 bytes --]

Jeff King <peff@peff.net> writes:

> This fixes a leak I found while discussing an unrelated leak in another
> thread[1]. As a bonus, this fixes some minor recent breakage of
> leak-reporting when running the test suite under prove. The patches can
> be split into separate topics if we want.

I think you meant to CC the other "Kaartic" :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

^ permalink raw reply

* [PATCH v4 2/2] Makefile: support universal macOS builds via RUST_TARGETS
From: Shardul Natu via GitGitGadget @ 2026-07-04 18:05 UTC (permalink / raw)
  To: git; +Cc: Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru, Shardul Natu
In-Reply-To: <pull.2288.v4.git.git.1783188355.gitgitgadget@gmail.com>

From: Shardul Natu <snatu@google.com>

On macOS, Universal Binaries contain native executable code for
multiple architectures (such as Intel x86_64 and Apple Silicon arm64)
bundled into a single file. This is standard practice for macOS
distribution and CI packaging (such as internal distribution packages
or tooling like Burrito/Homebrew), allowing a single build artifact
to run natively across all Macs without Rosetta emulation or
maintaining separate packages.

When building Git C code for multiple architectures on macOS, the
Apple toolchain (clang) natively supports universal builds via
CFLAGS/LDFLAGS. When "-arch x86_64 -arch arm64" is passed, clang
automatically compiles and links universal binaries for all C object
files and executables out of the box.

Cargo and rustc, however, do not support multiple "-arch" flags or
emitting universal binaries in a single invocation. Instead, Cargo
requires invoking each target triple independently (e.g., passing
"--target x86_64-apple-darwin" and "--target aarch64-apple-darwin").

To bridge this gap when Rust is enabled:
  1. Allow specifying space-separated target triples in RUST_TARGETS.
  2. Introduce declarative pattern rules (target/%/...) to compile
     each target-specific library slice via Cargo.
  3. On macOS, if multiple targets are specified, use "lipo" (part of
     the mandatory Xcode Command Line Tools) to combine the resulting
     static libraries into target/release/libgitcore.a.
  4. Ensure target directory creation before invoking lipo via
     mkdir_p_parent_template.

Once $(RUST_LIB) is compiled into a universal static archive, the
standard C linker seamlessly links it with the C object files to
produce universal Git executables.

Signed-off-by: Shardul Natu <snatu@google.com>
---
 Makefile | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 7db38ecce9..ecada0acb4 100644
--- a/Makefile
+++ b/Makefile
@@ -500,6 +500,14 @@ include shared.mak
 #
 # Building Rust code requires Cargo.
 #
+# Define RUST_TARGETS if you want to cross-compile. If left unspecified, it uses
+# the default rust target on the system.
+#
+# On macOS, this supports specifying multiple targets, separated by a space.
+# This will produce a Universal static library using `lipo`.
+#
+# Example: RUST_TARGETS="aarch64-apple-darwin x86_64-apple-darwin"
+#
 # == SHA-1 and SHA-256 defines ==
 #
 # === SHA-1 backend ===
@@ -941,16 +949,17 @@ LIB_FILE = libgit.a
 
 ifndef NO_RUST
 ifdef DEBUG
-RUST_TARGET_DIR = target/debug
+RUST_BUILD_CONFIG = debug
 else
-RUST_TARGET_DIR = target/release
+RUST_BUILD_CONFIG = release
 endif
 
 ifeq ($(uname_S),Windows)
-RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib
+RUST_LIB_NAME = gitcore.lib
 else
-RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a
+RUST_LIB_NAME = libgitcore.a
 endif
+RUST_LIB = target/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME)
 endif
 
 GITLIBS = common-main.o $(LIB_FILE)
@@ -3022,8 +3031,30 @@ $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 
 ifndef NO_RUST
+ifeq ($(RUST_TARGETS),)
 $(RUST_LIB): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
 	$(QUIET_CARGO)cargo build $(CARGO_ARGS)
+else
+ifneq ($(words $(RUST_TARGETS)),1)
+ifneq ($(uname_S),Darwin)
+$(error Building universal Rust libraries requires macOS (lipo is not available on $(uname_S)))
+endif
+endif
+
+RUST_MEMBER_LIBS = $(foreach target,$(RUST_TARGETS),target/$(target)/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME))
+$(RUST_MEMBER_LIBS): target/%/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
+	$(QUIET_CARGO)cargo build $(CARGO_ARGS) --target $*
+
+$(RUST_LIB): $(RUST_MEMBER_LIBS)
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN)\
+	if test $(words $(RUST_TARGETS)) -gt 1; \
+	then \
+		lipo -create $^ -output $@; \
+	else \
+		cp $< $@; \
+	fi
+endif
 
 .PHONY: rust
 rust: $(RUST_LIB)
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v4 1/2] Makefile: add $(RUST_LIB) prerequisite to osxkeychain
From: Shardul Natu via GitGitGadget @ 2026-07-04 18:05 UTC (permalink / raw)
  To: git; +Cc: Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru, Shardul Natu
In-Reply-To: <pull.2288.v4.git.git.1783188355.gitgitgadget@gmail.com>

From: Shardul Natu <snatu@google.com>

When Rust is enabled, the git-credential-osxkeychain helper depends on
Rust symbols compiled into $(RUST_LIB). While commit 522ea8ef7d
("osxkeychain: fix build with Rust") updated the linker command line to
use $(LIBS), it omitted $(RUST_LIB) from the target prerequisite list.
Without this prerequisite, running a parallel build ("make -j") from a
clean working tree can fail because Make does not know to invoke Cargo
to build libgitcore.a before linking git-credential-osxkeychain.

Add $(RUST_LIB) as a prerequisite dependency to the
git-credential-osxkeychain target.

Additionally, wrap the definitions of $(RUST_LIB) and the "rust" build
target in "ifndef NO_RUST". This ensures that when NO_RUST=1 is
specified, $(RUST_LIB) evaluates to empty, making the Rust dependency a
clean no-op without needing intermediate variables.

Signed-off-by: Shardul Natu <snatu@google.com>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1f3f099f5c..7db38ecce9 100644
--- a/Makefile
+++ b/Makefile
@@ -939,6 +939,7 @@ TEST_SHELL_PATH = $(SHELL_PATH)
 
 LIB_FILE = libgit.a
 
+ifndef NO_RUST
 ifdef DEBUG
 RUST_TARGET_DIR = target/debug
 else
@@ -950,6 +951,7 @@ RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib
 else
 RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a
 endif
+endif
 
 GITLIBS = common-main.o $(LIB_FILE)
 EXTLIBS =
@@ -3019,11 +3021,13 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 
+ifndef NO_RUST
 $(RUST_LIB): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
 	$(QUIET_CARGO)cargo build $(CARGO_ARGS)
 
 .PHONY: rust
 rust: $(RUST_LIB)
+endif
 
 export DEFAULT_EDITOR DEFAULT_PAGER
 
@@ -4074,7 +4078,8 @@ $(LIBGIT_HIDDEN_EXPORT): $(LIBGIT_PARTIAL_EXPORT)
 contrib/libgit-sys/libgitpub.a: $(LIBGIT_HIDDEN_EXPORT)
 	$(AR) $(ARFLAGS) $@ $^
 
-contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) GIT-LDFLAGS
+# When Rust is enabled, git-credential-osxkeychain depends on Rust symbols in $(RUST_LIB)
+contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) $(RUST_LIB) GIT-LDFLAGS
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
 		$(filter %.o,$^) $(LIBS) -framework Security -framework CoreFoundation
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v4 0/2] Makefile: link osxkeychain helper against Rust
From: Shardul Natu via GitGitGadget @ 2026-07-04 18:05 UTC (permalink / raw)
  To: git; +Cc: Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru
In-Reply-To: <pull.2288.v3.git.git.1783030971.gitgitgadget@gmail.com>

This series improves macOS build reliability and distribution support when
Rust is enabled in the Git build system. It addresses two distinct
challenges: a parallel build race condition in git-credential-osxkeychain
and support for macOS Universal Binaries (multi-architecture distribution).


Why This Series is Needed
=========================

 1. Parallel Build Race Condition ("make -j"): While commit 522ea8ef7d
    ("osxkeychain: fix build with Rust") updated the link command for
    git-credential-osxkeychain to pass $(LIBS), it omitted $(RUST_LIB) from
    the target prerequisite list. When running a parallel build ("make -j")
    from a clean working tree, Make can attempt to link
    git-credential-osxkeychain before Cargo has finished compiling
    libgitcore.a, causing linker failures.

 2. macOS Universal Binary (lipo) Support: On macOS, Universal Binaries
    bundle native executable code for multiple architectures (Intel x86_64
    and Apple Silicon arm64) into a single file. This is standard practice
    for macOS distribution and CI packaging (such as Burrito, Homebrew, and
    Git's macOS CI runners), allowing a single artifact to run natively
    across all Macs without Rosetta translation.

While Apple's C compiler (clang) natively supports universal builds by
passing "-arch x86_64 -arch arm64" in CFLAGS and LDFLAGS, Cargo and rustc do
not support multiple "-arch" flags in a single invocation. Instead, Cargo
must be invoked separately for each target triple ("--target
x86_64-apple-darwin" and "--target aarch64-apple-darwin"). This series
bridges that gap.


Overview of Patches
===================

 * Patch 1: Makefile: add $(RUST_LIB) prerequisite to osxkeychain Adds
   $(RUST_LIB) as a prerequisite dependency to the osxkeychain target,
   eliminating the parallel build race condition. Additionally, wraps the
   definitions of $(RUST_LIB) and the "rust" build target in "ifndef
   NO_RUST" so that disabling Rust cleanly makes the dependency a no-op.

 * Patch 2: Makefile: support universal macOS builds via RUST_TARGETS Allows
   users to specify space-separated target triples in RUST_TARGETS.
   Introduces declarative pattern rules (target/%/...) to compile each
   target slice via Cargo, and uses "lipo" (part of the mandatory Xcode
   Command Line Tools) to combine the resulting static archives into a
   universal library at target/release/libgitcore.a. Uses
   mkdir_p_parent_template to guarantee directory creation before lipo.

Changes since v2:

 * Split the original combined commit into a two-patch series to separate
   prerequisite bug fixes from Universal Binary features.
 * Added $(call mkdir_p_parent_template) prior to invoking lipo to guarantee
   that parent target directories exist.
 * 

Shardul Natu (2):
  Makefile: add $(RUST_LIB) prerequisite to osxkeychain
  Makefile: support universal macOS builds via RUST_TARGETS

 Makefile | 46 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)


base-commit: 602f6c329a7d99df269d382df353b4e1bbbbd8aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2288%2Fkiranani%2Fnext-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2288/kiranani/next-v4
Pull-Request: https://github.com/git/git/pull/2288

Range-diff vs v3:

 1:  41de7d391a = 1:  41de7d391a Makefile: add $(RUST_LIB) prerequisite to osxkeychain
 2:  257f5ef42f ! 2:  88fc2e0bd8 Makefile: support universal macOS builds via RUST_TARGETS
     @@ Makefile: $(LIB_FILE): $(LIB_OBJS)
      +	$(QUIET_CARGO)cargo build $(CARGO_ARGS) --target $*
      +
      +$(RUST_LIB): $(RUST_MEMBER_LIBS)
     -+	@$(call mkdir_p_parent_template)
     ++	$(call mkdir_p_parent_template)
      +	$(QUIET_GEN)\
     -+	if [ $(words $(RUST_TARGETS)) -gt 1 ]; then \
     ++	if test $(words $(RUST_TARGETS)) -gt 1; \
     ++	then \
      +		lipo -create $^ -output $@; \
      +	else \
      +		cp $< $@; \

-- 
gitgitgadget

^ permalink raw reply

* Re: [PATCH v4 8/8] doc: promisor: improve acceptFromServer entry
From: Kristoffer Haugsbakk @ 2026-07-04  9:49 UTC (permalink / raw)
  To: Christian Couder, git
  Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
	Elijah Newren, Toon Claes, Christian Couder
In-Reply-To: <20260527140820.1438165-9-christian.couder@gmail.com>

On Wed, May 27, 2026, at 16:08, Christian Couder wrote:
> The entry for the `promisor.acceptFromServer` in
> "Documentation/config/promisor.adoc" has a number of issues:

(This series is now in `next` so this is not a review comment)

>[snip]
>  Documentation/config/promisor.adoc | 53 ++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/config/promisor.adoc
> b/Documentation/config/promisor.adoc
> index 455ce40be8..f07a2e883b 100644
> --- a/Documentation/config/promisor.adoc
> +++ b/Documentation/config/promisor.adoc
> @@ -32,24 +32,41 @@ variable is set to "true", and the "name" and "url"
> fields are always
>  advertised regardless of this setting.
>[snip]
> ++
> +The available options are:
> ++
> +* `none` (default): No promisor remote advertised by a server will be
> +  accepted.

Why did you use an unordered/bullet list instead of a description list?

> ++
> +* `knownUrl`: The client will accept promisor remotes that are already
> +  configured on the client and have both the same name and the same URL
> +  as advertised by the server. This is more secure than `all` or
> +  `knownName`, and should be used if possible instead of those options.
> ++
> +* `knownName`: The client will accept promisor remotes that are already
> +  configured on the client and have the same name as those advertised
> +  by the server. This is not very secure, but could be used in a corporate
> +  setup where servers and clients are trusted to not switch names and URLs.
> ++
> +* `all`: The client will accept all the promisor remotes a server might
> +  advertise. This is the least secure option and should only be used in
> +  fully trusted environments.
> ++
> +Name and URL comparisons are case-sensitive. See linkgit:gitprotocol-v2[5]
> +for protocol details.
>
>  promisor.acceptFromServerUrl::
>  	A glob pattern to specify which server-advertised URLs a
> --
> 2.54.0.275.g96c817d129.dirty

^ permalink raw reply

* Re: [PATCH 08/13] submodule: fix cwd leak in get_superproject_working_tree()
From: Johannes Schindelin @ 2026-07-04  8:59 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Johannes Schindelin via GitGitGadget, git
In-Reply-To: <akTINO_S_NgWbGxG@pks.im>

Hi Patrick,

On Wed, 1 Jul 2026, Patrick Steinhardt wrote:

> On Wed, Jul 01, 2026 at 07:04:26AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/submodule.c b/submodule.c
> > index fd91201a92..8ddeebd8af 100644
> > --- a/submodule.c
> > +++ b/submodule.c
> > @@ -2627,10 +2627,10 @@ int get_superproject_working_tree(struct strbuf *buf)
> >  		 * We might have a superproject, but it is harder
> >  		 * to determine.
> >  		 */
> > -		return 0;
> > +		goto out;
> >  
> >  	if (!strbuf_realpath(&one_up, "../", 0))
> > -		return 0;
> > +		goto out;
> >  
> >  	subpath = relative_path(cwd, one_up.buf, &sb);
> >  	strbuf_release(&one_up);
> > @@ -2693,6 +2693,10 @@ int get_superproject_working_tree(struct strbuf *buf)
> >  		die(_("ls-tree returned unexpected return code %d"), code);
> >  
> >  	return ret;
> > +
> > +out:
> > +	free(cwd);
> > +	return 0;
> >  }
> 
> Okay. This is fine, but it feels a bit fragile as we also have a call to
> `free(cwd)` a bit further up. So if somebody were to add a `goto out`
> after that call we'd have a double free. Makes me wonder whether we want
> to have a single exit path for the complete function and then drop the
> other call to free(3p).

Agreed. In v2 the function has a single exit path: all late returns
fall through to the `out:` label, which additionally releases `sb`
and `one_up`.

A side effect worth noting is that consolidation also closes a latent
leak the original had on the `strbuf_realpath(&one_up, "../", 0)`
failure path. `strbuf_realpath_1()` calls `strbuf_reset(resolved)` on
error, which does not free the backing buffer, so `one_up` could
carry a residual allocation that the previous shape never released.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 07/13] dir: free allocations on parse-error paths in read_one_dir()
From: Johannes Schindelin @ 2026-07-04  8:58 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Johannes Schindelin via GitGitGadget, git
In-Reply-To: <akTIOeXFhNjJ7V3i@pks.im>

Hi Patrick,

On Wed, 1 Jul 2026, Patrick Steinhardt wrote:

> On Wed, Jul 01, 2026 at 07:04:25AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/dir.c b/dir.c
> > index 32430090dc..23335b9f7a 100644
> > --- a/dir.c
> > +++ b/dir.c
> > @@ -3792,13 +3792,18 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,
> >  		ALLOC_ARRAY(ud.untracked, ud.untracked_nr);
> >  
> >  	ud.dirs_alloc = ud.dirs_nr = decode_varint(&data);
> > -	if (data > end)
> > +	if (data > end) {
> > +		free(ud.untracked);
> >  		return -1;
> > +	}
> >  	ALLOC_ARRAY(ud.dirs, ud.dirs_nr);
> >  
> >  	eos = memchr(data, '\0', end - data);
> > -	if (!eos || eos == end)
> > +	if (!eos || eos == end) {
> > +		free(ud.untracked);
> > +		free(ud.dirs);
> >  		return -1;
> > +	}
> >  
> >  	*untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), eos - data, 1));
> >  	memcpy(untracked, &ud, sizeof(ud));
> 
> Hm. Here we assign ownership to the caller, but this still feels quite
> off to me as we also have two more early returns after this point that
> seem to leak memory. Do the callers make sure to always free the data?

Ownership transfers to the caller on the `xmalloc`/`memcpy` line: the
`memcpy` copies the `ud.untracked` and `ud.dirs` pointers into the freshly
xmalloc'd struct that becomes `*untracked_`. From there, any subsequent
failure in the caller reaches `free_untracked_cache()` and then
`free_untracked()`, which releases both arrays. So the two further early
returns are correct as-are.

I will fold that reasoning into the v2 commit message so a future
reader does not have to re-derive it.

Incidentally, and orthogonal to Coverity's leak report: on those same
failure paths, individual slots of `->dirs` and `->untracked` remain
uninitialised, so `free_untracked()` walks garbage pointers before it
ever reaches the two `free()` calls above. That is a separate
crash-on-cleanup bug and I would prefer to address it in a follow-up
rather than widen the scope of this series.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 05/13] run_diff_files: avoid memory leak
From: Johannes Schindelin @ 2026-07-04  8:58 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Johannes Schindelin via GitGitGadget, git
In-Reply-To: <akTIMM6qLfDNdg-a@pks.im>

Hi Patrick,

On Wed, 1 Jul 2026, Patrick Steinhardt wrote:

> On Wed, Jul 01, 2026 at 07:04:23AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/diff-lib.c b/diff-lib.c
> > index ae91027a02..7ba839b4a8 100644
> > --- a/diff-lib.c
> > +++ b/diff-lib.c
> > @@ -152,7 +152,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
> >  			continue;
> >  
> >  		if (ce_stage(ce)) {
> > -			struct combine_diff_path *dpath;
> > +			struct combine_diff_path *dpath = NULL;
> >  			struct diff_filepair *pair;
> >  			unsigned int wt_mode = 0;
> >  			int num_compare_stages = 0;
> > @@ -164,6 +164,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
> >  			else {
> >  				if (changed < 0) {
> >  					perror(ce->name);
> > +					free(dpath);
> >  					continue;
> >  				}
> >  				wt_mode = 0;
> 
> Huh. There is no assignment between the variable declaration and this
> call to `continue`, so how could this ever plug a memory leak? None of
> the other paths seem to leak the variable, either.

You are right; the patch as posted plugs nothing.

The reason it looks pointless is that the leak it was written against was
fixed independently in the meantime by 949bb8f74f4a (run_diff_files():
delay allocation of combine_diff_path, 2025-01-09), which moved the `dpath
= xmalloc(...)` to after the `check_removed()` call. Before that
reordering, the two `continue` statements did leak the just-allocated
`dpath` (originally introduced by 4fc970c43884, 2007-02-25).

I had missed that when picking back up the work on addressing Coverity
reports, sorry! I will drop this patch from v2.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 01/13] load_one_loose_object_map(): fix resource leak
From: Johannes Schindelin @ 2026-07-04  8:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git
In-Reply-To: <xmqqcxx6pths.fsf@gitster.g>

Hi Junio,

On Wed, 1 Jul 2026, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > +	ret = 0;
> 
> Or we can do
> 
> 	ret = ferror(fp) ? -1 : 0;
> 
> if we want to be sure that we have caught all the errors.

Agreed; that is what v2 will use.

To corroborate the diagnosis: `strbuf_getline_lf()` ultimately calls
`getdelim()`, which returns -1 on both EOF and I/O error, so `ferror(fp)`
on the underlying stream is the only reliable way to distinguish the two.
That also makes the `errno = 0;` I had added at the top of the loop dead,
so it goes away in v2.

Ciao,
Johannes

^ permalink raw reply

* [PATCH v7 3/3] graph: indent visual root in graph
From: Pablo Sabater @ 2026-07-04  8:52 UTC (permalink / raw)
  To: git
  Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
	christian.couder, gitster, jltobler, karthik.188, krka, peff,
	phillip.wood, siddharthasthana31
In-Reply-To: <20260704-ps-pre-commit-indent-v7-0-a94706cc8376@gmail.com>

When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.

A fix for this issue was already attempted [1] a while ago.

This happens because the commits fill the space from left to right and
when a visual root ends, its column becomes free for the following
commit even if they are not related. Once this happens the unrelated
commit is rendered below the visual root. Because there is no special
character or way to identify when a visual root is rendered making the
graph confusing.

By indenting the visual roots when there are still commits to show the
vertical adjacency can be avoided.

Add is_visual_root flag to git_graph making it visible in all graph states,
give graph_update() a new function, graph_is_visual_root() to know if the
current commit is a visual root and set is_visual_root.
The different handled cases are:

- If a visual root has children: similar to GRAPH_PRE_COMMIT state when
  octopus merges need space, an edge row needs to be printed to connect
  the child with the indented visual root. A new state GRAPH_PRE_ROOT is
  needed to connect the child with the visual root:

    * child of the visual root
     \ GRAPH_PRE_ROOT
      * visual root indented

- If a visual root is child-less we can skip GRAPH_PRE_ROOT state and
  render the indented commit directly.

      * visual root indented
    * unrelated commit

- If two or more visual roots are adjacent: by having a lookahead to the
  next commit that will be rendered, if the next commit is also a visual
  root and we are on a visual root, meaning two visual root adjacent in
  the history, the top one can omit the indent, making the one below to
  indent only once, if there are more adjacent visual commits, the
  indentation will increase for each adjacent one, cascading.

    * visual root
      * visual root
        * visual root
    * last commit

  Even if the last commit is a root, because there is nothing that will be
  rendered below we can omit the indentation on purpose.

[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/

Help-by: Kristofer Karlsson <krka@spotify.com>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c                          | 235 ++++++++++++++++++++
 t/meson.build                    |   1 +
 t/t4218-log-graph-indentation.sh | 453 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 689 insertions(+)

diff --git a/graph.c b/graph.c
index 300ae67669..75162cea23 100644
--- a/graph.c
+++ b/graph.c
@@ -60,12 +60,23 @@ struct column {
 	 * index into column_colors.
 	 */
 	unsigned short color;
+	/*
+	 * Marks if a commit is a non-first parent of a merge. These columns are
+	 * already visually connected to the merge commit and do not need
+	 * indentation.
+	 *
+	 * The first parent is the one that inherits the column and it can need
+	 * indentation if turns out to be a visual root and there's still
+	 * commits to render.
+	 */
+	unsigned is_merge_parent:1;
 };
 
 enum graph_state {
 	GRAPH_PADDING,
 	GRAPH_SKIP,
 	GRAPH_PRE_COMMIT,
+	GRAPH_PRE_ROOT,
 	GRAPH_COMMIT,
 	GRAPH_POST_MERGE,
 	GRAPH_COLLAPSING
@@ -323,6 +334,51 @@ struct git_graph {
 	 */
 	struct commit *lookahead[2];
 	int lookahead_nr;
+
+	/*
+	 * If a commit is a visual root, we need to indent it to prevent
+	 * unrelated commits from being vertically adjacent to it.
+	 */
+	unsigned is_visual_root:1;
+
+	/*
+	 * Indentation increases for each visual root adjacent to another visual
+	 * root, making visual root commits indentation cascade.
+	 */
+	unsigned int visual_root_depth;
+
+	/*
+	 * When a visual root is adjacent to other visual roots, the first one
+	 * can avoid indentation and the rest cascades, increasing the indentation
+	 * for each one.
+	 */
+	unsigned visual_root_cascade:1;
+
+	/*
+	 * Set when the current commit was already present in graph->columns
+	 * before being processed.
+	 */
+	unsigned commit_in_columns:1;
+};
+
+struct graph_lookahead_flags {
+
+	/*
+	 * Set when there will be a commit after the current one that will be
+	 * rendered.
+	 */
+	unsigned int is_next_visible:1;
+
+	/*
+	 * Set when the next visible commit is candidate to be a visual root.
+	 */
+	unsigned int is_next_visual_root:1;
+
+	/*
+	 * Set when the next visible commit will be rendered under the current
+	 * commit.
+	 */
+	unsigned int next_has_column:1;
 };
 
 static inline int graph_needs_truncation(struct git_graph *graph, int lane)
@@ -399,6 +455,8 @@ struct git_graph *graph_init(struct rev_info *opt)
 	graph->lookahead[0] = NULL;
 	graph->lookahead[1] = NULL;
 	graph->lookahead_nr = 0;
+	graph->visual_root_depth = 0;
+	graph->visual_root_cascade = 0;
 	/*
 	 * Start the column color at the maximum value, since we'll
 	 * always increment it for the first commit we output.
@@ -581,6 +639,11 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 					  struct commit *commit,
 					  int idx)
 {
+	/*
+	 * Get the initial merge_layout before it's modified to know if this
+	 * is a merge.
+	 */
+	int initial_merge_layout = graph->merge_layout;
 	int i = graph_find_new_column_by_commit(graph, commit);
 	int mapping_idx;
 
@@ -592,6 +655,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 		i = graph->num_new_columns++;
 		graph->new_columns[i].commit = commit;
 		graph->new_columns[i].color = graph_find_commit_color(graph, commit);
+		graph->new_columns[i].is_merge_parent = 0;
 	}
 
 	if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -630,6 +694,12 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 	}
 
 	graph->mapping[mapping_idx] = i;
+
+	/*
+	 * Mark non-first parents of a merge.
+	 */
+	if (graph->num_parents > 1 && initial_merge_layout >= 0 && idx > -1)
+		graph->new_columns[i].is_merge_parent = 1;
 }
 
 static void graph_update_columns(struct git_graph *graph)
@@ -721,10 +791,20 @@ static void graph_update_columns(struct git_graph *graph)
 			if (graph->num_parents == 0)
 				graph->width += 2;
 		} else {
+			int j;
 			graph_insert_into_new_columns(graph, col_commit, -1);
+			/*
+			 * This column is not the current commit, but we need to
+			 * propagate the flag until the commit is processed.
+			 */
+			j = graph_find_new_column_by_commit(graph, col_commit);
+			if (j >= 0 && graph->columns[i].is_merge_parent)
+				graph->new_columns[j].is_merge_parent = 1;
 		}
 	}
 
+	graph->commit_in_columns = is_commit_in_columns;
+
 	/*
 	 * If graph_max_lanes is set, cap the width
 	 */
@@ -810,9 +890,104 @@ void graph_push_lookahead(struct git_graph *graph, struct commit *c)
 	graph->lookahead[graph->lookahead_nr++] = c;
 }
 
+/*
+ * A commit can be a visual root when:
+ *
+ * - It has no parents.
+ *
+ * - It has parents but they are all filtered out and
+ *   commit->parents arrives NULL.
+ *
+ * - It is not a boundary commit. Boundary commits also have no visible
+ *   parents, but they are not selected as visual roots because they cannot
+ *   cause the ambiguity of being vertically adjacent because:
+ *
+ *   1. A boundary only appears because an included commit is its child.
+ *      Children are always above, and the renderer draws an edge down to
+ *      the boundary from that child. Rather than starting a column like a
+ *      visual root would do, it inherits its child column.
+ *
+ *   2. Included commits cannot appear below a boundary. Boundaries are
+ *      ancestors of the exclusion point; if an included commit were an
+ *      ancestor of the boundary it would be excluded and not rendered.
+ *      Boundaries therefore always sink to the bottom.
+ */
+static int graph_is_visual_root_candidate(struct commit *c)
+{
+	return c->parents == NULL && !(c->object.flags & BOUNDARY);
+}
+
+static int graph_is_visual_root(struct git_graph *graph,
+				struct graph_lookahead_flags *flags)
+{
+	/*
+	 * This must be only called for the current commit as graph contains
+	 * the state for the current commit only.
+	 *
+	 * To check if a commit is a visual root, call graph_is_visual_root_candidate()
+	 * but we won't know if it is really a visual root until we get to the
+	 * next commit state.
+	 *
+	 * The current commit is an actual visual root if it is a candidate and
+	 * the commit is not a non-first parent of a merge.
+	 *
+	 *   *
+	 *   |\
+	 *   | *    <- it is a visual root candidate but it shouldn't be indented
+	 *   *         because it is already connected by an edge.
+	 *   ^         if commit_in_columns && is_merge_parent means the commit
+	 *   |         was put by a merge and is connected.
+	 *   |
+	 *   `-------- if !is_next_visible means we're on the last commit, avoid
+	 *             indentation unless the one before is a visual root, then
+	 *             we need to differentiate from the one above.
+	 *
+	 * If next_has_columns means that the next commit has
+	 * already a column, so it will not be rendered below, the
+	 * current commit has to act as the last commit and omit
+	 * indentation.
+	 */
+	return graph_is_visual_root_candidate(graph->commit) &&
+	       !(graph->commit_in_columns &&
+		 graph->columns[graph->commit_index].is_merge_parent) &&
+	       flags->is_next_visible &&
+	       (!flags->next_has_column || graph->visual_root_depth > 0);
+}
+
+/*
+ * Peeks the next commits via the lookahead buffer and sets the lookahead flags.
+ */
+static void graph_peek_next_visible(struct git_graph *graph,
+				    struct graph_lookahead_flags *flags)
+{
+	flags->is_next_visible = 0;
+	flags->is_next_visual_root = 0;
+	flags->next_has_column = 0;
+
+	if (!graph->lookahead_nr)
+		return;
+
+	flags->is_next_visible = 1;
+	flags->next_has_column =
+		graph_find_new_column_by_commit(graph, graph->lookahead[0]) >= 0;
+
+	if (!graph_is_visual_root_candidate(graph->lookahead[0]))
+		return;
+
+	if (graph->lookahead_nr >= 2)
+		flags->is_next_visual_root = 1;
+}
+
+static int graph_needs_pre_root_line(struct git_graph *graph)
+{
+	return graph->commit_in_columns && graph->is_visual_root &&
+	       graph->num_columns > 0 && !graph->visual_root_cascade;
+}
+
 void graph_update(struct git_graph *graph, struct commit *commit)
 {
 	struct commit_list *parent;
+	struct graph_lookahead_flags flags;
 
 	/*
 	 * Set the new commit
@@ -843,6 +1018,23 @@ void graph_update(struct git_graph *graph, struct commit *commit)
 	 */
 	graph_update_columns(graph);
 
+	graph_peek_next_visible(graph, &flags);
+
+	graph->is_visual_root = graph_is_visual_root(graph, &flags);
+
+	if (graph->is_visual_root) {
+		/*
+		 * If next is a visual root we can omit the indent for the first
+		 * visual root and start cascading.
+		 */
+		if (!graph->visual_root_depth && flags.is_next_visual_root)
+			graph->visual_root_cascade = 1;
+		graph->visual_root_depth++;
+	} else {
+		graph->visual_root_depth = 0;
+		graph->visual_root_cascade = 0;
+	}
+
 	graph->expansion_row = 0;
 
 	/*
@@ -860,11 +1052,16 @@ void graph_update(struct git_graph *graph, struct commit *commit)
 	 * room for it.  We need to do this only if there is a branch row
 	 * (or more) to the right of this commit.
 	 *
+	 * If it is a visual root, we need to print an extra row to
+	 * connect the indentation.
+	 *
 	 * If there are less than 3 parents, we can immediately print the
 	 * commit line.
 	 */
 	if (graph->state != GRAPH_PADDING)
 		graph->state = GRAPH_SKIP;
+	else if (graph_needs_pre_root_line(graph))
+		graph->state = GRAPH_PRE_ROOT;
 	else if (graph_needs_pre_commit_line(graph))
 		graph->state = GRAPH_PRE_COMMIT;
 	else
@@ -1112,6 +1309,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 		if (col_commit == graph->commit) {
 			seen_this = 1;
+			if (graph->is_visual_root) {
+				int depth = graph->visual_root_depth;
+				/*
+				 * Each visual column is 2 characters wide.
+				 * Omit the indentation for the first visual
+				 * root in cascade mode.
+				 */
+				int padding = (depth - graph->visual_root_cascade) * 2;
+				graph_line_addchars(line, ' ', padding);
+				graph->width += padding;
+			}
 			graph_output_commit_char(graph, line);
 
 			if (graph_needs_truncation(graph, i)) {
@@ -1483,6 +1691,30 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 		graph_update_state(graph, GRAPH_PADDING);
 }
 
+static void graph_output_pre_root_line(struct git_graph *graph, struct graph_line *line)
+{
+	/*
+	 * This function adds a row before a visual root, to connect the
+	 * branch to the indented commit. It must only be called on a
+	 * visual root.
+	 */
+	if (!graph->is_visual_root)
+		BUG("commit must be a visual root to call pre_root_line");
+
+	for (size_t i = 0; i < graph->num_columns; i++) {
+		struct column *col = &graph->columns[i];
+		if (col->commit == graph->commit) {
+			graph_line_addch(line, ' ');
+			graph_line_write_column(line, col, '\\');
+		} else {
+			graph_line_write_column(line, col, '|');
+		}
+		graph_line_addch(line, ' ');
+	}
+
+	graph_update_state(graph, GRAPH_COMMIT);
+}
+
 int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 {
 	int shown_commit_line = 0;
@@ -1508,6 +1740,9 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 	case GRAPH_PRE_COMMIT:
 		graph_output_pre_commit_line(graph, &line);
 		break;
+	case GRAPH_PRE_ROOT:
+		graph_output_pre_root_line(graph, &line);
+		break;
 	case GRAPH_COMMIT:
 		graph_output_commit_line(graph, &line);
 		shown_commit_line = 1;
diff --git a/t/meson.build b/t/meson.build
index 3219264fe7..6093ff469b 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -576,6 +576,7 @@ integration_tests = [
   't4215-log-skewed-merges.sh',
   't4216-log-bloom.sh',
   't4217-log-limit.sh',
+  't4218-log-graph-indentation.sh',
   't4252-am-options.sh',
   't4253-am-keep-cr-dos.sh',
   't4254-am-corrupt.sh',
diff --git a/t/t4218-log-graph-indentation.sh b/t/t4218-log-graph-indentation.sh
new file mode 100755
index 0000000000..c70dab384f
--- /dev/null
+++ b/t/t4218-log-graph-indentation.sh
@@ -0,0 +1,453 @@
+#!/bin/sh
+
+test_description='git log --graph visual root indentations'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
+
+check_graph_with_description () {
+	cat >expect &&
+	lib_test_cmp_graph --format="%s%ndescription%nsecond-line" "$@"
+}
+
+create_orphan () {
+	git checkout --orphan "$1" &&
+	{ git rm -rf . || true; }
+}
+
+# disable commit-graph topo order to have the graph to render in different
+# ways (used in --first-parent tests to have multiple visual roots while a
+# column is active at the same time).
+unset_commit_graph() {
+	sane_unset GIT_TEST_COMMIT_GRAPH &&
+	rm -f .git/objects/info/commit-graph &&
+	rm -rf .git/objects/info/commit-graphs
+}
+
+test_expect_success 'single root commit is not indented' '
+	create_orphan _1 && test_commit 1_A &&
+	lib_test_check_graph _1 <<-\EOF
+	* 1_A
+	EOF
+'
+
+test_expect_success 'visual root indented before unrelated branch' '
+	create_orphan _2 && test_commit 2_A && test_commit 2_B &&
+	create_orphan _3 && test_commit 3_A &&
+	lib_test_check_graph _2 _3 <<-\EOF
+	  * 3_A
+	* 2_B
+	* 2_A
+	EOF
+'
+
+test_expect_success 'visual root indentation with --left-right' '
+	lib_test_check_graph --left-right _2..._3 <<-\EOF
+	  > 3_A
+	< 2_B
+	< 2_A
+	EOF
+'
+
+# A better case of why indentation is still needed with '--left-right' flag is
+# that unrelated branches can be on the same side, so it's needed to
+# differentiate visual roots on the same side.
+test_expect_success 'visual root indentation with --left-right having unrelated commits on the same side' '
+	lib_test_check_graph --left-right _2..._3 _1 <<-\EOF
+	  > 3_A
+	< 2_B
+	 \
+	  < 2_A
+	> 1_A
+	EOF
+'
+
+test_expect_success 'visual root indents the description also' '
+	check_graph_with_description _2 _3 <<-\EOF
+	  * 3_A
+	    description
+	    second-line
+	* 2_B
+	| description
+	| second-line
+	* 2_A
+	  description
+	  second-line
+	EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child' '
+	create_orphan _4 && test_commit 4_A && test_commit 4_B &&
+	create_orphan _5 && test_commit 5_A && test_commit 5_B &&
+	lib_test_check_graph _4 _5<<-\EOF
+	* 5_B
+	 \
+	  * 5_A
+	* 4_B
+	* 4_A
+	EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child with description' '
+	check_graph_with_description _4 _5 <<-\EOF
+	* 5_B
+	| description
+	| second-line
+	 \
+	  * 5_A
+	    description
+	    second-line
+	* 4_B
+	| description
+	| second-line
+	* 4_A
+	  description
+	  second-line
+	EOF
+'
+
+test_expect_success 'visual roots cascade and last root does not' '
+	create_orphan _7 && test_commit 7_A && test_commit 7_B &&
+	create_orphan _8 && test_commit 8_A &&
+	create_orphan _9 && test_commit 9_A &&
+	create_orphan _10 && test_commit 10_A &&
+	lib_test_check_graph _7 _8 _9 _10  <<-\EOF
+	* 10_A
+	  * 9_A
+	    * 8_A
+	* 7_B
+	* 7_A
+	EOF
+'
+
+test_expect_success 'last root does not cascade' '
+	lib_test_check_graph _8 _9 _10 <<-\EOF
+	* 10_A
+	  * 9_A
+	* 8_A
+	EOF
+'
+
+test_expect_success 'merge parents are roots between them but they do not indent' '
+	create_orphan _11 && test_commit 11_A &&
+	create_orphan _12 && test_commit 12_A &&
+	create_orphan _13 && test_commit 13_A &&
+	git checkout _11 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _11 -p _12 -p _13 -m 11_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _11 <<-\EOF
+	*-.   11_octopus
+	|\ \
+	| | * 13_A
+	| * 12_A
+	* 11_A
+	EOF
+'
+
+# The last parent of a merge can be indented if nothing related to it needs to
+# be rendered after, if it's another visual root, merge parent must not get
+# indented but rather activate cascading.
+test_expect_success 'merge then unrelated visual root and unrelated branch' '
+	create_orphan _16 && test_commit 16_A && test_commit 16_B &&
+	create_orphan _17 && test_commit 17_A &&
+	create_orphan _18 && test_commit 18_A &&
+	create_orphan _19 && test_commit 19_A &&
+	create_orphan _20 && test_commit 20_A &&
+	git checkout _18 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _18 -p _19 -p _20 -m 18_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _18 _17 _16 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	* 18_A
+	  * 17_A
+	* 16_B
+	* 16_A
+	EOF
+'
+
+# The last commit root does not get indented, if the next thing after the root
+# merge parent is the last commit, indent the merge parent.
+test_expect_success 'merge then unrelated root indents merge parent' '
+	lib_test_check_graph _18 _17 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	 \
+	  * 18_A
+	* 17_A
+	EOF
+'
+
+test_expect_success 'merge then unrelated branch indents merge parent' '
+	lib_test_check_graph _18 _16 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	 \
+	  * 18_A
+	* 16_B
+	* 16_A
+	EOF
+'
+
+test_expect_success 'two-parent merge of orphans' '
+	create_orphan _21 && test_commit 21_A &&
+	create_orphan _22 && test_commit 22_A &&
+	git checkout _21 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _21 -p _22 -m 21_merge) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _21 <<-\EOF
+	*   21_merge
+	|\
+	| * 22_A
+	* 21_A
+	EOF
+'
+
+test_expect_success 'commit with filtered parent becomes a visual root' '
+	create_orphan _23 &&
+	echo test >other.txt &&
+	git add other.txt &&
+	git commit -m "23_A" &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "23_B" &&
+	create_orphan _24 &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "24_A" &&
+	lib_test_check_graph _23 _24 -- foo.txt <<-\EOF
+	  * 23_B
+	* 24_A
+	EOF
+'
+
+test_expect_success 'filtered parent cascading edge case' '
+	create_orphan _27 &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	test_tick &&
+	git commit -m "D (last)" &&
+
+	create_orphan _25 &&
+	echo test >other.txt &&
+	git add other.txt &&
+	test_tick &&
+	git commit -m "C-filtered" &&
+
+	echo test >foo.txt &&
+	git add foo.txt &&
+	test_tick &&
+	git commit -m "B (child of filtered)" &&
+
+	create_orphan _26 &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	test_tick &&
+	git commit -m "A (visual root)" &&
+
+
+	lib_test_check_graph _25 _26 _27 -- foo.txt <<-\EOF
+	* A (visual root)
+	  * B (child of filtered)
+	* D (last)
+	EOF
+'
+
+test_expect_success 'multiple filtered parents in sequence' '
+	create_orphan _44 &&
+	echo a >other.txt && git add other.txt && git commit -m "44_F" &&
+	echo b >foo.txt && git add foo.txt && git commit -m "44_C" &&
+
+	create_orphan _45 &&
+	echo c >other.txt && git add other.txt && git commit -m "45_F" &&
+	echo d >foo.txt && git add foo.txt && git commit -m "45_C" &&
+
+	create_orphan _46 &&
+	echo e >foo.txt && git add foo.txt && git commit -m "46_A" &&
+
+	lib_test_check_graph _44 _45 _46 -- foo.txt <<-\EOF
+	* 44_C
+	  * 45_C
+	* 46_A
+	EOF
+'
+
+# This tests prove why there is no need to have indentation for boundary
+# commits.
+#
+# Boundary commits rather than starting a column they 'inherit' the one of
+# its child so there will always be an edge that connects it removing the
+# ambiguity.
+test_expect_success 'unrelated boundaries are not ambiguous' '
+	create_orphan _28 && test_commit 28_A && test_commit 28_B &&
+	test_commit 28_C &&
+	create_orphan _29 && test_commit 29_A && test_commit 29_B &&
+	lib_test_check_graph --boundary 28_A.._28 29_A.._29 <<-\EOF
+	* 29_B
+	| * 28_C
+	| * 28_B
+	| o 28_A
+	o 29_A
+	EOF
+'
+
+# Same structure as t6016
+test_expect_success 'boundary commits big test' '
+	# 3 commits on branch _30
+	create_orphan _30 &&
+	test_commit 30_A &&
+	test_commit 30_B &&
+	test_commit 30_C &&
+
+	# 2 commits on branch _31, started from 30_A
+	git checkout -b _31 30_A &&
+	test_commit 31_A &&
+	test_commit 31_B &&
+
+	# 2 commits on branch _32, started from 30_B
+	git checkout -b _32 30_B &&
+	test_commit 32_A &&
+	test_commit 32_B &&
+
+	# Octopus merge _31 and _32 into -30
+	git checkout _30 &&
+	git merge _31 _32 -m 30_D &&
+	git tag 30_D &&
+	test_commit 30_E &&
+
+	# More commits on _32, then merge _32 into _30
+	git checkout _32 &&
+	test_commit 32_C &&
+	test_commit 32_D &&
+	git checkout _30 &&
+	git merge -s ours _32 -m 30_F &&
+	git tag 30_F &&
+	test_commit 30_G &&
+	lib_test_check_graph --boundary _30 _31 _32 ^32_C <<-\EOF
+	* 30_G
+	*   30_F
+	|\
+	| * 32_D
+	* | 30_E
+	| |
+	|  \
+	*-. \   30_D
+	|\ \ \
+	| * | | 31_B
+	| * | | 31_A
+	* | | | 30_C
+	o | | | 30_B
+	|/ / /
+	o / / 30_A
+	 / /
+	| o 32_C
+	|/
+	o 32_B
+	EOF
+'
+
+# Filter by --first-parent and then forcing the filtered parents to be shown.
+test_expect_success '--first-parent flag with the filtered parents' '
+	(
+		unset_commit_graph &&
+		create_orphan _35 && test_commit 35_A && test_commit 35_B &&
+		create_orphan _36 && test_commit 36_A &&
+		create_orphan _37 && test_commit 37_A &&
+		git checkout _35 &&
+		TREE=$(git write-tree) &&
+		MERGE=$(git commit-tree $TREE -p _35 -p _36 -p _37 -m 35_octopus) &&
+		git reset --hard $MERGE &&
+		lib_test_check_graph --first-parent _35 _36 _37 <<-\EOF
+		* 35_octopus
+		| * 37_A
+		|   * 36_A
+		* 35_B
+		* 35_A
+		EOF
+	)
+'
+
+test_expect_success '--first-parent with filtered parents but one has a child' '
+	(
+		unset_commit_graph &&
+		create_orphan _38 && test_commit 38_A && test_commit 38_B &&
+		create_orphan _39 && test_commit 39_A &&
+		create_orphan _40 && test_commit 40_A && test_commit 40_B &&
+		git checkout _38 &&
+		TREE=$(git write-tree) &&
+		MERGE=$(git commit-tree $TREE -p _38 -p _39 -p _40 -m 38_octopus) &&
+		git reset --hard $MERGE &&
+		lib_test_check_graph --first-parent _38 _39 _40 <<-\EOF
+		* 38_octopus
+		| * 40_B
+		| * 40_A
+		|   * 39_A
+		* 38_B
+		* 38_A
+		EOF
+	)
+'
+
+test_expect_success '--first-parent with filtered parents but both have childs' '
+	(
+		unset_commit_graph &&
+		create_orphan _41 && test_commit 41_A && test_commit 41_B &&
+		create_orphan _42 && test_commit 42_A && test_commit 42_B &&
+		create_orphan _43 && test_commit 43_A && test_commit 43_B &&
+		git checkout _41 &&
+		TREE=$(git write-tree) &&
+		MERGE=$(git commit-tree $TREE -p _41 -p _42 -p _43 -m 41_octopus) &&
+		git reset --hard $MERGE &&
+		lib_test_check_graph --first-parent _41 _42 _43 <<-\EOF
+		* 41_octopus
+		| * 43_B
+		|  \
+		|   * 43_A
+		| * 42_B
+		| * 42_A
+		* 41_B
+		* 41_A
+		EOF
+	)
+'
+
+test_expect_success 'two unrelated merges' '
+	create_orphan _50 && test_commit 50_A &&
+	git checkout -b _51 &&
+	test_commit 51_A && test_commit 51_B &&
+	git checkout _50 &&
+	git merge --no-ff _51 -m 50_B &&
+
+	create_orphan _52 && test_commit 52_A &&
+	git checkout -b _53 &&
+	test_commit 53_A && test_commit 53_B &&
+	git checkout _52 &&
+	git merge --no-ff _53 -m 52_B &&
+
+	lib_test_check_graph _52 _50 <<-\EOF
+	*   52_B
+	|\
+	| * 53_B
+	| * 53_A
+	|/
+	 \
+	  * 52_A
+	*   50_B
+	|\
+	| * 51_B
+	| * 51_A
+	|/
+	* 50_A
+	EOF
+'
+
+test_done

-- 
2.54.0

^ permalink raw reply related

* [PATCH v7 2/3] graph: add a 2 commit buffer for lookahead
From: Pablo Sabater @ 2026-07-04  8:52 UTC (permalink / raw)
  To: git
  Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
	christian.couder, gitster, jltobler, karthik.188, krka, peff,
	phillip.wood, siddharthasthana31
In-Reply-To: <20260704-ps-pre-commit-indent-v7-0-a94706cc8376@gmail.com>

In a subsequent commit the graph renderer needs to know if the next
commit is a visual root or if it is the last commit to be shown. This
requires peeking 2 commits ahead.

Commits are pre-fetched at get_revision_internal() where they are also
marked as SHOWN.

Update graph_is_interesting() so it considers commits inside the
lookahead as interesting as well.

Helped-by: Kristofer Karlsson <krka@spotify.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 graph.h    | 17 +++++++++++++++++
 revision.c | 17 ++++++++++++++++-
 3 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/graph.c b/graph.c
index 842282685f..300ae67669 100644
--- a/graph.c
+++ b/graph.c
@@ -315,6 +315,14 @@ struct git_graph {
 	 * diff_output_prefix_callback().
 	 */
 	struct strbuf prefix_buf;
+
+	/*
+	 * Lookahead buffer: up to 2 pre-fetched commits that will be shown.
+	 * Populated by get_revision() so graph_peek_next_visible() can use
+	 * actual walk results instead of peeking at rev_info internals.
+	 */
+	struct commit *lookahead[2];
+	int lookahead_nr;
 };
 
 static inline int graph_needs_truncation(struct git_graph *graph, int lane)
@@ -388,6 +396,9 @@ struct git_graph *graph_init(struct rev_info *opt)
 	graph->num_columns = 0;
 	graph->num_new_columns = 0;
 	graph->mapping_size = 0;
+	graph->lookahead[0] = NULL;
+	graph->lookahead[1] = NULL;
+	graph->lookahead_nr = 0;
 	/*
 	 * Start the column color at the maximum value, since we'll
 	 * always increment it for the first commit we output.
@@ -456,6 +467,15 @@ static void graph_ensure_capacity(struct git_graph *graph, int num_columns)
  */
 static int graph_is_interesting(struct git_graph *graph, struct commit *commit)
 {
+	/*
+	 * Commits in the lookahead buffer have been pre-fetched by
+	 * get_revision() and will be shown in the future. They already
+	 * have the SHOWN flag set by get_revision_internal(), but the
+	 * graph still needs to treat them as interesting parents.
+	 */
+	for (int i = 0; i < graph->lookahead_nr; i++)
+		if (graph->lookahead[i] == commit)
+			return 1;
 	/*
 	 * If revs->boundary is set, commits whose children have
 	 * been shown are always interesting, even if they have the
@@ -763,6 +783,33 @@ static int graph_needs_pre_commit_line(struct git_graph *graph)
 	       graph->expansion_row < graph_num_expansion_rows(graph);
 }
 
+struct commit *graph_pop_lookahead(struct git_graph *graph)
+{
+	struct commit *c;
+
+	if (!graph->lookahead_nr)
+		return NULL;
+
+	c = graph->lookahead[0];
+	graph->lookahead[0] = graph->lookahead[1];
+	graph->lookahead[1] = NULL;
+	graph->lookahead_nr--;
+	return c;
+}
+
+int graph_get_lookahead_room(struct git_graph *graph)
+{
+	return 2 - graph->lookahead_nr;
+}
+
+void graph_push_lookahead(struct git_graph *graph, struct commit *c)
+{
+	if (!graph_get_lookahead_room(graph))
+		BUG("pushing into lookahead buffer when it is already full");
+
+	graph->lookahead[graph->lookahead_nr++] = c;
+}
+
 void graph_update(struct git_graph *graph, struct commit *commit)
 {
 	struct commit_list *parent;
diff --git a/graph.h b/graph.h
index 3fd1dcb2e9..281603b020 100644
--- a/graph.h
+++ b/graph.h
@@ -262,4 +262,21 @@ void graph_show_commit_msg(struct git_graph *graph,
 			   FILE *file,
 			   struct strbuf const *sb);
 
+/*
+ * Pop the first commit from the graph's lookahead buffer.
+ * Returns NULL if the buffer is empty.
+ */
+struct commit *graph_pop_lookahead(struct git_graph *graph);
+
+/*
+ * Returns how many more commits can be added to the lookahead buffer.
+ */
+int graph_get_lookahead_room(struct git_graph *graph);
+
+/*
+ * Push a commit into the lookahead buffer. Must only be called when
+ * graph_lookahead_room() returns > 0.
+ */
+void graph_push_lookahead(struct git_graph *graph, struct commit *c);
+
 #endif /* GRAPH_H */
diff --git a/revision.c b/revision.c
index e91d7e1f11..58351aeeff 100644
--- a/revision.c
+++ b/revision.c
@@ -4699,12 +4699,27 @@ struct commit *get_revision(struct rev_info *revs)
 				for (p = c->parents; p; p = p->next)
 					p->item->object.flags |= CHILD_SHOWN;
 		}
+	} else if (revs->graph) {
+		c = graph_pop_lookahead(revs->graph);
+		if (!c)
+			c = get_revision_internal(revs);
+
 	} else {
 		c = get_revision_internal(revs);
 	}
 
-	if (c && revs->graph)
+	if (c && revs->graph) {
+		if (!revs->max_count_stage && !revs->reverse_output_stage) {
+			while (graph_get_lookahead_room(revs->graph)) {
+				struct commit *next = get_revision_internal(revs);
+				if (!next)
+					break;
+				graph_push_lookahead(revs->graph, next);
+			}
+		}
 		graph_update(revs->graph, c);
+	}
+
 	if (!c) {
 		free_saved_parents(revs);
 		commit_list_free(revs->previous_parents);

-- 
2.54.0

^ permalink raw reply related

* [PATCH v7 1/3] lib-log-graph: move check_graph function
From: Pablo Sabater @ 2026-07-04  8:52 UTC (permalink / raw)
  To: git
  Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
	christian.couder, gitster, jltobler, karthik.188, krka, peff,
	phillip.wood, siddharthasthana31
In-Reply-To: <20260704-ps-pre-commit-indent-v7-0-a94706cc8376@gmail.com>

check_graph is a function shared in the test files t4215 and t6016 used
to format the output graph, but instead of being in a file called by
both test, the function code is repeated in each file.

Move check_graph to lib-log-graph.sh file which both tests already
import graph functions from, renaming it to lib_test_check_graph.

This function is needed for the following commit which includes graph
tests in a new file and requires check_graph.

Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 t/lib-log-graph.sh                         |  5 +++++
 t/t4215-log-skewed-merges.sh               | 33 +++++++++++++-----------------
 t/t6016-rev-list-graph-simplify-history.sh | 25 +++++++++-------------
 3 files changed, 29 insertions(+), 34 deletions(-)

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
index bf952ef920..1eae8f60c2 100644
--- a/t/lib-log-graph.sh
+++ b/t/lib-log-graph.sh
@@ -26,3 +26,8 @@ lib_test_cmp_colored_graph () {
 	test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
 	test_cmp expect.colors output.colors
 }
+
+lib_test_check_graph () {
+	cat >expect &&
+	lib_test_cmp_graph --format=%s "$@"
+}
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 1612f05f1b..eebab71039 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -5,11 +5,6 @@ test_description='git log --graph of skewed merges'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-log-graph.sh
 
-check_graph () {
-	cat >expect &&
-	lib_test_cmp_graph --format=%s "$@"
-}
-
 test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
 	git checkout --orphan _p &&
 	test_commit A &&
@@ -21,7 +16,7 @@ test_expect_success 'log --graph with merge fusing with its left and right neigh
 	git checkout _p && git merge --no-ff _r -m G &&
 	git checkout @^^ && git merge --no-ff _p -m H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   H
 	|\
 	| *   G
@@ -49,7 +44,7 @@ test_expect_success 'log --graph with left-skewed merge' '
 	git checkout 0_p && git merge --no-ff 0_s -m 0_G &&
 	git checkout @^ && git merge --no-ff 0_q 0_r 0_t 0_p -m 0_H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*-----.   0_H
 	|\ \ \ \
 	| | | | * 0_G
@@ -83,7 +78,7 @@ test_expect_success 'log --graph with nested left-skewed merge' '
 	git checkout 1_p && git merge --no-ff 1_r -m 1_G &&
 	git checkout @^^ && git merge --no-ff 1_p -m 1_H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   1_H
 	|\
 	| *   1_G
@@ -115,7 +110,7 @@ test_expect_success 'log --graph with nested left-skewed merge following normal
 	git checkout -b 2_s @^^ && git merge --no-ff 2_q -m 2_J &&
 	git checkout 2_p && git merge --no-ff 2_s -m 2_K &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   2_K
 	|\
 	| *   2_J
@@ -151,7 +146,7 @@ test_expect_success 'log --graph with nested right-skewed merge following left-s
 	git checkout 3_p && git merge --no-ff 3_r -m 3_H &&
 	git checkout @^^ && git merge --no-ff 3_p -m 3_J &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   3_J
 	|\
 	| *   3_H
@@ -182,7 +177,7 @@ test_expect_success 'log --graph with right-skewed merge following a left-skewed
 	git merge --no-ff 4_p -m 4_G &&
 	git checkout @^^ && git merge --no-ff 4_s -m 4_H &&
 
-	check_graph --date-order <<-\EOF
+	lib_test_check_graph --date-order <<-\EOF
 	*   4_H
 	|\
 	| *   4_G
@@ -218,7 +213,7 @@ test_expect_success 'log --graph with octopus merge with column joining its penu
 	git checkout 5_r &&
 	git merge --no-ff 5_s -m 5_H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   5_H
 	|\
 	| *-.   5_G
@@ -257,7 +252,7 @@ test_expect_success 'log --graph with multiple tips' '
 	git checkout 6_1 &&
 	git merge --no-ff 6_2 -m 6_I &&
 
-	check_graph 6_1 6_3 6_5 <<-\EOF
+	lib_test_check_graph 6_1 6_3 6_5 <<-\EOF
 	*   6_I
 	|\
 	| | *   6_H
@@ -334,7 +329,7 @@ test_expect_success 'log --graph with multiple tips' '
 	git checkout -b M_7 7_1 &&
 	git merge --no-ff 7_2 7_3 -m 7_M4 &&
 
-	check_graph M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
@@ -371,7 +366,7 @@ test_expect_success 'log --graph with multiple tips' '
 '
 
 test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
-	check_graph --graph-lane-limit=2 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=2 M_7 <<-\EOF
 	*-.   7_M4
 	|\ \
 	| | * 7_G
@@ -388,7 +383,7 @@ test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
 '
 
 test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge' '
-	check_graph --graph-lane-limit=1 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=1 M_7 <<-\EOF
 	*-~  7_M4
 	|\~
 	| ~ 7_G
@@ -405,7 +400,7 @@ test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge
 '
 
 test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
-	check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
@@ -441,7 +436,7 @@ test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
 '
 
 test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows first of 3 parent merge' '
-	check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
@@ -478,7 +473,7 @@ test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows fir
 '
 
 test_expect_success 'log --graph --graph-lane-limit=7 check if it shows all 3 parent merge' '
-	check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh
index 54b0a6f5f8..e0d9c3c1ac 100755
--- a/t/t6016-rev-list-graph-simplify-history.sh
+++ b/t/t6016-rev-list-graph-simplify-history.sh
@@ -13,11 +13,6 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-log-graph.sh
 
-check_graph () {
-	cat >expect &&
-	lib_test_cmp_graph --format=%s "$@"
-}
-
 test_expect_success 'set up rev-list --graph test' '
 	# 3 commits on branch A
 	test_commit A1 foo.txt &&
@@ -54,7 +49,7 @@ test_expect_success 'set up rev-list --graph test' '
 '
 
 test_expect_success '--graph --all' '
-	check_graph --all <<-\EOF
+	lib_test_check_graph --all <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -82,7 +77,7 @@ test_expect_success '--graph --all' '
 # that undecorated merges are interesting, even with --simplify-by-decoration
 test_expect_success '--graph --simplify-by-decoration' '
 	git tag -d A4 &&
-	check_graph --all --simplify-by-decoration <<-\EOF
+	lib_test_check_graph --all --simplify-by-decoration <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -114,7 +109,7 @@ test_expect_success 'setup: get rid of decorations on B' '
 
 # Graph with branch B simplified away
 test_expect_success '--graph --simplify-by-decoration prune branch B' '
-	check_graph --simplify-by-decoration --all <<-\EOF
+	lib_test_check_graph --simplify-by-decoration --all <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -133,7 +128,7 @@ test_expect_success '--graph --simplify-by-decoration prune branch B' '
 '
 
 test_expect_success '--graph --full-history -- bar.txt' '
-	check_graph --full-history --all -- bar.txt <<-\EOF
+	lib_test_check_graph --full-history --all -- bar.txt <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -148,7 +143,7 @@ test_expect_success '--graph --full-history -- bar.txt' '
 '
 
 test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
-	check_graph --full-history --simplify-merges --all -- bar.txt <<-\EOF
+	lib_test_check_graph --full-history --simplify-merges --all -- bar.txt <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -161,7 +156,7 @@ test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
 '
 
 test_expect_success '--graph -- bar.txt' '
-	check_graph --all -- bar.txt <<-\EOF
+	lib_test_check_graph --all -- bar.txt <<-\EOF
 	* A7
 	* A5
 	* A3
@@ -172,7 +167,7 @@ test_expect_success '--graph -- bar.txt' '
 '
 
 test_expect_success '--graph --sparse -- bar.txt' '
-	check_graph --sparse --all -- bar.txt <<-\EOF
+	lib_test_check_graph --sparse --all -- bar.txt <<-\EOF
 	* A7
 	* A6
 	* A5
@@ -189,7 +184,7 @@ test_expect_success '--graph --sparse -- bar.txt' '
 '
 
 test_expect_success '--graph ^C4' '
-	check_graph --all ^C4 <<-\EOF
+	lib_test_check_graph --all ^C4 <<-\EOF
 	* A7
 	* A6
 	* A5
@@ -202,7 +197,7 @@ test_expect_success '--graph ^C4' '
 '
 
 test_expect_success '--graph ^C3' '
-	check_graph --all ^C3 <<-\EOF
+	lib_test_check_graph --all ^C3 <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -220,7 +215,7 @@ test_expect_success '--graph ^C3' '
 # that important, but this test depends on it.  If the ordering ever changes
 # in the code, we'll need to update this test.
 test_expect_success '--graph --boundary ^C3' '
-	check_graph --boundary --all ^C3 <<-\EOF
+	lib_test_check_graph --boundary --all ^C3 <<-\EOF
 	* A7
 	*   A6
 	|\

-- 
2.54.0

^ permalink raw reply related

* [PATCH v7 0/3] graph: indent visual roots in graph
From: Pablo Sabater @ 2026-07-04  8:52 UTC (permalink / raw)
  To: git
  Cc: pabloosabaterr, ayu.chandekar, chandrapratap3519,
	christian.couder, gitster, jltobler, karthik.188, krka, peff,
	phillip.wood, siddharthasthana31
In-Reply-To: <20260620-ps-pre-commit-indent-v6-0-cdc6d8fd5fbc@gmail.com>

When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.

A fix for this issue was already attempted [1] a while ago.

This series adds indentation to the visual root commits, so they cannot be
vertically adjacent anymore making it easier to identify them.

Before indentation:

	* A
	* B1
	* B2
	* C1
	* C2

After indentation:

	  * A
	* B1
	 \
	  * B2
	* C1
	* C2

Indents the visual root commits that have still commits to show after
them, and if they have children it connects them with an edge at a new
row.

If there are multiple visual roots adjacent in history, the indentation
starts with the second one, avoiding redundant indentation of the first
one and cascades after the second.

	* A
	  * B
	    * C
	* D1
	* D2

This series first commit is a cleanup that brings a common function
from t4215 and t6016 to a graph functions file which they both use, so
the new test file for indentation, t4218, can use it as well.

[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/

V6 DIFF:

- Replaced the queue peeking with a 2-entry lookahead buffer populated by
  get_revision_internal() (second commit and graph_peek_next_visible()).

- Changed assert() with BUG() at graph_output_pre_root_line().

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Pablo Sabater (3):
      lib-log-graph: move check_graph function
      graph: add a 2 commit buffer for lookahead
      graph: indent visual root in graph

 graph.c                                    | 282 ++++++++++++++++++
 graph.h                                    |  17 ++
 revision.c                                 |  17 +-
 t/lib-log-graph.sh                         |   5 +
 t/meson.build                              |   1 +
 t/t4215-log-skewed-merges.sh               |  33 +--
 t/t4218-log-graph-indentation.sh           | 453 +++++++++++++++++++++++++++++
 t/t6016-rev-list-graph-simplify-history.sh |  25 +-
 8 files changed, 798 insertions(+), 35 deletions(-)
---
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
change-id: 20260612-ps-pre-commit-indent-39ca72816382

Best regards,
--  
Pablo Sabater <pabloosabaterr@gmail.com>

^ permalink raw reply

* [PATCH v2] ci(dockerized): raise the PID limit for private repositories
From: Johannes Schindelin via GitGitGadget @ 2026-07-04  8:52 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2164.git.1782889484346.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Every once in a while I need to verify that Microsoft Git's test suite
passes for changes that are not yet meant for public consumption, and
since it was (made) too difficult to keep up a working Azure Pipeline
definition, I have to use GitHub Actions in a private GitHub repository
for that purpose.

In these tests, basically all Dockerized CI jobs fail consistently. The
symptom is something like:

  error: cannot create async thread: Resource temporarily unavailable

in the middle of a test, typically in the t5xxx-t6xxx range. The first
such error is immediately followed by plenty more of these errors, and
not a single test succeeds afterwards.

At first, I thought that maybe the massive parallelism I enjoy there is
the problem, and I thought that the cgroups limits might be shared
between the many containers that run on essentially the same physical
machine. But even reducing the matrix to just a single of those
Dockerized jobs runs into the very same problems.

The underlying reason seems to be a substantial difference in the hosted
runners that execute these Dockerized jobs: forcing the PID limit of the
container to a high number lets the jobs pass, even when running the
complete matrix of all 13 Dockerized jobs concurrently. But that's not
the only difference: The jobs seem to take a lot longer in these
containers than, say, in the containers made available to
https://github.com/git/git.

When forcing a PID limit of 64k in that private repository, the jobs
completed successfully, but they also took a lot longer, between 2x to
2.5x longer, i.e. painfully much longer. Reducing the PID limit to 16k,
the CI jobs still passed, but took an equally long amount of time.
Reducing the PID limit to 8k caused the errors to reappear.

Here are the numbers from three example runs, the first one forcing the
PID and nproc limit to 65536, the second one to 16384, the third run is
from the public git/git repository:

Job                           | 64k     | 16k     | reference
------------------------------|---------|---------|---------
almalinux-8                   | 19m 3s  | 16m 0s  | 9m 36s
debian-11                     | 20m 31s | 20m 3s  | 8m 5s
fedora-breaking-changes-meson | 16m 29s | 19m 19s | 9m 40s
linux-asan-ubsan              | 1h 10m  | 1h 11m  | 34m 36s
linux-breaking-changes        | 25m 39s | 25m 58s | 13m 15s
linux-leaks                   | 1h 9m   | 1h 10m  | 33m 30s
linux-meson                   | 28m 9s  | 27m 4s  | 13m 45s
linux-musl-meson              | 16m 32s | 13m 39s | 8m 6s
linux-reftable-leaks          | 1h 13m  | 1h 13m  | 34m 34s
linux-reftable                | 26m 2s  | 25m 48s | 13m 31s
linux-sha256                  | 26m 12s | 26m 3s  | 12m 36s
linux-TEST-vars               | 26m 5s  | 25m 21s | 13m 25s
linux32                       | 21m 16s | 19m 57s | 10m 44s

It does not look as if the PID limit is the reason for the longer
runtime, seeing as the 64k vs 16k timings deviate no more than as is
usual with GitHub workflows. So let's go for 16k.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    ci(dockerized): reduce the PID limit for private repositories
    
    I needed to craft this patch while developing fixes for vulnerabilities
    which eventually were published as Git for Windows v2.53.0(3).
    
    Changes since v1:
    
     * Reworded the commit message's title to reflect the actual intent.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2164%2Fdscho%2Fraise-pid-limit-in-private-repositories-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2164/dscho/raise-pid-limit-in-private-repositories-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2164

Range-diff vs v1:

 1:  77aa18442c ! 1:  671d03ad33 ci(dockerized): reduce the PID limit for private repositories
     @@ Metadata
      Author: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       ## Commit message ##
     -    ci(dockerized): reduce the PID limit for private repositories
     +    ci(dockerized): raise the PID limit for private repositories
      
          Every once in a while I need to verify that Microsoft Git's test suite
          passes for changes that are not yet meant for public consumption, and


 .github/workflows/main.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index cf341d74db..85cfedf5b0 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -420,7 +420,9 @@ jobs:
       CI_JOB_IMAGE: ${{matrix.vector.image}}
       CUSTOM_PATH: /custom
     runs-on: ubuntu-latest
-    container: ${{matrix.vector.image}}
+    container:
+      image: ${{ matrix.vector.image }}
+      options: ${{ github.repository_visibility == 'private' && '--pids-limit 16384 --ulimit nproc=16384:16384 --ulimit nofile=32768:32768' || '' }}
     steps:
     - name: prepare libc6 for actions
       if: matrix.vector.jobname == 'linux32'

base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH] ci(dockerized): reduce the PID limit for private repositories
From: Johannes Schindelin @ 2026-07-04  8:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git
In-Reply-To: <xmqq5x2yps4c.fsf@gitster.g>

Hi Junio,

On Wed, 1 Jul 2026, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > The underlying reason seems to be a substantial difference in the hosted
> > runners that execute these Dockerized jobs: forcing the PID limit of the
> > container to a high number lets the jobs pass, even when running the
> > complete matrix of all 13 Dockerized jobs concurrently.
> 
> Is the "reduce" in the title accurate?  The above description tells
> me that what you did was to "raise" the PID limit (i.e., forcing the
> PID limit to a high number), presumably because the default PID
> limit is way too low for the tests to pass?

You are right; The subject is wrong. The patch raises the limit from
the hosted-runner default to 16384. The subject is a left-over from a
hypothesis that my experiments refuted.

v2 will retitle to "ci(dockerized): raise the PID limit for private
repositories". No code change: 16k remains the sweet spot between the
failures we saw at 8k and the slow runs at 64k.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH v2 1/4] t1517: skip svn tests if svn is not installed
From: Jeff King @ 2026-07-04  4:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git
In-Reply-To: <xmqq33xzize2.fsf@gitster.g>

On Fri, Jul 03, 2026 at 01:36:37PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > fed to perl (no quotes around 1.1.0). We sometimes catch these cases
> > automatically it results in an extra argument to test_expect_success,
> > etc. But here you are unlucky enough that it does not (and anyway, we do
> > not seem to have the same safety check for test_lazy_prereq; we'd just
> > ignore the extra arguments).
> >
> > And of course being perl, it doesn't complain. I'm not sure how it is
> > interpreted,
> 
> I happen to know ;-).
> 
> When you have more than two sequences of digits separated by dot,
> like IP address 192.168.1.1, you are telling Perl to interpret the
> sequence as a string, each byte of it is the number denoted by these
> digits.  I believe this was invented primarily for IP addresses, but
> it does not have to be just four digits.  To wit:
> 
>     $ perl -e 'print 65.66.67;'
>     ABC
>     $ perl -e 'print 65.66.67.68.69;'
>     ABCDE
> 
> Of course, 65.66 is not AB, but a floating-point number that is
> between integers 65 and 66:
> 
>     $ perl -e 'print 65.66;'
>     65.66

Ah, thanks. It is both exciting and horrifying that in 2026 I can still
learn new perl esoterica. :)

-Peff

^ permalink raw reply

* Re: [GSoC Patch] repo: support category-based prefix querying for info keys
From: Junio C Hamano @ 2026-07-03 22:49 UTC (permalink / raw)
  To: K Jayatheerth; +Cc: git, jltobler, lucasseikioshiro
In-Reply-To: <20260703164709.22723-1-jayatheerthkulkarni2005@gmail.com>

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> Currently, git repo info relies on an all-or-nothing query model
> where users must either know the exact, fully-qualified key name or use
> the --all flag to dump the entire repository state.
> As the number of supported keys expands, dumping all metadata and
> relying on external filters like grep becomes an inefficient bottleneck
> for a plumbing command.
>
> Enable category-based prefix querying so users can request
> entire groups of related keys natively

You mean "repo info" takes layout.bare and layout.shallow (right
now, later we may gain a lot more), so you want to say "everything
under 'layout' to grab these two values?

Why should we limit ourselves to "prefix match"?  Would a glob like
"layout.*", or "path.*.absolute", work better?  Especially the
latter, i.e., "I want the path variables, but am not interested in
their .relative values, only the .absolute ones."  It is especially
puzzling as you are going to do a dumb linear search in this mode
anyway.

Perhaps during each iteration of the loop over argv[], you can first
look for exact match using the existing bsearch() codepath.  If that
succeeds, you have a single key to return the value for.  If it does
not match exactly any key, use the new "prefix" (or "glob" which I
think would make far more sense) match codepath to find which key(s)
to return values for, so iterate over them (or say "Hey, that pattern
does not match any key!" and fail).

^ permalink raw reply

* Re: [PATCH v6 3/3] replay: offer an option to linearize the commit topology
From: Junio C Hamano @ 2026-07-03 20:57 UTC (permalink / raw)
  To: Toon Claes; +Cc: git, Elijah Newren, Johannes Schindelin
In-Reply-To: <20260702-toon-git-replay-drop-merges-v6-3-78a07cdd0382@iotcl.com>

Toon Claes <toon@iotcl.com> writes:

> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ...
> Linearizing is a distinct operation, and flattening merge commits is
> just one aspect of that. Recreating merges would be a separate mode, so
> rather than mirror git-rebase(1)'s `--rebase-merges[=<mode>]` interface,
> git-replay(1) uses its own `--linearize` option.
>
> Co-authored-by: Toon Claes <toon@iotcl.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  Documentation/git-replay.adoc |  21 ++++++-
>  builtin/replay.c              |   6 +-
>  replay.c                      |  54 ++++++++++------
>  replay.h                      |   5 ++
>  t/t3650-replay-basics.sh      | 140 +++++++++++++++++++++++++++++++++++++++++-
>  5 files changed, 203 insertions(+), 23 deletions(-)

With such an extensive change in behaviour, I wonder if Dscho is
still responsible for latent bugs in this round of implementation
and documentation, or should you take the responsibility over?

> +--linearize::
> +	In this mode, each replayed commit is stacked on top of the
> +	previously replayed one, so all replayed commits are flattened into
> +	a single linear history.
> ++
> +When a merge commit is encountered, the behavior of git-rebase(1)'s
> +option `--no-rebase-merges` is imitated. All commits in the range
> +reachable from the merge commit are replayed into a linear history, and
> +the merge commit itself is dropped. A ref that pointed to a merge commit
> +is updated to the merge's last replayed ancestor.
> ++
> +This flattens the `<revision-range>` as a whole. When multiple revision
> +ranges are given they are stacked on top of each other into one linear
> +history. Each of their refs is updated to point to its position in that
> +history. To linearize ranges separately, replay them in separate `git
> +replay` invocations.

OK, very much understandable.

> +This option is incompatible with `--revert`.

Definitely it is OK to leave it outside the scope, but I am not sure
if reverting a group of commits that happens to be "closed" and
happens to contain merges, is inherently incompatible with
flattening.  If you have

    ----O--A
         \  \
          B--M--C

and you want to revert what happened while the history advanced from
O to M, I would naïvely expect that I can arrive at

    ----O--A
         \  \
          B--M--C-B'-A'

by linearly applying the inverse of A and B (in either order).

If it is an inherent limitation, then the sentence may want "because
..." at the end.  Otherwise, it would make more sense to strike the
sentence from the main text, and have BUGS (or LIMITATIONS) section
at the end of the page, perhaps?

^ permalink raw reply

* Re: [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*'
From: Junio C Hamano @ 2026-07-03 20:48 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Marcelo Machado Lage, git, Vinicius Lira de Freitas
In-Reply-To: <akdwp_a2EuhVoGVW@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

>> -		test -f f1 && test -f f2 && test -f file_with_\$metachar &&
>> +		test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar &&
>
> While at it we could split this line into three lines -- it's getting
> overly long, and we typically don't chain multiple commands on one line
> nowadays.

Excellent.

>> -		! test -f main/f10 &&
>> +		test_path_is_missing main/f10 &&
>
> This is a stronger guarantee compared to before, as we only checked
> whether the path is not a file. Now we verify that it doesn't exist at
> all, which would be equivalent to `test -e`. That's a strict improvement
> though, but may be worth pointing out in the commit message so that the
> reviewer is not surprised.

Good.

^ permalink raw reply

* Re: [PATCH 2/2] format-patch: fix leak of rev_info in prepare_bases()
From: Junio C Hamano @ 2026-07-03 20:45 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Jeff King, git, Karthik Nayak
In-Reply-To: <akY4u02vdBkVqs7m@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

>> Likewise I find the dual clang/gcc jobs to be overkill. Compiling with
>> both is useful, as they have different warnings. But have we ever seen a
>> case where running the tests showed a different result with different
>> compilers?
>
> Not that I'd know of. As you say, I think it makes sense to use
> different compilers in general. But I don't really think we need to have
> this as a full "compiler x tests" matrix.

Very true.  Different configurations with TEST-vars are great
combination to test, but we are not in the business of hunting bugs
in clang/gcc so we long as they compile (instead of warning "hey,
that construct gives you undefined behaviour"), we shouldn't have to
run the test suite with the same configuration for both.

> I'm certainly on board with reducing the test matrix a bit. I'm sure
> that we can have a cleverer selection of jobs where we both have the
> same test coverage as we have right now while running less jobs overall.

Yeah, and if we can spend the saved cycles for better coverage, that
would be grat.


^ permalink raw reply

* Re: Unexpected recursion in 'git rm'
From: Junio C Hamano @ 2026-07-03 20:41 UTC (permalink / raw)
  To: Mikael Magnusson
  Cc: Евгений Плискин,
	git
In-Reply-To: <CAHYJk3RXY5-YgcYWY2y8vOcHG5Frf91ehNiZRr66sJJH5F=qLQ@mail.gmail.com>

Mikael Magnusson <mikachu@gmail.com> writes:

> ..., though you might overall get less surprised if you
> set the failglob option in bash.

Excellent suggestion.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox