Subject: Re: [PATCH v2 2/9] Makefile: generate "po/git.pot" from stable LOCALIZED_C

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 19 May 2022 11:18:17 +0200

On Thu, May 19 2022, Jiang Xin wrote:

> From: Jiang Xin <[email-masked]>
>
> When running "make pot" on different platform, we may get a different
> message template file "po/git.pot". This is because the "LOCALIZED_C"
> variable may have different C source files on different platforms or
> different compiler conditions.

As noted in my review of 1/9 this part of the rationale makes perfect
sense, no matter what we should always get the same results from "make
pot" on different platforms.

And I like the simplicity of using FOUND_C_SOURCES, unfortunately...

> We can make a stable "LOCALIZED_C" variable by applying patch snippets
> as follows:
>
>      ifdef NO_LIBGEN_H
>          COMPAT_CFLAGS += -DNO_LIBGEN_H
>          COMPAT_OBJS += compat/basename.o
>     +else
>     +    LOCALIZED_C += compat/basename.c
>      endif
>
> But it is much simpler to use variables "$(FOUND_C_SOURCES)" and
> "$(FOUND_C_SOURCES)" to form a stable "LOCALIZED_C".
>
> With this update, the newly generated "po/git.pot" will have 30 new
> entries coming from the following C source files:
>
>  * compat/fsmonitor/fsm-listen-win32.c
>  * compat/mingw.c
>  * compat/regex/regcomp.c
>  * compat/simple-ipc/ipc-win32.c

Those files which list "our" code look like obvious bug fixes, i.e. we
are the primary maintainer of that compat/* code, so we should do the
translation.

But the inclusion of compat/regex/regcomp.c there is a bug IMO. It's
code we got from glibc/awk. Do we really want to be using translator
time on that?

Hrm, maybe. After all those systems (most notably Windows) won't have
glibc/awk's translation catalogs, and in any case even if they did we'd
be using some ~decade old version of the code, so the two won't match up
well.

So maybe this is fine....

> Signed-off-by: Jiang Xin <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 83e968e2a4..60ca42c268 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2714,7 +2714,8 @@ XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
>  	--keyword=gettextln --keyword=eval_gettextln
>  XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \
>  	--keyword=__ --keyword=N__ --keyword="__n:1,2"
> -LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
> +LOCALIZED_C = $(FOUND_C_SOURCES) $(SCALAR_SOURCES) \
> +	      $(FOUND_H_SOURCES) $(GENERATED_H)

The SCALAR_SOURCES seems snuck in here, but it's worth noting in the
commit message that we effectively had it here before as part of C_OBJ.