| Message ID | 20220523012531.4505-1-worldhello.net@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Incremental po/git.pot update and new l10n workflow | expand |
On Mon, May 23 2022, Jiang Xin wrote: > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > A workflow change for translators are being proposed. > > Changes since v2: > > 1. Patch 1/9: reword. > 2. Patch 2/9: reword. > 3. Patch 3/9: reword, and add "FORCE" to prerequisites of "po/git.pot". > 4. Patch 6/9: remove "FORCE" from prerequisites of "po/git.pot". > 5. Patch 8/9: reword, and reuse "$(gen_pot_header)" to prepare pot > header for "po/git-core.pot". > 6. Patch 9/9: various updates on po/README.md. From skimming this the *.c.po v.s. *.c extension is still left in comments. I'm not saying you need to go for my suggestions, but it would be very useful in CL's to note things that were suggested but not changed, such as that. Right now I haven't paged that v2 discussion into my brain again, so I don't know if that was the only thing, it's the only thing I remember right now... But let's read on: > Range-diff vs v2: > > 1: c45f34f233 ! 1: 362cd0cbe1 Makefile: sort "po/git.pot" by file location > @@ Metadata > ## Commit message ## > Makefile: sort "po/git.pot" by file location > > - Before feeding xgettext with more C souce files which may be ignored > - by various compiler conditions, add new option "--sort-by-file" to > - xgettext program to create stable message template file "po/git.pot". > + We will feed xgettext with more C souce files and in different order in > + subsequent commit. To generate a stable "po/git.pot" regardless of the > + number and order of input source files, we add a new option > + "--sort-by-file" to xgettext program. > > With this update, the newly generated "po/git.pot" will has the same > - entries while in a different order. We won't checkin the newly generated > - "po/git.pot", because we will remove it from tree in a later commit. > + entries while in a different order. > + > + With the help of a custom diff driver as shown below, > + > + git config --global diff.gettext-fmt.textconv \ > + "msgcat --no-location --sort-by-file" > + > + and appending a new entry "*.po diff=gettext-fmt" to git attributes, > + we can see that there are no substantial changes in "po/git.pot". > + > + We won't checkin the newly generated "po/git.pot", because we will > + remove it from tree in a later commit. Does this actually work? This seems to suggest adding a driver for *.po, but using it against the *.pot file. Isn't that a typo (I haven't tested it)> > But it is much simpler to use variables "$(FOUND_C_SOURCES)" and > - "$(FOUND_C_SOURCES)" to form a stable "LOCALIZED_C". > + "$(FOUND_C_SOURCES)" to form a stable "LOCALIZED_C". We also add > + "$(SCALAR_SOURCES)" files, which are part of C_OBJ but not included in > + "$(FOUND_C_SOURCES)" because they are in the "contrib/" directory. Thanks, good to note that. [snipped the rest, will re-read individual commits]
On Mon, May 23 2022, Ævar Arnfjörð Bjarmason wrote: > On Mon, May 23 2022, Jiang Xin wrote: > >> From: Jiang Xin <zhiyou.jx@alibaba-inc.com> >> >> A workflow change for translators are being proposed. >> >> Changes since v2: >> >> 1. Patch 1/9: reword. >> 2. Patch 2/9: reword. >> 3. Patch 3/9: reword, and add "FORCE" to prerequisites of "po/git.pot". >> 4. Patch 6/9: remove "FORCE" from prerequisites of "po/git.pot". >> 5. Patch 8/9: reword, and reuse "$(gen_pot_header)" to prepare pot >> header for "po/git-core.pot". >> 6. Patch 9/9: various updates on po/README.md. > > From skimming this the *.c.po v.s. *.c extension is still left in > comments. I'm not saying you need to go for my suggestions, but it would > be very useful in CL's to note things that were suggested but not > changed, such as that. > > Right now I haven't paged that v2 discussion into my brain again, so I > don't know if that was the only thing, it's the only thing I remember > right now... This fix-up below implements what I suggested on v2, so now the comments in the generated file are correct, and don't refer to our intermediate files: $ grep '#-#' po/git.pot #. #-#-#-#-# git-add--interactive.perl #-#-#-#-# #. #-#-#-#-# add-patch.c #-#-#-#-# #. #-#-#-#-# git-add--interactive.perl #-#-#-#-# #. #-#-#-#-# branch.c #-#-#-#-# #. #-#-#-#-# object-name.c #-#-#-#-# #. #-#-#-#-# grep.c #-#-#-#-# I gathered that the reason you preferred the whole "grep -q PRItime" was because you wanted to mitigate the effects of your IDE discovering these files. With the below you can define AGGRESSIVE_INTERMEDIATE and when you "make pot" the generated *.c files will only exist for as long as they're needed to generate the next step. But if you do a subsequent "make pot" will be slower, as we'll of course need to generate them again. I think it's better to go in this direction, and rename that AGGRESSIVE_INTERMEDIATE to something like MAKE_AVOID_REAL_EXTENSIONS_IN_GITIGNORED_FILES or whatever. I.e. our correctness shouldn't suffer because we're trying to work around some issue in a specific (and optional) developer tooling. There's also the fix there for the "header" dependency, but as noted in another reply it should probably be dropped altogether... diff --git a/Makefile b/Makefile index d3eae150de9..0b96b55b63f 100644 --- a/Makefile +++ b/Makefile @@ -2736,6 +2736,7 @@ endif ## "po/git.pot" file. LOCALIZED_ALL_GEN_PO = +LOCALIZED_C_GEN_C = $(LOCALIZED_C:%=.build/pot/po-munged/%) LOCALIZED_C_GEN_PO = $(LOCALIZED_C:%=.build/pot/po/%.po) LOCALIZED_ALL_GEN_PO += $(LOCALIZED_C_GEN_PO) @@ -2745,26 +2746,19 @@ 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: % +ifdef AGGRESSIVE_INTERMEDIATE +.INTERMEDIATE: $(LOCALIZED_C_GEN_C) +endif +$(LOCALIZED_C_GEN_C): .build/pot/po-munged/%: % $(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 + $(QUIET_GEN)sed -e 's|PRItime|PRIuMAX|g' <$< >$@ + +$(LOCALIZED_C_GEN_PO): .build/pot/po/%.po: .build/pot/po-munged/% + $(call mkdir_p_parent_template) + $(QUIET_XGETTEXT)( \ + cd $(<D) && \ + $(XGETTEXT) $(XGETTEXT_FLAGS_C) --omit-header -o - $(<F) \ + ) >$@ $(LOCALIZED_SH_GEN_PO): .build/pot/po/%.po: % $(call mkdir_p_parent_template) @@ -2786,11 +2780,24 @@ sed -e 's|charset=CHARSET|charset=UTF-8|' \ echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@ endef -.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO) +.build/pot/git.header: $(call mkdir_p_parent_template) $(QUIET_GEN)$(gen_pot_header) -po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO) +# We go through this dance of having a prepared +# e.g. .build/pot/po/grep.c.po and copying it to +# .build/pot/to-cat/grep.c only because some IDEs (e.g. VSCode) pick +# up on the "real" extension for the purposes of auto-completion, even +# if the .build directiory is in .gitignore. +LOCALIZED_ALL_GEN_TO_CAT = $(LOCALIZED_ALL_GEN_PO:.build/pot/po/%.po=.build/pot/to-cat/%) +ifdef AGGRESSIVE_INTERMEDIATE +.INTERMEDIATE: $(LOCALIZED_ALL_GEN_TO_CAT) +endif +$(LOCALIZED_ALL_GEN_TO_CAT): .build/pot/to-cat/%: .build/pot/po/%.po + $(call mkdir_p_parent_template) + $(QUIET_GEN)cat $< >$@ + +po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_TO_CAT) $(QUIET_GEN)$(MSGCAT) $(MSGCAT_FLAGS) $^ >$@ .PHONY: pot
On Mon, May 23, 2022 at 3:25 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Mon, May 23 2022, Jiang Xin wrote: > > > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > > > A workflow change for translators are being proposed. > > > > Changes since v2: > > > > 1. Patch 1/9: reword. > > 2. Patch 2/9: reword. > > 3. Patch 3/9: reword, and add "FORCE" to prerequisites of "po/git.pot". > > 4. Patch 6/9: remove "FORCE" from prerequisites of "po/git.pot". > > 5. Patch 8/9: reword, and reuse "$(gen_pot_header)" to prepare pot > > header for "po/git-core.pot". > > 6. Patch 9/9: various updates on po/README.md. > > From skimming this the *.c.po v.s. *.c extension is still left in > comments. I'm not saying you need to go for my suggestions, but it would > be very useful in CL's to note things that were suggested but not > changed, such as that. I've tried to improve some commit logs to make my point that we should name the po files in ".build/po/" with ".po" extension instead of ".c" extension. We can choose plan A, move forward with this patch series, and start using the new workflow in 2.37. If you want, we can try Plan B during next release cycle. > Right now I haven't paged that v2 discussion into my brain again, so I > don't know if that was the only thing, it's the only thing I remember > right now... > > But let's read on: > > > Range-diff vs v2: > > > > 1: c45f34f233 ! 1: 362cd0cbe1 Makefile: sort "po/git.pot" by file location > > @@ Metadata > > ## Commit message ## > > Makefile: sort "po/git.pot" by file location > > > > - Before feeding xgettext with more C souce files which may be ignored > > - by various compiler conditions, add new option "--sort-by-file" to > > - xgettext program to create stable message template file "po/git.pot". > > + We will feed xgettext with more C souce files and in different order in > > + subsequent commit. To generate a stable "po/git.pot" regardless of the > > + number and order of input source files, we add a new option > > + "--sort-by-file" to xgettext program. > > > > With this update, the newly generated "po/git.pot" will has the same > > - entries while in a different order. We won't checkin the newly generated > > - "po/git.pot", because we will remove it from tree in a later commit. > > + entries while in a different order. > > + > > + With the help of a custom diff driver as shown below, > > + > > + git config --global diff.gettext-fmt.textconv \ > > + "msgcat --no-location --sort-by-file" > > + > > + and appending a new entry "*.po diff=gettext-fmt" to git attributes, > > + we can see that there are no substantial changes in "po/git.pot". > > + > > + We won't checkin the newly generated "po/git.pot", because we will > > + remove it from tree in a later commit. > > > Does this actually work? This seems to suggest adding a driver for *.po, > but using it against the *.pot file. Isn't that a typo (I haven't tested > it)> Thanks, it's really a typo. s/*.po/*.pot/ > > > But it is much simpler to use variables "$(FOUND_C_SOURCES)" and > > - "$(FOUND_C_SOURCES)" to form a stable "LOCALIZED_C". > > + "$(FOUND_C_SOURCES)" to form a stable "LOCALIZED_C". We also add > > + "$(SCALAR_SOURCES)" files, which are part of C_OBJ but not included in > > + "$(FOUND_C_SOURCES)" because they are in the "contrib/" directory. > > Thanks, good to note that. > > [snipped the rest, will re-read individual commits]
On Mon, May 23, 2022 at 4:19 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Mon, May 23 2022, Ævar Arnfjörð Bjarmason wrote: > > > On Mon, May 23 2022, Jiang Xin wrote: > > > >> From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > >> > >> A workflow change for translators are being proposed. > >> > >> Changes since v2: > >> > >> 1. Patch 1/9: reword. > >> 2. Patch 2/9: reword. > >> 3. Patch 3/9: reword, and add "FORCE" to prerequisites of "po/git.pot". > >> 4. Patch 6/9: remove "FORCE" from prerequisites of "po/git.pot". > >> 5. Patch 8/9: reword, and reuse "$(gen_pot_header)" to prepare pot > >> header for "po/git-core.pot". > >> 6. Patch 9/9: various updates on po/README.md. > > > > From skimming this the *.c.po v.s. *.c extension is still left in > > comments. I'm not saying you need to go for my suggestions, but it would > > be very useful in CL's to note things that were suggested but not > > changed, such as that. > > > > Right now I haven't paged that v2 discussion into my brain again, so I > > don't know if that was the only thing, it's the only thing I remember > > right now... > > This fix-up below implements what I suggested on v2, so now the comments > in the generated file are correct, and don't refer to our intermediate > files: > > $ grep '#-#' po/git.pot > #. #-#-#-#-# git-add--interactive.perl #-#-#-#-# > #. #-#-#-#-# add-patch.c #-#-#-#-# > #. #-#-#-#-# git-add--interactive.perl #-#-#-#-# > #. #-#-#-#-# branch.c #-#-#-#-# > #. #-#-#-#-# object-name.c #-#-#-#-# > #. #-#-#-#-# grep.c #-#-#-#-# > > I gathered that the reason you preferred the whole "grep -q PRItime" was > because you wanted to mitigate the effects of your IDE discovering these > files. > > With the below you can define AGGRESSIVE_INTERMEDIATE and when you "make > pot" the generated *.c files will only exist for as long as they're > needed to generate the next step. > > But if you do a subsequent "make pot" will be slower, as we'll of course > need to generate them again. > > I think it's better to go in this direction, and rename that > AGGRESSIVE_INTERMEDIATE to something like > MAKE_AVOID_REAL_EXTENSIONS_IN_GITIGNORED_FILES or whatever. > > I.e. our correctness shouldn't suffer because we're trying to work > around some issue in a specific (and optional) developer tooling. > > There's also the fix there for the "header" dependency, but as noted in > another reply it should probably be dropped altogether... > > diff --git a/Makefile b/Makefile > index d3eae150de9..0b96b55b63f 100644 > --- a/Makefile > +++ b/Makefile > @@ -2736,6 +2736,7 @@ endif > ## "po/git.pot" file. > LOCALIZED_ALL_GEN_PO = > > +LOCALIZED_C_GEN_C = $(LOCALIZED_C:%=.build/pot/po-munged/%) Intermediate C source files copied from the original location, and PRItime will be replaced by PRIuMAX, if any. > LOCALIZED_C_GEN_PO = $(LOCALIZED_C:%=.build/pot/po/%.po) > LOCALIZED_ALL_GEN_PO += $(LOCALIZED_C_GEN_PO) > > @@ -2745,26 +2746,19 @@ 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: % > +ifdef AGGRESSIVE_INTERMEDIATE > +.INTERMEDIATE: $(LOCALIZED_C_GEN_C) Intermediate files "$(LOCALIZED_C_GEN_C)" will be removed automatically. > +endif > +$(LOCALIZED_C_GEN_C): .build/pot/po-munged/%: % > $(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 > + $(QUIET_GEN)sed -e 's|PRItime|PRIuMAX|g' <$< >$@ Copy each source files in $(LOCALIZED_C) to corresponding intermediate file ($(LOCALIZED_C_GEN_C)) in ".build/pot/po-munged/", replacing PRItime with PRIuMAX if any. > + > +$(LOCALIZED_C_GEN_PO): .build/pot/po/%.po: .build/pot/po-munged/% > + $(call mkdir_p_parent_template) > + $(QUIET_XGETTEXT)( \ > + cd $(<D) && \ > + $(XGETTEXT) $(XGETTEXT_FLAGS_C) --omit-header -o - $(<F) \ > + ) >$@ For each intermediate C source files in ".build/pot/po-munged/", call xgettext to create corresponding po file in ".build/pot/po/" directory. > $(LOCALIZED_SH_GEN_PO): .build/pot/po/%.po: % > $(call mkdir_p_parent_template) > @@ -2786,11 +2780,24 @@ sed -e 's|charset=CHARSET|charset=UTF-8|' \ > echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@ > endef > > -.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO) > +.build/pot/git.header: No. We should rebuild the pot header if any po file need to be update, because we want to refresh the timestamp in the "POT-Creation-Date:" filed of the pot header. > $(call mkdir_p_parent_template) > $(QUIET_GEN)$(gen_pot_header) > > -po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO) > +# We go through this dance of having a prepared > +# e.g. .build/pot/po/grep.c.po and copying it to > +# .build/pot/to-cat/grep.c only because some IDEs (e.g. VSCode) pick > +# up on the "real" extension for the purposes of auto-completion, even > +# if the .build directiory is in .gitignore. > +LOCALIZED_ALL_GEN_TO_CAT = $(LOCALIZED_ALL_GEN_PO:.build/pot/po/%.po=.build/pot/to-cat/%) > +ifdef AGGRESSIVE_INTERMEDIATE > +.INTERMEDIATE: $(LOCALIZED_ALL_GEN_TO_CAT) > +endif > +$(LOCALIZED_ALL_GEN_TO_CAT): .build/pot/to-cat/%: .build/pot/po/%.po > + $(call mkdir_p_parent_template) > + $(QUIET_GEN)cat $< >$@ Copy each po file in ".build/pot/po/" to another location ".build/pot/to-cat/", but without the ".po" extension. Let's take "date.c" as an example: 1. Copy "date.c" to an intermediate C source file ".build/pot/po-munged/date.c" and replace PRItime with PRIuMAX in it. 2. Call xgettext to create ".build/pot/po/date.c.po" from the intermediate C source file ".build/pot/po-munged/date.c". 3. Optionally remove intermediate C source files like ".build/pot/po-munged/date.c". To have two identical C source files in the same worktree is not good, some software may break. So I choose to remove them. 4. Copy the po file (".build/pot/po/date.c.po") created in step 2 to an intermediate fake C source file ".build/pot/to-cat/date.c" which is a file without the ".po" extension. Please note this intermediate fake C source file ".build/pot/to-cat/date.c" is not a valid C file, but a PO file. 5. Call msgcat to create "po/git.pot" from all the intermediate fake C source files including ".build/pot/to-cat/date.c". 6. Optionally remove all the intermediate fake C source files in ".build/pot/to-cat/". I choose to remove them, because leave lots of invalid C source files in worktree is not good. For example, ".build/pot/po/date.c.po" was created from > + > +po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_TO_CAT) > $(QUIET_GEN)$(MSGCAT) $(MSGCAT_FLAGS) $^ >$@ 7. "po/git.pot" depends on the intermediate fake C source files. If any single C source file has been changed, will run step 6 to copy all po files in ".build/pot/po" to corresponding fake C source files in ".build/pot/to-cat/", if we choose to remove these intermediate fake C source files. This implementation is too heavy to solve a trivial issue. I think we can push forward this patch series and leave these comments in "po/git.pot": > $ grep '#-#' po/git.pot > #. #-#-#-#-# git-add--interactive.perl.po #-#-#-#-# > #. #-#-#-#-# add-patch.c.po #-#-#-#-# > #. #-#-#-#-# git-add--interactive.perl.po #-#-#-#-# > #. #-#-#-#-# branch.c.po #-#-#-#-# > #. #-#-#-#-# object-name.c.po #-#-#-#-# > #. #-#-#-#-# grep.c.po #-#-#-#-#
On Mon, May 23 2022, Jiang Xin wrote: > On Mon, May 23, 2022 at 4:19 PM Ævar Arnfjörð Bjarmason >> $(LOCALIZED_SH_GEN_PO): .build/pot/po/%.po: % >> $(call mkdir_p_parent_template) >> @@ -2786,11 +2780,24 @@ sed -e 's|charset=CHARSET|charset=UTF-8|' \ >> echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@ >> endef >> >> -.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO) >> +.build/pot/git.header: > > No. We should rebuild the pot header if any po file need to be update, > because we want to refresh the timestamp in the "POT-Creation-Date:" > filed of the pot header. Okey, I did leave a question about this in an earlier E-Mail though, i.e. does anything actually rely on this, or the header at all, or is this just cargo-culting? I haven't found anything in our toolchain that cares about the header at all (for the *.pot, not *.po!) let alone the update timestamp. Except insofar as e.g. Emacs will add a timestamp or update it if it finds a header already. >> $(call mkdir_p_parent_template) >> $(QUIET_GEN)$(gen_pot_header) >> >> -po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO) >> +# We go through this dance of having a prepared >> +# e.g. .build/pot/po/grep.c.po and copying it to >> +# .build/pot/to-cat/grep.c only because some IDEs (e.g. VSCode) pick >> +# up on the "real" extension for the purposes of auto-completion, even >> +# if the .build directiory is in .gitignore. >> +LOCALIZED_ALL_GEN_TO_CAT = $(LOCALIZED_ALL_GEN_PO:.build/pot/po/%.po=.build/pot/to-cat/%) >> +ifdef AGGRESSIVE_INTERMEDIATE >> +.INTERMEDIATE: $(LOCALIZED_ALL_GEN_TO_CAT) >> +endif >> +$(LOCALIZED_ALL_GEN_TO_CAT): .build/pot/to-cat/%: .build/pot/po/%.po >> + $(call mkdir_p_parent_template) >> + $(QUIET_GEN)cat $< >$@ > > Copy each po file in ".build/pot/po/" to another location > ".build/pot/to-cat/", but without the ".po" extension. > > Let's take "date.c" as an example: > > 1. Copy "date.c" to an intermediate C source file > ".build/pot/po-munged/date.c" and replace PRItime with PRIuMAX in it. > > 2. Call xgettext to create ".build/pot/po/date.c.po" from the > intermediate C source file ".build/pot/po-munged/date.c". > > 3. Optionally remove intermediate C source files like > ".build/pot/po-munged/date.c". To have two identical C source files in > the same worktree is not good, some software may break. So I choose to > remove them. > > 4. Copy the po file (".build/pot/po/date.c.po") created in step 2 to > an intermediate fake C source file ".build/pot/to-cat/date.c" which is > a file without the ".po" extension. Please note this intermediate fake > C source file ".build/pot/to-cat/date.c" is not a valid C file, but a > PO file. > > 5. Call msgcat to create "po/git.pot" from all the intermediate fake C > source files including ".build/pot/to-cat/date.c". > > 6. Optionally remove all the intermediate fake C source files in > ".build/pot/to-cat/". I choose to remove them, because leave lots of > invalid C source files in worktree is not good. > > For example, ".build/pot/po/date.c.po" was created from >> + >> +po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_TO_CAT) >> $(QUIET_GEN)$(MSGCAT) $(MSGCAT_FLAGS) $^ >$@ > > 7. "po/git.pot" depends on the intermediate fake C source files. If > any single C source file has been changed, will run step 6 to copy all > po files in ".build/pot/po" to corresponding fake C source files in > ".build/pot/to-cat/", if we choose to remove these intermediate fake C > source files. > > This implementation is too heavy to solve a trivial issue. I think we > can push forward this patch series and leave these comments in > "po/git.pot": If you find it too "heavy" & are trying to optimize it for some reason then that whole extra special-dance can be made conditional on MAKE_AVOID_REAL_EXTENSIONS_IN_GITIGNORED_FILES. But really, it's 15MB of .build/pot in my local HEAD with this fix-up, it's 1.4MB without it, but this whole thing just seems like premature optimization. Especially given: $ git hyperfine -r 3 -L rev origin/master,HEAD~,HEAD,avar/Makefile-incremental-po-git-pot-rule~,avar/Makefile-incremental-po-git-pot-rule -p 'git clean -dxf; git reset --hard' 'make pot' --warmup 1 Benchmark 1: make pot' in 'origin/master Time (mean ± σ): 1.970 s ± 0.014 s [User: 1.683 s, System: 0.353 s] Range (min … max): 1.955 s … 1.982 s 3 runs Benchmark 2: make pot' in 'HEAD~ Time (mean ± σ): 931.3 ms ± 4.7 ms [User: 3358.5 ms, System: 1088.7 ms] Range (min … max): 927.0 ms … 936.3 ms 3 runs Benchmark 3: make pot' in 'HEAD Time (mean ± σ): 1.506 s ± 0.389 s [User: 4.655 s, System: 1.363 s] Range (min … max): 1.257 s … 1.955 s 3 runs Benchmark 4: make pot' in 'avar/Makefile-incremental-po-git-pot-rule~ Time (mean ± σ): 1.015 s ± 0.002 s [User: 3.615 s, System: 1.224 s] Range (min … max): 1.013 s … 1.017 s 3 runs Benchmark 5: make pot' in 'avar/Makefile-incremental-po-git-pot-rule Time (mean ± σ): 1.014 s ± 0.008 s [User: 3.540 s, System: 1.068 s] Range (min … max): 1.007 s … 1.023 s 3 runs Summary 'make pot' in 'HEAD~' ran 1.09 ± 0.01 times faster than 'make pot' in 'avar/Makefile-incremental-po-git-pot-rule' 1.09 ± 0.01 times faster than 'make pot' in 'avar/Makefile-incremental-po-git-pot-rule~' 1.62 ± 0.42 times faster than 'make pot' in 'HEAD' 2.12 ± 0.02 times faster than 'make pot' in 'origin/master' I.e. all of this is much faster than what we have on "master" now. My 22434ef36ae (Makefile: avoid "sed" on C files that don't need it, 2022-04-08) (avar/Makefile-incremental-po-git-pot-rule) is then just 10% slower than the "grep or xgettext", its "~" is the corresponding unoptimized. The HEAD here is with my fix-up, and HEAD~ is your series here. Anyway, if you really feel strongly about it let's go with your way of doing it. It just sounded like you weren't actually trying top optimize anything, but to work around your editor. So if we had a method to do that.... ...except it seems you also care about making it much faster than "master" (or care about <20MB of disk space), which to be blunt seems a bit crazy to me :) Last I checked "make test" ended up creating ~1GB of data (not all at once, but in parallel testing a lot more than 10MB is often in play at once). As this was a pretty obscure target that I only expect CI, you, translators & me to run in practice a small difference in the initial run didn't seem to matter, especially as it's all an improvement over "master". Anyway, you do whatever you think is best with that :) >> $ grep '#-#' po/git.pot >> #. #-#-#-#-# git-add--interactive.perl.po #-#-#-#-# >> #. #-#-#-#-# add-patch.c.po #-#-#-#-# >> #. #-#-#-#-# git-add--interactive.perl.po #-#-#-#-# >> #. #-#-#-#-# branch.c.po #-#-#-#-# >> #. #-#-#-#-# object-name.c.po #-#-#-#-# >> #. #-#-#-#-# grep.c.po #-#-#-#-#
On Mon, May 23, 2022 at 10:56 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Mon, May 23 2022, Jiang Xin wrote: > > > On Mon, May 23, 2022 at 4:19 PM Ævar Arnfjörð Bjarmason > >> $(LOCALIZED_SH_GEN_PO): .build/pot/po/%.po: % > >> $(call mkdir_p_parent_template) > >> @@ -2786,11 +2780,24 @@ sed -e 's|charset=CHARSET|charset=UTF-8|' \ > >> echo '"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n"' >>$@ > >> endef > >> > >> -.build/pot/git.header: $(LOCALIZED_ALL_GEN_PO) > >> +.build/pot/git.header: > > > > No. We should rebuild the pot header if any po file need to be update, > > because we want to refresh the timestamp in the "POT-Creation-Date:" > > filed of the pot header. > > Okey, I did leave a question about this in an earlier E-Mail though, > i.e. does anything actually rely on this, or the header at all, or is > this just cargo-culting? > > I haven't found anything in our toolchain that cares about the header at > all (for the *.pot, not *.po!) let alone the update timestamp. When creating a new po/XX.po manually using msginit from POT file with or without a header, the new generated po/XX.po has different header. $ msginit -i po/git.pot -o po/XX-with-header.po \ --locale=ja --no-translator $ msginit -i po/git-headless.pot -o po/XX-without-header.po \ --locale=ja --no-translator $ diff po/XX-with-header.po po/XX-without-header.po 1,5d0 < # Japanese translations for Git package. < # Copyright (C) 2022 THE Git'S COPYRIGHT HOLDER < # This file is distributed under the same license as the Git package. < # Automatically generated, 2022. < # 8,11c3 < "Project-Id-Version: Git\n" < "Report-Msgid-Bugs-To: Git Mailing List <git@vger.kernel.org>\n" < "POT-Creation-Date: 2022-05-23 23:27+0800\n" < "PO-Revision-Date: 2022-05-23 23:27+0800\n" --- > "Project-Id-Version: git 2.36.0.7.g31429651cf.dirty\n" 16c8 < "Content-Type: text/plain; charset=UTF-8\n" --- > "Content-Type: text/plain; charset=ASCII\n" Should we ignore this change? > > >> $(call mkdir_p_parent_template) > >> $(QUIET_GEN)$(gen_pot_header) > >> > >> -po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_PO) > >> +# We go through this dance of having a prepared > >> +# e.g. .build/pot/po/grep.c.po and copying it to > >> +# .build/pot/to-cat/grep.c only because some IDEs (e.g. VSCode) pick > >> +# up on the "real" extension for the purposes of auto-completion, even > >> +# if the .build directiory is in .gitignore. > >> +LOCALIZED_ALL_GEN_TO_CAT = $(LOCALIZED_ALL_GEN_PO:.build/pot/po/%.po=.build/pot/to-cat/%) > >> +ifdef AGGRESSIVE_INTERMEDIATE > >> +.INTERMEDIATE: $(LOCALIZED_ALL_GEN_TO_CAT) > >> +endif > >> +$(LOCALIZED_ALL_GEN_TO_CAT): .build/pot/to-cat/%: .build/pot/po/%.po > >> + $(call mkdir_p_parent_template) > >> + $(QUIET_GEN)cat $< >$@ > > > > Copy each po file in ".build/pot/po/" to another location > > ".build/pot/to-cat/", but without the ".po" extension. > > > > Let's take "date.c" as an example: > > > > 1. Copy "date.c" to an intermediate C source file > > ".build/pot/po-munged/date.c" and replace PRItime with PRIuMAX in it. > > > > 2. Call xgettext to create ".build/pot/po/date.c.po" from the > > intermediate C source file ".build/pot/po-munged/date.c". > > > > 3. Optionally remove intermediate C source files like > > ".build/pot/po-munged/date.c". To have two identical C source files in > > the same worktree is not good, some software may break. So I choose to > > remove them. > > > > 4. Copy the po file (".build/pot/po/date.c.po") created in step 2 to > > an intermediate fake C source file ".build/pot/to-cat/date.c" which is > > a file without the ".po" extension. Please note this intermediate fake > > C source file ".build/pot/to-cat/date.c" is not a valid C file, but a > > PO file. > > > > 5. Call msgcat to create "po/git.pot" from all the intermediate fake C > > source files including ".build/pot/to-cat/date.c". > > > > 6. Optionally remove all the intermediate fake C source files in > > ".build/pot/to-cat/". I choose to remove them, because leave lots of > > invalid C source files in worktree is not good. > > > > For example, ".build/pot/po/date.c.po" was created from > >> + > >> +po/git.pot: .build/pot/git.header $(LOCALIZED_ALL_GEN_TO_CAT) > >> $(QUIET_GEN)$(MSGCAT) $(MSGCAT_FLAGS) $^ >$@ > > > > 7. "po/git.pot" depends on the intermediate fake C source files. If > > any single C source file has been changed, will run step 6 to copy all > > po files in ".build/pot/po" to corresponding fake C source files in > > ".build/pot/to-cat/", if we choose to remove these intermediate fake C > > source files. > > > > This implementation is too heavy to solve a trivial issue. I think we > > can push forward this patch series and leave these comments in > > "po/git.pot": > > If you find it too "heavy" & are trying to optimize it for some reason > then that whole extra special-dance can be made conditional on > MAKE_AVOID_REAL_EXTENSIONS_IN_GITIGNORED_FILES. > > But really, it's 15MB of .build/pot in my local HEAD with this fix-up, > it's 1.4MB without it, but this whole thing just seems like premature > optimization. Especially given: > > $ git hyperfine -r 3 -L rev origin/master,HEAD~,HEAD,avar/Makefile-incremental-po-git-pot-rule~,avar/Makefile-incremental-po-git-pot-rule -p 'git clean -dxf; git reset --hard' 'make pot' --warmup 1 > Benchmark 1: make pot' in 'origin/master > Time (mean ± σ): 1.970 s ± 0.014 s [User: 1.683 s, System: 0.353 s] > Range (min … max): 1.955 s … 1.982 s 3 runs > > Benchmark 2: make pot' in 'HEAD~ > Time (mean ± σ): 931.3 ms ± 4.7 ms [User: 3358.5 ms, System: 1088.7 ms] > Range (min … max): 927.0 ms … 936.3 ms 3 runs > > Benchmark 3: make pot' in 'HEAD > Time (mean ± σ): 1.506 s ± 0.389 s [User: 4.655 s, System: 1.363 s] > Range (min … max): 1.257 s … 1.955 s 3 runs > > Benchmark 4: make pot' in 'avar/Makefile-incremental-po-git-pot-rule~ > Time (mean ± σ): 1.015 s ± 0.002 s [User: 3.615 s, System: 1.224 s] > Range (min … max): 1.013 s … 1.017 s 3 runs > > Benchmark 5: make pot' in 'avar/Makefile-incremental-po-git-pot-rule > Time (mean ± σ): 1.014 s ± 0.008 s [User: 3.540 s, System: 1.068 s] > Range (min … max): 1.007 s … 1.023 s 3 runs > > Summary > 'make pot' in 'HEAD~' ran > 1.09 ± 0.01 times faster than 'make pot' in 'avar/Makefile-incremental-po-git-pot-rule' > 1.09 ± 0.01 times faster than 'make pot' in 'avar/Makefile-incremental-po-git-pot-rule~' > 1.62 ± 0.42 times faster than 'make pot' in 'HEAD' > 2.12 ± 0.02 times faster than 'make pot' in 'origin/master' > > I.e. all of this is much faster than what we have on "master" now. My > 22434ef36ae (Makefile: avoid "sed" on C files that don't need it, > 2022-04-08) (avar/Makefile-incremental-po-git-pot-rule) is then just 10% > slower than the "grep or xgettext", its "~" is the corresponding > unoptimized. > > The HEAD here is with my fix-up, and HEAD~ is your series here. > > Anyway, if you really feel strongly about it let's go with your way of > doing it. > > It just sounded like you weren't actually trying top optimize anything, > but to work around your editor. So if we had a method to do that.... But for users who prefer to delete all the intermediate files in ".build/pot/to-cat" and ".build/pot/po-munged/", then they will get performance penalty. > ...except it seems you also care about making it much faster than > "master" (or care about <20MB of disk space), which to be blunt seems a > bit crazy to me :) Last I checked "make test" ended up creating ~1GB of > data (not all at once, but in parallel testing a lot more than 10MB is > often in play at once). > > As this was a pretty obscure target that I only expect CI, you, > translators & me to run in practice a small difference in the initial > run didn't seem to matter, especially as it's all an improvement over > "master". > > Anyway, you do whatever you think is best with that :) > > >> $ grep '#-#' po/git.pot > >> #. #-#-#-#-# git-add--interactive.perl.po #-#-#-#-# > >> #. #-#-#-#-# add-patch.c.po #-#-#-#-# > >> #. #-#-#-#-# git-add--interactive.perl.po #-#-#-#-# > >> #. #-#-#-#-# branch.c.po #-#-#-#-# > >> #. #-#-#-#-# object-name.c.po #-#-#-#-# > >> #. #-#-#-#-# grep.c.po #-#-#-#-# >