Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
15 changes: 13 additions & 2 deletions pkgs/build-support/cc-wrapper/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
, isGNU ? false, isClang ? cc.isClang or false, gnugrep ? null
, buildPackages ? {}, hostPlatform, targetPlatform
, runCommand ? null
, useMacosReexportHack ? false
}:

with stdenv.lib;
Expand Down Expand Up @@ -295,7 +296,15 @@ stdenv.mkDerivation {
ln -s $ldPath/${prefix}as $out/bin/${prefix}as
fi

wrap ${prefix}ld ${preWrap ./ld-wrapper.sh} ''${ld:-$ldPath/${prefix}ld}
'' + (if !useMacosReexportHack then ''
wrap ${prefix}ld ${./ld-wrapper.sh} ''${ld:-$ldPath/${prefix}ld}
'' else ''
export binPrefix=${prefix}
ldInner="${prefix}ld-reexport-delegate"
wrap "$ldInner" ${./macos-sierra-reexport-hack.bash} ''${ld:-$ldPath/${prefix}ld}
wrap "${prefix}ld" ${./ld-wrapper.sh} "$out/bin/$ldInner"
unset ldInner
'') + ''

if [ -e ${binutils_bin}/bin/${prefix}ld.gold ]; then
wrap ${prefix}ld.gold ${preWrap ./ld-wrapper.sh} ${binutils_bin}/bin/${prefix}ld.gold
Expand Down Expand Up @@ -399,5 +408,7 @@ stdenv.mkDerivation {
{ description =
stdenv.lib.attrByPath ["meta" "description"] "System C compiler" cc_
+ " (wrapper script)";
};
} // optionalAttrs useMacosReexportHack {
platforms = stdenv.lib.platforms.darwin;
};
}
103 changes: 103 additions & 0 deletions pkgs/build-support/cc-wrapper/macos-sierra-reexport-hack.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#! @shell@

set -eu -o pipefail

path_backup="$PATH"
if [ -n "@coreutils_bin@" ]; then
PATH="@coreutils_bin@/bin"
fi

declare -r recurThreshold=300

declare overflowCount=0
for ((n=0; n < $#; ++n)); do
case "${!n}" in
-l*) let overflowCount+=1 ;;
-reexport-l*) let overflowCount+=1 ;;
*) ;;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we handle -- here or elsewhere?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh I wasn't aware of it; looking up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You mean just plain paths to dylibs passed in? I didn't see any -- in the man page.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I just mean the standard -- to separate command line flags from files that start with --, no idea if ld supports it but most tools do.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@shlevy Well I don't yet handle libraries passed in by path yet at all, in fact.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@shlevy Well I don't yet handle libraries passed in by path yet at all, in fact. I'll look into -- and that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Eh, let's just do this as followup if needed.

esac
done

declare -a allArgs=()

if (( "$overflowCount" <= "$recurThreshold" )); then
allArgs=("$@")
else
declare -a childrenLookup=() childrenLink=()

