On Mon, May 23, 2022 at 4:07 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > > On Mon, May 23 2022, Jiang Xin wrote: > > > From: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> > > > > Before commit fc0fd5b23b (Makefile: help gettext tools to cope with our > > custom PRItime format, 2017-07-20) we'd consider source files as-is > > with gettext, but because we need to understand PRItime in the same way > > that gettext itself understands PRIuMAX we'd first check if we had a > > clean checkout, then munge all of the processed files in-place with > > "sed", generate "po/git.pot", and then finally "reset --hard" to undo > > our changes. > > > > By generating "pot" snippets in ".build/pot/po" for each source file > > and rewriting certain source files with PRItime macros to temporary > > files in ".build/pot/po", we can avoid running "make pot" by altering > > files in place and doing a "reset --hard" afterwards. > > > > This speed of "make pot" is slower than before on an initial run, > > because we run "xgettext" many times (once per source file), but it > > can be boosted by parallelization. It is *much* faster for incremental > > runs, and will allow us to implement related targets in subsequent > > commits. > > > > When the "pot" target was originally added in cd5513a7168 (i18n: > > Makefile: "pot" target to extract messages marked for translation, > > 2011-02-22) it behaved like a "normal" target. I.e. we'd skip the > > re-generation of the po/git.pot if nothing had to be done. > > > > Then after po/git.pot was checked in in dce37b66fb0 (l10n: initial > > git.pot for 1.7.10 upcoming release, 2012-02-13) the target was broken > > until 1f31963e921 (i18n: treat "make pot" as an explicitly-invoked > > target, 2014-08-22) when it was made to depend on "FORCE". I.e. the > > Makefile's dependency resolution inherently can't handle incremental > > building when the target file may be updated by git (or something else > > external to "make"). But this case no longer applies, so FORCE is no > > longer needed. > > > > That out of the way, the main logic change here is getting rid of the > > "reset --hard": > > > > We'll generate intermediate ".build/pot/po/%.po" files from "%", which > > is handy to see at a glance what strings (if any) in a given file are > > marked for translation: > > > > $ make .build/pot/po/pretty.c.po > > [...] > > $ cat .build/pot/po/pretty.c.po > > #: pretty.c:1051 > > msgid "unable to parse --pretty format" > > msgstr "" > > $ > > > > For these C source files which contain the PRItime macros, we will > > create temporary munged "*.c" files in a tree in ".build/pot/po" > > corresponding to our source tree, and have "xgettext" consider those. > > The rule needs to be careful to "(cd .build/pot/po && ...)", because > > otherwise the comments in the po/git.pot file wouldn't refer to the > > correct source locations (they'd be prefixed with ".build/pot/po"). > > These temporary munged "*.c” files will be removed immediately after > > the corresponding po files are generated, because some development tools > > cannot ignore the duplicate source files in the ".build" directory > > according to the ".gitignore" file, and that may cause trouble. > > > > The output of the generated po/git.pot file is changed in one minor > > way: Because we're using msgcat(1) instead of xgettext(1) to > > concatenate the output we'll now disambiguate where "TRANSLATORS" > > comments come from, in cases where a message is the same in N files, > > and either only one has a "TRANSLATORS" comment, or they're > > different. E.g. for the "Your edited hunk[...]" message we'll now > > apply this change (comment content elided): > > > > +#. #-#-#-#-# add-patch.c.po #-#-#-#-# > > #. TRANSLATORS: do not translate [y/n] > > [...] > > +#. #-#-#-#-# git-add--interactive.perl.po #-#-#-#-# > > #. TRANSLATORS: do not translate [y/n] > > [...] > > #: add-patch.c:1253 git-add--interactive.perl:1244 > > msgid "" > > "Your edited hunk does not apply. Edit again (saying \"no\" discards!) [y/n]? " > > msgstr "" > > > > There are six such changes, and they all make the context more > > understandable, as msgcat(1) is better at handling these edge cases > > than xgettext(1)'s previously used "--join-existing" flag. > > > > But filenames in the above disambiguation lines of extracted-comments > > have an extra ".po" extension compared to the filenames at the file > > locations. While we could rename the intermediate ".build/pot/po/%.po" > > files without the ".po" extension to use more intuitive filenames in > > the disambiguation lines of extracted-comments, but that will confuse > > developer tools with lots of invalid C or other source files in > > ".build/pot/po" directory. > > > > The addition of "--omit-header" option for xgettext makes the "pot" > > snippets in ".build/pot/po/*.po" smaller. But as we'll see in a > > subsequent commit this header behavior has been hiding an > > encoding-related bug from us, so let's carry it forward instead of > > re-generating it with xgettext(1). > > > > The "po/git.pot" file should have a header entry, because a proper > > header entry will increase the speed of creating a new po file using > > msginit and set a proper "POT-Creation-Date:" field in the header > > entry of a "po/XX.po" file. We use xgettext to generate a separate > > header file at ".build/pot/git.header" from "/dev/null", and use this > > header to assemble "po/git.pot". > > > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> > > Signed-off-by: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx> > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > > --- > > .gitignore | 1 + > > Makefile | 80 ++++++++++++++++++++++++++++++++++++++++-------------- > > 2 files changed, 60 insertions(+), 21 deletions(-) > > > > diff --git a/.gitignore b/.gitignore > > index e81de1063a..a452215764 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -200,6 +200,7 @@ > > *.[aos] > > *.o.json > > *.py[co] > > +.build/ > > .depend/ > > *.gcda > > *.gcno > > diff --git a/Makefile b/Makefile > > index 46914dcd80..1962999c18 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -569,6 +569,7 @@ INSTALL = install > > TCL_PATH = tclsh > > TCLTK_PATH = wish > > XGETTEXT = xgettext > > +MSGCAT = msgcat > > MSGFMT = msgfmt > > CURL_CONFIG = curl-config > > GCOV = gcov > > @@ -2706,6 +2707,7 @@ XGETTEXT_FLAGS = \ > > --force-po \ > > --add-comments=TRANSLATORS: \ > > --msgid-bugs-address="Git Mailing List <git@xxxxxxxxxxxxxxx>" \ > > + --package-name=Git \ > > --sort-by-file \ > > --from-code=UTF-8 > > XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ > > @@ -2714,6 +2716,7 @@ 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" > > +MSGCAT_FLAGS = --sort-by-file > > LOCALIZED_C = $(FOUND_C_SOURCES) $(FOUND_H_SOURCES) $(SCALAR_SOURCES) \ > > $(GENERATED_H) > > LOCALIZED_SH = $(SCRIPT_SH) > > @@ -2726,34 +2729,68 @@ LOCALIZED_SH += t/t0200/test.sh > > LOCALIZED_PERL += t/t0200/test.perl > > endif > > > > -## Note that this is meant to be run only by the localization coordinator > > -## under a very controlled condition, i.e. (1) it is to be run in a > > -## Git repository (not a tarball extract), (2) any local modifications > > -## will be lost. > > +## We generate intermediate .build/pot/po/%.po files containing a > > +## extract of the translations we find in each file in the source > > +## tree. We will assemble them using msgcat to create the final > > +## "po/git.pot" file. > > +LOCALIZED_ALL_GEN_PO = > > + > > +LOCALIZED_C_GEN_PO = $(LOCALIZED_C:%=.build/pot/po/%.po) > > +LOCALIZED_ALL_GEN_PO += $(LOCALIZED_C_GEN_PO) > > + > > +LOCALIZED_SH_GEN_PO = $(LOCALIZED_SH:%=.build/pot/po/%.po) > > +LOCALIZED_ALL_GEN_PO += $(LOCALIZED_SH_GEN_PO) > > + > > +LOCALIZED_PERL_GEN_PO = $(LOCALIZED_PERL:%=.build/pot/po/%.po) > > +LOCALIZED_ALL_GEN_PO += $(LOCALIZED_PERL_GEN_PO) > > + > > ## Gettext tools cannot work with our own custom PRItime type, so > > ## we replace PRItime with PRIuMAX. We need to update this to > > ## PRIdMAX if we switch to a signed type later. > > +$(LOCALIZED_C_GEN_PO): .build/pot/po/%.po: % > > + $(call mkdir_p_parent_template) > > + $(QUIET_XGETTEXT) \ > > + if grep -q PRItime $<; then \ > > + (\ > > + sed -e 's|PRItime|PRIuMAX|g' <$< \ > > + >.build/pot/po/$< && \ > > + cd .build/pot/po && \ > > + $(XGETTEXT) --omit-header \ > > + -o $(@:.build/pot/po/%=%) \ > > + $(XGETTEXT_FLAGS_C) $< && \ > > + rm $<; \ > > + ); \ > > + else \ > > + $(XGETTEXT) --omit-header \ > > + -o $@ $(XGETTEXT_FLAGS_C) $<; \ > > + fi > > > > -po/git.pot: $(GENERATED_H) FORCE > > - # All modifications will be reverted at the end, so we do not > > - # want to have any local change. > > - git diff --quiet HEAD && git diff --quiet --cached > > +$(LOCALIZED_SH_GEN_PO): .build/pot/po/%.po: % > > + $(call mkdir_p_parent_template) > > + $(QUIET_XGETTEXT)$(XGETTEXT) --omit-header \ > > + -o$@ $(XGETTEXT_FLAGS_SH) $< > > > > - @for s in $(LOCALIZED_C) $(LOCALIZED_SH) $(LOCALIZED_PERL); \ > > - do \ > > - sed -e 's|PRItime|PRIuMAX|g' <"$$s" >"$$s+" && \ > > - cat "$$s+" >"$$s" && rm "$$s+"; \ > > - done > > +$(LOCALIZED_PERL_GEN_PO): .build/pot/po/%.po: % > > + $(call mkdir_p_parent_template) > > + $(QUIET_XGETTEXT)$(XGETTEXT) --omit-header \ > > + -o$@ $(XGETTEXT_FLAGS_PERL) $< > > +q > > +define gen_pot_header > > +$(XGETTEXT) $(XGETTEXT_FLAGS_C) \ > > + -o - /dev/null | \ > > +sed -e 's|charset=CHARSET|charset=UTF-8|' \ > > + -e 's|\(Last-Translator: \)FULL NAME <.*>|\1make by the Makefile|' \ > > + -e 's|\(Language-Team: \)LANGUAGE <.*>|\1Git Mailing List <git@xxxxxxxxxxxxxxx>|' \ > > + >$@ && \ > > +echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@ > > +endef > > > > - $(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ $(XGETTEXT_FLAGS_C) $(LOCALIZED_C) > > - $(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ --join-existing $(XGETTEXT_FLAGS_SH) \ > > - $(LOCALIZED_SH) > > - $(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ --join-existing $(XGETTEXT_FLAGS_PERL) \ > > - $(LOCALIZED_PERL) > > +.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO) > > + $(call mkdir_p_parent_template) > > + $(QUIET_GEN)$(gen_pot_header) > > So re a previous round I think the addition of a header to po/git.pot > doesn't make sense, i.e. it's not needed, so we can just drop this > entirely. > > But if you really want to include it anyway then this dependency is > broken, the header just depends on this gen_pot_header you've defined, > we don't need to re-generate it every time any single source file > changes. > > Or is the intent to have its timestamp bumped if any of the source files > change? > > If so I think that to the extent we need headers at all (for the *.po > files) it might be worthwhile to just have the timestamp be the same > epoch we use for the tests in t/, i.e. never update it and avoid the > needless churn. If people need to check when things were last changed, > or initially added, they can use this little thing called "git" :) After rereading all the replies, I feel it's better to to leave a note here to make my point clear. As we removed the "po/git.pot" file, it is not easy to find the base version of a "po/XX.po" file, and we need the "POT-Creation-Date:" field which comes from the header of the generated POT file to tell us the version of the base. $ git -P grep POT-Creation-Date -- po/ po/bg.po:"POT-Creation-Date: 2022-01-17 08:34+0800\n" po/ca.po:"POT-Creation-Date: 2022-04-06 14:40+0800\n" po/de.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/el.po:"POT-Creation-Date: 2019-02-15 10:09+0800\n" po/es.po:"POT-Creation-Date: 2021-11-04 08:34+0800\n" po/fr.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/id.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/is.po:"POT-Creation-Date: 2016-06-17 18:55+0000\n" po/it.po:"POT-Creation-Date: 2020-10-10 09:32+0800\n" po/ko.po:"POT-Creation-Date: 2018-06-16 22:06+0800\n" po/pl.po:"POT-Creation-Date: 2021-11-10 08:55+0800\n" po/pt_PT.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/ru.po:"POT-Creation-Date: 2020-12-21 07:10+0800\n" po/sv.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/tr.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/vi.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/zh_CN.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n" po/zh_TW.po:"POT-Creation-Date: 2022-04-13 14:52+0800\n"