Package started failing with unrecognized -Wthread-safety

[mdorier]

Steps to reproduce

I am posting this as a bug rather than a build error because although the problem appears when building a package, it was caused by a recent commit to spack, not the package itself (I narrowed it down to commit 330832c, authored by @trws )

$ spack install leveldb@1.22

This command will fail, with g++ complaining that it doesn't know -Wthread-safety (I reproduced it on 3 platforms with various versions of gcc).

The leveldb package wasn't changed. I looked at its cmake file and the weird thing is that it has a mechanism that checks whether this flag is supported (here: https://github.com/google/leveldb/blob/1.22/CMakeLists.txt#L53) so I don't know if spack is interfering with it in some way.

Error message

No response

Information on your system

• Spack: 0.18.0.dev0 (330832c)
• Python: 3.9.5
• Platform: linux-ubuntu21.04-sandybridge
• Concretizer: clingo

General information

• I have run spack debug report and reported the version of Spack/Python/Platform
• I have searched the issues of this repo and believe this is not a duplicate
• I have run the failing commands in debug mode and reported the output

[mdorier]

From looking at the logs, the only warning flags that I see passed down to g++ are -Werror and -Wthread-safety. I'm under the impression that the commit I mentioned above is making the compiler wrapper eliminate warning flags. In leveldb, this make cmake think that -Wthread-safety is supported, since the compiler wrapper eliminates it before g++ gets the chance to actual evaluate it. Then at this line https://github.com/google/leveldb/blob/1.22/CMakeLists.txt#L242 cmake will add -Wthread-safety as a compile option. Why is the spack compiler wrapper not eliminating it this time, I don't know.

[trws]

It doesn't remove warning flags, only the "treat warnings as errors" flags, specifically -Werror if the specific value is used and -Werror=* as well if it's all.

I managed to reproduce the problem, but the cause is a bit strange, the -Wthread-safety flag isn't passed in the test, only -Werror actually gets added, and it happens to work when -Werror is there because the attributes in the test file only exist in versions that have the flag. Here's the flag file generated by cmake for the test:

# compile CXX with /home/scogland1/Repos/spack/lib/spack/env/gcc/g++
CXX_DEFINES = -DHAVE_CLANG_THREAD_SAFETY
CXX_INCLUDES = 
CXX_FLAGS =  -Werror -std=c++11

Since we're now stripping -Werror, the bug in the check is becoming a problem. This is because CMAKE_REQUIRED_FLAGS is not a cmake list it's a plain string. When the list gets interpreted the second argument is lost. Changing the list append operation for the check to the following fixes the issue:

set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror -Wthread-safety")

We could incorporate a patch for that, but it really needs to be fixed upstream as well.

[mdorier]

I see, ok. It looks like they use a different way of checking the flag in version 1.23 and greater:
https://github.com/google/leveldb/blob/main/CMakeLists.txt#L79
(unfortunately there is an issue with 1.23+ that prevents us from using it).

So yes, it needs a patch.

[tgamblin]

I think in this case patching is the right thing to do -- either with a patch() method or a patch file -- @mdorier can you add that?

[trws]

Yeah, 1.23+ is doing it correctly, so a patch seems like the best option.

[mdorier]

Patch added.

[alalazo]

Was fixed by #30810