while (( $# )); do
case "$1" in
-L/*)
childrenLookup+=("$1")
allArgs+=("$1")
;;
-L)
echo "cctools LD does not support '-L foo' or '-l foo'" >&2
exit 1
;;
-l)
echo "cctools LD does not support '-L foo' or '-l foo'" >&2
exit 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How can a general purpose linker just fail to process these args? This is bound to break linking something.

@Ericson2314 Ericson2314 Aug 1, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, I suppose the man page could be wrong. I thought i tested too but let me double check.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note that this isn't a failure to parse -Lpath or -llib, just -L path and -l lib. The latter is supported by the GNU toolchain nowadays but can't be expected to work everywhere (and, FWIW, from my understanding this is a limitation of the real linker program, not this script)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't know that macOS ld does not support these args without spaces, then there is no problem. This is funny, since it has quite a few arguments that start with -l (-lazy-l* -lazy_library -lazy_framework -lto_library).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@orivej Yes I should special case the rest of those--I only did -lto_library.

;;
-lto_library) allArgs+=("$1") ;;
# We aren't linking any "to_library"
-lSystem) allArgs+=("$1") ;;
# Special case as indirection seems like a bad idea for something
# so fundamental. Can be removed for simplicity.
-l?*) childrenLink+=("$1") ;;
-reexport-l?*) childrenLink+=("$1") ;;
*) allArgs+=("$1") ;;
esac

shift
done

declare n=0
while (( $n < "${#childrenLink[@]}" )); do
if [[ "${childrenLink[n]}" = -l* ]]; then
childrenLink[n]="-reexport${childrenLink[n]}"
fi
let ++n
done
unset n

declare -r outputNameLibless=$(basename $( \
if [[ -z "${outputName:+isUndefined}" ]]; then
echo unnamed
elif [[ "${outputName:0:3}" = lib ]]; then
echo "${outputName:3}"
else
echo "${outputName}"
fi))
declare -ra children=("$outputNameLibless-reexport-delegate-0" \
"$outputNameLibless-reexport-delegate-1")

mkdir -p "$out/lib"

PATH="$PATH:@out@/bin"

symbolBloatObject=$outputNameLibless-symbol-hack.o
if [[ ! -e $symbolBloatObject ]]; then
printf '.private_extern _______child_hack_foo\nchild_hack_foo:\n' \
| @binPrefix@as -- -o $symbolBloatObject
fi

# first half of libs
@binPrefix@ld -macosx_version_min 10.10 -arch x86_64 -dylib \

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copumpkin @LnL7 / other Darwin people. I just cargo-culted -macosx_version_min 10.10 from elsewhere, and it's probably necessary. What should it be, if it is passed anything at all?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can use $MACOSX_DEPLOYMENT_TARGET, that's defined in the stdenv.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks!

-o "$out/lib/lib${children[0]}.dylib" \
-install_name "$out/lib/lib${children[0]}.dylib" \
"${childrenLookup[@]}" "$symbolBloatObject" \
"${childrenLink[@]:0:$((${#childrenLink[@]} / 2 ))}"

# second half of libs
@binPrefix@ld -macosx_version_min 10.10 -arch x86_64 -dylib \
-o "$out/lib/lib${children[1]}.dylib" \
-install_name "$out/lib/lib${children[1]}.dylib" \
"${childrenLookup[@]}" "$symbolBloatObject" \
"${childrenLink[@]:$((${#childrenLink[@]} / 2 ))}"

allArgs+=("-L$out/lib" "-l${children[0]}" "-l${children[1]}")
fi

PATH="$path_backup"
exec @prog@ "${allArgs[@]}"
2 changes: 1 addition & 1 deletion pkgs/stdenv/darwin/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ in rec {
initialPath = import ../common-path.nix { inherit pkgs; };
shell = "${pkgs.bash}/bin/bash";

cc = import ../../build-support/cc-wrapper {
cc = lib.callPackageWith {} ../../build-support/cc-wrapper {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does this do?

@Ericson2314 Ericson2314 Aug 1, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It adds the override field so I can replace cc while leaving everything else the same. Probably should add a note for that.

inherit (pkgs) stdenv;
inherit shell;
nativeTools = false;
Expand Down
89 changes: 89 additions & 0 deletions pkgs/test/macos-sierra-shared/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
{ lib, clangStdenv, clang-sierraHack-stdenv, stdenvNoCC }:

let
makeBigExe = stdenv: prefix: rec {

count = 500;

sillyLibs = lib.genList (i: stdenv.mkDerivation rec {
name = "${prefix}-fluff-${toString i}";
unpackPhase = ''
src=$PWD
cat << 'EOF' > ${name}.c
unsigned int asdf_${toString i}(void) {
return ${toString i};
}
EOF
'';
buildPhase = ''
$CC -std=c99 -shared ${name}.c -o lib${name}.dylib -Wl,-install_name,$out/lib/lib${name}.dylib
'';
installPhase = ''
mkdir -p "$out/lib"
mv lib${name}.dylib "$out/lib"
'';
meta.platforms = lib.platforms.darwin;
}) count;

finalExe = stdenv.mkDerivation rec {
name = "${prefix}-final-asdf";
unpackPhase = ''
src=$PWD
cat << 'EOF' > main.cxx

#include <cstdlib>
#include <iostream>

${toString (lib.genList (i: "extern \"C\" unsigned int asdf_${toString i}(void); ") count)}

unsigned int (*funs[])(void) = {
${toString (lib.genList (i: "asdf_${toString i},") count)}
};

int main(int argc, char **argv) {
bool ret;
unsigned int i = 0;
for (auto f : funs) {
if (f() != i++) {
std::cerr << "Failed to get expected response from function #" << i << std::endl;
return EXIT_FAILURE;
}
}
return EXIT_SUCCESS;
}
EOF
'';
buildPhase = ''
$CXX -std=c++11 main.cxx ${toString (map (x: "-l${x.name}") sillyLibs)} -o ${prefix}-asdf
'';
buildInputs = sillyLibs;
installPhase = ''
mkdir -p "$out/bin"
mv ${prefix}-asdf "$out/bin"
'';
meta.platforms = lib.platforms.darwin;
};

};

good = makeBigExe clang-sierraHack-stdenv "good";

bad = makeBigExe clangStdenv "bad";

in stdenvNoCC.mkDerivation {
name = "macos-sierra-shared-test";
buildInputs = [ good.finalExe bad.finalExe ];
# TODO(@Ericson2314): Be impure or require exact MacOS version of builder?
buildCommand = ''
if bad-asdf
then echo "bad-asdf can succeed on non-sierra, OK" >&2
else echo "bad-asdf should fail on sierra, OK" >&2
fi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't love that we won't be able to catch regressions due to the exe just no longer breaking the limit for some reason

@Ericson2314 Ericson2314 Aug 1, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Neither do I. But I couldn't think of anything better without much more work.


# Must succeed on all supported MacOS versions
good-asdf

touch $out
'';
meta.platforms = lib.platforms.darwin;
}
11 changes: 11 additions & 0 deletions pkgs/top-level/all-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -5162,6 +5162,11 @@ with pkgs;

clang = llvmPackages.clang;

clang-sierraHack = clang.override {
name = "clang-wrapper-with-reexport-hack";
useMacosReexportHack = true;
};

clang_4 = llvmPackages_4.clang;
clang_39 = llvmPackages_39.clang;
clang_38 = llvmPackages_38.clang;
Expand Down Expand Up @@ -5189,6 +5194,7 @@ with pkgs;

#Use this instead of stdenv to build with clang
clangStdenv = if stdenv.isDarwin then stdenv else lowPrio llvmPackages.stdenv;
clang-sierraHack-stdenv = overrideCC stdenv clang-sierraHack;
libcxxStdenv = lowPrio llvmPackages.libcxxStdenv;

clean = callPackage ../development/compilers/clean { };
Expand Down Expand Up @@ -19217,4 +19223,9 @@ with pkgs;
undaemonize = callPackage ../tools/system/undaemonize {};

houdini = callPackage ../applications/misc/houdini {};

# No `recurseIntoAttrs` because there's no need to nix-env these.
tests = {
macOSSierraShared = callPackage ../test/macos-sierra-shared {};
};
}
1 change: 1 addition & 0 deletions pkgs/top-level/release.nix
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ let
jobs.python.x86_64-darwin
jobs.rustc.x86_64-darwin
jobs.go.x86_64-darwin
jobs.tests.macOSSierraShared
];
};

Expand Down