[PATCH] Makefile: link osxkeychain helper against Rust

[PATCH] Makefile: link osxkeychain helper against Rust

[Shardul Natu via GitGitGadget]

From: Shnatu <snatu@google.com>

When Rust is enabled, ensure that the git-credential-osxkeychain
helper is linked with the necessary Rust libraries.

Introduce the RUST_LIBS variable inside ifndef NO_RUST block
to hold the Rust library dependency, and use it in the helper's
build target. This cleanly handles cases where Rust is disabled,
making it a no-op and avoiding any build failures on systems
without Cargo.

This addresses reviewer feedback from internal CL 910223487
by simplifying the variables and avoiding confusing "LINK"
terminology.

Signed-off-by: Shnatu <snatu@google.com>
---
    Makefile: link osxkeychain helper against Rust

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

 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index f86173f93a..a17dca22b1 100644
--- a/Makefile
+++ b/Makefile
@@ -1593,6 +1593,7 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)
 ifndef NO_RUST
 BASIC_CFLAGS += -DWITH_RUST
 GITLIBS += $(RUST_LIB)
+RUST_LIBS = $(RUST_LIB)
 ifeq ($(uname_S),Windows)
 EXTLIBS += -luserenv
 endif
@@ -4082,9 +4083,9 @@ $(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
+contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) $(RUST_LIBS) GIT-LDFLAGS
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
-		$(filter %.o,$^) $(LIB_FILE) $(EXTLIBS) -framework Security -framework CoreFoundation
+		$(filter %.o,$^) $(LIB_FILE) $(RUST_LIBS) $(EXTLIBS) -framework Security -framework CoreFoundation
 
 contrib/credential/osxkeychain/git-credential-osxkeychain.o: contrib/credential/osxkeychain/git-credential-osxkeychain.c GIT-CFLAGS
 	$(QUIET_LINK)$(CC) -o $@ -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<

base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2
-- 
gitgitgadget

Re: [PATCH] Makefile: link osxkeychain helper against Rust

[Kristoffer Haugsbakk]

On Tue, May 5, 2026, at 19:26, Shardul Natu via GitGitGadget wrote:
> From: Shnatu <snatu@google.com>
>
> When Rust is enabled, ensure that the git-credential-osxkeychain
> helper is linked with the necessary Rust libraries.
>
> Introduce the RUST_LIBS variable inside ifndef NO_RUST block
> to hold the Rust library dependency, and use it in the helper's
> build target. This cleanly handles cases where Rust is disabled,
> making it a no-op and avoiding any build failures on systems
> without Cargo.
>
> This addresses reviewer feedback from internal CL 910223487
> by simplifying the variables and avoiding confusing "LINK"
> terminology.

This pararagraph is meaningless to those outside internal.

>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
>[snip]

Re: [PATCH] Makefile: link osxkeychain helper against Rust

[Shnatu]

I have remove the Google specific paragraph, in addition to updating the description

Re: [PATCH] Makefile: link osxkeychain helper against Rust

[Junio C Hamano]

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

> From: Shnatu <snatu@google.com>

If your name is "Shardul Natu", we'd prefer (not 'require', but
'prefer') that the patches authored by you also identify with that
name, both on "From:" and "Signed-off-by:"..

> When Rust is enabled, ensure that the git-credential-osxkeychain
> helper is linked with the necessary Rust libraries.
>
> Introduce the RUST_LIBS variable inside ifndef NO_RUST block
> to hold the Rust library dependency, and use it in the helper's
> build target. This cleanly handles cases where Rust is disabled,
> making it a no-op and avoiding any build failures on systems
> without Cargo.
>
> This addresses reviewer feedback from internal CL 910223487
> by simplifying the variables and avoiding confusing "LINK"
> terminology.
>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
>     Makefile: link osxkeychain helper against Rust

Thanks.  I've added to CC: a few folks who may be more clueful in
the affected area than I am.  It somehow feels strange that we have
to have RUST_LIB and RUST_LIBS separately, and apparently with the
new definition the latter is expected to be a superset of the
former, and it is unclear what are the things that should be added
to the latter without getting added to the former.

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2288%2Fkiranani%2Fnext-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2288/kiranani/next-v1
> Pull-Request: https://github.com/git/git/pull/2288
>
>  Makefile | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f86173f93a..a17dca22b1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1593,6 +1593,7 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)
>  ifndef NO_RUST
>  BASIC_CFLAGS += -DWITH_RUST
>  GITLIBS += $(RUST_LIB)
> +RUST_LIBS = $(RUST_LIB)
>  ifeq ($(uname_S),Windows)
>  EXTLIBS += -luserenv
>  endif
> @@ -4082,9 +4083,9 @@ $(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
> +contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) $(RUST_LIBS) GIT-LDFLAGS
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> -		$(filter %.o,$^) $(LIB_FILE) $(EXTLIBS) -framework Security -framework CoreFoundation
> +		$(filter %.o,$^) $(LIB_FILE) $(RUST_LIBS) $(EXTLIBS) -framework Security -framework CoreFoundation
>  
>  contrib/credential/osxkeychain/git-credential-osxkeychain.o: contrib/credential/osxkeychain/git-credential-osxkeychain.c GIT-CFLAGS
>  	$(QUIET_LINK)$(CC) -o $@ -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
>
> base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2

