Re: meson: avoid PATH bloat from NLS .mo targets in tmp_install test setup - Mailing list pgsql-hackers
| From | Tristan Partin |
|---|---|
| Subject | Re: meson: avoid PATH bloat from NLS .mo targets in tmp_install test setup |
| Date | |
| Msg-id | DLY3DK18YLCF.2QTLVQ7LVVN4O@partin.io Whole thread |
| In response to | Re: meson: avoid PATH bloat from NLS .mo targets in tmp_install test setup (Andrew Dunstan <andrew@dunslane.net>) |
| List | pgsql-hackers |
On Sun Oct 4, 2026 at 12:52 PM UTC, Andrew Dunstan wrote: > > On 2026-09-21 Mo 5:50 PM, Tristan Partin wrote: >> On Wed Jul 29, 2026 at 11:28 AM CDT, Tristan Partin wrote: >>> On Wed Jul 15, 2026 at 8:49 PM UTC, Tristan Partin wrote: >>>> On Wed Jul 15, 2026 at 8:44 PM UTC, Andrew Dunstan wrote: >>>>> On Wed, Jul 15, 2026 at 12:30 PM Tristan Partin <tristan@partin.io> wrote: >>>>> >>>>>> On Tue Jul 14, 2026 at 9:24 PM UTC, Andrew Dunstan wrote: >>>>>>> Hi >>>>>>> >>>>>>> While setting up a Windows/MSVC buildfarm animal with NLS enabled, the >>>>>>> meson 'tmp_install' and 'initdb_cache' setup tests failed immediately >>>>>>> with exit status 1 and no stdout/stderr at all. >>>>>>> >>>>>>> With a little help from Claude, I (eventually) found this diagnosis and >>>>>>> solution: >>>>>>> >>>>>>> The root cause turned out to be in meson itself, not in our meson.build. >>>>>>> On Windows, determine_windows_extra_paths() in >>>>>>> mesonbuild/backend/backends.py builds a test's PATH by walking every >>>>>>> target passed via that test's 'depends:' kwarg (extra_bdeps) and adding >>>>>>> each one's build directory unconditionally, with no check for whether >>>>>>> the target actually produces a DLL. That's fine when 'depends:' lists a >>>>>>> handful of real link dependencies, but the 'tmp_install' test depends on >>>>>>> installed_targets, which includes nls_mo_targets - one custom_target per >>>>>>> locale/domain of compiled .mo catalogs. With NLS enabled that's several >>>>>>> hundred targets, none of them DLLs, none of them ever looked up via >>>>>>> PATH, and each one still gets its own entry. >>>>>>> >>>>>>> In our case this inflated PATH to ~39000 characters across 584 entries >>>>>>> (448 of them po/*/LC_MESSAGES directories), comfortably past practical >>>>>>> Windows environment-variable/command-line length limits, which is why >>>>>>> the test failed silently - the failure happens before the child process >>>>>>> gets a chance to produce any output. >>>>>>> >>>>>>> I think this is arguably a meson bug (determine_windows_extra_paths() >>>>>>> should filter extra_bdeps the same way it already filters a test >>>>>>> executable's own link dependencies), but regardless of whether that >>>>>>> gets fixed upstream, we can sidestep it on our end cheaply: depend on a >>>>>>> trivial stamp custom_target instead of installed_targets directly. Its >>>>>>> own 'depends:' still forces installed_targets to build first, so build >>>>>>> ordering is unaffected, but since a custom_target is not a >>>>>>> build.BuildTarget, meson doesn't recurse into its dependencies when >>>>>>> computing the test PATH - it contributes at most one harmless directory >>>>>>> instead of hundreds. >>>>>>> >>>>>>> I initially tried wrapping installed_targets in an alias_target() >>>>>>> instead, which would avoid the recursion the same way, but test()'s >>>>>>> 'depends:' kwarg is typechecked to only accept >>>>>>> BuildTarget | CustomTarget | CustomTargetIndex and rejects AliasTarget >>>>>>> outright: >>>>>>> >>>>>>> meson.build:NNNN:0: ERROR: test keyword argument 'depends' was of >>>>>>> type array[AliasTarget] but should have been type >>>>>>> array[BuildTarget | CustomTarget | CustomTargetIndex] >>>>>>> >>>>>>> The attached patch uses a custom_target instead, which satisfies that >>>>>>> type check. >>>>>>> >>>>>>> Tested on Windows/MSVC (meson 1.11.1), building with -Dnls=enabled: >>>>>>> >>>>>>> before: tmp_install test PATH = 39193 chars, 584 entries, 448 >>>>>>> LC_MESSAGES - tmp_install and initdb_cache setup tests FAIL >>>>>>> (exit status 1, no output) >>>>>>> after: tmp_install test PATH = 2189 chars, 43 entries, 0 >>>>>>> LC_MESSAGES - all three setup tests (tmp_install, >>>>>>> install_test_files, initdb_cache) OK >>>>>>> >>>>>>> I did not attempt to fix the same class of problem for any other test >>>>>>> in the tree - this patch only touches the one setup test that was >>>>>>> actually failing for us. If there's interest, the same technique could >>>>>>> presumably be applied wherever else a test's 'depends:' pulls in a >>>>>>> large target list. >>>>>> Hey Andrew, >>>>>> >>>>>> I took some time to raise this issue with the Meson team in their >>>>>> Matrix/IRC channel. I'll see if the discussion goes anywhere. >>>>>> >>>>>> As for the fix and the results, they look good to me. I couldn't >>>>>> identify any other problems where this could be an issue, so this seems >>>>>> like a one-off. Did you notice any other problems? I only noticed >>>>>> installed_targets depended on in tmp_install and install-quiet. >>>>>> >>>>> >>>>> I should have mentioned that I raised an issue at < >>>>> https://github.com/mesonbuild/meson/issues/16010> >>>> Thanks! Currently discussing potential solutions. >>> I posted a PR[0] to fix this. Just waiting on review. Thanks for your >>> initial investigation. >>> >>> [0]: https://github.com/mesonbuild/meson/pull/16039 >> I ended up using a different solution, but this is now fixed[1] in >> Meson, and will be released in 1.13.0. >> >> [1]: https://github.com/mesonbuild/meson/pull/16039 > > > Hi Tristan > > > Thanks for following up. Meanwhile I intend to apply the fix to > meson.build I originally proposed, which is fairly harmless and small, > so we don't have to wait for everyone to catch up to the latest meson > (which isn't out yet anyway). Yep, makes complete sense to me. One thing that you could do is guard on meson version within the build script: if meson.version().version_compare('<=1.13') tmp_install_depends = ... else tmp_install_depeends = installed_targets endif It might also be valuable to add a comment explaining the indirection can be removed some time after we depend on 1.13 fully. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
pgsql-hackers by date: