On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
>
> We obviously do not want to bundle `.mo` files during `make
> artifacts-tar NO_GETTEXT`, but that was the case.
Should be:
make artifacts-tar NO_GETTEXT=YesPlease
(Without the =<something> we try to find a "NO_GETTEXT" target)
> To fix that, go a step beyond just fixing the symptom, and simply
> define the lists of `.po` and `.mo` files as empty if `NO_GETTEXT` is
> set.
How about fixing the cause instead of the symptom...
> Helped-by: Matthias Aßhauer <mha1993@xxxxxxx>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> Makefile | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index c3565fc0f8f..04e852be015 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2675,10 +2675,13 @@ po/git.pot: $(GENERATED_H) FORCE
> .PHONY: pot
> pot: po/git.pot
>
> +ifdef NO_GETTEXT
> +POFILES :=
> +MOFILES :=
> +else
> POFILES := $(wildcard po/*.po)
> MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
>
> -ifndef NO_GETTEXT
> all:: $(MOFILES)
> endif
...i.e. this patch just seems like odd (ab)use of Makefile logic.
Later on in the artifacts-tar rule we rely on our immediate dependency
list in $^ to feed to "tar czf", and here we're going to set $(MOFILES)
to an empty list, just to later interpolate that empty list into that
list of dependencies.
Wouldn't the mores straightforward thing to do be the diff I've got at
the end here, perhaps with a preceding commit just for the split-up of
the dependency list?
This matches how we do things elsewhere, i.e. we don't ifdef e.g. this away:
LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
rather we keep the list as-is, and ifdef the actual addition of the
dependency, e.g.:
ifndef NO_PERL
all:: $(LIB_PERL_GEN)
[...]
endif
One reason we do it like this is because we *don't* want to forget what
the MOFILES were, because you want e.g. "make clean" to clean them up
(not that it matters in this case, we rm -rf po/build).
Doesn't matter much here, but following this pattern leads to subtle
"bugs", e.g. an outstanding issue in your 179227d6e21 (Optionally skip
linking/copying the built-ins, 2020-09-21) (which I noted on-list in
passing before, IIRC) where during a build we end up with stale
built-ins from a previous build in the build directory, because we
pruned the list during definition time, as opposed to adding an inverse
"I should remove this then" rule.
("bug" because it doesn't have any actual effect I know of other than
bothering me that I have e.g. a git-add in my build-dir still :)
diff --git a/Makefile b/Makefile
index c3565fc0f8f..7fb1d4b6caa 100644
--- a/Makefile
+++ b/Makefile
@@ -3146,9 +3146,18 @@ ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)
endif
-artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
- GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
- $(MOFILES)
+ARTIFACTS_TAR =
+ARTIFACTS_TAR += GIT-BUILD-OPTIONS
+ARTIFACTS_TAR += $(ALL_COMMANDS_TO_INSTALL)
+ARTIFACTS_TAR += $(SCRIPT_LIB)
+ARTIFACTS_TAR += $(OTHER_PROGRAMS)
+ARTIFACTS_TAR += $(TEST_PROGRAMS)
+ARTIFACTS_TAR += $(test_bindir_programs)
+ifndef NO_GETTEXT
+ARTIFACTS_TAR += $(MOFILES)
+endif
+
+artifacts-tar:: $(ARTIFACTS_TAR)
$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
test -n "$(ARTIFACTS_DIRECTORY)"
