pr-git-2288/kiranani/next-v6
tagged this
06 Jul 22:52
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
Submitted-As: https://lore.kernel.org/git/pull.2288.v6.git.git.1783378333.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2288.git.git.1778001976709.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2288.v2.git.git.1782943303219.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2288.v3.git.git.1783030971.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2288.v4.git.git.1783188355.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2288.v5.git.git.1783358097.gitgitgadget@gmail.com