Skip to content

erlang: refactor to generalize Erlang/OTP derivations #26381

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

Merged
merged 4 commits into from
Jun 11, 2017

Conversation

gleber
Copy link
Contributor

@gleber gleber commented Jun 4, 2017

Motivation for this change

This PR does not change anything feature-wise, and instead aims at unify how different Erlang versions are built.

This is a subset of #17240 (and is a heavily-inspired re-implementation of it). It contains:

  • move all Beam packages into beam-packages.nix file which lives by the side of all-packages.nix;
  • deduplicates contents of pkgs/development/interpreters/erlang/R{16-19}.nix into a generic builder;
  • supports OTP build features like odbc- and javasupport

It does not contain:

  • a fixpoint to build Erlang packages using different OTP versions;
  • Elixir generalization;
  • LFE generalization.
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.

/cc @LnL7 @ankhers @ericbmerritt @yurrriq

Sorry, something went wrong.

erlang erlang_odbc erlang_javac erlang_odbc_javac
elixir
lfe
erlangR16 erlangR16_odbc
Copy link
Member

Choose a reason for hiding this comment

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

Do we really have to expose all these versions toplevel? See #24047 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not, but let's leave these two aspects separate. I want this PR to be purely a refactoring.

@yurrriq
Copy link
Member

yurrriq commented Jun 4, 2017

I'm mobile atm. Does this include the generic LFE builder from my fork?

@gleber
Copy link
Contributor Author

gleber commented Jun 4, 2017

@yurrriq It does not, on purpose. This PR is supposed to be purely a refactoring.

@gleber
Copy link
Contributor Author

gleber commented Jun 5, 2017

It builds on Ubuntu, but still fails on NixOs, debugging

@ankhers
Copy link
Contributor

ankhers commented Jun 5, 2017

I'm also getting a failure on OSX 10.11.6

You can see the entire output of nox-review here

@gleber gleber force-pushed the generalize-beam branch 5 times, most recently from c9d5451 to 39cf2ae Compare June 7, 2017 17:17
@gleber
Copy link
Contributor Author

gleber commented Jun 7, 2017

It compiles now both on Ubuntu and NixOs (or more specifically all Erlang/OTP derivations compile and nox preview has been running for 3 hours now and still did not fail).

Please review.

