Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite
From: Junio C Hamano @ 2026-07-06 20:16 UTC (permalink / raw)
  To: Ian Jackson; +Cc: git, Colin Stagner, Johannes Schindelin
In-Reply-To: <27211.50096.133710.528147@chiark.greenend.org.uk>

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

>> 	if git rev-parse --verify -q "$1:$config"
>> 	then
>> 		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
>> 	fi
>> 
>> Overly long output does not look very easy to read, but I kept it
>> the same as the original.
>
> I'm not a great fan of the long error message myself, but it seemed to
> be what the rest of the script was doing.  I didn't find any
> multi-line calls to die, so that's why I did it this way.
>
> I'm happy to reformat this to your taste.

Nah, it seems your plan is to deprecate this script over time and
move everybody to a newer implementation, so as long as "die" does
its job to stop and prevent breakages from spreading, that would be
fine.

Thanks.

^ permalink raw reply

* Re: [PATCH v5 1/2] Makefile: add $(GITLIBS) prerequisite to osxkeychain
From: Junio C Hamano @ 2026-07-06 19:52 UTC (permalink / raw)
  To: Shardul Natu via GitGitGadget
  Cc: git, Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru,
	Patrick Steinhardt, Shardul Natu, Ben Knoble
In-Reply-To: <e0bb18ff0191de384ea3c947bf26ee07834782cb.1783358097.git.gitgitgadget@gmail.com>

"Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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.
>
> All other core Git targets that link $(LIBS) already depend on
> $(GITLIBS), which bundles common-main.o, $(LIB_FILE), and $(RUST_LIB)
> when Rust is enabled. Add $(GITLIBS) as a prerequisite dependency to the
> git-credential-osxkeychain target to make it consistent with the rest of
> the codebase.

I do not work with macOS but doesn't this change introduce a
build/link failure?

Sorry if I am mistaken, but as far as I can see, $(GITLIBS) includes
common-main.o (and it being .o, not .a, it is always included in the
result), and git-credential-osxkeychain.c comes with its own main()
function.  

Using a list of things to link that contains common-main.o does not
sound like a right thing to do; in other words, linking too many is
just as bad as linking too little.

^ permalink raw reply

* Re: [PATCH v2] builtin/rm.c: use die_errno() when system call fails
From: Junio C Hamano @ 2026-07-06 19:44 UTC (permalink / raw)
  To: Gatla Vishweshwar Reddy; +Cc: git
In-Reply-To: <20260706191024.93788-1-gatlavishweshwarreddy26@gmail.com>

Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:

> When remove_file_from_index() fails, the errno value contains the
> reason for the failure.

Is that true?

Together with the subject that mentions "system call", I think the
above is not quite right.  Offhand I do not think of any system call
or library call we make, whose failure that sets errnor would cause
the callchain to return an error back to this caller.

Please show us a call graph that originates at this location that
calls remove_file_from_index(), and identify which system/library
call sets errno to cause this function return non-zero.

Otherwise, we would end up feeding an irrelevant garbage value in
error and have die_errno() pick it up.

> Using die() instead of die_errno() loses
> this information, making it harder to diagnose failures. Switch to
> die_errno() to include the system error message in the output.
>
> Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
> ---
>  builtin/rm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/rm.c b/builtin/rm.c
> index 081d0bc375..3f5e6e232d 100644
> --- a/builtin/rm.c
> +++ b/builtin/rm.c
> @@ -396,7 +396,7 @@ int cmd_rm(int argc,
>  			printf("rm '%s'\n", path);
>  
>  		if (remove_file_from_index(the_repository->index, path))
> -			die(_("git rm: unable to remove %s"), path);
> +			die_errno(_("git rm: unable to remove %s"), path);
>  	}
>  
>  	if (show_only)

^ permalink raw reply

* Re: [PATCH v2] setup.c: use die_errno() when chdir() system call fails
From: Junio C Hamano @ 2026-07-06 19:37 UTC (permalink / raw)
  To: Gatla Vishweshwar Reddy; +Cc: git
In-Reply-To: <20260706191421.94453-1-gatlavishweshwarreddy26@gmail.com>

Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:

Make sure that v(N+1) patch is sent as a reply to vN patch, so that
people who look at vN immediately notice there is a newer iteration
and they should spend their time on it rather than the stale one.

> When chdir() fails, the errno value contains the reason for the
> failure. Using die() instead of die_errno() loses this information,
> making it harder to diagnose failures. Switch to die_errno() to
> include the system error message in the output.
>
> Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>

Perfect.

> ---
>  setup.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> diff --git a/setup.c b/setup.c
> index b4652651df..e2e98d1126 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1954,13 +1954,13 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
>  		break;
>  	case GIT_DIR_DISCOVERED:
>  		if (dir.len < cwd.len && chdir(dir.buf))
> -			die(_("cannot change to '%s'"), dir.buf);
> +			die_errno(_("cannot change to '%s'"), dir.buf);
>  		prefix = setup_discovered_git_dir(repo, gitdir.buf, &cwd, dir.len,
>  						  &repo_fmt, nongit_ok);
>  		break;
>  	case GIT_DIR_BARE:
>  		if (dir.len < cwd.len && chdir(dir.buf))
> -			die(_("cannot change to '%s'"), dir.buf);
> +			die_errno(_("cannot change to '%s'"), dir.buf);
>  		prefix = setup_bare_git_dir(repo, &cwd, dir.len, &repo_fmt, nongit_ok);
>  		break;
>  	case GIT_DIR_HIT_CEILING:

Will queue.  Thanks.

^ permalink raw reply

* Re: [PATCH] setup.c: use die_errno() when chdir() system call fails
From: Junio C Hamano @ 2026-07-06 19:35 UTC (permalink / raw)
  To: Gatla Vishweshwar Reddy; +Cc: git
In-Reply-To: <20260706183851.90517-1-gatlavishweshwarreddy26@gmail.com>

Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:

> ---

