On Thu, May 19 2022, Jiang Xin wrote: > On Thu, May 19, 2022 at 6:39 PM Ævar Arnfjörð Bjarmason > <avarab@xxxxxxxxx> 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 :)