erlangR19: 19.1.6 -> 19.2 #21775

Merged
merged 1 commit into from Jan 11, 2017

Projects

None yet

6 participants

@yurrriq
Contributor
yurrriq commented Jan 10, 2017
Motivation for this change

Update erlangR19 from 19.1.6 to 19.2.

N.B. I've added a sw_vers substitution for darwin.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot

@yurrriq, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DerTim1, @mdaiter and @dezgeg to be potential reviewers.

@DerTim1

On NixOS

@@ -45,6 +45,8 @@ stdenv.mkDerivation rec {
preConfigure = ''
./otp_build autoconf
+ '' + optionalString stdenv.isDarwin ''
+ substituteInPlace configure --replace "sw_vers" "/usr/bin/sw_vers"
@mdaiter
mdaiter Jan 10, 2017 Contributor

So for me, running the "sw_vers" command without the prepended "/usr/bin/" works perfectly alright. I want to see if NixOS contains this already

@mdaiter
mdaiter Jan 10, 2017 Contributor

@grahamc I see it done here https://github.com/NixOS/nixpkgs/blob/5a31ad1bbf6acceba531f0995de4378370d87e31/pkgs/tools/admin/certbot/default.nix#L35 . Must be a Darwin-specific protocol for getting the system version.

@LnL7
LnL7 Jan 10, 2017 edited Contributor

yes it's darwin specific, what's this used for? If you can replace the call to the binary with a string would be a better approach.

@mdaiter
mdaiter Jan 10, 2017 Contributor

@LnL7 it's used to obtain specific Mac OS version info

@LnL7
LnL7 Jan 10, 2017 Contributor

I'm saying that the build should probably not use the system version because our stdenv is pure. The result of a build on el capitan should be exactly the same as a build on sierra.

@mdaiter
mdaiter Jan 10, 2017 edited Contributor

@LnL7 this is true; how does the other system then work with this tool?
I'm also curious to see how the build system in Erlang 19.2 has changed from 19.1.6 to use this tool. Is this tool even necessary for the compilation step?
Can we try manually linking all .frameworks necessary into the build expression, and then remove the call to this tool by substituteInPlace?

@mdaiter
mdaiter Jan 10, 2017 edited Contributor

@LnL7 it looks like system info is contained in CoreServices, as listed here: https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man1/sw_vers.1.html .

@LnL7
LnL7 Jan 10, 2017 edited Contributor

Here's an example of a package that patches this with a hardcoded value: https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/science/misc/root/sw_vers.patch

@yurrriq
yurrriq Jan 10, 2017 edited Contributor
@mdaiter, here's the what. I'm still not sure about the why.
git diff tags/OTP-19.1.6 tags/OTP-19.2 -- configure.in
diff --git a/configure.in b/configure.in
index 8dd4e588d..a149acbb5 100644
--- a/configure.in
+++ b/configure.in
@@ -105,7 +105,6 @@ else
 fi
 AC_SUBST(CROSS_COMPILING)
 
-
 AC_ARG_ENABLE(bootstrap-only,
 AS_HELP_STRING([--enable-bootstrap-only],
                [enable bootstrap only configuration]),
@@ -371,6 +370,49 @@ if test X${enable_native_libs} = Xyes -a X${enable_hipe} != Xno; then
 fi
 AC_SUBST(NATIVE_LIBS_ENABLED)
 
+if test $CROSS_COMPILING = no; then
+   case $host_os in
+   	darwin*)
+	   macosx_version=`sw_vers -productVersion`
+	   test $? -eq 0 || {
+	   	AC_MSG_ERROR([Failed to execute 'sw_vers'; please provide it in PATH])
+	   }
+	   [case "$macosx_version" in
+	       [1-9][0-9].[0-9])
+	          int_macosx_version=`echo $macosx_version | sed 's|\([^\.]*\)\.\([^\.]*\)|\1\2|'`;;
+	       [1-9][0-9].[0-9].[0-9])
+	          int_macosx_version=`echo $macosx_version | sed 's|\([^\.]*\)\.\([^\.]*\)\.\([^\.]*\)|\1\2\3|'`;;
+	       [1-9][0-9].[1-9][0-9])
+	          int_macosx_version=`echo $macosx_version | sed 's|\([^\.]*\)\.\([^\.]*\)|\1\200|'`;;
+	       [1-9][0-9].[1-9][0-9].[0-9])
+	          int_macosx_version=`echo $macosx_version | sed 's|\([^\.]*\)\.\([^\.]*\)\.\([^\.]*\)|\1\20\3|'`;;
+	       [1-9][0-9].[1-9][0-9].[1-9][0-9])
+	          int_macosx_version=`echo $macosx_version | sed 's|\([^\.]*\)\.\([^\.]*\)\.\([^\.]*\)|\1\2\3|'`;;
+	       *)
+		  int_macosx_version=unexpected;;
+	   esac]
+	   test $int_macosx_version != unexpected || {
+	   	AC_MSG_ERROR([Unexpected MacOSX version ($macosx_version) returned by 'sw_vers -productVersion'; this configure script probably needs to be updated])
+	   }
+	   AC_TRY_COMPILE([
+#if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ > $int_macosx_version
+#error Compiling for a newer MacOSX version...
+#endif
+		], [;],
+		[],
+		[AC_MSG_ERROR([
+
+  You are natively building Erlang/OTP for a later version of MacOSX
+  than current version ($macosx_version). You either need to
+  cross-build Erlang/OTP, or set the environment variable
+  MACOSX_DEPLOYMENT_TARGET to $macosx_version (or a lower version).
+
+])])
+	   ;;
+	*)
+	   ;;
+   esac
+fi
 
 rm -f $ERL_TOP/lib/SKIP-APPLICATIONS
 for app in `cd lib && ls -d *`; do
