Re: [PATCH v7 3/3] contrib: wire up osxkeychain in contrib/Makefile on macOS

[Junio C Hamano <gitster@pobox.com>]

"Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Shardul Natu <snatu@google.com>
>
> When running "make test" with TEST_CONTRIB_TOO=yes (which is 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 built or verified during
> standard CI test runs.
>
> Add a "test" target to contrib/credential/osxkeychain/Makefile that
> depends on building git-credential-osxkeychain. Additionally, wire up
> credential/osxkeychain in contrib/Makefile under "all", "test", and
> "clean" whenever running on macOS (Darwin).
>
> This ensures that running "make test" or "make all" in contrib on macOS
> automatically builds and links git-credential-osxkeychain, preventing
> future build or symbol linking regressions from slipping through CI.
>
> Signed-off-by: Shardul Natu <snatu@google.com>
> ---
>  contrib/Makefile                        | 10 ++++++++++
>  contrib/credential/osxkeychain/Makefile |  4 +++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/Makefile b/contrib/Makefile
> index 787cd07f52..7962a9ff12 100644
> --- a/contrib/Makefile
> +++ b/contrib/Makefile
> @@ -1,10 +1,20 @@
> +-include ../config.mak.autogen
> +-include ../config.mak
> +
> +ifeq ($(uname_S),Darwin)
> +OS_CONTRIB += credential/osxkeychain
> +endif

Is $(uname_S) defined here at this point with only the above two
includes?  Don't you need to include ../config.mak.uname as well?

The top-level Makefile does this:

        include config.mak.uname
        -include config.mak.autogen
        -include config.mak

and so should this one, I think, in exactly the same order.

>  all::
> +	$(foreach dir,$(OS_CONTRIB),$(MAKE) -C $(dir) $@;)
>  
>  test::
>  	$(MAKE) -C diff-highlight $@
>  	$(MAKE) -C subtree $@
> +	$(foreach dir,$(OS_CONTRIB),$(MAKE) -C $(dir) $@;)
>  
>  clean::
>  	$(MAKE) -C contacts $@
>  	$(MAKE) -C diff-highlight $@
>  	$(MAKE) -C subtree $@
> +	$(foreach dir,$(OS_CONTRIB),$(MAKE) -C $(dir) $@;)
> diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
> index 219b0d7f49..d9fba07e8d 100644
> --- a/contrib/credential/osxkeychain/Makefile
> +++ b/contrib/credential/osxkeychain/Makefile
> @@ -10,4 +10,6 @@ install:
>  clean:
>  	$(MAKE) -C ../../.. clean-git-credential-osxkeychain
>  
> -.PHONY: all git-credential-osxkeychain install clean
> +test: git-credential-osxkeychain
> +
> +.PHONY: all git-credential-osxkeychain install clean test