make update-po@master stops at pg_upgrade

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

I find it annoying that make update-po stops at pg_upgrade on master.
The cause is that a file is renamed from relfilenode.c to
relfilenumber.c so just fixing the name works. (attached first).

Ooops.

I wonder if we can use $(wildcard *.c) instead of explicitly
enumerating every *.c file in the directory then maintaining the
list.

+1. We've repeatedly forgotten to update the nls.mk files when
adding/removing files; they're just not on most hackers' radar.
Anything we can do to make that more automatic seems like a win.

Since backend does that way, I think we can do that the same way
also for the tools. Attached second does that except for tools that
have only one *.c. The patch doesn't make a difference in the result
of make update-po.

I wonder if there's anything we can do about the manual cross-references
to src/common. We could wildcard that, but then every FE program would
have to contain translations for all messages in src/common/, even for
modules it doesn't use.

Still, wildcarding the local *.c references seems like a clear step
forward. I'll go push that part.

regards, tom lane

I wrote:

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

Since backend does that way, I think we can do that the same way
also for the tools. Attached second does that except for tools that
have only one *.c. The patch doesn't make a difference in the result
of make update-po.

Still, wildcarding the local *.c references seems like a clear step
forward. I'll go push that part.

I had to recreate the patch almost from scratch, because 88dad06b4
touched adjacent lines in most of these files, scaring patch(1)
away from applying the changes. That being the case, I decided
to use $(wildcard *.c) everywhere, including the places where there's
currently just one *.c file. It seems to work for me, but please check.

regards, tom lane

On 2022-Jul-13, Tom Lane wrote:

I had to recreate the patch almost from scratch, because 88dad06b4
touched adjacent lines in most of these files, scaring patch(1)
away from applying the changes. That being the case, I decided
to use $(wildcard *.c) everywhere, including the places where there's
currently just one *.c file. It seems to work for me, but please check.

Hmm, I got this failure:

make[4]: se entra en el directorio '/home/alvherre/Code/pgsql-build/master/src/interfaces/ecpg/ecpglib'
/usr/bin/xgettext -ctranslator --copyright-holder='PostgreSQL Global Development Group' --msgid-bugs-address=pgsql-bugs@lists.postgresql.org --no-wrap --sort-by-file --package-name='ecpglib (PostgreSQL)' --package-version='16' -D /pgsql/source/master/src/interfaces/ecpg/ecpglib -D . -n -kecpg_gettext -k_ --flag=ecpg_gettext:1:pass-c-format --flag=_:1:pass-c-format
/usr/bin/xgettext: no se especificó el fichero de entrada
Pruebe '/usr/bin/xgettext --help' para más información.
make[4]: *** [/pgsql/source/master/src/nls-global.mk:108: po/ecpglib.pot] Error 1

The error from xgettext, of course, is
/usr/bin/xgettext: no input file given

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Puedes vivir sólo una vez, pero si lo haces bien, una vez es suficiente"

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

Hmm, I got this failure:
/usr/bin/xgettext: no se especificó el fichero de entrada

Hmm ... are you doing this in a VPATH setup? Does it help
if you make the entry be