@yurrriq
yurrriq Jan 10, 2017 edited Contributor

Here's the PR that added it and the bug report that inspired it.

@LnL7
LnL7 Jan 10, 2017 edited Contributor

We currently still use the 10.9 MacOS SDK so I would suggest using:

  prePatch = ''
    substituteInPlace configure.in \
      --replace '`sw_vers -productVersion`' '10.9'
  '';
@yurrriq
yurrriq Jan 10, 2017 Contributor

@LnL7, I'm giving that a try locally now. If it works, I'll update this PR.

@yurrriq
yurrriq Jan 10, 2017 Contributor
configure: error:

  You are natively building Erlang/OTP for a later version of MacOSX
  than current version (10.9). You either need to
  cross-build Erlang/OTP, or set the environment variable
  MACOSX_DEPLOYMENT_TARGET to 10.9 (or a lower version).

Note: On my system, sw_vers -productVersion = 10.12.2

@LnL7
LnL7 Jan 11, 2017 edited Contributor

I just checked our stdenv sets MACOSX_DEPLOYMENT_TARGET to 10.10, that's probably what we want to use.

@yurrriq
yurrriq Jan 11, 2017 Contributor

I'm hacking nix-darwin atm, but I'll give that a go after.

@yurrriq
yurrriq Jan 11, 2017 Contributor

Your suggestion worked for me and I've included it in 5bb4d2b, @LnL7. I'd be happy to squash these two commits if y'all prefer.

@LnL7
LnL7 Jan 11, 2017 Contributor

Yes, please.

@yurrriq
yurrriq Jan 11, 2017 Contributor

Rebased and squashed!

@grahamc
Member
grahamc commented Jan 10, 2017

Hey, @DerTim1! I appreciate the reviews you've been doing, thank you and keep up the good work!

This change LGTM but just want to get a darwin person (like LnL ) to look at the change. Can you explain what that substitute does?

@DerTim1
Contributor
DerTim1 commented Jan 10, 2017

@grahamc No problem. I thought a build von a linux-system is always useful. :)

@LnL7
Contributor
LnL7 commented Jan 10, 2017

@DerTim1 Definitely! Donating some cpu cycles is always appreciated.

@yurrriq
Contributor
yurrriq commented Jan 11, 2017

Rebased.

@yurrriq yurrriq erlangR19: 19.1.6 -> 19.2
Include sw_vers patch as discussed on #21775.
ad3e589
@LnL7
LnL7 approved these changes Jan 11, 2017 View changes

Looks good now, it's building but it might be some time before I can check the results.

@LnL7 LnL7 merged commit 8ff396a into NixOS:master Jan 11, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@yurrriq yurrriq deleted the yurrriq:update/pkgs/development/interpreters/erlang/R19 branch Jan 11, 2017
@volth volth added a commit to volth/nixpkgs that referenced this pull request Jan 11, 2017
@yurrriq @volth yurrriq + volth erlangR19: 19.1.6 -> 19.2
Include sw_vers patch as discussed on #21775.
6bbedd6
@LnL7 LnL7 referenced this pull request Jan 15, 2017
Merged

DarwinTools: init at 1 #21898

3 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment