diff mbox series

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

Message ID 20220519081548.3380-3-worldhello.net@gmail.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Jiang Xin May 19, 2022, 8:15 a.m. UTC
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

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.

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

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Ævar Arnfjörð Bjarmason May 19, 2022, 9:18 a.m. UTC | #1
On Thu, May 19 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> 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 <zhiyou.jx@alibaba-inc.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  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.
Jiang Xin May 19, 2022, 12:48 p.m. UTC | #2
On Thu, May 19, 2022 at 5:25 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > 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.

Agree.
diff mbox series

Patch

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)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)