mbox series

[v2,0/9] Incremental po/git.pot update and new l10n workflow

Message ID 20220519081548.3380-1-worldhello.net@gmail.com (mailing list archive)
Headers show
Series Incremental po/git.pot update and new l10n workflow | expand

Message

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

A workflow change for translators are being proposed.

Changes since v1:

* Patch 4/9: When running "make check-pot", do not update "po/git.pot".
* Patch 6/9: reword: the removal of "po/git.pot" is in preceding commit.
* Patch 7/9 and 8/9: Add new funciton "check_po_file_envvar" for reuse in both
  po-update and po-init targets.


Range-diff vs v1:

 1:  15c4090757 =  1:  15c4090757 Makefile: sort "po/git.pot" by file location
 2:  846a9e72c5 =  2:  846a9e72c5 Makefile: generate "po/git.pot" from stable LOCALIZED_C
 3:  b5cf2836ea =  3:  b5cf2836ea Makefile: have "make pot" not "reset --hard"
 4:  a508f6e18b !  4:  8b30e332df 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)
    @@ Makefile: po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
      pot: po/git.pot
      
     +.PHONY: check-pot
    -+check-pot: pot
    ++check-pot: $(LOCALIZED_ALL_GEN_PO)
     +
      ifdef NO_GETTEXT
      POFILES :=
 5:  720a00b348 =  5:  65f87c5503 po/git.pot: this is now a generated file
 6:  f97113a370 !  6:  fe5f27a88b po/git.pot: don't check in result of "make pot"
    @@ Commit message
         Makefile: "pot" target to extract messages marked for translation,
         2011-02-22).
     
    -    The actual "git rm" of po/git.pot is deferred until the subsequent
    -    commit, to make this change easier to review, and to preempt the
    -    mailing list from blocking it due to it being too large.
    +    The actual "git rm" of po/git.pot was in preceding commit to make this
    +    change easier to review, and to preempt the mailing list from blocking
    +    it due to it being too large.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
 7:  be23a1c8b0 !  7:  710aff9d66 Makefile: add "po-update" rule to update po/XX.po
    @@ Makefile: po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
      .PHONY: pot
      pot: po/git.pot
      
    ++define check_po_file_envvar
    ++	$(if $(PO_FILE), \
    ++		$(if $(filter po/%.po,$(PO_FILE)), , \
    ++			$(error PO_FILE should match pattern: "po/%.po")), \
    ++		$(error PO_FILE is not defined))
    ++endef
    ++
    ++.PHONY: po-update
     +po-update: po/git.pot
    -+ifndef PO_FILE
    -+	$(error not define variable "PO_FILE")
    -+else
    -+ifeq ($(filter po/%.po,$(PO_FILE)),)
    -+	$(error PO_FILE should match pattern: "po/%.po")
    -+endif
    -+endif
    ++	$(check_po_file_envvar)
     +	@if test ! -e $(PO_FILE); then \
     +		echo >&2 "error: $(PO_FILE) does not exist"; \
     +		exit 1; \
    @@ Makefile: po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
     +	$(QUIET_MSGMERGE)$(MSGMERGE) $(MSGMERGE_FLAGS) $(PO_FILE) po/git.pot
     +
      .PHONY: check-pot
    - check-pot: pot
    + check-pot: $(LOCALIZED_ALL_GEN_PO)
      
     
      ## shared.mak ##
 8:  176a80587a !  8:  fc9d3f5603 Makefile: add "po-init" rule to initialize po/XX.po
    @@ Commit message
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## Makefile ##
    -@@ Makefile: endif
    - endif
    +@@ Makefile: po-update: po/git.pot
    + 	$(check_po_file_envvar)
      	@if test ! -e $(PO_FILE); then \
      		echo >&2 "error: $(PO_FILE) does not exist"; \
     +		echo >&2 'To create an initial po file, use: "make po-init PO_FILE=po/XX.po"'; \
      		exit 1; \
      	fi
      	$(QUIET_MSGMERGE)$(MSGMERGE) $(MSGMERGE_FLAGS) $(PO_FILE) po/git.pot
    -@@ Makefile: endif
    +@@ Makefile: po-update: po/git.pot
      .PHONY: check-pot
    - check-pot: pot
    + check-pot: $(LOCALIZED_ALL_GEN_PO)
      
     +### TODO FIXME: Translating everything in these files is a bad
     +### heuristic for "core", as we'll translate obscure error() messages
    @@ Makefile: endif
     +
     +.PHONY: po-init
     +po-init: po/git-core.pot
    -+ifndef PO_FILE
    -+	$(error not define variable "PO_FILE")
    -+else
    -+ifeq ($(filter po/%.po,$(PO_FILE)),)
    -+	$(error PO_FILE should match pattern: "po/%.po")
    -+endif
    -+endif
    ++	$(check_po_file_envvar)
     +	@if test -e $(PO_FILE); then \
     +		echo >&2 "error: $(PO_FILE) exists already"; \
     +		exit 1; \
 9:  f2491b96ab =  9:  10fafa5bf9 l10n: Document the new l10n workflow

---

