New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkgsStatic: set BUILD_SHARED_LIBS=OFF for cmake #76659
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this make any existing pkg overrides obsolete? If so, you should remove those from the explicit set in static.nix
.
@tobim Yes, it does, but only two or three cases. I will make sure to address those. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me!
See @orivej comments on this:
from #56391 (comment) It may still be okay to just enable it in the static overlay though. The above PR was doing this for everything. |
Yes, I was considering doing that. I've also seen other other custom options like |
ff397be
to
3128ed0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more packages need to be changed since #75798 was merged.
Static libraries are to be provided by the pkgsStatic.fmt package.
pkgsStatic.gtest already has CMAKE_BUILD_SHARED set to OFF.
pkgsStatic.glog already has CMAKE_BUILD_SHARED set to OFF.
pkgsStatic.double-conversion already has CMAKE_BUILD_SHARED set to OFF.
3128ed0
to
bb890c4
Compare
Motivation for this change
While reviewing #75798 I've noticed a repetitive pattern of setting BUILD_SHARED_LIBS=OFF for many of the cmake builds. It makes sense to just set it for all builds.
Background
CMake has a BUILD_SHARED_LIBS option which governs the behaviour of the add_library() function in case when the library type is not specified. Its default value is "OFF" meaning default to STATIC. It is not uncommon to explicitly redefine the option:
This adds the BUILD_SHARED_LIBS option to the list of options in the CMake gui. There is a different variation of the redefinition where the default value is changed to "ON".
Some packages allow to provide both shared and static libraries. Additional custom options are defined to facilitate this. These options may or may not break the default convention for the BUILD_SHARED_LIBS.
In Nixpkgs
Here are a few data points for nixpkgs:
This PR should help with 39 cases where we enabled building of shared libraries. It will also help in cases when the default value of the BUILD_STATIC_LIBS option was changed to "ON"
In upstream
fmt
: chose to not override BUILD_STATIC_LIBS (Please make BUILD_SHARED_LIBS a cmake option fmtlib/fmt#600).gtest
: BUILD_STATIC_LIBS overridden with default to "OFF" https://github.com/google/googletest/blob/306f3754a71d6d1ac644681d3544d06744914228/googletest/CMakeLists.txt#L69-L71hyperscan
: BUILD_SHARED_LIBS overridden with default to "OFF", additional option BUILD_SHARED_AND_STATIC_LIBS available https://github.com/intel/hyperscan/blob/d79973efb1fcf5ed338122882c1f896829767fb6/CMakeLists.txt#L109-L110pugixml
: BUILD_SHARED_LIBS overridden with default to "OFF", additional option BUILD_SHARED_AND_STATIC_LIBS availablehttps://github.com/zeux/pugixml/blob/41b6ff21c455865bb8ef67c5952b7f895b62bacc/CMakeLists.txt#L26-L28
Things done
fmt
gtest