The usual way to compose a log message (i.e., what the readers would
have seen above that three-dash line we see) of this project is to

 - Give an observation on how the current system works in the
   present tense (so no need to say "Currently X is Y", or
   "Previously X was Y" to describe the state before your change;
   just "X is Y" is enough), and discuss what you perceive as a
   problem in it.

 - Propose a solution (optional---often, problem description
   trivially leads to an obvious solution in reader's minds).

 - Give commands to somebody editing the codebase to "make it so",
   instead of saying "This commit does X".

in this order.  And then to conclude the message, add your sign-off
(see Documentation/SubmittingPatches:[[sign-off]]).

>  setup.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

The changes are trivially correct, in that you call die_errno()
immediately after seeing chdir() fail (which means there is no other
potential failures that can contaminate errno---what your die_errno()
will report cannot be anything but what we got from chdir()).

> diff --git a/setup.c b/setup.c
> index b4652651df..e2e98d1126 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1954,13 +1954,13 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
>  		break;
>  	case GIT_DIR_DISCOVERED:
>  		if (dir.len < cwd.len && chdir(dir.buf))
> -			die(_("cannot change to '%s'"), dir.buf);
> +			die_errno(_("cannot change to '%s'"), dir.buf);
>  		prefix = setup_discovered_git_dir(repo, gitdir.buf, &cwd, dir.len,
>  						  &repo_fmt, nongit_ok);
>  		break;
>  	case GIT_DIR_BARE:
>  		if (dir.len < cwd.len && chdir(dir.buf))
> -			die(_("cannot change to '%s'"), dir.buf);
> +			die_errno(_("cannot change to '%s'"), dir.buf);
>  		prefix = setup_bare_git_dir(repo, &cwd, dir.len, &repo_fmt, nongit_ok);
>  		break;
>  	case GIT_DIR_HIT_CEILING:

^ permalink raw reply

* [PATCH v2] setup.c: use die_errno() when chdir() system call fails
From: Gatla Vishweshwar Reddy @ 2026-07-06 19:14 UTC (permalink / raw)
  To: git; +Cc: Gatla Vishweshwar Reddy

When chdir() fails, the errno value contains the reason for the
failure. Using die() instead of die_errno() loses this information,
making it harder to diagnose failures. Switch to die_errno() to
include the system error message in the output.

Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
 setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index b4652651df..e2e98d1126 100644
--- a/setup.c
+++ b/setup.c
@@ -1954,13 +1954,13 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
 		break;
 	case GIT_DIR_DISCOVERED:
 		if (dir.len < cwd.len && chdir(dir.buf))
-			die(_("cannot change to '%s'"), dir.buf);
+			die_errno(_("cannot change to '%s'"), dir.buf);
 		prefix = setup_discovered_git_dir(repo, gitdir.buf, &cwd, dir.len,
 						  &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_BARE:
 		if (dir.len < cwd.len && chdir(dir.buf))
-			die(_("cannot change to '%s'"), dir.buf);
+			die_errno(_("cannot change to '%s'"), dir.buf);
 		prefix = setup_bare_git_dir(repo, &cwd, dir.len, &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_HIT_CEILING:
-- 
2.54.0


^ permalink raw reply related

* [PATCH v2] builtin/rm.c: use die_errno() when system call fails
From: Gatla Vishweshwar Reddy @ 2026-07-06 19:10 UTC (permalink / raw)
  To: git; +Cc: Gatla Vishweshwar Reddy

When remove_file_from_index() fails, the errno value contains the
reason for the failure. Using die() instead of die_errno() loses
this information, making it harder to diagnose failures. Switch to
die_errno() to include the system error message in the output.

Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
 builtin/rm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 081d0bc375..3f5e6e232d 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -396,7 +396,7 @@ int cmd_rm(int argc,
 			printf("rm '%s'\n", path);
 
 		if (remove_file_from_index(the_repository->index, path))
-			die(_("git rm: unable to remove %s"), path);
+			die_errno(_("git rm: unable to remove %s"), path);
 	}
 
 	if (show_only)
-- 
2.54.0


^ permalink raw reply related

* [PATCH] setup.c: use die_errno() when chdir() system call fails
From: Gatla Vishweshwar Reddy @ 2026-07-06 18:38 UTC (permalink / raw)
  To: git; +Cc: Gatla Vishweshwar Reddy

---
 setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index b4652651df..e2e98d1126 100644
--- a/setup.c
+++ b/setup.c
@@ -1954,13 +1954,13 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
 		break;
 	case GIT_DIR_DISCOVERED:
 		if (dir.len < cwd.len && chdir(dir.buf))
-			die(_("cannot change to '%s'"), dir.buf);
+			die_errno(_("cannot change to '%s'"), dir.buf);
 		prefix = setup_discovered_git_dir(repo, gitdir.buf, &cwd, dir.len,
 						  &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_BARE:
 		if (dir.len < cwd.len && chdir(dir.buf))
-			die(_("cannot change to '%s'"), dir.buf);
+			die_errno(_("cannot change to '%s'"), dir.buf);
 		prefix = setup_bare_git_dir(repo, &cwd, dir.len, &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_HIT_CEILING:
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH v4 2/2] Makefile: support universal macOS builds via RUST_TARGETS
From: Shardul Natu @ 2026-07-06 18:36 UTC (permalink / raw)
  To: ps
  Cc: git, Shardul Natu via GitGitGadget, Koji Nakamaru,
	Kristoffer Haugsbakk, Shardul Natu

> I was wondering why no other target declares an explicit dependency on
> RUST_LIB. As it turns out, all the other targets that link "$(LIBS)" all
> already depend on "$(GITLIBS)", which includes both "$(LIB_FILE)" and
> "$(RUST_LIB)". So shouldn't we also depend depend on "$(GITLIBS)" here
> instead of on either of the other two variables?

Ah, a much cleaner cleanup! Done


> s/rust/Rust/
Done


> With this we now have both:
>
> - target/$ARCH/$BUILD_CONFIG/
>
> - target/$BUILD_CONFIG/
>
> Is there any reason why we have to have those two different layouts
> instead of swapping the order in the first item so that all artifacts
> are in "target/$BUILD_CONFIG/"? Essentially, what I'm proposing instead
> is:
>
> - "target/$BUILD_CONFIG/" for the final universal executable.
>
> - "target/$BUILD_CONFIG/$ARCH" for the per-arch artifacts.

When you invoke "cargo build --release --target x86_64-apple-darwin",
Cargo automatically places the resulting artifacts under
"target/x86_64-apple-darwin/release/".

If we tried to force Cargo to output under "target/release/$ARCH" by
passing a custom "--target-dir", Cargo would still append its required
"$ARCH/$BUILD_CONFIG/" structure inside that custom directory, resulting
in nested paths like "target/release/$ARCH/$ARCH/release/libgitcore.a",
or otherwise breaking Cargo's internal dependency caching and artifact
sharing across builds.

And so, we have to have "target/$ARCH/$BUILD_CONFIG/" for per-arch artifacts.

^ permalink raw reply

* [PATCH] builtin/rm.c: use die_errno() when system call fails
From: Gatla Vishweshwar Reddy @ 2026-07-06 18:21 UTC (permalink / raw)
  To: git; +Cc: Gatla Vishweshwar Reddy

---
 builtin/rm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 081d0bc375..3f5e6e232d 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -396,7 +396,7 @@ int cmd_rm(int argc,
 			printf("rm '%s'\n", path);
 
 		if (remove_file_from_index(the_repository->index, path))
-			die(_("git rm: unable to remove %s"), path);
+			die_errno(_("git rm: unable to remove %s"), path);
 	}
 
 	if (show_only)
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH v7 4/9] environment: move pager_program into repo_config_values
From: Junio C Hamano @ 2026-07-06 17:38 UTC (permalink / raw)
  To: Tian Yuchen
  Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-5-cat@malon.dev>

Tian Yuchen <cat@malon.dev> writes:

>  	if (data.value)
> -		pager_program = data.value;
> +		repo_config_values(r)->pager_program = data.value;
>  	return data.want;
>  }

May not be a new problem, but does the old value or pager_program
leak here, if callers call this function more than once (or
pager_program gets assigned elsewhere)?

^ permalink raw reply

* [PATCH] t0213: skip ancestry tests under user-mode emulation
From: Jamie Magee via GitGitGadget @ 2026-07-06 17:34 UTC (permalink / raw)
  To: git; +Cc: Jamie Magee, Jamie Magee

From: Jamie Magee <jamie.magee@gmail.com>

The tests added in 3c8c638df6 (t0213: add trace2 cmd_ancestry tests,
2026-02-13) expect the cmd_ancestry event to name "test-tool" and
"git". On Linux those names come from the "comm" field of
/proc/<pid>/stat. Under user-mode emulation (e.g. qemu-user) /proc
reports the emulator ("qemu-riscv64") instead, so the event is still
emitted, the TRACE2_ANCESTRY probe enables the tests, and tests 2-5
fail even though they pass on native riscv64.

Require the probe to see "test-tool" in the ancestry of a test-tool
spawned from test-tool, so the tests skip when the names are unreliable.

Cc: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
---
    t0213: skip ancestry tests under user-mode emulation

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2168%2FJamieMagee%2Ft0213-skip-under-emulation-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2168/JamieMagee/t0213-skip-under-emulation-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2168

 t/t0213-trace2-ancestry.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t0213-trace2-ancestry.sh b/t/t0213-trace2-ancestry.sh
index a2b9536da8..2eb86c1952 100755
--- a/t/t0213-trace2-ancestry.sh
+++ b/t/t0213-trace2-ancestry.sh
@@ -31,12 +31,15 @@ PATH="$TTDIR:$PATH" && export PATH
 # no cmd_ancestry event is emitted.  We detect this at runtime and
 # skip the format-specific tests accordingly.
 
-# Determine if cmd_ancestry is supported on this platform.
+# Enable these tests only when cmd_ancestry reports real process names.
+# The procinfo stub emits no event; under user-mode emulation (e.g.
+# qemu-user) /proc reports the emulator, not the guest. Spawn test-tool
+# from test-tool and require "test-tool" in the child's ancestry.
 test_expect_success 'detect cmd_ancestry support' '
 	test_when_finished "rm -f trace.detect" &&
 	GIT_TRACE2_BRIEF=1 GIT_TRACE2="$(pwd)/trace.detect" \
-		test-tool trace2 001return 0 &&
-	if grep -q "^cmd_ancestry" trace.detect
+		test-tool trace2 004child test-tool trace2 001return 0 &&
+	if grep -q "^cmd_ancestry.*test-tool" trace.detect
 	then
 		test_set_prereq TRACE2_ANCESTRY
 	fi

base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v5 2/2] Makefile: support universal macOS builds via RUST_TARGETS
From: Shardul Natu via GitGitGadget @ 2026-07-06 17:14 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru,
	Patrick Steinhardt, Shardul Natu, Ben Knoble, Shardul Natu
In-Reply-To: <pull.2288.v5.git.git.1783358097.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.

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 7f4be97b90..335fd056fa 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 v5 1/2] Makefile: add $(GITLIBS) prerequisite to osxkeychain
From: Shardul Natu via GitGitGadget @ 2026-07-06 17:14 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru,
	Patrick Steinhardt, Shardul Natu, Ben Knoble, Shardul Natu
