Subject: Re: [PATCH v8 2/4] libgit-sys: introduce Rust wrapper for libgit.a

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Jan 2025 14:43:56 -0800

Josh Steadmon <[email-masked]> writes:

> diff --git a/Makefile b/Makefile
> index 27e68ac039..f2c08df4cb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -416,6 +416,9 @@ include shared.mak
>  # Define LINK_FUZZ_PROGRAMS if you want `make all` to also build the fuzz test
>  # programs in oss-fuzz/.
>  #
> +# Define INCLUDE_LIBGIT_RS if you want `make all` and `make test` to build and
> +# test the Rust crate in contrib/libgit-sys.
> +#
>  # === Optional library: libintl ===
>  #
>  # Define NO_GETTEXT if you don't want Git output to be translated.
> @@ -657,6 +660,8 @@ CURL_CONFIG = curl-config
>  GCOV = gcov
>  STRIP = strip
>  SPATCH = spatch
> +LD = ld
> +OBJCOPY = objcopy
>  
>  export TCL_PATH TCLTK_PATH

Good.

> +LIBGIT_PUB_OBJS = contrib/libgit-sys/public_symbol_export.o
> +LIBGIT_PUB_OBJS += libgit.a
> +LIBGIT_PUB_OBJS += reftable/libreftable.a
> +LIBGIT_PUB_OBJS += xdiff/lib.a

This is a fairly minor point, but if we look at the places where
OBJECTS, PROGRAMS, and friends are prepared in the Makefile, they
all start with empty, i.e.

	LIBGIT_PUB_OBJS =

at a fairly early place in the file, and then appends real contents
to the symbol with += assignment.  It probably makes sense to follow
suit, although the result would be the same.

Other than that, nothing jumped at me as fishy.  Nicely done.

Thanks.  Will replace.