Re: [PATCH v2] Makefile: link osxkeychain & support universal Rust

[Patrick Steinhardt <ps@pks.im>]

On Wed, Jul 01, 2026 at 10:01:43PM +0000, 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.
> 
> Also, introduce native support for macOS Universal Binaries
> (multi-architecture builds) in the Git build system by allowing
> the user to specify a list of target triples in the RUST_TARGETS
> environment variable.

These are fundamentally unrelated things, aren't they? So I'd argue they
should be split up into two commits.

I think we could also use an explanation here what the universal binary
buys us for those who are not deeply familiar with the macOS platform.
What are they, and why do we want/need to support them?

> To implement this cleanly without complex shell scripting in recipes:
>   1. We introduce a declarative Make pattern rule (target/%/...) to
>      compile each target-specific library slice (e.g.,
>      target/aarch64-apple-darwin/...).
>   2. We update the $(RUST_LIB) recipe to depend on the list of
>      compiled target-specific member libraries ($(RUST_MEMBER_LIBS)).
>   3. On macOS, if multiple targets are specified, we use lipo to
>      combine them into a single Universal static library at
>      target/release/libgitcore.a.
>   4. If only one target is specified, we copy it to the standard
>      path.
>   5. We enforce that building for multiple targets requires macOS
>      (as lipo is only available there), raising a clear make error
>      on other platforms.
> 
> This is a highly elegant and native Makefile solution that avoids
> complex shell scripting in recipes and fully supports macOS Universal
> Binaries.

As Junio already pointed out this self-praise reads quite weird. I'm
just going to assume that this is AI-generated fluff.

> diff --git a/Makefile b/Makefile
> index 1f3f099f5c..8d49ecc897 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3019,11 +3030,33 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
>  $(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)
> +	$(QUIET_GEN)\
> +	if [ $(words $(RUST_TARGETS)) -gt 1 ]; then \
> +		lipo -create $^ -output $@; \

Can we assume lipo to be generally available on macOS? Also, is it
sufficient to just do this for the library? I would have expected that
binaries would also need some treatment there.

In other words: what does it help us to have the Rust treated this way
if the rest isn't?

Thanks!

Patrick