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

From: Shardul Natu <shardul.27591@gmail.com>
Date: Jul 2, 2026, 22:30 UTC
Message-ID: <CABw8Y3H7P3JKwaSrUGjifcDh7rMR2nCFgqPjw8q6vfZnLc730w@mail.gmail.com>
In-Reply-To: <akZQmDYe9MtTdGM2@pks.im>

Show 6 quoted lines

> "Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Shnatu <snatu@google.com>
> > Signed-off-by: Shardul Natu <snatu@google.com>
>
> You'd want to make sure these two match.

Good catch. Done!

Show 6 quoted lines

> > This is a highly elegant and native Makefile solution that avoids
> > complex shell scripting in recipes and fully supports macOS Universal
> > Binaries.
>
> You're the second person on this list I saw who calls their own
> creation "elegant" ;-).

Removed! It was AI generated description

Show 10 quoted lines

> Do we know that leading directories to $(RUST_LIB) target has
> already been created at this point? If not, we may want to have
>
> $(RUST_LIB): $(RUST_MEMBER_LIBS)
> + $(call mkdir_p_parent_template)
> $(QUIET_GEN)\
> if [ $(words $(RUST_TARGETS)) -gt 1 ]; then \
> lipo -create $^ -output $@; \
>
> on top.

Added $(call mkdir_p_parent_template).

Show 10 quoted lines

> > 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.

You're right; these address two fundamentally different
problems. In v3, I have split this into a two commits:
1. Makefile: add $(RUST_LIB) prerequisite to osxkeychain
2. Makefile: support universal macOS builds via RUST_TARGETS

> 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?

I have added this background to the relevant commit.

Show 6 quoted lines

> 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?

Yes, "lipo" is part of the Apple Xcode CLT, which
is already a hard prerequisite for invoking clang or make on macOS.
The reason only Rust needs special treatment in the Makefile is due to
how the respective toolchains handle multi-architecture builds:
1. Apple's C toolchain (clang) natively supports universal builds via
CFLAGS and 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.
2. Cargo and rustc, however, do not support multiple "-arch" flags or
emitting universal binaries in a single invocation. Instead, Cargo must
be invoked separately for each target triple ("--target x86_64-apple-darwin"
and "--target aarch64-apple-darwin").

By using "lipo" to combine those target-specific Rust static libraries into a single universal archive at "target/release/libgitcore.a", we bridge this gap. Once $(RUST_LIB) is a universal archive, the standard C linker seamlessly links it with the C object files to produce the final universal Git executables.

On Thu, Jul 2, 2026 at 4:57 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 81 quoted lines

>
> 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
>