diff mbox series

[v2,4/9] i18n CI: stop allowing non-ASCII source messages in po/git.pot

Message ID 20220519081548.3380-5-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: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

In the preceding commit we moved away from using xgettext(1) to both
generate the po/git.pot, and to merge the incrementally generated
po/git.pot+ file as we sourced translations from C, shell and Perl.

Doing it this way, which dates back to my initial
implementation[1][2][3] was conflating two things: With xgettext(1)
the --from-code both controls what encoding is specified in the
po/git.pot's header, and what encoding we allow in source messages.

We don't ever want to allow non-ASCII in *source messages*, and doing
so has hid e.g. a buggy message introduced in
a6226fd772b (submodule--helper: convert the bulk of cmd_add() to C,
2021-08-10) from us, we'd warn about it before, but only when running
"make pot", but the operation would still succeed. Now we'll error out
on it when running "make pot".

Since the preceding Makefile changes made this easy: let's add a "make
check-pot" target with the same prerequisites as the "po/git.pot"
target, but without changing the file "po/git.pot". Running it as part
of the "static-analysis" CI target will ensure that we catch any such
issues in the future. E.g.:

    $ make check-pot
        XGETTEXT .build/pot/po/builtin/submodule--helper.c.po
    xgettext: Non-ASCII string at builtin/submodule--helper.c:3381.
              Please specify the source encoding through --from-code.
    make: *** [.build/pot/po/builtin/submodule--helper.c.po] Error 1

1. cd5513a7168 (i18n: Makefile: "pot" target to extract messages
   marked for translation, 2011-02-22)
2. adc3b2b2767 (Makefile: add xgettext target for *.sh files,
   2011-05-14)
3. 5e9637c6297 (i18n: add infrastructure for translating Git with
   gettext, 2011-11-18)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile                    | 6 ++++--
 builtin/submodule--helper.c | 2 +-
 ci/run-static-analysis.sh   | 2 ++
 3 files changed, 7 insertions(+), 3 deletions(-)

Comments

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

