Re: Buf fix: update-po for PGXS does not work

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            not tested

Greetings,

I took a look at the v2 patch for the PGXS update-po bug, and it looks solid.

I built PostgreSQL from source with --enable-nls and set up a test installation. To actually see the bug in action and
verifythe fix, I put together a simple external extension with NLS support - just a basic C file with some
errmsg/errhintcalls, a PGXS Makefile with the usual CATALOG_NAME and AVAIL_LANGUAGES settings, and a po/ directory with
French,German, and Japanese translations. I also added an nls.mk file and a po/LINGUAS file, since they are required
forthe NLS machinery to work.
 

Before the patch, running make update-po in my extension directory did absolutely nothing—no .po.new files were
created.The problem is pretty straightforward once you look at it: nls-global.mk searches $(top_srcdir) for .po files,
butin PGXS mode, $ (top_srcdir) points to the PostgreSQL install directory, not the extension's source tree, where the
actual.po files live.
 

I applied the patch and everything works as expected. The patch adds the PGXS conditional to nls-global.mk so it
searchesthe current directory instead, and make update-po successfully created all three .po.new files (fr, de, ja)
usingmsgmerge. The generated files look good—proper msgid/msgstr entries and valid translation content. The extension
buildsfine afterward as well.
 

I also double-checked that regular in-tree builds still work correctly with contrib modules, and they do.

The fix is straightforward and does exactly what it needs to—using ‘find .’ for PGXS mode instead of ‘find
$(top_srcdir)’for regular builds. Both ALL_LANGUAGES and all_compendia get the same treatment, and the ifdef nesting
looksright to me.
 

The patch is clean, and I didn't encounter any issues during testing.  There is no documentation, but for this BUG FIX,
Idid not expect any.
 

Looks good to me— I think it is ready for commit.

Bryan Green