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

From: Junio C Hamano <[email-masked]>
Date: Mon, 23 May 2022 01:05:09 -0700

Jiang Xin <[email-masked]> writes:

> From: Jiang Xin <[email-masked]>
>
> When running "make pot" on different platform, we may get a different

"on a"

> 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.
>
> 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

While that is true, going into such a detail of an approach that you
rejected does not sound like a good use of readers' time.  Instead,
perhaps mention why ...

> But it is much simpler to use variables "$(FOUND_C_SOURCES)" and
> "$(FOUND_C_SOURCES)" to form a stable "LOCALIZED_C".

... FOUND_C_SOURCES approach works better by reminding the readers
what it is.  I'd probably discard everything after "This is
because..." up to point and replace it with something like:

    "$(LOCALIZED_C)" is supposed to list all the sources that we
    extract the strings to be translated from.  Instead of using
    "$(C_OBJ)", which only lists the source files used in the
    current build, to compute it, use "$(FOUND_C_SOURCES)", which
    lists all source files we keep track of (or ship in a tarball
    extract).

if I were writing this.

> We also add
> "$(SCALAR_SOURCES)" files, which are part of C_OBJ but not included in
> "$(FOUND_C_SOURCES)" because they are in the "contrib/" directory.

Good.

> 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

Good.

> 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..46914dcd80 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) $(FOUND_H_SOURCES) $(SCALAR_SOURCES) \
> +	      $(GENERATED_H)
>  LOCALIZED_SH = $(SCRIPT_SH)
>  LOCALIZED_SH += git-sh-setup.sh
>  LOCALIZED_PERL = $(SCRIPT_PERL)