> From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> In the preceding commit we moved away from using xgettext(1) to both
> generate the po/git.pot, and to merge the incrementally generated
> po/git.pot+ file as we sourced translations from C, shell and Perl.
>
> Doing it this way, which dates back to my initial
> implementation[1][2][3] was conflating two things: With xgettext(1)
> the --from-code both controls what encoding is specified in the
> po/git.pot's header, and what encoding we allow in source messages.
>
> We don't ever want to allow non-ASCII in *source messages*, and doing
> so has hid e.g. a buggy message introduced in
> a6226fd772b (submodule--helper: convert the bulk of cmd_add() to C,
> 2021-08-10) from us, we'd warn about it before, but only when running
> "make pot", but the operation would still succeed. Now we'll error out
> on it when running "make pot".
>
> Since the preceding Makefile changes made this easy: let's add a "make
> check-pot" target with the same prerequisites as the "po/git.pot"
> target, but without changing the file "po/git.pot". Running it as part
> of the "static-analysis" CI target will ensure that we catch any such
> issues in the future. E.g.:
>
>     $ make check-pot
>         XGETTEXT .build/pot/po/builtin/submodule--helper.c.po
>     xgettext: Non-ASCII string at builtin/submodule--helper.c:3381.
>               Please specify the source encoding through --from-code.
>     make: *** [.build/pot/po/builtin/submodule--helper.c.po] Error 1
>
> 1. cd5513a7168 (i18n: Makefile: "pot" target to extract messages
>    marked for translation, 2011-02-22)
> 2. adc3b2b2767 (Makefile: add xgettext target for *.sh files,
>    2011-05-14)
> 3. 5e9637c6297 (i18n: add infrastructure for translating Git with
>    gettext, 2011-11-18)
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Makefile                    | 6 ++++--
>  builtin/submodule--helper.c | 2 +-
>  ci/run-static-analysis.sh   | 2 ++
>  3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c32ac4ca30..304cd03276 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2708,8 +2708,7 @@ XGETTEXT_FLAGS = \
>  	--add-comments=TRANSLATORS: \
>  	--msgid-bugs-address="Git Mailing List <git@vger.kernel.org>" \
>  	--package-name=Git \
> -	--sort-by-file \
> -	--from-code=UTF-8
> +	--sort-by-file
>  XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
>  	--keyword=_ --keyword=N_ --keyword="Q_:1,2"
>  XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
> @@ -2788,6 +2787,9 @@ po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
>  .PHONY: pot
>  pot: po/git.pot
>  
> +.PHONY: check-pot
> +check-pot: $(LOCALIZED_ALL_GEN_PO)
> +
>  ifdef NO_GETTEXT
>  POFILES :=
>  MOFILES :=
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 2c87ef9364..b97f02eed5 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -3378,7 +3378,7 @@ static int module_add(int argc, const char **argv, const char *prefix)
>  			   N_("reference repository")),
>  		OPT_BOOL(0, "dissociate", &dissociate, N_("borrow the objects from reference repositories")),
>  		OPT_STRING(0, "name", &add_data.sm_name, N_("name"),
> -			   N_("sets the submodule’s name to the given string "
> +			   N_("sets the submodule's name to the given string "
>  			      "instead of defaulting to its path")),
>  		OPT_INTEGER(0, "depth", &add_data.depth, N_("depth for shallow clones")),
>  		OPT_END()
> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> index 65bcebda41..0d51e5ce0e 100755
> --- a/ci/run-static-analysis.sh
> +++ b/ci/run-static-analysis.sh
> @@ -29,4 +29,6 @@ fi
>  make hdr-check ||
>  exit 1
>  
> +make check-pot
> +
>  save_good_tree

In my latest version of this (range-diff below, yours is the RHS) I had
this depend on the full po.git pot file, i.e. .build/pot/git.pot. You're
instead making it depend on only the intermediate files we msgcat from
it.

What I was going for was to detect that non-ASCII issue, which your
version will also do, but out of general paranoia I thought running it
thorugh the same toolchain and produce the end result made sense.

So if there's any issue that the msgcat that produces po.git would spot
we'll miss it here, but I don't know of any such problem.

So maybe there's nothing to worry about here, but it's probably worth
noting that we're confident that we won't have any trouble because of
not checking the end result po.git in the commit message...

1:  a7f8122d43f ! 1:  e2cfb1a2408 i18n CI: stop allowing non-ASCII source messages in po/git.pot
    @@ Commit message
         on it when running "make pot".
     
         Since the preceding Makefile changes made this easy: let's add a "make
    -    check-pot" target and run it as part of the "static-analysis" CI
    -    target, this will ensure that we catch any such issues in the future.
    +    check-pot" target with the same prerequisites as the "po/git.pot"
    +    target, but without changing the file "po/git.pot". Running it as part
    +    of the "static-analysis" CI target will ensure that we catch any such
    +    issues in the future. E.g.:
    +
    +        $ make check-pot
    +            XGETTEXT .build/pot/po/builtin/submodule--helper.c.po
    +        xgettext: Non-ASCII string at builtin/submodule--helper.c:3381.
    +                  Please specify the source encoding through --from-code.
    +        make: *** [.build/pot/po/builtin/submodule--helper.c.po] Error 1
     
         1. cd5513a7168 (i18n: Makefile: "pot" target to extract messages
            marked for translation, 2011-02-22)
    @@ Commit message
            gettext, 2011-11-18)
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    +    Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## Makefile ##
    -@@ Makefile: pdf:
    - XGETTEXT_FLAGS = \
    - 	--force-po \
    +@@ Makefile: XGETTEXT_FLAGS = \
      	--add-comments=TRANSLATORS: \
    --	--from-code=UTF-8 \
    - 	--omit-header
    - 
    + 	--msgid-bugs-address="Git Mailing List <git@vger.kernel.org>" \
    + 	--package-name=Git \
    +-	--sort-by-file \
    +-	--from-code=UTF-8
    ++	--sort-by-file
      XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
    -@@ Makefile: po/git.pot: .build/pot/git.pot FORCE
    + 	--keyword=_ --keyword=N_ --keyword="Q_:1,2"
    + XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
    +@@ Makefile: po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
      .PHONY: pot
      pot: po/git.pot
      
     +.PHONY: check-pot
    -+check-pot: .build/pot/git.pot
    ++check-pot: $(LOCALIZED_ALL_GEN_PO)
     +
      ifdef NO_GETTEXT
      POFILES :=
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index c32ac4ca30..304cd03276 100644
--- a/Makefile
+++ b/Makefile
@@ -2708,8 +2708,7 @@  XGETTEXT_FLAGS = \
 	--add-comments=TRANSLATORS: \
 	--msgid-bugs-address="Git Mailing List <git@vger.kernel.org>" \
 	--package-name=Git \
-	--sort-by-file \
-	--from-code=UTF-8
+	--sort-by-file
 XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
 	--keyword=_ --keyword=N_ --keyword="Q_:1,2"
 XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
@@ -2788,6 +2787,9 @@  po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
 .PHONY: pot
 pot: po/git.pot
 
+.PHONY: check-pot
+check-pot: $(LOCALIZED_ALL_GEN_PO)
+
 ifdef NO_GETTEXT
 POFILES :=
 MOFILES :=
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 2c87ef9364..b97f02eed5 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3378,7 +3378,7 @@  static int module_add(int argc, const char **argv, const char *prefix)
 			   N_("reference repository")),
 		OPT_BOOL(0, "dissociate", &dissociate, N_("borrow the objects from reference repositories")),
 		OPT_STRING(0, "name", &add_data.sm_name, N_("name"),
-			   N_("sets the submodule’s name to the given string "
+			   N_("sets the submodule's name to the given string "
 			      "instead of defaulting to its path")),
 		OPT_INTEGER(0, "depth", &add_data.depth, N_("depth for shallow clones")),
 		OPT_END()
diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
index 65bcebda41..0d51e5ce0e 100755
--- a/ci/run-static-analysis.sh
+++ b/ci/run-static-analysis.sh
@@ -29,4 +29,6 @@  fi
 make hdr-check ||
 exit 1
 
+make check-pot
+
 save_good_tree