In-Reply-To: <pull.2288.v5.git.git.1783358097.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.

All other core Git targets that link $(LIBS) already depend on
$(GITLIBS), which bundles common-main.o, $(LIB_FILE), and $(RUST_LIB)
when Rust is enabled. Add $(GITLIBS) as a prerequisite dependency to the
git-credential-osxkeychain target to make it consistent with the rest of
the codebase.

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..7f4be97b90 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 $(GITLIBS) GIT-LDFLAGS
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
 		$(filter %.o,$^) $(LIBS) -framework Security -framework CoreFoundation
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v5 0/2] Makefile: link osxkeychain helper against Rust
From: Shardul Natu via GitGitGadget @ 2026-07-06 17:14 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru,
	Patrick Steinhardt, Shardul Natu, Ben Knoble
In-Reply-To: <pull.2288.v4.git.git.1783188355.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 v4:

 * Changed the osxkeychain prerequisite dependency from $(LIB_FILE)
   $(RUST_LIB) to $(GITLIBS) to match the canonical prerequisite pattern
   used by all other core Git targets linking $(LIBS).

Changes since v3:

 * Removed leading @ from $(call mkdir_p_parent_template) so it relies on
   the built-in $(QUIET_MKDIR_P_PARENT) behavior, matching existing Makefile
   conventions.
 * Replaced if [ with if test in Bourne shell recipe snippets to strictly
   adhere to the project's CodingGuidelines.

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 $(GITLIBS) 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-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2288/kiranani/next-v5
Pull-Request: https://github.com/git/git/pull/2288

Range-diff vs v4:

 1:  41de7d391a ! 1:  e0bb18ff01 Makefile: add $(RUST_LIB) prerequisite to osxkeychain
     @@ Metadata
      Author: Shardul Natu <snatu@google.com>
      
       ## Commit message ##
     -    Makefile: add $(RUST_LIB) prerequisite to osxkeychain
     +    Makefile: add $(GITLIBS) prerequisite to osxkeychain
      
          When Rust is enabled, the git-credential-osxkeychain helper depends on
          Rust symbols compiled into $(RUST_LIB). While commit 522ea8ef7d
     @@ Commit message
          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.
     +    All other core Git targets that link $(LIBS) already depend on
     +    $(GITLIBS), which bundles common-main.o, $(LIB_FILE), and $(RUST_LIB)
     +    when Rust is enabled. Add $(GITLIBS) as a prerequisite dependency to the
     +    git-credential-osxkeychain target to make it consistent with the rest of
     +    the codebase.
      
          Additionally, wrap the definitions of $(RUST_LIB) and the "rust" build
          target in "ifndef NO_RUST". This ensures that when NO_RUST=1 is
     @@ Makefile: $(LIBGIT_HIDDEN_EXPORT): $(LIBGIT_PARTIAL_EXPORT)
       
      -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
     ++contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(GITLIBS) GIT-LDFLAGS
       	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
       		$(filter %.o,$^) $(LIBS) -framework Security -framework CoreFoundation
       
 2:  88fc2e0bd8 ! 2:  66f71fb0d7 Makefile: support universal macOS builds via RUST_TARGETS
     @@ Commit message
            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
     @@ Makefile: 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.
     ++# 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`.

-- 
gitgitgadget

^ permalink raw reply

* Re: [PATCH v7 2/3] graph: add a 2 commit buffer for lookahead
From: Chandra Pratap @ 2026-07-06 15:33 UTC (permalink / raw)
  To: Kristofer Karlsson
  Cc: Pablo Sabater, git, ayu.chandekar, christian.couder, gitster,
	jltobler, karthik.188, peff, phillip.wood, siddharthasthana31
In-Reply-To: <CAL71e4O1tLE_VSDeeZQ_p=8kAXvk9JQ9EqdPaYMZnNs+Xj+RYA@mail.gmail.com>

On Mon, 6 Jul 2026 at 19:15, Kristofer Karlsson <krka@spotify.com> wrote:
>
> The hardcoded size-2 lookahead buffer was my suggestion,
> so I am responding inline with my thoughts although Pablo is
> the right person for making further changes (if any).
>
> On Mon, 6 Jul 2026, Chandra Pratap <chandrapratap3519@gmail.com> wrote:
> > Do we need to NULL out the retrieved buffer entries? If so, it is
> > worthwhile asserting that the entire buffer is NULLed out in the
> > !graph->lookahead_nr check above.
>
> You're right, it's not technically needed, and there are many places
> in the repo where stale data remains in buffers, and it would be possible
> to do that here too. I don't think it matters much in practice though,
> and NULLing them out would perhaps prevent some accidental reuse on bugs
> (NULL would crash instead).
>
> As for asserting: rather than checking that empty slots are NULL
> (which just verifies our own cleanup), it might be more useful to
> assert that a slot is non-NULL when lookahead_nr says it should be
> populated, i.e. assert on read rather than on empty. But even that
> may be overkill for a 2-element internal buffer.

True. But since we're already going through the pains of initializing the
buffer and NULLing it upon a pop, I'd much rather go the extra length
and verify what we're trying to do, shouldn't be that complicated anyway.

Whether that means checking for NULL here, on a push, or on a read
is something I don't feel strongly about, either is fine with me.

> > Not the best engineering practice, but I guess it is fine to constrain
> > the logic to _only_ a 2-entry buffer since that's what we'll always
> > deal with anyway.
>
> I did consider making it a proper ring buffer, but it felt like
> overkill (and I could not find any other existing ring buffer to
> piggy-back on in the repo), and the lookahead depth is
> structurally tied to the algorithm - we only ever need two more
> elements.
>
> It also helps that this is entirely internal to graph.c. If the
> buffer were part of a broader API, a less hardcoded approach
> would be more appropriate indeed.

Agreed.

> > We should use ARRAY_SIZE(graph->lookahead) instead of hardcoding
> > the value 2.
>
> Agreed, that is a nice improvement. What do you think Pablo?
>
> Thanks,
> Kristofer

^ permalink raw reply

* Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite
From: Ian Jackson @ 2026-07-06 15:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Colin Stagner, Johannes Schindelin
In-Reply-To: <xmqqy0fob2kl.fsf@gitster.g>

Hi.  Thanks for the quick review.

Junio C Hamano writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite"):
> If (global) $rev is not set here, we'd check :.git-subtree/config in
> the index in order to detect the v2's configuration.  It seems to me
> that this code however wants to inspect HEAD's tree.

This was a slip.  The code in reject_if_v2_config is supposed to use
its argument (as per the usage comment I added), not a global.  I'll
fix this with a respin.

(I think it may somehow work by accident in my tests.)

> The above helper may work for one caller that passes "$rev" but not
> for the other caller that passes "HEAD", no?

HEAD is a valid revision spec for git-rev-parse, but the
function should use $1 (which in that case would be HEAD), not $rev.

> 	if git rev-parse --verify -q "$1:$config"
> 	then
> 		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
> 	fi
> 
> Overly long output does not look very easy to read, but I kept it
> the same as the original.

I'm not a great fan of the long error message myself, but it seemed to
be what the rest of the script was doing.  I didn't find any
multi-line calls to die, so that's why I did it this way.

I'm happy to reformat this to your taste.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.

^ permalink raw reply

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

Em sex., 3 de jul. de 2026 às 05:20, Patrick Steinhardt <ps@pks.im> escreveu:
>
> On Thu, Jul 02, 2026 at 11:07:04AM -0300, Marcelo Machado Lage wrote:
> > Replace the basic shell commands 'test -f', with more modern test
> > helpers 'test_path_is_file' and 'test_path_is_missing'.
>
> Nit: it might make sense to briefly mention why we do this exercise.
> Like, what does `test_path_is_file` et al give us over `test -f`?

We'll add this in v2.

>
> > diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
> > index 7614dfbd95..93d6b4c479 100755
> > --- a/t/t9811-git-p4-label-import.sh
> > +++ b/t/t9811-git-p4-label-import.sh
> > @@ -62,9 +62,9 @@ test_expect_success 'basic p4 labels' '
> >
> >               cd main &&
> >               git checkout TAG_F1_ONLY &&
> > -             ! test -f f2 &&
> > +             test_path_is_missing f2 &&
> >               git checkout TAG_WITH\$_SHELL_CHAR &&
> > -             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.

We'll do this for v2 as well and make it into a patch series to
separate test interface modernization from formatting changes.

While on this, there are some other places in the file where multiple
commands in a && chain appear in a single line, e.g. in line 244:
> p4 edit f2 && date >f2 && p4 submit -d "change" f2 &&
Should we split these into multiple lines as well, even though they
are under the 80 characters limit?

>
> > @@ -135,9 +135,9 @@ test_expect_success 'export git tags to p4' '
> >               p4 labels ... | grep LIGHTWEIGHT_TAG &&
> >               p4 label -o GIT_TAG_1 | grep "tag created in git:xyzzy" &&
> >               p4 sync ...@GIT_TAG_1 &&
> > -             ! 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.

We overlooked this improvement at first, but we'll add a proper note
about it in v2.

>
> > @@ -168,9 +168,9 @@ test_expect_success 'export git tags to p4 with deletion' '
> >               cd "$cli" &&
> >               p4 sync ... &&
> >               p4 sync ...@GIT_TAG_ON_DELETED &&
> > -             test -f main/deleted_file &&
> > +             test_path_is_file main/deleted_file &&
> >               p4 sync ...@GIT_TAG_AFTER_DELETION &&
> > -             ! test -f main/deleted_file &&
> > +             test_path_is_missing main/deleted_file &&
>
> Same here.
>
> Other than that the patch looks good to me, thanks!

Thanks for the detailed feedback, Patrick!

Best,
Marcelo

>
> Patrick

^ permalink raw reply

* Re: [PATCH v4 0/5] builtin/refs: add ability to write references
From: Junio C Hamano @ 2026-07-06 14:57 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Toon Claes
In-Reply-To: <20260706-pks-refs-writing-subcommands-v4-0-d51f6ce7f830@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> Reference-related functionality in Git is currently spread across many
> different commands: git-update-ref(1), git-for-each-ref(1),
> git-show-ref(1), git-pack-refs(1) and git-symbolic-ref(1). This makes it
> hard for users to discover what functionality we have available to work
> with references.
>
> We have thus started to consolidate this functionality into git-refs(1),
> which is a toolbox of everything related to references. Until now, the
> command doesn't handle functionality of git-update-ref(1).
>
> This patch series backfills most of the functionality by introducing
> three new commands:
>
>   - `git refs delete` to delete references. This is the equivalent of
>     `git update-ref -d`.
>
>   - `git refs update` to update references. This is the equivalent of
>     `git update-ref <refname> <oldvalue> <newvalue>`.
>
>   - `git refs rename` to rename a reference, including its reflog. This
>     does not have an equivalent in git-update-ref(1), but is inspired by
>     and supersedes [1].
>
> Changes in v4:
>   - Add a couple more tests around symrefs.
>   - Use a subshell in one of the tests for consistency.
>   - Link to v3: https://patch.msgid.link/20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@pks.im

Good that this came just in time before I started merging things
down to 'next' ;-)  Will replace.