GETTEXT_FILES = $(wildcard $(srcdir)/*.c)

I'd supposed we didn't need to be careful about that, because
I saw uses of $(wildcard) without it ... but I now see other uses
with.

regards, tom lane

On 13.07.22 18:07, Tom Lane wrote:

Still, wildcarding the local *.c references seems like a clear step
forward. I'll go push that part.

In some cases, the listed files are build output from another rule, for
example sql_help.c. By using a wildcard, you just take whatever files
happen to be there, not observing proper make dependencies.
(Conversely, you could also include files that are not part of the
build, maybe leftover from something aborted.) So I'm not sure this is
such a clear win. In any case, someone should check that this creates
identical output before and after.

On 13.07.22 20:09, Peter Eisentraut wrote:

In any case, someone should check that this creates identical output
before and after.

A quick check shows differences in

pg_rewind.pot
psql.pot
ecpg.pot
libpq.pot
plpgsql.pot

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

In some cases, the listed files are build output from another rule, for
example sql_help.c. By using a wildcard, you just take whatever files
happen to be there, not observing proper make dependencies.

Hmm. We could list built files explicitly, perhaps, and still be
a good step ahead on the maintenance burden. Does xgettext get
upset if the same input file is mentioned twice, ie would we have
to filter sql_help.c out of the wildcard result?

regards, tom lane

On 13.07.22 19:41, Tom Lane wrote:

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

Hmm, I got this failure:
/usr/bin/xgettext: no se especificó el fichero de entrada

Hmm ... are you doing this in a VPATH setup? Does it help
if you make the entry be

GETTEXT_FILES = $(wildcard $(srcdir)/*.c)

I'd supposed we didn't need to be careful about that, because
I saw uses of $(wildcard) without it ... but I now see other uses
with.

Note that we have this in nls-global.mk which tries to avoid having the
vpath details sneak into the output:

po/$(CATALOG_NAME).pot: $(GETTEXT_FILES) $(MAKEFILE_LIST)
# Change to srcdir explicitly, don't rely on $^. That way we get
# consistent #: file references in the po files.
...
$(XGETTEXT) -D $(srcdir) -D . -n $(addprefix -k, $(GETTEXT_TRIGGERS)) $(addprefix --flag=, $(GETTEXT_FLAGS)) $(GETTEXT_FILES)

On 13.07.22 20:19, Tom Lane wrote:

Hmm. We could list built files explicitly, perhaps, and still be
a good step ahead on the maintenance burden. Does xgettext get
upset if the same input file is mentioned twice, ie would we have
to filter sql_help.c out of the wildcard result?

It seems it would be ok with that.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Note that we have this in nls-global.mk which tries to avoid having the
vpath details sneak into the output:

po/$(CATALOG_NAME).pot: $(GETTEXT_FILES) $(MAKEFILE_LIST)
# Change to srcdir explicitly, don't rely on $^. That way we get
# consistent #: file references in the po files.
...
$(XGETTEXT) -D $(srcdir) -D . -n $(addprefix -k, $(GETTEXT_TRIGGERS)) $(addprefix --flag=, $(GETTEXT_FLAGS)) $(GETTEXT_FILES)

Hmm ... so how does that work with built files in a VPATH build today?

Anyway, I'll go revert the patch for now, since it's clearly got
at least a couple problems that need sorting.

regards, tom lane

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

On 13.07.22 20:19, Tom Lane wrote:

Hmm. We could list built files explicitly, perhaps, and still be
a good step ahead on the maintenance burden. Does xgettext get
upset if the same input file is mentioned twice, ie would we have
to filter sql_help.c out of the wildcard result?

It seems it would be ok with that.

Actually, we can get rid of those easily enough anyway with $(sort).
Here's a draft that might solve these problems. The idea is to use
$(wildcard) for files in the srcdir, and manually enumerate only
built files.

regards, tom lane

On 2022-Jul-13, Tom Lane wrote:

Actually, we can get rid of those easily enough anyway with $(sort).
Here's a draft that might solve these problems. The idea is to use
$(wildcard) for files in the srcdir, and manually enumerate only
built files.

I checked the files in src/bin/scripts and they look OK; there are no
missing messages now. I also checked the es.po.new files with and
without patch; they look good, nothing is lost.

Files that weren't previously being processed:
src/interfaces/libpq/fe-print.c
src/interfaces/ecpg/preproc/parser.c

Incidentally, this patch is pretty similar to what Kyotaro-san sent when
opening the thread, with the addition of the $(notdir) call.

In short, +1 to this patch.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"El sentido de las cosas no viene de las cosas, sino de
las inteligencias que las aplican a sus problemas diarios
en busca del progreso." (Ernesto Hernández-Novich)

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

In short, +1 to this patch.

Thanks for testing it. I think the only remaining concern is
Peter's objection that $(wildcard) might pick up random junk files
that end in ".c". That's true, but the backend's nls.mk also
picks up everything matching "*.c" (over the whole backend tree,
not just one directory!), and I don't recall people complaining
about that. So I think the reduction in maintenance burden
justifies the risk. What do others think?

regards, tom lane

On 2022-Jul-28, Tom Lane wrote:

Thanks for testing it. I think the only remaining concern is
Peter's objection that $(wildcard) might pick up random junk files
that end in ".c". That's true, but the backend's nls.mk also
picks up everything matching "*.c" (over the whole backend tree,
not just one directory!),

Now that I did the translation chores again after a few years I am
reminded of a point about this argument: in reality, few people ever
run this recipe manually (I know I never do), because it's easier to
grab the already-merged files from the NLS website. It all happens
mechanically and there's nobody leaving random junnk files.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"El que vive para el futuro es un iluso, y el que vive para el pasado,
un imbécil" (Luis Adler, "Los tripulantes de la noche")

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

On 2022-Jul-28, Tom Lane wrote:

Thanks for testing it. I think the only remaining concern is
Peter's objection that $(wildcard) might pick up random junk files
that end in ".c". That's true, but the backend's nls.mk also
picks up everything matching "*.c" (over the whole backend tree,
not just one directory!),

Now that I did the translation chores again after a few years I am
reminded of a point about this argument: in reality, few people ever
run this recipe manually (I know I never do), because it's easier to
grab the already-merged files from the NLS website. It all happens
mechanically and there's nobody leaving random junnk files.

Hmm, so where does the NLS website get its data?

I'd be all for flushing the recipe altogether if no one uses it.
However, the existence of this thread suggests otherwise.

regards, tom lane

On 2022-Aug-08, Tom Lane wrote:

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

Now that I did the translation chores again after a few years I am
reminded of a point about this argument: in reality, few people ever
run this recipe manually (I know I never do), because it's easier to
grab the already-merged files from the NLS website. It all happens
mechanically and there's nobody leaving random junnk files.

Hmm, so where does the NLS website get its data?

Well, the NLS website does invoke the recipe. Just not manually.

I'd be all for flushing the recipe altogether if no one uses it.
However, the existence of this thread suggests otherwise.

I just meant it's not normally run manually. But if you do run it
manually, and you translate a file that has a few extra messages because
of the hypothetical junk source file, then you'll upload a catalog with
those extra messages; these extra messages will be dropped the next time
your file is merged through the NLS website. Maybe you'll do some extra
work (translating useless messages) but there'll be no harm.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/