Skip to content
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.

Sign up for GitHub

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

stdenv: make darwin builds reproducable #77632

Merged
merged 5 commits into from
Jan 31, 2020
Merged

stdenv: make darwin builds reproducable #77632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a826b49
stdenv: make darwin builds reproducable
LnL7 Jan 13, 2020
6567823
stdenv: introduce appleSdkVersion and macosVersionMin
LnL7 Jan 13, 2020
9b57984
treewide: use stdenv.macosVersionMin
LnL7 Jan 13, 2020
b984c22
cmake: remove CMAKE_OSX_DEPLOYMENT_TARGET overrides
LnL7 Jan 14, 2020
68513e4
cmake: move CMAKE_OSX_ARCHITECTURES out of the stdenv
LnL7 Jan 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions pkgs/development/tools/build-managers/cmake/setup-hook.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,12 @@ cmakeConfigurePhase() {
# because we usually do not package the framework
cmakeFlags="-DCMAKE_FIND_FRAMEWORK=last $cmakeFlags"

# on macOS i686 was only relevant for 10.5 or earlier.
cmakeFlags="-DCMAKE_OSX_ARCHITECTURES=x86_64 $cmakeFlags"

# we never want to use the global macOS SDK
cmakeFlags="-DCMAKE_OSX_SYSROOT= $cmakeFlags"

# disable OSX deployment target
# we don't want our binaries to have a "minimum" OSX version
cmakeFlags="-DCMAKE_OSX_DEPLOYMENT_TARGET= $cmakeFlags"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this isn't needed? How does CMake choose this value when it's unset?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not set explicitly the value is initialized by the MACOSX_DEPLOYMENT_TARGET environment variable, if set, and otherwise computed based on the host platform.

So if I understand this correctly this was equivalent to unsetting MACOSX_DEPLOYMENT_TARGET from the stdenv.


# correctly detect our clang compiler
cmakeFlags="-DCMAKE_POLICY_DEFAULT_CMP0025=NEW $cmakeFlags"

Expand Down
5 changes: 1 addition & 4 deletions pkgs/development/tools/misc/ccls/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ stdenv.mkDerivation rec {
nativeBuildInputs = [ cmake ];
buildInputs = with llvmPackages; [ clang-unwrapped llvm rapidjson ];

cmakeFlags = [
"-DCCLS_VERSION=${version}"
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.12"
];
cmakeFlags = [ "-DCCLS_VERSION=${version}" ];

preConfigure = ''
cmakeFlagsArray+=(-DCMAKE_CXX_FLAGS="-fvisibility=hidden -fno-rtti")
Expand Down
1 change: 0 additions & 1 deletion pkgs/development/tools/misc/cquery/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ stdenv.mkDerivation {
cmakeFlags = [
"-DSYSTEM_CLANG=ON"
"-DCLANG_CXX=ON"
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.12"
];

shell = runtimeShell;
Expand Down
7 changes: 6 additions & 1 deletion pkgs/os-specific/darwin/apple-sdk/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
{ stdenv, fetchurl, xar, cpio, pkgs, python3, pbzx, lib }:

let version = "10.12"; in

# Ensure appleSdkVersion is up to date.
assert stdenv.isDarwin -> stdenv.appleSdkVersion == version;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we just use stdenv.appleSdkVersion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attribute is macOS specific, without the condition evaluating darwin only packages on linux fails on the assertion instead of meta.platforms with a nice message.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was trying to ask is: if version must be equal to stdenv.appleSdkVersion, then why is one of them not initialized from the other, as in version = stdenv.appleSdkVersion or "invalid platform";?


let
# sadly needs to be exported because security_tool needs it
sdk = stdenv.mkDerivation rec {
version = "10.12";
pname = "MacOS_SDK";
inherit version;

# This URL comes from https://swscan.apple.com/content/catalogs/others/index-10.12.merged-1.sucatalog, which we found by:
# 1. Google: site:swscan.apple.com and look for a name that seems appropriate for your version
Expand Down
1 change: 0 additions & 1 deletion pkgs/servers/sql/mysql/8.0.x.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ self = stdenv.mkDerivation rec {
outputs = [ "out" "static" ];

cmakeFlags = [
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.12" # For std::shared_timed_mutex.
"-DCMAKE_SKIP_BUILD_RPATH=OFF" # To run libmysql/libmysql_api_test during build.
"-DFORCE_UNSUPPORTED_COMPILER=1" # To configure on Darwin.
"-DWITH_ROUTER=OFF" # It may be packaged separately.
Expand Down
26 changes: 16 additions & 10 deletions pkgs/stdenv/darwin/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{ lib
, localSystem, crossSystem, config, overlays, crossOverlays ? []

# The version of darwin.apple_sdk used for sources provided by apple.
, appleSdkVersion ? "10.12"
# Minimum required macOS version, used both for compatibility as well as reproducability.
, macosVersionMin ? "10.12"
# Allow passing in bootstrap files directly so we can test the stdenv bootstrap process when changing the bootstrap tools
, bootstrapFiles ? let
fetch = { file, sha256, executable ? true }: import <nix/fetchurl.nix> {
Expand Down Expand Up @@ -28,15 +31,19 @@ let
];
in rec {
commonPreHook = ''
export NIX_ENFORCE_PURITY="''${NIX_ENFORCE_PURITY-1}"
export NIX_ENFORCE_NO_NATIVE="''${NIX_ENFORCE_NO_NATIVE-1}"
export NIX_ENFORCE_NO_NATIVE=''${NIX_ENFORCE_NO_NATIVE-1}
export NIX_ENFORCE_PURITY=''${NIX_ENFORCE_PURITY-1}
export NIX_IGNORE_LD_THROUGH_GCC=1
stripAllFlags=" " # the Darwin "strip" command doesn't know "-s"
export MACOSX_DEPLOYMENT_TARGET=10.12
export SDKROOT=
export CMAKE_OSX_ARCHITECTURES=x86_64

# Ensure consistent LC_VERSION_MIN_MACOSX and remove LC_UUID.
export MACOSX_DEPLOYMENT_TARGET=${macosVersionMin}
export NIX_LDFLAGS+=" -macosx_version_min ${macosVersionMin} -sdk_version ${appleSdkVersion} -no_uuid"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how the linker works when you pass multiple values here. For instance, is -macosx_version_min 10.12 -macosx_version_min 10.14 a valid argument list?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MACOSX_DEPLOYMENT_TARGET and -macos_version_min are closely related but not the same. AFAIK the variable is used by build systems and headers for version specific pragmas while the linker flag just configures the load command.

Alltho since it looks like the compiler always adds the flag based on the variable, overriding this original one, it's not really necessary. Added it just in case but should be fine to leave out if you're concerned about it.


# Workaround for https://openradar.appspot.com/22671534 on 10.11.
export gl_cv_func_getcwd_abort_bug=no

stripAllFlags=" " # the Darwin "strip" command doesn't know "-s"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this a space is kind of hacky, but I think it is because of the - in ${stripAllFlags:--s} in setup.sh?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but doesn't that do the opposite of what we want here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs say that :- means:

If parameter is unset or null, the expansion of word is substituted. Otherwise, the value of parameter is substituted.

So yeah it sounds like the space is needed

'';

bootstrapTools = derivation {
Expand Down Expand Up @@ -130,8 +137,7 @@ in rec {
__extraImpureHostDeps = commonImpureHostDeps;

extraAttrs = {
inherit platform;
parent = last;
inherit macosVersionMin appleSdkVersion platform;
};
overrides = self: super: (overrides self super) // { fetchurl = thisStdenv.fetchurlBoot; };
};
Expand Down Expand Up @@ -400,9 +406,9 @@ in rec {
extraBuildInputs = [ pkgs.darwin.CF ];

extraAttrs = {
inherit platform bootstrapTools;
libc = pkgs.darwin.Libsystem;
libc = pkgs.darwin.Libsystem;
shellPackage = pkgs.bash;
inherit macosVersionMin appleSdkVersion platform bootstrapTools;
};

allowedRequisites = (with pkgs; [
Expand Down