^ permalink raw reply

* Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite
From: Junio C Hamano @ 2026-07-06 14:44 UTC (permalink / raw)
  To: Ian Jackson; +Cc: git, Colin Stagner, Johannes Schindelin
In-Reply-To: <20260706115816.20267-2-ijackson@chiark.greenend.org.uk>

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

> +# Usage: reject_if_v2_config REV
> +#
> +# Bails if we find .git-subtree/config.  This file is used by the RIIR
> +# git-subtree, which can read data from this script, but which generates
> +# data that this script cannot cope with.  So if we find that the user's
> +# project has already been processed with the new tool, we stop, to
> +# avoid generating broken output.
> +reject_if_v2_config () {
> +	local config=.git-subtree/config
> +	if git rev-parse --verify -q "$rev:$config"; then
> +		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
> +	fi
> +}

[warning: I have no idea what is going on in the code we see here,
as I do not use subtree script at all]

The above helper may work for one caller that passes "$rev" but not
for the other caller that passes "HEAD", no?


	if git rev-parse --verify -q "$1:$config"
	then
		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
	fi

Overly long output does not look very easy to read, but I kept it
the same as the original.

> @@ -846,6 +860,7 @@ process_split_commit () {
>  #    Or: cmd_add REPOSITORY REF
>  cmd_add () {
>  
> +	reject_if_v2_config HEAD
>  	ensure_clean

If (global) $rev is not set here, we'd check :.git-subtree/config in
the index in order to detect the v2's configuration.  It seems to me
that this code however wants to inspect HEAD's tree.

> @@ -934,6 +949,8 @@ cmd_split () {
>  		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
>  	fi
>  
> +	reject_if_v2_config "$rev"

This would happen to work, as the global "$rev" visible here is the
same one as what the new helper function sees and uses.

>  	# Now validate prefix against the commit, not the working tree
>  	if ! git cat-file -e "$rev:$dir" 2>/dev/null
>  	then
> @@ -1034,6 +1051,7 @@ cmd_merge () {
>  	then
>  		repository="$2"
>  	fi
> +	reject_if_v2_config HEAD
>  	ensure_clean

The same comment as the one for cmd_add's usage.

>  	if test -n "$arg_addmerge_squash"

^ permalink raw reply

* [PATCH v7 9/9] environment: move object_creation_mode into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
  To: git
  Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-1-cat@malon.dev>

The global variable 'object_creation_mode' controls how Git creates
object files, specifically determining whether to use hardlinks or
renames when moving temporary files into the object database. Move
it into 'struct repo_config_values' to continue the libification
effort.

Move the 'enum object_creation_mode' definition higher up in
'environment.h' to ensure it is visible to the structure. Initialize
the per-repository value to its default macro value
OBJECT_CREATION_MODE inside 'repo_config_values_init()'.

Update configuration parsing in 'git_default_core_config()' to write
directly to the repository-specific configuration structure.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c |  6 +++---
 environment.h | 12 ++++++------
 object-file.c |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/environment.c b/environment.c
index 7701aa3bc0..e50beda918 100644
--- a/environment.c
+++ b/environment.c
@@ -61,7 +61,6 @@ char *check_roundtrip_encoding;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
-enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
 unsigned long pack_size_limit_cfg;
 
@@ -511,9 +510,9 @@ int git_default_core_config(const char *var, const char *value,
 		if (!value)
 			return config_error_nonbool(var);
 		if (!strcmp(value, "rename"))
-			object_creation_mode = OBJECT_CREATION_USES_RENAMES;
+			cfg->object_creation_mode = OBJECT_CREATION_USES_RENAMES;
 		else if (!strcmp(value, "link"))
-			object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
+			cfg->object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
 		else
 			die(_("invalid mode for object creation: %s"), value);
 		return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
 	cfg->autorebase = AUTOREBASE_NEVER;
+	cfg->object_creation_mode = OBJECT_CREATION_MODE;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 464ff73136..eaa0aba7bc 100644
--- a/environment.h
+++ b/environment.h
@@ -109,6 +109,11 @@ enum rebase_setup_type {
 	AUTOREBASE_ALWAYS
 };
 
+enum object_creation_mode {
+	OBJECT_CREATION_USES_HARDLINKS = 0,
+	OBJECT_CREATION_USES_RENAMES = 1
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -120,6 +125,7 @@ struct repo_config_values {
 	char *apply_default_ignorewhitespace;
 	enum push_default_type push_default;
 	enum rebase_setup_type autorebase;
+	enum object_creation_mode object_creation_mode;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -213,12 +219,6 @@ extern unsigned long pack_size_limit_cfg;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-enum object_creation_mode {
-	OBJECT_CREATION_USES_HARDLINKS = 0,
-	OBJECT_CREATION_USES_RENAMES = 1
-};
-extern enum object_creation_mode object_creation_mode;
-
 extern int grafts_keep_true_parents;
 
 const char *get_log_output_encoding(void);
diff --git a/object-file.c b/object-file.c
index 9afa842da2..cbbfc8f1dc 100644
--- a/object-file.c
+++ b/object-file.c
@@ -415,7 +415,7 @@ int finalize_object_file_flags(struct repository *repo,
 retry:
 	ret = 0;
 
-	if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)
+	if (repo_config_values(repo)->object_creation_mode == OBJECT_CREATION_USES_RENAMES)
 		goto try_rename;
 	else if (link(tmpfile, filename))
 		ret = errno;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v7 8/9] environment: move autorebase into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
  To: git
  Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-1-cat@malon.dev>

The global variable 'autorebase' dictates whether a newly created
branch should be configured to automatically rebase by default.
Move it into 'struct repo_config_values' to continue the
libification effort.

The 'enum rebase_setup_type' definition is moved higher up in
'environment.h' so that it is visible to the repository-specific
structure. The default state AUTOREBASE_NEVER is now correctly
initialized in 'repo_config_values_init()'.

Configuration parsing in 'git_default_branch_config()' is updated to
write directly to the repository's configuration instance.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 branch.c      |  2 +-
 environment.c | 10 +++++-----
 environment.h | 16 ++++++++--------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/branch.c b/branch.c
index 243db7d0fc..e1c1f8c89d 100644
--- a/branch.c
+++ b/branch.c
@@ -61,7 +61,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
 
 static int should_setup_rebase(const char *origin)
 {
-	switch (autorebase) {
+	switch (repo_config_values(the_repository)->autorebase) {
 	case AUTOREBASE_NEVER:
 		return 0;
 	case AUTOREBASE_LOCAL:
diff --git a/environment.c b/environment.c
index 09de2fee87..7701aa3bc0 100644
--- a/environment.c
+++ b/environment.c
@@ -58,7 +58,6 @@ enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding;
-enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
@@ -600,13 +599,13 @@ static int git_default_branch_config(const char *var, const char *value)
 		if (!value)
 			return config_error_nonbool(var);
 		else if (!strcmp(value, "never"))
-			autorebase = AUTOREBASE_NEVER;
+			cfg->autorebase = AUTOREBASE_NEVER;
 		else if (!strcmp(value, "local"))
-			autorebase = AUTOREBASE_LOCAL;
+			cfg->autorebase = AUTOREBASE_LOCAL;
 		else if (!strcmp(value, "remote"))
-			autorebase = AUTOREBASE_REMOTE;
+			cfg->autorebase = AUTOREBASE_REMOTE;
 		else if (!strcmp(value, "always"))
-			autorebase = AUTOREBASE_ALWAYS;
+			cfg->autorebase = AUTOREBASE_ALWAYS;
 		else
 			return error(_("malformed value for %s"), var);
 		return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->apply_default_whitespace = NULL;
 	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
+	cfg->autorebase = AUTOREBASE_NEVER;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 72859b5d76..464ff73136 100644
--- a/environment.h
+++ b/environment.h
@@ -102,6 +102,13 @@ enum push_default_type {
 	PUSH_DEFAULT_UNSPECIFIED
 };
 
+enum rebase_setup_type {
+	AUTOREBASE_NEVER = 0,
+	AUTOREBASE_LOCAL,
+	AUTOREBASE_REMOTE,
+	AUTOREBASE_ALWAYS
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -112,6 +119,7 @@ struct repo_config_values {
 	char *apply_default_whitespace;
 	char *apply_default_ignorewhitespace;
 	enum push_default_type push_default;
+	enum rebase_setup_type autorebase;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -205,14 +213,6 @@ extern unsigned long pack_size_limit_cfg;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-enum rebase_setup_type {
-	AUTOREBASE_NEVER = 0,
-	AUTOREBASE_LOCAL,
-	AUTOREBASE_REMOTE,
-	AUTOREBASE_ALWAYS
-};
-extern enum rebase_setup_type autorebase;
-
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
 	OBJECT_CREATION_USES_RENAMES = 1
-- 
2.43.0


^ permalink raw reply related

* [PATCH v7 7/9] environment: move push_default into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
  To: git
  Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-1-cat@malon.dev>

The global variable 'push_default' specifies the default behavior of
'git push' when no explicit refspec is provided. Move 'push_default'
into 'struct repo_config_values' to continue the libification effort.

While 'enum push_default_type' ideally belongs in 'remote.h', moving it
there introduces a circular dependency chain:

  remote.h -> hash.h -> repository.h -> environment.h.

Therefore, the enum definition is kept in 'environment.h' just above
'struct repo_config_values' with a NEEDSWORK comment for future cleanup.

Modify the configuration parsing in environment.c to update the
per-repository structure directly, and update caller across the
codebase to access the value via 'repo_config_values()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 builtin/push.c |  8 ++++----
 environment.c  | 16 +++++++++-------
 environment.h  | 26 ++++++++++++++++----------
 remote.c       |  2 +-
 4 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..6dc3224b60 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -88,7 +88,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,
 		}
 	}
 
-	if (push_default == PUSH_DEFAULT_UPSTREAM &&
+	if (repo_config_values(the_repository)->push_default == PUSH_DEFAULT_UPSTREAM &&
 	    skip_prefix(matched->name, "refs/heads/", &branch_name)) {
 		struct branch *branch = branch_get(branch_name);
 		if (branch->merge_nr == 1 && branch->merge[0]->src) {
@@ -160,7 +160,7 @@ static NORETURN void die_push_simple(struct branch *branch,
 	 * Don't show advice for people who explicitly set
 	 * push.default.
 	 */
-	if (push_default == PUSH_DEFAULT_UNSPECIFIED)
+	if (cfg->push_default == PUSH_DEFAULT_UNSPECIFIED)
 		advice_pushdefault_maybe = _("\n"
 				 "To choose either option permanently, "
 				 "see push.default in 'git help config'.\n");
@@ -232,7 +232,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 	const char *dst;
 	int same_remote;
 
-	switch (push_default) {
+	switch (repo_config_values(the_repository)->push_default) {
 	case PUSH_DEFAULT_MATCHING:
 		refspec_append(&rs, ":");
 		return;
@@ -252,7 +252,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 	dst = branch->refname;
 	same_remote = !strcmp(remote->name, remote_for_branch(branch, NULL));
 
-	switch (push_default) {
+	switch (repo_config_values(the_repository)->push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 	case PUSH_DEFAULT_SIMPLE:
diff --git a/environment.c b/environment.c
index 8744790219..09de2fee87 100644
--- a/environment.c
+++ b/environment.c
@@ -59,7 +59,6 @@ enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
-enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
@@ -619,21 +618,23 @@ static int git_default_branch_config(const char *var, const char *value)
 
 static int git_default_push_config(const char *var, const char *value)
 {
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	if (!strcmp(var, "push.default")) {
 		if (!value)
 			return config_error_nonbool(var);
 		else if (!strcmp(value, "nothing"))
-			push_default = PUSH_DEFAULT_NOTHING;
+			cfg->push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
-			push_default = PUSH_DEFAULT_MATCHING;
+			cfg->push_default = PUSH_DEFAULT_MATCHING;
 		else if (!strcmp(value, "simple"))
-			push_default = PUSH_DEFAULT_SIMPLE;
+			cfg->push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
-			push_default = PUSH_DEFAULT_UPSTREAM;
+			cfg->push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
-			push_default = PUSH_DEFAULT_UPSTREAM;
+			cfg->push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "current"))
-			push_default = PUSH_DEFAULT_CURRENT;
+			cfg->push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error(_("malformed value for %s: %s"), var, value);
 			return error(_("must be one of nothing, matching, simple, "
@@ -725,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->askpass_program = NULL;
 	cfg->apply_default_whitespace = NULL;
 	cfg->apply_default_ignorewhitespace = NULL;
+	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 9aecd64152..72859b5d76 100644
--- a/environment.h
+++ b/environment.h
@@ -87,6 +87,21 @@ extern const char * const local_repo_env[];
 struct strvec;
 
 struct repository;
+
+/*
+ * NEEDSWORK: It would be better if these definitions could be moved to
+ * other more specific files, but care is needed to avoid circular
+ * inclusion issues.
+ */
+enum push_default_type {
+	PUSH_DEFAULT_NOTHING = 0,
+	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
+	PUSH_DEFAULT_UPSTREAM,
+	PUSH_DEFAULT_CURRENT,
+	PUSH_DEFAULT_UNSPECIFIED
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -96,6 +111,7 @@ struct repo_config_values {
 	char *askpass_program;
 	char *apply_default_whitespace;
 	char *apply_default_ignorewhitespace;
+	enum push_default_type push_default;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -197,16 +213,6 @@ enum rebase_setup_type {
 };
 extern enum rebase_setup_type autorebase;
 
-enum push_default_type {
-	PUSH_DEFAULT_NOTHING = 0,
-	PUSH_DEFAULT_MATCHING,
-	PUSH_DEFAULT_SIMPLE,
-	PUSH_DEFAULT_UPSTREAM,
-	PUSH_DEFAULT_CURRENT,
-	PUSH_DEFAULT_UNSPECIFIED
-};
-extern enum push_default_type push_default;
-
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
 	OBJECT_CREATION_USES_RENAMES = 1
diff --git a/remote.c b/remote.c
index 00723b385e..d48c01d375 100644
--- a/remote.c
+++ b/remote.c
@@ -1933,7 +1933,7 @@ static char *branch_get_push_1(struct repository *repo,
 	if (remote->mirror)
 		return tracking_for_push_dest(remote, branch->refname, err);
 
-	switch (push_default) {
+	switch (repo_config_values(repo)->push_default) {
 	case PUSH_DEFAULT_NOTHING:
 		return error_buf(err, _("push has no destination (push.default is 'nothing')"));
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v7 6/9] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
  To: git
  Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-1-cat@malon.dev>

The global variables 'apply_default_whitespace' and
'apply_default_ignorewhitespace' are used to store the default
whitespace configuration for 'git apply'. Move these variables
into 'struct repo_config_values' to continue the libification
effort.

Dynamically allocated strings fetched via 'repo_config_get_string()'
are now tracked per-repository and safely freed in
'repo_config_values_clear()'.

As part of this transition, update 'git_apply_config()' to accept a
'struct repository *' argument rather than relying on the
'the_repository' global.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 apply.c       | 20 ++++++++++++--------
 environment.c |  6 ++++--
 environment.h |  4 ++--
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/apply.c b/apply.c
index 249248d4f2..66db9b7678 100644
--- a/apply.c
+++ b/apply.c
@@ -47,11 +47,13 @@ struct gitdiff_data {
 	int p_value;
 };
 
-static void git_apply_config(void)
+static void git_apply_config(struct repository *repo)
 {
-	repo_config_get_string(the_repository, "apply.whitespace", &apply_default_whitespace);
-	repo_config_get_string(the_repository, "apply.ignorewhitespace", &apply_default_ignorewhitespace);
-	repo_config(the_repository, git_xmerge_config, NULL);
+	repo_config_get_string(repo, "apply.whitespace",
+			       &repo_config_values(repo)->apply_default_whitespace);
+	repo_config_get_string(repo, "apply.ignorewhitespace",
+			       &repo_config_values(repo)->apply_default_ignorewhitespace);
+	repo_config(repo, git_xmerge_config, NULL);
 }
 
 static int parse_whitespace_option(struct apply_state *state, const char *option)
@@ -126,10 +128,12 @@ int init_apply_state(struct apply_state *state,
 	strset_init(&state->kept_symlinks);
 	strbuf_init(&state->root, 0);
 
-	git_apply_config();
-	if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
+	git_apply_config(repo);
+	if (repo_config_values(repo)->apply_default_whitespace &&
+	    parse_whitespace_option(state, repo_config_values(repo)->apply_default_whitespace))
 		return -1;
-	if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+	if (repo_config_values(repo)->apply_default_ignorewhitespace &&
+	    parse_ignorewhitespace_option(state, repo_config_values(repo)->apply_default_ignorewhitespace))
 		return -1;
 	return 0;
 }
@@ -192,7 +196,7 @@ int check_apply_state(struct apply_state *state, int force_apply)
 
 static void set_default_whitespace_mode(struct apply_state *state)
 {
-	if (!state->whitespace_option && !apply_default_whitespace)
+	if (!state->whitespace_option && !repo_config_values(state->repo)->apply_default_whitespace)
 		state->ws_error_action = (state->apply ? warn_on_ws_error : nowarn_ws_error);
 }
 
diff --git a/environment.c b/environment.c
index 3782bf68aa..8744790219 100644
--- a/environment.c
+++ b/environment.c
@@ -49,8 +49,6 @@ int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
 char *git_commit_encoding;
 char *git_log_output_encoding;
-char *apply_default_whitespace;
-char *apply_default_ignorewhitespace;
 int fsync_object_files = -1;
 int use_fsync = -1;
 enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
@@ -725,6 +723,8 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->editor_program = NULL;
 	cfg->pager_program = NULL;
 	cfg->askpass_program = NULL;
+	cfg->apply_default_whitespace = NULL;
+	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -758,4 +758,6 @@ void repo_config_values_clear(struct repository *repo)
 	FREE_AND_NULL(cfg->editor_program);
 	FREE_AND_NULL(cfg->pager_program);
 	FREE_AND_NULL(cfg->askpass_program);
+	FREE_AND_NULL(cfg->apply_default_whitespace);
+	FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
 }
diff --git a/environment.h b/environment.h
index d55b1ba073..9aecd64152 100644
--- a/environment.h
+++ b/environment.h
@@ -94,6 +94,8 @@ struct repo_config_values {
 	char *editor_program;
 	char *pager_program;
 	char *askpass_program;
+	char *apply_default_whitespace;
+	char *apply_default_ignorewhitespace;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -182,8 +184,6 @@ extern int has_symlinks;
 extern int minimum_abbrev, default_abbrev;
 extern int ignore_case;
 extern int assume_unchanged;
-extern char *apply_default_whitespace;
-extern char *apply_default_ignorewhitespace;
 extern unsigned long pack_size_limit_cfg;
 
 extern int protect_hfs;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v7 5/9] environment: move askpass_program into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
  To: git
  Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-1-cat@malon.dev>

The global variable 'askpass_program' stores the path to the program
used to prompt the user for credentials. Move it into repo_config_values
to continue the libification effort.

While it is uncommon for a single process to require different askpass
programs for different repositories, maintaining this value as a mutable
global string is a blocker for libification. Global heap-allocated
strings introduce thread-safety issues in a multi-repo environment.

Move 'askpass_program' into 'struct repo_config_values' to eliminate
this global state. The memory is now safely managed and freed via
'repo_config_values_clear()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c | 6 ++++--
 environment.h | 1 +
 prompt.c      | 3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/environment.c b/environment.c
index a1204fdcb2..3782bf68aa 100644
--- a/environment.c
+++ b/environment.c
@@ -462,8 +462,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.askpass")) {
-		FREE_AND_NULL(askpass_program);
-		return git_config_string(&askpass_program, var, value);
+		FREE_AND_NULL(cfg->askpass_program);
+		return git_config_string(&cfg->askpass_program, var, value);
 	}
 
 	if (!strcmp(var, "core.excludesfile")) {
@@ -724,6 +724,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->excludes_file = NULL;
 	cfg->editor_program = NULL;
 	cfg->pager_program = NULL;
+	cfg->askpass_program = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -756,4 +757,5 @@ void repo_config_values_clear(struct repository *repo)
 	FREE_AND_NULL(cfg->excludes_file);
 	FREE_AND_NULL(cfg->editor_program);
 	FREE_AND_NULL(cfg->pager_program);
+	FREE_AND_NULL(cfg->askpass_program);
 }
diff --git a/environment.h b/environment.h
index 22f6697c52..d55b1ba073 100644
--- a/environment.h
+++ b/environment.h
@@ -93,6 +93,7 @@ struct repo_config_values {
 	char *excludes_file;
 	char *editor_program;
 	char *pager_program;
+	char *askpass_program;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
diff --git a/prompt.c b/prompt.c
index 706fba2a50..d8d74c7e37 100644
--- a/prompt.c
+++ b/prompt.c
@@ -3,6 +3,7 @@
 #include "git-compat-util.h"
 #include "parse.h"
 #include "environment.h"
+#include "repository.h"
 #include "run-command.h"
 #include "strbuf.h"
 #include "prompt.h"
@@ -51,7 +52,7 @@ char *git_prompt(const char *prompt, int flags)
 
 		askpass = getenv("GIT_ASKPASS");
 		if (!askpass)
-			askpass = askpass_program;
+			askpass = repo_config_values(the_repository)->askpass_program;
 		if (!askpass)
 			askpass = getenv("SSH_ASKPASS");
 		if (askpass && *askpass)
-- 
2.43.0


^ permalink raw reply related


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