Re: [PATCH] make strip: include `scalar`
Hi Junio,
On Mon, 17 Nov 2025, Junio C Hamano wrote:
Show 35 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > When Scalar was made a canonical part of Git in 7b5c93c6c68 (scalar:
> > include in standard Git build & installation, 2022-09-02), it was added
> > to all relevant Makefile targets except for the `strip` target.
> >
> > Let's correct that.
>
> The motivation makes perfect sense.
>
> > diff --git a/Makefile b/Makefile
> > index 7e0f77e298..62f7f7bf56 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2565,7 +2565,7 @@ please_set_SHELL_PATH_to_a_more_modern_shell:
> >
> > shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
> >
> > -strip: $(PROGRAMS) git$X
> > +strip: $(PROGRAMS) git$X scalar$X
> > $(STRIP) $(STRIP_OPTS) $^
>
> I wonder why the original names git$X here explicitly, instead of
> using say $(OTHER_PROGRAMS) that covers both of these. I know that
> the undocumented INCLUDE_DLLS_IN_ARTIFACTS knob uses OTHER_PROGRAMS
> by throwing in non-programs like DLLs to it, so that artifacts-tar
> target would include them, but perhaps instead of working around the
> misdesign of that target, wouldn't it be better to correct its use
> of OTHER_PROGRAMS and use it here instead?
>
> The change (including the "strip scalar, too!" part) should look
> like this, I think.
> Also do we need a matching change to CMake and meson?
I am unfamiliar with Meson, and do not see anything about stripping in `meson.build` apart from a `--strip` option that is mentioned in a comment (and which I would assume already handles all executables, otherwise the move to Meson really is not worth all the hassle).
About CMake: It was always meant as a tool to help Visual Studio users to build and debug Git for Windows conveniently (something that Meson distinctly fails to accomplish). As such, there is no support for stripping executables in the CMake definition, that's completely up to how the Release builds are set up.
Besides, since Meson was picked over CMake as the modern build setup, I am seriously playing with the idea of abandoning Git's CMake definition (and with that, all Visual Studio-based developers, of course).
Ciao, Johannes
Show 40 quoted lines
>
> Makefile | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git c/Makefile w/Makefile
> index 70d1543b6b..a63a4adbc7 100644
> --- c/Makefile
> +++ w/Makefile
> @@ -682,6 +682,7 @@ LIB_OBJS =
> LIBGIT_PUB_OBJS =
> SCALAR_OBJS =
> OBJECTS =
> +OTHER_ARTIFACTS =
> OTHER_PROGRAMS =
> PROGRAM_OBJS =
> PROGRAMS =
> @@ -2499,7 +2500,7 @@ please_set_SHELL_PATH_to_a_more_modern_shell:
>
> shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>
> -strip: $(PROGRAMS) git$X
> +strip: $(PROGRAMS) $(OTHER_PROGRAMS)
> $(STRIP) $(STRIP_OPTS) $^
>
> ### Target-specific flags and dependencies
> @@ -3697,10 +3698,11 @@ rpm::
> .PHONY: rpm
>
> ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
> -OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll t/unit-tests/bin/*.dll)
> +OTHER_ARTIFACTS += $(shell echo *.dll t/helper/*.dll t/unit-tests/bin/*.dll)
> endif
>
> artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
> + $(OTHER_ARTIFACTS) \
> GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
> $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) $(MOFILES)
> $(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
>
>
Johannes Schindelin via GitGitGadget
