Add more CMake flags #56391
Add more CMake flags #56391
Conversation
GrahamcOfBorg
added
6.topic: python
6.topic: qt/kde
6.topic: stdenv
This means we can avoid building test suites that will never be run.
matthewbauer
force-pushed
the
cmake-treewide
branch
4 times, most recently
from
6417a1f to
476b3f3
Compare
GrahamcOfBorg
added
10.rebuild-darwin-stdenv
matthewbauer
force-pushed
the
cmake-treewide
branch
2 times, most recently
from
4a43c9b to
ff0d179
Compare
| preCheck = if enableShared | ||
| then "export LD_LIBRARY_PATH=\"$PWD\"" | ||
| else ""; | ||
| preCheck = "export LD_LIBRARY_PATH=\"$PWD\""; |
There was a problem hiding this comment.
This can be replaced by -DCMAKE_SKIP_BUILD_RPATH=OFF in cmakeFlags.
Overall, -DBUILD_SHARED_LIBS=ON has a high potential to break packages that use add_library without STATIC or SHARED for internal components and add_library(SHARED) (or STATIC with something like BUILD_STATIC_LIBS) for final artifacts. (This is a good practice for development.) One way they are likely to break is during testing due to our current default of -DCMAKE_SKIP_BUILD_RPATH=OFF. If -DBUILD_SHARED_LIBS=ON is to be the default, -DCMAKE_SKIP_BUILD_RPATH=OFF should probably be removed from the default.
There was a problem hiding this comment.
Yeah that makes sense to me. I'll leave of the BUILD_*_LIBS changes for now and just do the BUILD_TESTING change.
| @@ -17,8 +17,6 @@ stdenv.mkDerivation rec { | |||
| preConfigure = "rm BUILD"; | |||
|
|
|||
| cmakeFlags = [ | |||
| "-DBUILD_SHARED_LIBS=ON" | |||
| "-DBUILD_STATIC_LIBS=ON" | |||
There was a problem hiding this comment.
Building the static library in all cases is intentional (needed by some packages that do not expect shared gflags).
| @@ -60,6 +60,10 @@ cmakeConfigurePhase() { | |||
| # correctly detect our clang compiler | |||
| cmakeFlags="-DCMAKE_POLICY_DEFAULT_CMP0025=NEW $cmakeFlags" | |||
|
|
|||
| if [ -z "$dontDisableStatic" ]; then | |||
| cmakeFlags="-DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=OFF $cmakeFlags" | |||
There was a problem hiding this comment.
CMake does not have BUILD_STATIC_LIBS option: its meaning is project-dependent and by default it's unset. I don't think we should disable it here.
My impression is that smaller (single library) projects indeed depend on BUILD_SHARED_LIBS to select between static and shared build, as the new variable dontDisableStatic implies, but the effect of BUILD_SHARED_LIBS on larger projects is completely different.
This is now set by CMake
matthewbauer
force-pushed
the
cmake-treewide
branch
from
ff0d179 to
6ed4926
Compare
GrahamcOfBorg
removed
6.topic: stdenv
Motivation for this change
Adds BUILD_TESTING, BUILD_SHARED_LIBS, and BUILD_STATIC_LIBS flags to Nixpkgs. BUILD_TESTING=OFF is set if doCheck = false. BUILD_SHARED_LIBS=ON, and BUILD_STATIC_LIBS=OFF to tell CMake to build the rights libs. These are reversed in static overlay.