Subject: Re: [PATCH v2 0/9] Incremental po/git.pot update and new l10n workflow

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

On Thu, May 19 2022, Jiang Xin wrote:

> On Thu, May 19, 2022 at 6:39 PM Ævar Arnfjörð Bjarmason
> <[email-masked]> 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 :)