Re: [PATCH] Makefile: link osxkeychain helper against Rust

[Koji Nakamaru]

On Fri, May 8, 2026 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Shnatu <snatu@google.com>
>
> If your name is "Shardul Natu", we'd prefer (not 'require', but
> 'prefer') that the patches authored by you also identify with that
> name, both on "From:" and "Signed-off-by:"..
>
> > When Rust is enabled, ensure that the git-credential-osxkeychain
> > helper is linked with the necessary Rust libraries.
> >
> > Introduce the RUST_LIBS variable inside ifndef NO_RUST block
> > to hold the Rust library dependency, and use it in the helper's
> > build target. This cleanly handles cases where Rust is disabled,
> > making it a no-op and avoiding any build failures on systems
> > without Cargo.
> >
> > This addresses reviewer feedback from internal CL 910223487
> > by simplifying the variables and avoiding confusing "LINK"
> > terminology.
> >
> > Signed-off-by: Shnatu <snatu@google.com>
> > ---
> >     Makefile: link osxkeychain helper against Rust
>
> Thanks.  I've added to CC: a few folks who may be more clueful in
> the affected area than I am.  It somehow feels strange that we have
> to have RUST_LIB and RUST_LIBS separately, and apparently with the
> new definition the latter is expected to be a superset of the
> former, and it is unclear what are the things that should be added
> to the latter without getting added to the former.
>
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2288%2Fkiranani%2Fnext-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2288/kiranani/next-v1
> > Pull-Request: https://github.com/git/git/pull/2288
> >
> >  Makefile | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index f86173f93a..a17dca22b1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1593,6 +1593,7 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)
> >  ifndef NO_RUST
> >  BASIC_CFLAGS += -DWITH_RUST
> >  GITLIBS += $(RUST_LIB)
> > +RUST_LIBS = $(RUST_LIB)
> >  ifeq ($(uname_S),Windows)
> >  EXTLIBS += -luserenv
> >  endif
> > @@ -4082,9 +4083,9 @@ $(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
> > +contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) $(RUST_LIBS) GIT-LDFLAGS
> >       $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> > -             $(filter %.o,$^) $(LIB_FILE) $(EXTLIBS) -framework Security -framework CoreFoundation
> > +             $(filter %.o,$^) $(LIB_FILE) $(RUST_LIBS) $(EXTLIBS) -framework Security -framework CoreFoundation
> >
> >  contrib/credential/osxkeychain/git-credential-osxkeychain.o: contrib/credential/osxkeychain/git-credential-osxkeychain.c GIT-CFLAGS
> >       $(QUIET_LINK)$(CC) -o $@ -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
> >
> > base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2

How about simply wrapping the RUST_LIB-related sections in ifndef
NO_RUST, as shown below? This way, we can avoid defining
RUST_LIBS.

diff --git a/Makefile b/Makefile
index f86173f93a..daa1691950 100644
--- a/Makefile
+++ b/Makefile
@@ -947,11 +947,13 @@ else
 RUST_TARGET_DIR = target/release
 endif

+ifndef NO_RUST
 ifeq ($(uname_S),Windows)
 RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib
 else
 RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a
 endif
+endif

 GITLIBS = common-main.o $(LIB_FILE)
 EXTLIBS =
@@ -3027,11 +3029,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

@@ -4082,9 +4086,9 @@ $(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
+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,$^) $(LIB_FILE) $(EXTLIBS) -framework
Security -framework CoreFoundation
+               $(filter %.o,$^) $(LIB_FILE) $(RUST_LIB) $(EXTLIBS)
-framework Security -framework CoreFoundation

 contrib/credential/osxkeychain/git-credential-osxkeychain.o:
contrib/credential/osxkeychain/git-credential-osxkeychain.c GIT-CFLAGS
        $(QUIET_LINK)$(CC) -o $@ -c $(dep_args) $(compdb_args)
$(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<

Re: [PATCH] Makefile: link osxkeychain helper against Rust

[Shnatu]

Thank you for the suggestion! This is indeed a much cleaner approach.

By wrapping `RUST_LIB` and the rust targets in `ifndef NO_RUST`, we can link `git-credential-osxkeychain` directly against `$(RUST_LIB)` without needing to introduce an intermediate `RUST_LIBS` variable. When `NO_RUST` is defined, `$(RUST_LIB)` evaluates to empty, and Make naturally links it as a pure C binary without any Rust dependencies.

To integrate this with the universal build support (`RUST_TARGETS`/`lipo`) introduced in this PR, I have updated the changes to:

- Wrap the `RUST_LIB` definition block (which resolves target-specific paths for universal builds) in ifndef `NO_RUST`.
- Wrap the entire universal compilation and lipo combining block in ifndef `NO_RUST`.
- Remove the `RUST_LIBS` helper variable and use `$(RUST_LIB)` directly in `git-credential-osxkeychain`.