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

On 2025.01.28 14:43, Junio C Hamano wrote:

Josh Steadmon [off-list ref] writes:

quoted

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.

quoted

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

Ack, will send a V9 soon with this & a fix for Phillip's feedback.
Thanks.