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 verify the fix, I put together a simple external extension with NLS support - just a basic C file with some errmsg/errhint calls, 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 for the 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, but in 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 searches the current directory instead, and make update-po successfully created all three .po.new files (fr, de, ja) using msgmerge. The generated files look good—proper msgid/msgstr entries and valid translation content. The extension builds fine 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 looks right to me. The patch is clean, and I didn't encounter any issues during testing. There is no documentation, but for this BUG FIX, I did not expect any. Looks good to me— I think it is ready for commit. Bryan Green
