nasm: Fixes for using under MSVC standalone

[amyspark]

:wave:

This PR fixes the issues I found when trying to use NASM to build a library under MSVC. The associated issues need to be fixed in tandem in order for the test to pass.

First, I override get_crt_link_args in the MSVS mixin to ensure all linkage always get a full set of default libraries as per the Microsoft docs; then, I added the MSVC linkers to the corresponding trial list for NASM.

TODO: check if the default library set is needed for Clang/CL as well.

Fixes #11082 Fixes #11083

[codecov[bot]]

Codecov Report

Merging #11127 (fbe2769) into master (b2473b6) will increase coverage by 2.97%. The diff coverage is n/a.

:exclamation: Current head fbe2769 differs from pull request most recent head cc176e8. Consider uploading reports for the commit cc176e8 to get more accurate results

@@            Coverage Diff             @@
##           master   #11127      +/-   ##
==========================================
+ Coverage   65.81%   68.78%   +2.97%     
==========================================
  Files         414      207     -207     
  Lines       90654    45349   -45305     
  Branches    20126     9384   -10742     
==========================================
- Hits        59665    31194   -28471     
+ Misses      26476    11759   -14717     
+ Partials     4513     2396    -2117     

Impacted Files	Coverage Δ
compilers/mixins/visualstudio.py	32.08% <0.00%> (-1.14%)	:arrow_down:
build.py	78.54% <0.00%> (-0.33%)	:arrow_down:
mesonbuild/build.py
mesonbuild/compilers/asm.py
mesonbuild/compilers/compilers.py
mesonbuild/compilers/detect.py
mesonbuild/compilers/mixins/visualstudio.py
mesonbuild/interpreter/primitives/boolean.py
mesonbuild/optinterpreter.py
mesonbuild/scripts/gtkdochelper.py
... and 203 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[tristan957]

Looking at one of the test failures.

https://github.com/mesonbuild/meson/actions/runs/3588697942/jobs/6040366906#step:4:2095

That seems completely incorrect. CC @xclaesse for nasm stuff

[amyspark]

@xclaesse How could I test the link flags of an executable? I thought (blindly) that Meson accepted per-language link flags, and I didn't find any other examples of checking link flags in the existing suite.

[xclaesse]

I can actually reproduce your issue pretty easily, and even in our CI: https://github.com/mesonbuild/meson/pull/11146. Could you cherry-pick that commit into this PR, so we can ensure we fix it properly please?

I've been trying to learn a bit on this topic, but to be honest I'm still a bit confused. Do I understand correctly that as soon as we link at least 1 .c file together with the nasm code, that pulls CRT libs correctly and all is fine?

I think get_crt_link_args() should be implemented on the NasmCompiler class instead of MSVCCompiler because C compiler does not need this. That way those libs will be used only if nasm is the linker which happens if there are no other languages in the same target, I think.

[amyspark]

hat way those libs will be used only if nasm is the linker which happens if there are no other languages in the same target

This is required if all the objects are generated by NASM and the linker is MSVC's LINK.exe. I'm not sure how NASM could be the linker here? I'm not sure what ld.bfd (MinGW) output would be here, as I only tried NASM with MSVC.

[amyspark]

I've just tested and with these changes under CLANG64 and UCRT64 flavors of MSYS:

The Meson build system
Version: 0.64.1
Source dir: C:\Users\Amalia\Desktop\meson-nasm
Build dir: C:\Users\Amalia\Desktop\meson-nasm\build
Build type: native build
Project name: nasm only
Project version: undefined
Host machine cpu family: x86_64
Host machine cpu: x86_64
Nasm compiler for the host machine: nasm (nasm 2.15.05)
Nasm linker for the host machine: cc ld.lld 15.0.5
Build targets in project: 1

Found ninja-1.11.1 at D:\msys64\clang64\bin\ninja.EXE

The Meson build system
Version: 0.64.1
Source dir: C:\Users\Amalia\Desktop\meson-nasm
Build dir: C:\Users\Amalia\Desktop\meson-nasm\build
Build type: native build
Project name: nasm only
Project version: undefined
Host machine cpu family: x86_64
Host machine cpu: x86_64
Nasm compiler for the host machine: nasm (nasm 2.15.05)
Nasm linker for the host machine: cc ld.bfd 2.39
Build targets in project: 1

Found ninja-1.11.1 at D:\msys64\ucrt64\bin\ninja.EXE

Both compile and link their CRT successfully, so get_crt_link_args is exclusively needed for MSVC.

[xclaesse]

@amyspark I think this is enough: https://github.com/xclaesse/meson/commit/2346e8e59e816682046aad0b4358a16e016a70f0. As you said, it is similar to D compiler.

[amyspark]

@xclaesse let me see first if I've nailed the test suite. Then I'll nitpick the implementation of get_crt_link_args.

[amyspark]

@xclaesse This error looks like the MSVS integration wasn't tested. I'll try and see if I can fix it along with the rest.

[xclaesse]

@xclaesse This error looks like the MSVS integration wasn't tested. I'll try and see if I can fix it along with the rest.

ah right, nasm language does not work with vs backend, that can be skipped indeed.

[amyspark]

@xclaesse I can't find any failed tests in the cygwin CI logs, even when grepping. Do you know what are those failures?

[xclaesse]

@xclaesse I can't find any failed tests in the cygwin CI logs, even when grepping. Do you know what are those failures?

What do you mean, cygwin CI seems to have passed. msys2 fails but that seems unrelated, I saw it failing in other PRs too.

[xclaesse]

@amyspark I made a much smaller PR that should be enough to fix your issue: https://github.com/mesonbuild/meson/pull/11405. We already had a unit test that build nasm-only target, but it was skipped because nasm was not installed on CI runners. With the my PR that unit test now pass.

[amyspark]

Fixed by #11405 and 744e6ebe1d5f214fd54727abde0726160218a1f0, I'll reopen if we need the remaining changes!