Restore the possibility of forcing "fat" static library links on a per-target basis - #9453

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.30%. Comparing base (18e4b3f) to head (81d630d).
Report is 3091 commits behind head on master.

@@            Coverage Diff             @@
##           master    #9453      +/-   ##
==========================================
- Coverage   67.31%   67.30%   -0.01%     
==========================================
  Files         396      396              
  Lines       85468    85474       +6     
  Branches    17661    17663       +2     
==========================================
+ Hits        57530    57532       +2     
- Misses      23249    23251       +2     
- Partials     4689     4691       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This PR looks good to me, just missing a unit test ideally. However, we are not supposed to add new API in a point release. Since thin archives is only a performance optimisation and I don't think it has any functional difference, I think we should for 0.60.1 revert to not use thin archives by default and add this API for 0.61.0. What do you think?

@xclaesse Favourably disposed.

@jpakkane @dcbaker @eli-schwartz what do you think? Should we revert the thin archive stuff and postpone it to 0.61.0?

Well, git master is already for 0.61, the question is whether to revert it in 0.60.1 :p

In #9294 I wasn't sure whether this might sometimes be unwanted, but I wasn't expecting cases where it's unwanted because of what the meson.build does with the output lol. I wish we'd discovered this earlier... no one seemed to be quite sure what the potential pitfalls of thin archives might be.

Is this a problem for meson's own cuda compiler too?

@eli-schwartz By pure luck, the Meson testcases pass for CUDA because they are shielded with an -Xlinker flag: -Xlinker=libblabla.a. This will break, however, if the testcases would have attempted to link only static_library()s together in one target without sources, because then NVCC, not seeing anything but opaque -Xlinker=... flags, thinks it's not been given any compilation/linking work to do. But if I fully manually invoke NVCC through custom_target('name', [nvcc, ...]) on a (obviously not-installed) static_library(), it vomits on me nvcc: elfLinker: something something format not recognized or not an ELF file.

@obilaniu I'm not sure your test case actually test much, we have no way to verify the generated archive actually is thin or not, do we? Are there tools that can tell?

Actually, seems pretty easy to check: https://stackoverflow.com/questions/17297761/detect-whether-static-library-is-a-thin-archive.

@xclaesse If the Filesystem's module's fs.size(filename) accepted build targets, perhaps... but even if it did, there is no guarantee that the toolchain will support thinning, so I can't assert anything more strict than <=, but that would test almost nothing.

Is !<thin> actually reliable on all platforms that support thinning?

There are not that many platforms that supports thin archives in the first place... Tbh as long as it pass on CI platforms it's fine.

specifically, it's nvcc being driven with custom flags

Has this issue been reported to NVidia? This seems like a thing that they should handle.

Continuing with my shower thoughts... there wouldn't happen to be some way we could detect that the archive is going to be used by a custom_target and disable generating thin archives in that case, would there be? In order to have a slightly safer/more conservative default...

@jpakkane @eli-schwartz Here is a scenario where you can quickly observe all kinds of dysfunctions of NVCC and (in some places) our CUDA support.

In this file, I compile four static_library()'s (with different flags, but this is immaterial here), and fuse them into one shared library. Obviously, the component static libraries shouldn't be installed, but the shared library perhaps.

At the top of meson.build there are two knobs: variant = 'a' or 'b' flag and a fat = true or false flag.

• variant='a': Uses shared_library() for fusion. Fails with nvcc fatal : No input files specified; use option --help for more information because all input libraries are made opaque by their wrapping -Xlinker= flag. I don't know why we're injecting those. Whether thin-link or fat-link doesn't matter here.
• variant='b', fat=false: Uses custom_target() for fusion, with a quasi-identical line (but see footnote) except for stripping the -Xlinker=flags. Unfortunately, that fails with both nvlink fatal : Memory allocation failure and nvlink fatal : elfLink internal error.
• variant='b', fat=true: Idem, but sets install: true on the static libraries, thereby forcing them to be built "fat". The build works.

I have tested this with CUDA Toolkit 10.2 (old) and 11.4 (newest aside from before-before-yesterday's 11.5 release), both with similar results. The odds are 11.5 will be affected as well. Even if NVIDIA fixed this tomorrow, there's a huge legacy of slightly-broken compilers to support.

I therefore use a custom_target() to do that step of the build, and this broke with Meson 0.60 due to the switchover to thin libraries internally. And there is nothing that I can do to make it build "fat" archives currently, short of installing the temporary libraries. Fixing Meson's strange -Xlinker= prefixing habit (going from variant 'a' -> 'b') is will not be a complete solution because it would simply expose the thin-archive problem more broadly.

For this reason, I made this PR, which proposes returning control to the user through the thin: [true|false] flag if they're dealing with broken toolchains like NVCC, or other post-processing tools. Default would be thin: true in the spirit of speeding up builds.

Footnote: When using custom_target(), I normally require the use of -Xcompiler=-Wl\,foo\,bar and because the , is special to both NVCC's -Xcompiler and the linker's -Wl itself, I need to escape the backslashes, thus: -Xcompiler=-Wl\\,foo\\,bar. Unfortunately, custom_target() somehow turns \ into / (???), and this breaks the build. I didn't absolutely require an -Xcompiler= flag to demonstrate the thin-archive problem, but it's a separate issue in Meson itself.

This seems more like a task for a built-in option, than a keyword argument.