Meson build fails on windows

[o-mdr]

Describe the bug
Meson build fails on Windows platform

To Reproduce
Steps to reproduce the behaviour:

1. On Windows platform with MSVC tools run a developer command line (this comes with MS Visual Studio. NB: NOT vscode, but full VS)
2. Download source code of zstd (main or release 1.4.5)
3. In developer console execute first time setup

cd build\meson
meson setup builddir
---
C:\Users\om\Downloads\zstd-dev\build\meson>meson setup builddir
The Meson build system
Version: 0.55.3
Source dir: C:\Users\om\Downloads\zstd-dev\build\meson
Build dir: C:\Users\om\Downloads\zstd-dev\build\meson\builddir
Build type: native build
Project name: zstd
Project version: DUMMY
C compiler for the host machine: cl (msvc 19.27.29111)
C linker for the host machine: link link 14.27.29111.0
C++ compiler for the host machine: cl (msvc 19.27.29111)
C++ linker for the host machine: link link 14.27.29111.0
Host machine cpu family: x86
Host machine cpu: x86
Program GetZstdLibraryVersion.py found: YES
Message: Project version is now: 1.4.6
Library m found: NO
Run-time dependency threads found: YES
Did not find pkg-config by name 'pkg-config'
Found Pkg-config: NO
Found CMake: C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.EXE (3.17.20032601)
Run-time dependency zlib found: NO (tried pkgconfig and cmake)
Run-time dependency liblzma found: NO (tried pkgconfig and cmake)
Run-time dependency liblz4 found: NO (tried pkgconfig and cmake)
Message: Enable legacy support back to version 0.5
Message: Enable multi-threading support
Windows resource compiler: Microsoft (R) Windows (R) Resource Compiler Version 10.0.10011.16384
Program ../InstallSymlink.py found: YES (C:\Program Files\Meson\meson.exe runpython C:\Users\om\Downloads\zstd-dev\build\meson\programs\../InstallSymlink.py)
Build targets in project: 5

Found ninja-1.10.0 at "C:\Program Files\Meson\ninja.EXE"

4. Compile code

meson compile -C builddir

