Re: [PATCH RFC v2 2/7] Makefile: introduce infrastructure to build internal Rust library

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Fri, Sep 05, 2025 at 01:50:58PM +0200, Patrick Steinhardt wrote:
> Introduce infrastructure to build the internal Rust library. This
> mirrors the infrastructure we have added to Meson in the preceding
> commit. Developers can enable the infrastructure by passing the new
> `WITH_RUST` build toggle.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  .gitignore |  2 ++
>  Cargo.toml |  9 +++++++++
>  Makefile   | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 54 insertions(+), 2 deletions(-)

> diff --git a/Makefile b/Makefile
> index 555b7f4dc3..e7b3c8e57b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -483,6 +483,14 @@ include shared.mak
>  # Define LIBPCREDIR=/foo/bar if your PCRE header and library files are
>  # in /foo/bar/include and /foo/bar/lib directories.
>  #
> +# == Optional Rust support ==
> +#
> +# Define WITH_RUST if you want to include features and subsystems written in
> +# Rust into Git. For now, Rust is still an optional feature of the build
> +# process. With Git 3.0 though, Rust will always be enabled.
> +#
> +# Building Rust code requires Cargo.
> +#
>  # == SHA-1 and SHA-256 defines ==
>  #
>  # === SHA-1 backend ===
> @@ -918,6 +926,11 @@ TEST_SHELL_PATH = $(SHELL_PATH)
>  LIB_FILE = libgit.a
>  XDIFF_LIB = xdiff/lib.a
>  REFTABLE_LIB = reftable/libreftable.a
> +ifdef DEBUG
> +RUST_LIB = target/debug/libgit.a
> +else
> +RUST_LIB = target/release/libgit.a
> +endif
>  
>  GENERATED_H += command-list.h
>  GENERATED_H += config-list.h
> @@ -1387,8 +1400,12 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
>  
>  UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
>  
> -# xdiff and reftable libs may in turn depend on what is in libgit.a
> -GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)
> +GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB)
> +ifdef WITH_RUST
> +GITLIBS += $(RUST_LIB)
> +endif
> +# Other libs may in turn depend on what is in libgit.a.
> +GITLIBS += $(LIB_FILE)
>  EXTLIBS =
>  
>  GIT_USER_AGENT = git/$(GIT_VERSION)
> @@ -1411,6 +1428,19 @@ BASIC_LDFLAGS =
>  ARFLAGS = rcs
>  PTHREAD_CFLAGS =
>  
> +# Rust flags
> +CARGO_ARGS =
> +ifndef V
> +CARGO_ARGS += --quiet
> +endif
> +ifndef DEBUG
> +CARGO_ARGS += --release
> +endif
> +
> +ifdef WITH_RUST
> +BASIC_CFLAGS += -DWITH_RUST
> +endif
> +
>  # For the 'sparse' target
>  SPARSE_FLAGS ?= -std=gnu99 -D__STDC_NO_VLA__
>  SP_EXTRA_FLAGS =
> @@ -2918,6 +2948,16 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
>  $(LIB_FILE): $(LIB_OBJS)
>  	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
>  
> +$(RUST_LIB): FORCE

Why is this target FORCE-d instead of declaring its dependencies?

> +	@OLD_STAT="$$(stat $@ 2>/dev/null)"; \
> +	    cargo build $(CARGO_ARGS); \
> +	    if test $$? != 0 || test x"$$OLD_STAT" != x"$$(stat $@ 2>/dev/null)"; then \
> +		echo '   ' CARGO $@; \
> +	    fi

This hides 'cargo's exit code from 'make', so a failure to build the
Rust components won't abort the build.  So once we have a successfully
built Rust library and were to inadvertently introduce a syntax error
into the Rust code:

  $ echo foo >src/varint.rs 
  $ make WITH_RUST=1
  error: expected one of `!` or `::`, found `<eof>`
   --> src/varint.rs:1:1
    |
  1 | foo
    | ^^^ expected one of `!` or `::`

  error: could not compile `git` (lib) due to previous error
      CARGO target/release/libgit.a
      SUBDIR git-gui
      SUBDIR gitk-git
      SUBDIR templates
  $ echo $?
  0

Also note that the error message is printed before the command that
produced that error; it should be the other way around.

> +
> +.PHONY: rust
> +rust: $(RUST_LIB)
> +
>  $(XDIFF_LIB): $(XDIFF_OBJS)
>  	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
>  
> @@ -3768,6 +3808,7 @@ clean: profile-clean coverage-clean cocciclean
>  	$(RM) $(FUZZ_PROGRAMS)
>  	$(RM) $(SP_OBJ)
>  	$(RM) $(HCC)
> +	$(RM) -r target/ Cargo.lock
>  	$(RM) version-def.h
>  	$(RM) -r $(dep_dirs) $(compdb_dir) compile_commands.json
>  	$(RM) $(test_bindir_programs)
> 
> -- 
> 2.51.0.417.g1ba7204a04.dirty
>