Subject: Re: [PATCH v4 5/5] Makefile: add option to build and test libgit-rs and libgit-rs-sys

From: Junio C Hamano <[email-masked]>
Date: Tue, 08 Oct 2024 17:01:07 -0700

Josh Steadmon <[email-masked]> writes:

> From: Calvin Wan <[email-masked]>
>
> Add libgitrs, libgitrs-sys, libgitrs-test, and libgitrs-sys-test targets
> to their respective Makefiles so they can be built and tested without
> having to run cargo build/test.
>
> Add environment variable, INCLUDE_LIBGIT_RS, that when set,
> automatically builds and tests libgit-rs and libgit-rs-sys when `make
> all` is ran.
>
> Signed-off-by: Calvin Wan <[email-masked]>
> Signed-off-by: Josh Steadmon <[email-masked]>
> ---
>  Makefile   | 16 ++++++++++++++++
>  t/Makefile | 16 ++++++++++++++++
>  2 files changed, 32 insertions(+)

Interesting.

I tried

	$ make INCLUDE_LIBGIT_RS=YesPlease

which did not fail, and then did the same

	$ make INCLUDE_LIBGIT_RS=YesPlease

and was surprised to see that not only the libgit-sys part but
everything was recompiled and rebuilt.

> diff --git a/Makefile b/Makefile
> ...
> +.PHONY: libgitrs
> +libgitrs:
> +	$(QUIET)(\
> +		cd contrib/libgit-rs && \
> +		cargo build \
> +	)
> +ifdef INCLUDE_LIBGIT_RS
> +all:: libgitrs
> +endif
> +
>  contrib/libgit-rs/libgit-sys/partial_symbol_export.o: contrib/libgit-rs/libgit-sys/public_symbol_export.o libgit.a reftable/libreftable.a xdiff/lib.a
>  	$(LD) -r $^ -o $@

I can see libgitrs is a phony target designed to run every time it
gets triggered, and I would imagine "cargo build" itself would avoid
repeating unnecessary work, but I do not see this patch screwing up
with the dependencies for other object files.

Is it fair to say this is still a WIP?  Showing a WIP to others and
asking for help is OK, but it is fair to make sure that others know
what is expected of them.

Thanks.