---
C:\Users\om\Downloads\zstd-dev\build\meson>meson compile -C builddir
Found runner: ['C:\\Program Files\\Meson\\ninja.EXE']
ninja: Entering directory `builddir'
[1/52] Generating Windows resource for file 'lib_.._.._.._build_VS2010_libzstd-dll_libzstd-dll.rc' with a custom command
FAILED: lib/lib_.._.._.._build_VS2010_libzstd-dll_libzstd-dll.rc_libzstd-dll.res
"C:/Program Files (x86)/Windows Kits/10/bin/10.0.18362.0/x86/rc.EXE" "/nologo" "/folib/lib_.._.._.._build_VS2010_libzstd-dll_libzstd-dll.rc_libzstd-dll.res" "../lib/../../../build/VS2010/libzstd-dll/libzstd-dll.rc"
../lib/../../../build/VS2010/libzstd-dll/libzstd-dll.rc(4) : fatal error RC1015: cannot open include file 'zstd.h'.
[10/52] Compiling C object lib/zstd-1.dll.p/.._.._.._lib_common_fse_decompress.c.obj
ninja: build stopped: subcommand failed.

Expected behavior
Compilation should just work

Screenshots and charts

Desktop (please complete the following information):

• OS: Windows x64
• Version build 2004
• Compiler msvc 19.27.29111
• Flags: none, default build
• Other relevant hardware specs: this works on Linux and Ubuntu (because that line is not hit)
• Build system: Meson

Additional context
This seems to be because of the following:
Step 1. Meson build file calls a resource file and compiles it
Step 2. Resource file includes zstd.h on line 4, which fails.

Originally found when working on a patch

[Cyan4973]

cc @lzutao

[ghost]

Possible related issue: #2304

[tesuji]

It's my mistake that our meson config has never worked on Windows.
I'm trying to debug the issue right now. However I don't have Windows machine,
I have to rely on CI so iteration speed might be slow.

[o-mdr]

@lzutao not a problem, it's easy to mess up code for multiplatform use. I have a Windows machine and happy test/give you a hand with this, just tag me here. I think the script includes /lib folder where zstd.h file is located, so I am a bit unsure why the compiler doesn't see it. Maybe it is included at the wrong stage.

While you are on it, could you please add some simple comments to that area of the build script? I think it's related to changing a version file and compiling a dll for Windows, but there is already some code to update a version above, will be nice to know why we do this. Thank you.

[tesuji]

I don't think the code base tested on Visual C/C++ compiler,
I cannot get the code to statically compile on msvc: some are syntax errors.
I haven't tried cmake/VS* solution project.

[tesuji]

I think until the Visual C/C++ compiler properly supported in the codebase,
I will gate meson with gcc-liked compiler.

Edit: Filed #2363

[o-mdr]

@lzutao I've added just one change on my branch, I think it should be enough - the include_directories statement, so

libzstd_sources += [
        windows_mod.compile_resources(
          join_paths(zstd_rootdir, 'build/VS2010/libzstd-dll/libzstd-dll.rc'),
          include_directories: include_directories(join_paths(zstd_rootdir, 'lib'))  <<<<<<< HERE
      )]

I would have expected windows_mod to pass the include directories to the resource compiler, but it doesn't seem to be happening (error output seems to be out of order, but ok to figure out):

C:\git\zstd\build\meson>meson compile -C builddir -v

...
[31/52] "C:/Program Files (x86)/Windows Kits/10/bin/10.0.18362.0/x86/rc.EXE" "/nologo" "/foprograms/programs_.._.._.._build_VS2010_zstd_zstd.rc_zstd.res" "../programs/../../../build/VS2010/zstd/zstd.rc"
FAILED: programs/programs_.._.._.._build_VS2010_zstd_zstd.rc_zstd.res
"C:/Program Files (x86)/Windows Kits/10/bin/10.0.18362.0/x86/rc.EXE" "/nologo" "/foprograms/programs_.._.._.._build_VS2010_zstd_zstd.rc_zstd.res" "../programs/../../../build/VS2010/zstd/zstd.rc"
../programs/../../../build/VS2010/zstd/zstd.rc(4) : fatal error RC1015: cannot open include file 'zstd.h'.
...

I wonder if it is a problem in the windows_mod windows module, that it doesn't actually pass the include_directory to resource compiler. I will check out that module to see how the include dirs are implemented. I've not found any github source code that actually use that module with include dirs.

[tesuji]

There are one more place needed to change like that, see 7cbc04a

Also, zstd codebase doesn't pass compilation on MSVC 2019. I'd recommend using gcc
or clang in the meantime.

[yoniko]

While the suggested diffs in this issue do fix some of the issues, Meson compilation with MSVC toolchain is still fragile and prone to failures for other reasons (I've had multiple failures with the VS 2022 toolchain when I tried).

Furthermore, while we appreciate and glad to see contributions to the Meson build system, it's not a build systems that's officially supported by the project, especially on Windows.

If anyone feels like tackling this issue, please do, I suggest starting with the patches mentioned in the comments, and continuing with setting up a CI.
For now, due to low community engagement on the topic and low priority of the issue, I'm closing it.

[eli-schwartz]

Hmm, this should actually work though? I wasn't aware of this ticket, but I did submit commit 5b2c6c7 as part of #3039 and I have (external) CI that successfully builds zstd on Windows using Meson.

[yoniko]

@eli-schwartz - Makes sense that you missed it, as it's an older issues.
On my system, running VS 2022 toolchain + Meson didn't build. I've encountered three issues:

1. The one specified in this issue which was resolved with the suggested fixes.
2. The default command line tool uses x86 but the linking target is x64. This is okay when working specifically with the x64 toolchain.
3. Issue with flags when building pzstd: cl : Command line error D8021 : invalid numeric argument '/Wno-shadow', at which point I stopped my attempt.

What toolchain are you using? I'll be surprised if it works with MSVC.

I'm glad to hear that you have a CI that tests it, would you consider porting it to be a part of the official repo CI so we can get a quick signal if something breaks?

[eli-schwartz]

My CI build logs this:

zstd| C compiler for the host machine: cl (msvc 19.33.31631 "Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31631 for x64")
zstd| C linker for the host machine: link link 14.33.31631.0
zstd| C++ compiler for the host machine: cl (msvc 19.33.31631 "Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31631 for x64")
zstd| C++ linker for the host machine: link link 14.33.31631.0

It builds fine, tests fail due to #3120

However, I did miss enabling the contrib directory, so I'm not sure about that one... let me investigate...

[eli-schwartz]

I investigated, and I wrote this fix for pzstd: 490a439

I get a bit further, but it still fails...

[83/90] Compiling C++ object subprojects/zstd-pr3121/build/meson/contrib/pzstd/pzstd.exe.p/.._.._.._.._contrib_pzstd_Pzstd.cpp.obj
FAILED: subprojects/zstd-pr3121/build/meson/contrib/pzstd/pzstd.exe.p/.._.._.._.._contrib_pzstd_Pzstd.cpp.obj 
"cl" "-Isubprojects\zstd-pr3121\build/meson/contrib\pzstd\pzstd.exe.p" "-Isubprojects\zstd-pr3121\build/meson/contrib\pzstd" "-I..\subprojects\zstd-pr3121\build\meson\contrib\pzstd" "-I..\subprojects\zstd-pr3121\programs" "-I..\subprojects\zstd-pr3121\contrib\pzstd" "-I..\subprojects\zstd-pr3121\lib" "-I..\subprojects\zstd-pr3121\lib\common" "-I..\subprojects\zstd-pr3121\lib\compress" "-I..\subprojects\zstd-pr3121\lib\decompress" "-I..\subprojects\zstd-pr3121\lib\dictBuilder" "-I..\subprojects\zstd-pr3121\lib\legacy" "-DNDEBUG" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/Zc:__cplusplus" "/W4" "/EHsc" "/std:c++14" "/permissive-" "/Od" "/Zi" "/D_UNICODE" "/DUNICODE" "/MP" "/Fdsubprojects\zstd-pr3121\build/meson/contrib\pzstd\pzstd.exe.p\.._.._.._.._contrib_pzstd_Pzstd.cpp.pdb" /Fosubprojects/zstd-pr3121/build/meson/contrib/pzstd/pzstd.exe.p/.._.._.._.._contrib_pzstd_Pzstd.cpp.obj "/c" ../subprojects/zstd-pr3121/contrib/pzstd/Pzstd.cpp
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): error C2589: '(': illegal token on right side of '::'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(126): note: see reference to class template instantiation 'pzstd::Range<Iter>' being compiled
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): error C3878: syntax error: unexpected token '(' following 'expression'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): note: error recovery skipped: '( ( identifier'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): error C2760: syntax error: ')' was unexpected here; expected ';'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): error C3878: syntax error: unexpected token ')' following 'jump_statement'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): note: error recovery skipped: ') <'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): error C3878: syntax error: unexpected token ')' following 'expression_statement'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): note: error recovery skipped: ') ?'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): error C2760: syntax error: ':' was unexpected here; expected ';'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): error C3878: syntax error: unexpected token ':' following 'expression_statement'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): note: error recovery skipped: ':'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/Range.h(124): note: error recovery skipped: ') )'
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/FileSystem.h(85): warning C4245: 'return': conversion from 'int' to 'uintmax_t', signed/unsigned mismatch
D:\a\wrapdb\wrapdb\subprojects\zstd-pr3121\contrib\pzstd\utils/FileSystem.h(89): warning C4245: 'return': conversion from 'int' to 'uintmax_t', signed/unsigned mismatch
../subprojects/zstd-pr3121/contrib/pzstd/Pzstd.cpp(366): error C2589: '(': illegal token on right side of '::'
../subprojects/zstd-pr3121/contrib/pzstd/Pzstd.cpp(366): error C2062: type 'unknown-type' unexpected
../subprojects/zstd-pr3121/contrib/pzstd/Pzstd.cpp(366): error C2059: syntax error: ')'
../subprojects/zstd-pr3121/contrib/pzstd/Pzstd.cpp(367): error C3536: 'bytesRead': cannot be used before it is initialized
../subprojects/zstd-pr3121/contrib/pzstd/Pzstd.cpp(590): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[84/90] Compiling C++ object subprojects/zstd-pr3121/build/meson/contrib/pzstd/pzstd.exe.p/.._.._.._.._contrib_pzstd_main.cpp.obj
[85/90] Compiling C++ object subprojects/zstd-pr3121/build/meson/contrib/pzstd/pzstd.exe.p/.._.._.._.._contrib_pzstd_Options.cpp.obj
../subprojects/zstd-pr3121/contrib/pzstd/Options.cpp(325): warning C4858: discarding return value: The 'remove' and 'remove_if' algorithms return the iterator past the last element that should be kept. You need to call container.erase(result, container.end()) afterwards. In C++20, 'std::erase' and 'std::erase_if' are simpler replacements for these two steps.
../subprojects/zstd-pr3121/contrib/pzstd/Options.cpp(414): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data

From a quick look, no Windows jobs attempt to compile pzstd anyway... it simply does not compile there AFAICT.

[eli-schwartz]

I'm glad to hear that you have a CI that tests it, would you consider porting it to be a part of the official repo CI so we can get a quick signal if something breaks?

Which CI shall I add it to? Probably not travis... .github/workflows/dev-short-tests.yml?

[Cyan4973]

Yeah, we do no longer use travis.
Prefer Github Actions, for any test it can handle (we have other CIs for corner cases that GA cannot handle).

dev-short-tests.yml is likely a good place to host this test,
assuming the test is designed to last < 7mn (approximately).

You can have multiple tests (one per host system).

[eli-schwartz]

The test time for Ubuntu is 30 minutes because of tests/playTests.sh, which is not run on Windows -- so the MSVC build finishes for me in 6 minutes.

Sounds like a plan.

[Cyan4973]

tests/playTests.sh contains 2 sections.

The first section manages only "fast" tests, and is the main one.
It's deemed fast enough to be run with make check.
And by default, it should be the only section to run.

The second section contains "long" tests, such as compressing / decompressing large amounts of data, or complex dictionary scenarios. It's only triggered if the directive --test-large-data is passed.

I would expect only the first section to make sense for meson on CI.
Limiting playTests.sh to the "short" tests should bring the total test time way below 30mn,
unless there are other larger contributors beyond playTests.sh.

[eli-schwartz]

I wonder if it makes sense instead to have a make check-short and a make check-large? Meson would have the large test skipped by default.

The runtime of each test is listed in Meson's test collection:

 1/10 test-fullbench-1         OK               26.44s
 2/10 test-fullbench-2         OK               20.12s
 3/10 test-fuzzer              OK              255.95s
 4/10 test-zstream-1           OK              135.35s
 5/10 test-zstream-3           OK              101.60s
 6/10 test-longmatch           OK                8.02s
 7/10 test-invalidDictionaries OK                0.01s
 8/10 test-decodecorpus        OK               38.96s
 9/10 test-poolTests           OK                0.60s
10/10 test-zstd                OK             1905.81s

[yoniko]

Currently we the main Makefile has make test which runs the long tests and make shortest / make check (they are aliases) that run the shorter tests.
Does meson need similar targets in tests/Makefile?

[Cyan4973]

I would recommend to only port the equivalent of make check.

make test is much deeper, it's designed to find errors during development.

[eli-schwartz]

I investigated, and I wrote this fix for pzstd: 490a439

#3357

[eli-schwartz]

Currently we the main Makefile has make test which runs the long tests and make shortest / make check (they are aliases) that run the shorter tests.
Does meson need similar targets in tests/Makefile?

Meson has a single test entrypoint: meson test. It takes options such as the suite to run, or the individual tests to run. Defining two mutually exclusive tests is a bit complicated, but can be done by tagging one as "fast" and the other as "slow", then making test setups that run only one of the two. It's simpler to just have them as two different tests, but I'll make do.

[eli-schwartz]

I have a branch that adds Meson CI. (Including for Linux, because it was only being run on Travis, and if you're no longer running Travis at all, then that means it's no longer being tested!)

It does, predictably, fail on Windows as I mentioned above. Waiting on a resolution to #3120 before submitting my CI additions.