@gleber gleber changed the title WIP: erlang: refactor to generalize Erlang/OTP derivations erlang: refactor to generalize Erlang/OTP derivations Jun 7, 2017
rec {
lib = import ../development/beam-modules/lib.nix { inherit pkgs; };

inherit (pkgs) darwin wxGTK30;
Copy link
Member

Choose a reason for hiding this comment

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

these can be moved to the arguments, now they end up in the beam attrset

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I replaced 'foo' with 'pkgs.foo' everywhere below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved these to arguments of this file (i.e. lambda defined at top level of this file)


buildInputs = [ perl gnum4 ncurses openssl autoconf libxslt libxml2 makeWrapper ]
++ optionals wxSupport wxPackages2
++ optional odbcSupport odbcPackages
Copy link
Member

Choose a reason for hiding this comment

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

use optionals since it's already a list, nested buildInputs work but it should be avoided

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Please note that "mkDerivation" defined here is the one called from R16.nix and similar files.
*/
callErlang = drv: gargs: args: pkgs.callPackage drv (
Copy link
Member

Choose a reason for hiding this comment

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

Using makeOverridable for the arguments instead here would be more flexible and also makes the top-level definitions a bit nicer.

LnL7@de6ccfe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have applied this patch

@ankhers
Copy link
Contributor

ankhers commented Jun 8, 2017

nox-review passed on OSX 10.11.6

gleber added 2 commits June 8, 2017 11:28
This applies to Erlang, Elixir and LFE packages.

beam-packages provides interpreters and packages sets separately. This
is in preparation of generalizing BEAM interpreters definitions.
Switch official Erlang versions to use a common builder.
@gleber gleber force-pushed the generalize-beam branch from 39cf2ae to ff80b17 Compare June 8, 2017 09:31
@gleber
Copy link
Contributor Author

gleber commented Jun 8, 2017

@LnL7 All fixed. Please take a look again (PTAL)

@gleber
Copy link
Contributor Author

gleber commented Jun 8, 2017

@LnL7 I have added a commit which moves basho's fork over too. PTAL

@gleber gleber force-pushed the generalize-beam branch from ac3d3f0 to 60682c0 Compare June 8, 2017 20:13
@ankhers
Copy link
Contributor

ankhers commented Jun 9, 2017

Failing again on OSX for the basho stuff. https://gist.github.com/ankhers/e36e7e6f4908446ec6de6c0ee30ff592

@gleber gleber force-pushed the generalize-beam branch from 60682c0 to a800173 Compare June 9, 2017 15:16
@LnL7
Copy link
Member

LnL7 commented Jun 9, 2017

I'm seeing an issue with /usr/bin/env again for R16

/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/make/emd2exml /tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/HOWTO/DTRACE.md DTRACE.xml
/nix/store/wb34dgkpmnssjkq7yj4qbjqxpnapq0lw-bash-4.4-p12/bin/bash: /tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/make/emd2exml: /usr/bin/env: bad interpreter: No such file or directory
make[5]: *** [Makefile:86: DTRACE.xml] Error 126
make[5]: Leaving directory '/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/lib/runtime_tools/doc/src'
make[4]: *** [/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/make/otp_release_targets.mk:146: release_docs] Error 2
make[4]: Leaving directory '/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/lib/runtime_tools/doc/src'
make[3]: *** [/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/make/otp_subdir.mk:28: release_docs] Error 2
make[3]: Leaving directory '/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/lib/runtime_tools'
make[2]: *** [/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/make/otp_subdir.mk:28: release_docs] Error 2
make[2]: Leaving directory '/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/lib'
make[1]: *** [Makefile:408: release_docs] Error 2
make[1]: Leaving directory '/tmp/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src'
make: *** [Makefile:929: install-docs] Error 2
<3>builder for ‘/nix/store/p13fgcipawc4sxdw54yxb71zxqvaip45-erlang-16B02.drv’ failed with exit code 2

@gleber gleber force-pushed the generalize-beam branch from a800173 to d4577e5 Compare June 10, 2017 14:16
@gleber
Copy link
Contributor Author

gleber commented Jun 10, 2017

@LnL Fixed emd2exml failure for basho version.

@ankhers I also submitted a version which use clang instead of gcc on Darwin for the diameterc.

@LnL7
Copy link
Member

LnL7 commented Jun 10, 2017

Another shebang issue (on darwin), not sure what's going on here.

../bin/diameterc -o gen -i ../ebin dict/base_rfc3588.dia
/nix/store/vkh1qq7m28v6wajva1k50a4i8x3l0r9c-bash-4.4-p12/bin/bash: ../bin/diameterc: /private/var/folders/xb/qd021gp54z3d0_rsjn6nhhn40000gn/T/nix-build-erlang-16B02.drv-0/otp-OTP_R16B02_basho8-src/bin/x86_64-ap: bad interpreter: No such file or directory
make[3]: *** [Makefile:123: gen/diameter_gen_base_rfc3588.erl] Error 126

@gleber gleber force-pushed the generalize-beam branch from d4577e5 to c9cb640 Compare June 11, 2017 11:31
@gleber
Copy link
Contributor Author

gleber commented Jun 11, 2017

@LnL7 That's this line:
https://github.com/NixOS/nixpkgs/pull/26381/files#diff-2b1b48d1f8b621ebb9fbae7dfc03bb6eR20

Previously Basho Erlang fork would require another Erlang to be already installed so that the diameterc script runs correctly during build phase.

There are three options:

  • disable diameterc use somehow (not sure if it is necessary in practice);
  • use anotehr erlang when building basho erlang fork (as it used to be);
  • find a way to predict where erlang build phase would put escript script.

I think I have found a solution using the last option. Can you retest?

@gleber
Copy link
Contributor Author

gleber commented Jun 11, 2017

I think I have found a less hacky way which should be more resilient. Can you retest it on OS X?

@ankhers
Copy link
Contributor

ankhers commented Jun 11, 2017

Nox review just finished and everything built on OSX.

Copy link
Member

@LnL7 LnL7 left a comment

Choose a reason for hiding this comment

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

Everything looks good now.

@LnL7 LnL7 merged commit 4010313 into NixOS:master Jun 11, 2017
@LnL7
Copy link
Member

LnL7 commented Jun 11, 2017

Thanks for finishing this!

@gleber
Copy link
Contributor Author

gleber commented Jun 11, 2017

Yay! There's still plenty to do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants