All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
	Shardul Natu <snatu@google.com>,
	Koji Nakamaru <koji.nakamaru@gree.net>,
	Patrick Steinhardt <ps@pks.im>,
	Shardul Natu <shardul.27591@gmail.com>,
	Ben Knoble <ben.knoble@gmail.com>
Subject: [PATCH v6 0/3] Makefile: link osxkeychain helper against Rust
Date: Mon, 06 Jul 2026 22:52:10 +0000	[thread overview]
Message-ID: <pull.2288.v6.git.git.1783378333.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2288.v5.git.git.1783358097.gitgitgadget@gmail.com>

This series improves macOS build reliability, automated CI verification, and
distribution support when Rust is enabled in the Git build system. It
addresses three distinct challenges: a parallel build race condition in
git-credential-osxkeychain, support for macOS Universal Binaries
(multi-architecture distribution), and missing automated CI test wiring for
macOS contrib utilities.


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.

 3. Automated CI Verification for Contrib on macOS: When running make test
    with TEST_CONTRIB_TOO=yes (default in macOS CI workflows), $(MAKE) -C
    contrib/ test is invoked. However, contrib/Makefile only invoked tests
    for diff-highlight and subtree, meaning git-credential-osxkeychain was
    never compiled or verified during standard CI test runs.


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.
   
   * Patch 3: contrib: wire up osxkeychain in contrib/Makefile on macOS Adds
     a test target to contrib/credential/osxkeychain/Makefile that depends
     on building git-credential-osxkeychain. Introduces a generic OS_CONTRIB
     variable in contrib/Makefile to conditionally wire
     credential/osxkeychain into all, test, and clean whenever running on
     macOS (Darwin). This guarantees that standard CI test runs on macOS
     automatically compile and link the helper, preventing build
     regressions.

Changes since v5:

 * Reverted Patch 1 to depend explicitly on $(LIB_FILE) $(RUST_LIB) rather
   than $(GITLIBS). Unlike Git builtins or scalar (which define cmd_main()),
   git-credential-osxkeychain.c defines its own standalone main(), meaning
   $(GITLIBS) caused a duplicate symbol error for _main during linking.
 * Added Patch 3 ("contrib: wire up osxkeychain in contrib/Makefile on
   macOS") using a scalable OS_CONTRIB variable so that running make test
   with TEST_CONTRIB_TOO=yes in macOS CI workflows automatically verifies
   compilation and linking integrity.

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 (3):
  Makefile: add $(RUST_LIB) prerequisite to osxkeychain
  Makefile: support universal macOS builds via RUST_TARGETS
  contrib: wire up osxkeychain in contrib/Makefile on macOS

 Makefile                                | 46 ++++++++++++++++++++++---
 contrib/Makefile                        | 10 ++++++
 contrib/credential/osxkeychain/Makefile |  4 ++-
 3 files changed, 54 insertions(+), 6 deletions(-)


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

Range-diff vs v5:

 1:  e0bb18ff01 ! 1:  0d21513940 Makefile: add $(GITLIBS) prerequisite to osxkeychain
     @@ Metadata
      Author: Shardul Natu <snatu@google.com>
      
       ## Commit message ##
     -    Makefile: add $(GITLIBS) prerequisite to osxkeychain
     +    Makefile: add $(RUST_LIB) 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.
      
     -    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.
     +    Note that we depend explicitly on $(LIB_FILE) and $(RUST_LIB) rather
     +    than $(GITLIBS). Unlike standard Git builtins and programs like scalar
     +    (which define cmd_main() and rely on common-main.o to supply main()),
     +    git-credential-osxkeychain.c defines its own standalone int main().
     +    If $(GITLIBS) were used, $(filter %.o,$^) in the link recipe would
     +    match both git-credential-osxkeychain.o and common-main.o, causing a
     +    duplicate symbol linking error for _main on macOS.
      
          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 $(GITLIBS) 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,$^) $(LIBS) -framework Security -framework CoreFoundation
       
 2:  66f71fb0d7 = 2:  21dedb91f0 Makefile: support universal macOS builds via RUST_TARGETS
 -:  ---------- > 3:  8455e449f3 contrib: wire up osxkeychain in contrib/Makefile on macOS

-- 
gitgitgadget

  parent reply	other threads:[~2026-07-06 22:52 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05 17:26 [PATCH] Makefile: link osxkeychain helper against Rust Shardul Natu via GitGitGadget
2026-05-05 19:08 ` Kristoffer Haugsbakk
2026-05-07  0:39   ` Shnatu
2026-05-08  2:54 ` Junio C Hamano
2026-05-08  9:33   ` Koji Nakamaru
2026-05-08 17:44     ` Shnatu
2026-07-01 22:01 ` [PATCH v2] Makefile: link osxkeychain & support universal Rust Shardul Natu via GitGitGadget
2026-07-02  1:35   ` Junio C Hamano
2026-07-02 11:50   ` Patrick Steinhardt
2026-07-02 22:30     ` Shardul Natu
2026-07-03  5:15       ` Patrick Steinhardt
2026-07-03 12:02     ` lipo availability [was: [PATCH v2] Makefile: link osxkeychain & support universal Rust] Ben Knoble
2026-07-02 22:22   ` [PATCH v3 0/2] Makefile: link osxkeychain helper against Rust Shardul Natu via GitGitGadget
2026-07-02 22:22     ` [PATCH v3 1/2] Makefile: add $(RUST_LIB) prerequisite to osxkeychain Shardul Natu via GitGitGadget
2026-07-02 22:22     ` [PATCH v3 2/2] Makefile: support universal macOS builds via RUST_TARGETS Shardul Natu via GitGitGadget
2026-07-03  5:36       ` Junio C Hamano
2026-07-03 17:37         ` Shardul Natu
2026-07-04 18:05     ` [PATCH v4 0/2] Makefile: link osxkeychain helper against Rust Shardul Natu via GitGitGadget
2026-07-04 18:05       ` [PATCH v4 1/2] Makefile: add $(RUST_LIB) prerequisite to osxkeychain Shardul Natu via GitGitGadget
2026-07-06 10:49         ` Patrick Steinhardt
2026-07-04 18:05       ` [PATCH v4 2/2] Makefile: support universal macOS builds via RUST_TARGETS Shardul Natu via GitGitGadget
2026-07-06 10:49         ` Patrick Steinhardt
2026-07-05  4:08       ` [PATCH v4 0/2] Makefile: link osxkeychain helper against Rust Junio C Hamano
2026-07-05 17:38         ` Shardul Natu
2026-07-06 17:14       ` [PATCH v5 " Shardul Natu via GitGitGadget
2026-07-06 17:14         ` [PATCH v5 1/2] Makefile: add $(GITLIBS) prerequisite to osxkeychain Shardul Natu via GitGitGadget
2026-07-06 19:52           ` Junio C Hamano
2026-07-06 17:14         ` [PATCH v5 2/2] Makefile: support universal macOS builds via RUST_TARGETS Shardul Natu via GitGitGadget
2026-07-06 22:52         ` Shardul Natu via GitGitGadget [this message]
2026-07-06 22:52           ` [PATCH v6 1/3] Makefile: add $(RUST_LIB) prerequisite to osxkeychain Shardul Natu via GitGitGadget
2026-07-06 22:52           ` [PATCH v6 2/3] Makefile: support universal macOS builds via RUST_TARGETS Shardul Natu via GitGitGadget
2026-07-06 22:52           ` [PATCH v6 3/3] contrib: wire up osxkeychain in contrib/Makefile on macOS Shardul Natu via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.2288.v6.git.git.1783378333.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=koji.nakamaru@gree.net \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=ps@pks.im \
    --cc=shardul.27591@gmail.com \
    --cc=snatu@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.