Jiang Xin (3):
  Makefile: sort "po/git.pot" by file location
  Makefile: generate "po/git.pot" from stable LOCALIZED_C
  Makefile: add "po-update" rule to update po/XX.po

Junio C Hamano (1):
  po/git.pot: this is now a generated file

Ævar Arnfjörð Bjarmason (5):
  Makefile: have "make pot" not "reset --hard"
  i18n CI: stop allowing non-ASCII source messages in po/git.pot
  po/git.pot: don't check in result of "make pot"
  Makefile: add "po-init" rule to initialize po/XX.po
  l10n: Document the new l10n workflow

 .gitignore                  |     1 +
 Makefile                    |   144 +-
 builtin/submodule--helper.c |     2 +-
 ci/run-static-analysis.sh   |     2 +
 po/.gitignore               |     2 +
 po/README.md                |   191 +-
 po/git.pot                  | 25151 ----------------------------------
 shared.mak                  |     2 +
 8 files changed, 217 insertions(+), 25278 deletions(-)
 delete mode 100644 po/git.pot

Comments

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

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> A workflow change for translators are being proposed.
>
> Changes since v1:
>
> * Patch 4/9: When running "make check-pot", do not update "po/git.pot".
> * Patch 6/9: reword: the removal of "po/git.pot" is in preceding commit.
> * Patch 7/9 and 8/9: Add new funciton "check_po_file_envvar" for reuse in both
>   po-update and po-init targets.

Thanks a lot for picking this up. I left some detailed comments on
individual commits.

My own latest WIP version of an approximation of this topic was
https://github.com/avar/git/tree/avar/Makefile-incremental-po-git-pot-rule,
which is what I used for some of the range-diffs.

(I think the first E-Mail I sent had a range-diff against the latest
version I found in your fork, but I found that was probably the v1
version, but I think those comments applied to your v2 (which I read
on-list))

Aside from differences already noted I spotted that your "make pot" ends
up with a po/git.pot that has a header, but I omitted it in
mine. Perhaps that explains some of the headers in 8/9? I.e. we don't
need the header on po/git.pot, perhaps that explains the difference
noted in my comment in 8/9?

Also: shouldn't "make clean" remove the generated po/git.pot and
po/git-core.pot? I see you added it to "distclean", maybe that's better
(or maybe that's from a version of mine...).

Just from some last minute testing I think you want this squashed in
(and move that "sed" to the "init" and/or "update" of the individual
po/XX.po files):
	
	diff --git a/Makefile b/Makefile
	index 65a7558261a..57db37db556 100644
	--- a/Makefile
	+++ b/Makefile
	@@ -2778,14 +2778,7 @@ $(LOCALIZED_PERL_GEN_PO): .build/pot/po/%.po: %
	 	$(QUIET_XGETTEXT)$(XGETTEXT) --omit-header \
	 		-o$@ $(XGETTEXT_FLAGS_PERL) $<
	 
	-.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO)
	-	$(call mkdir_p_parent_template)
	-	$(QUIET_XGETTEXT)$(XGETTEXT) $(XGETTEXT_FLAGS_C) \
	-		-o - /dev/null | \
	-	sed -e 's|charset=CHARSET|charset=UTF-8|g' >$@ && \
	-	echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@
	-
	-po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
	+po/git.pot: $(LOCALIZED_ALL_GEN_PO)
	 	$(QUIET_GEN)$(MSGCAT) $(MSGCAT_FLAGS) $^ >$@
	 
	 .PHONY: pot

I.e. we can just msgcat po/git.pot without the header. For both
"po-init" and "po-update" that seems to do the right thing for me...
Jiang Xin May 19, 2022, 2:32 p.m. UTC | #2
On Thu, May 19, 2022 at 6:39 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Thanks a lot for picking this up. I left some detailed comments on
> individual commits.
>
> My own latest WIP version of an approximation of this topic was
> https://github.com/avar/git/tree/avar/Makefile-incremental-po-git-pot-rule,
> which is what I used for some of the range-diffs.
>
> (I think the first E-Mail I sent had a range-diff against the latest
> version I found in your fork, but I found that was probably the v1
> version, but I think those comments applied to your v2 (which I read
> on-list))
>
> Aside from differences already noted I spotted that your "make pot" ends
> up with a po/git.pot that has a header, but I omitted it in
> mine. Perhaps that explains some of the headers in 8/9? I.e. we don't
> need the header on po/git.pot, perhaps that explains the difference
> noted in my comment in 8/9?
>
> Also: shouldn't "make clean" remove the generated po/git.pot and
> po/git-core.pot? I see you added it to "distclean", maybe that's better
> (or maybe that's from a version of mine...).
>
> Just from some last minute testing I think you want this squashed in
> (and move that "sed" to the "init" and/or "update" of the individual
> po/XX.po files):
>
>         diff --git a/Makefile b/Makefile
>         index 65a7558261a..57db37db556 100644
>         --- a/Makefile
>         +++ b/Makefile
>         @@ -2778,14 +2778,7 @@ $(LOCALIZED_PERL_GEN_PO): .build/pot/po/%.po: %
>                 $(QUIET_XGETTEXT)$(XGETTEXT) --omit-header \
>                         -o$@ $(XGETTEXT_FLAGS_PERL) $<
>
>         -.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO)
>         -       $(call mkdir_p_parent_template)
>         -       $(QUIET_XGETTEXT)$(XGETTEXT) $(XGETTEXT_FLAGS_C) \
>         -               -o - /dev/null | \
>         -       sed -e 's|charset=CHARSET|charset=UTF-8|g' >$@ && \
>         -       echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@
>         -
>         -po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
>         +po/git.pot: $(LOCALIZED_ALL_GEN_PO)
>                 $(QUIET_GEN)$(MSGCAT) $(MSGCAT_FLAGS) $^ >$@
>
>          .PHONY: pot
>
> I.e. we can just msgcat po/git.pot without the header. For both
> "po-init" and "po-update" that seems to do the right thing for me...

Benefits of having a header for "po/git.pot" file:
1. Have a nice field "Project-Id-Version: Git" in the head of a new
    generated po file.
2. We can identify the base version of "po/git.pot" by inspecting
    the "POT-Creation-Date" field in the header of a po file.
Ævar Arnfjörð Bjarmason May 19, 2022, 2:41 p.m. UTC | #3
On Thu, May 19 2022, Jiang Xin wrote:

> On Thu, May 19, 2022 at 6:39 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Thanks a lot for picking this up. I left some detailed comments on
>> individual commits.
>>
>> My own latest WIP version of an approximation of this topic was
>> https://github.com/avar/git/tree/avar/Makefile-incremental-po-git-pot-rule,
>> which is what I used for some of the range-diffs.
>>
>> (I think the first E-Mail I sent had a range-diff against the latest
>> version I found in your fork, but I found that was probably the v1
>> version, but I think those comments applied to your v2 (which I read
>> on-list))
>>
>> Aside from differences already noted I spotted that your "make pot" ends
>> up with a po/git.pot that has a header, but I omitted it in
>> mine. Perhaps that explains some of the headers in 8/9? I.e. we don't
>> need the header on po/git.pot, perhaps that explains the difference
>> noted in my comment in 8/9?
>>
>> Also: shouldn't "make clean" remove the generated po/git.pot and
>> po/git-core.pot? I see you added it to "distclean", maybe that's better
>> (or maybe that's from a version of mine...).
>>
>> Just from some last minute testing I think you want this squashed in
>> (and move that "sed" to the "init" and/or "update" of the individual
>> po/XX.po files):
>>
>>         diff --git a/Makefile b/Makefile
>>         index 65a7558261a..57db37db556 100644
>>         --- a/Makefile
>>         +++ b/Makefile
>>         @@ -2778,14 +2778,7 @@ $(LOCALIZED_PERL_GEN_PO): .build/pot/po/%.po: %
>>                 $(QUIET_XGETTEXT)$(XGETTEXT) --omit-header \
>>                         -o$@ $(XGETTEXT_FLAGS_PERL) $<
>>
>>         -.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO)
>>         -       $(call mkdir_p_parent_template)
>>         -       $(QUIET_XGETTEXT)$(XGETTEXT) $(XGETTEXT_FLAGS_C) \
>>         -               -o - /dev/null | \
>>         -       sed -e 's|charset=CHARSET|charset=UTF-8|g' >$@ && \
>>         -       echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@
>>         -
>>         -po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO)
>>         +po/git.pot: $(LOCALIZED_ALL_GEN_PO)
>>                 $(QUIET_GEN)$(MSGCAT) $(MSGCAT_FLAGS) $^ >$@
>>
>>          .PHONY: pot
>>
>> I.e. we can just msgcat po/git.pot without the header. For both
>> "po-init" and "po-update" that seems to do the right thing for me...
>
> Benefits of having a header for "po/git.pot" file:
> 1. Have a nice field "Project-Id-Version: Git" in the head of a new
>     generated po file.
> 2. We can identify the base version of "po/git.pot" by inspecting
>     the "POT-Creation-Date" field in the header of a po file.

For 1: Yes, we should have a header, I'm saying we don't need it for
po/git.pot, just po/XX.po, and not having it in po/git.pot makes things
a bit simpler, since when you "msgmerge" it you only worry about merging
the content, not the header.

The header you can then create with msginit, which in both our versions
we'd "sed" or otherwise correctly invoke msginit to add the correct
fields.

For 2: I think that having such fields in a world where everyone uses
version control (and the git project certainly does) is rather useless,
they're there in the PO format because it pre-dates version control
being ubiquitous.

The time-related fields I left in I left there because it seemed that
some PO tooling (e.g. Emacs's po-mode) insists on it.

Anyway, this is all small potatoes. I only pointed this out because when
I was hacking this up & debugging it I found it much easier to deal with
being able to piece together things with just msgcat, which we can do
with po/git.pot if it doesn't have a header.

But for adding the header we either need to msgcat a header file (which
an early version of my patches did), or just skip it and have it only
added for the XX.po files.

I think it's simpler just to omit it :)