Generalize building of Elixir interpreter #26716

ankhers · 2017-06-19T17:28:37Z

Motivation for this change

This is a continuation of #26381 based on #17240

This only includes Elixir generalization. It also includes derivations for Elixir 1.3 and 1.4

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.

ankhers · 2017-06-19T17:29:24Z

/cc @gleber, @LnL7, @ericbmerritt, @yurrriq

gleber · 2017-06-19T17:37:21Z

Nice! This was my next step, thanks for doing it! :)

ankhers · 2017-06-19T17:44:42Z

Glad I could help. I can also take a crack at LFE if you would prefer?

ankhers · 2017-06-19T18:13:57Z

#26668 should be merged before this. Once it is, I will need to make a couple modifications.

gleber · 2017-06-19T18:45:32Z

Current state of PR built successfully built on NixOS with sandboxes:

$ nix-shell -p nox --run "nox-review pr 26668"
...
Result in /run/user/1000/nox-review-982bk1l1
total 0
lrwxrwxrwx 1 gleber users 57 Jun 19 20:36 result -> /nix/store/lnxk39nmhg03pi5lniz3v8kih6vz0sxa-hex2nix-0.0.5
lrwxrwxrwx 1 gleber users 56 Jun 19 20:36 result-2 -> /nix/store/4npkcs2ds1dga0903mjbvj4ya3h3rq77-rebar3-3.3.2
lrwxrwxrwx 1 gleber users 59 Jun 19 20:36 result-3 -> /nix/store/mfdf6z2hjxpyk1rrnj0knpzxxdnrmlva-relx-exe-3.18.0
lrwxrwxrwx 1 gleber users 53 Jun 19 20:36 result-4 -> /nix/store/wx7bvpfimz0n68dkgjkai0kajxsx1h0a-lfe-1.2.1
lrwxrwxrwx 1 gleber users 56 Jun 19 20:36 result-5 -> /nix/store/n3j5kw7glc12g9gdvj83h39airmfinp1-rebar3-3.3.2

gleber · 2017-06-19T19:00:26Z

Oops, that comment was actually for wrong PR. Results for this PR are also positive:

$ nix-shell -p nox --run "nox-review pr 26716"
...
Result in /run/user/1000/nox-review-bmd3mwgi
total 0
lrwxrwxrwx 1 gleber users 57 Jun 19 20:36 result -> /nix/store/lvx8d5cw7nm3780ahw0gvi4h74hj12qf-hex2nix-0.0.5
lrwxrwxrwx 1 gleber users 56 Jun 19 20:36 result-2 -> /nix/store/jizvhrng4p4wqk3wdagr74d8fqavs8vl-elixir-1.4.4

yurrriq · 2017-06-19T19:40:29Z

@ankhers my previous work on LFE might be helpful if you do. There should be a link on the old generalized Erlang PR. Please don't update to 1.3.0 in the process because there are some known issues and we're waiting on 1.3.1.

ankhers · 2017-06-19T19:55:20Z

@yurrriq Noted. Do you have any idea how long until 1.3.1 will be released?

yurrriq · 2017-06-19T20:00:35Z

@rvirding has identified the problem and is working on a fix, but I'm not sure about the timeline.

ankhers · 2017-06-23T00:52:27Z

I just did a rebase from master. Unfortunately I do not seem to have Elixir being built for different version of Erlang. nix-shell -A beam.packages.erlangR18.elixir . and nix-shell -A beam.packages.erlangR19.elixir . both seem to be using R18. Though, it does properly throw an error when you try beam.packages.erlangR17.elixir.

ankhers · 2017-06-23T01:19:40Z

I fixed the issue with the incorrect Erlang version being loaded for Elixir. I am not sure if this is the best solution or not.

I am running nix-build -A beam.packages.erlangR19.elixir-1_3 . and it is building Elixir 1.3 for me (I tested to make sure I was getting the proper version). When I run nix-shell -A beam.packakges.erlangR19.elixir-1_3 . I seem to get Elixir 1.4.

ankhers · 2017-06-23T15:35:48Z

And today I am receiving the following error when trying to build Elixir with R19

Uncaught error in rebar_core: {'EXIT',
                               {undef,
                                [{crypto,start,[],[]},
                                 {rebar,run_aux,2,
                                  [{file,"src/rebar.erl"},{line,165}]},
                                 {rebar,main,1,
                                  [{file,"src/rebar.erl"},{line,58}]},
                                 {escript,run,2,
                                  [{file,"escript.erl"},{line,757}]},
                                 {escript,start,1,
                                  [{file,"escript.erl"},{line,277}]},
                                 {init,start_it,1,[]},
                                 {init,start_em,1,[]}]}}

=ERROR REPORT==== 23-Jun-2017::15:23:46 ===
Unable to load crypto library. Failed with error:
"bad_lib, Library version (2.11) not compatible (with 2.10)."
OpenSSL might not be installed on this system.
make: *** [Makefile:68: erlang] Error 1

I will look into it a little bit later.

ankhers · 2017-06-23T16:40:06Z

The various Elixir versions build with the given erlang versions.

If anyone knows of a way to have the erlang and rebar packages that are in scope be passed into the generic builder instead of having to specify for each one, I would appreciate the help.

gleber · 2017-06-24T09:56:16Z

pkgs/development/beam-modules/default.nix

-        elixir = if versionAtLeast (lib.getVersion erlang) "18"
-                 then callPackage ../interpreters/elixir { debugInfo = true; }
-                 else throw "Elixir requires at least Erlang/OTP R18.";
+        elixir = defaultScope.elixir-1_4;


You can drop default scope. from all the new code if you add rec just before opening "{" of this block. This will also allow you to write inherit rebar erlang; instead of erlang = ...; and rebar = ...; inside args of lib.callElixir.

Another option is to replace defaultScope. with self..

These have slightly different semantics, but in this case they seem to be equivalent.

gleber · 2017-06-24T09:59:57Z

pkgs/development/beam-modules/lib.nix

+  callElixir = drv: vsn: args:
+    let
+      inherit (stdenv.lib) versionAtLeast;
+      builder = callPackage ../../development/interpreters/elixir/generic-builder.nix args;


You can drop ../development in this path, since you are already under development directory

gleber · 2017-06-26T17:30:05Z

pkgs/top-level/beam-packages.nix

@@ -49,6 +49,10 @@ rec {
    # access for example elixir built with different version of Erlang, use
    # `beam.packages.erlangR19.elixir`.
    elixir = packages.erlang.elixir;


You can replace these 4 lines with

inherit (packages.erlang) elixir elixir-1_5 elixir-1_4 elixir-1_3;

LnL7 · 2017-06-26T17:41:29Z

pkgs/top-level/beam-packages.nix

@@ -49,6 +49,10 @@ rec {
    # access for example elixir built with different version of Erlang, use
    # `beam.packages.erlangR19.elixir`.
    elixir = packages.erlang.elixir;
+    elixir-1_5 = packages.erlang.elixir-1_5;


the naming convention for attributes is elixir_1_5 or elixir15 and since this is an rc release it should be suffixed to make that explicit.

LnL7 · 2017-06-26T17:53:27Z

pkgs/development/beam-modules/default.nix

+                       debugInfo = true;
+                     };
+
+        elixir-1_4 = lib.callElixir ../interpreters/elixir/1.4.nix "18" {


I think it makes more sense to add the otp version as an optional argument for the elixir builder instead, then it could be overridden by the specific elixir expression if needed.

Forgive me if I am misunderstanding your comment here. But that OTP version ("18"), is the minimum version required to build Elixir. It isn't the version to build Elixir with.

Yes, I mean adding an argument and asserting it here then it's contained in the elixir builder. And if elixir16 would only work with R19 it could specify it here along with it's version, etc.

Also removed superfluous path segments from elixir generic builder

Rename attributes from elixir-1_x to elixir_1_x

Changed Elixir 1.5 to include the fact it is an rc release

gleber · 2017-06-27T06:07:29Z

pkgs/development/interpreters/elixir/generic-builder.nix

+  inherit (stdenv.lib) getVersion versionAtLeast;
+
+in
+  assert versionAtLeast (getVersion erlang) minimumOTPVersion;


stdenv.lib.getVersion does not work at least for erlangR16 (I don't remember about other versions). That's the reason I have introduced erlang-specific version of this function here:

nixpkgs/pkgs/development/beam-modules/lib.nix

Line 15 in aba574c

/* Erlang/OTP-specific version retrieval, returns 19 for OTP R19 */

Can you double check it works well for all Erlang versions present in Nixpkgs?

I built all the three Elixir versions included in this PR against OTP 18, 19 and 20. I also made sure it failed when attempting to build on R17 and R16.

This is the results of nox-review.

/nix/store/gmqllgyna6kav9zz770fkczlh928hd8f-erlang-20.0-odbc /nix/store/5421cdrj4zf5dkqfld4pcba2jfjrk8k4-erlang-19.3-javac-odbc /nix/store/hs7jj5niyx13awgi73kqdsb00a62jz8f-erlang-18.3.4.4-javac /nix/store/gfks6rlfrg37k20gmw3by6j5pv32lwhi-erlang-16B03-1 /nix/store/b39vr4vay0cx9mvlzwb9a28yp3lc9gs6-erlang-17.5-javac-odbc /nix/store/55alxr80yb0s0k4cm7a3svw7kh4rwz42-elixir-1.5.0-rc.0 /nix/store/dgi75xy4xngpjvn9ijzkabpp1ppwf5w6-erlang-17.5-odbc /nix/store/8d2m82zrvq8r247zhx99yj63vmrb8zaw-erlang-19.3-javac /nix/store/pmiwscdddsxyvc2la6fiygjh9nqgnr6q-erlang-20.0-javac-odbc /nix/store/sjz5ijsvrysas3swr1aygay85sy6ayb4-erlang-17.5-javac /nix/store/29gn1vapx5mqdka3cixh8flzgnspggwr-erlang-18.3.4.4-odbc /nix/store/5b6djrg39bbrfd8k9dfpcsa8vrkygjpd-erlang-16B03-1-odbc /nix/store/ddc3ix61gs6nml93wr1yddfawlyxpg0f-erlang-20.0-javac /nix/store/b2c6whz71kxaa9jrg7zc25l8s1a1zcr6-elixir-1.3.4 /nix/store/dy1g580h1cy06gzh2pv2z99mf0c3kw4j-erlang-16B02-odbc /nix/store/nmi33wz63dwld6xnxn7w70cpsbiljs9f-erlang-18.3.4.4-javac-odbc /nix/store/asrc5783fmkjrvs1k6r2fbh1p5m6b51x-erlang-19.3-odbc Result in /var/folders/lp/n8j6lzmj5dj34zgzb4zxw35c0000gn/T/nox-review-cooxkh3s total 68 lrwxr-xr-x 1 woodjk staff 60 Jun 27 00:36 result -> /nix/store/gmqllgyna6kav9zz770fkczlh928hd8f-erlang-20.0-odbc lrwxr-xr-x 1 woodjk staff 61 Jun 27 00:36 result-10 -> /nix/store/sjz5ijsvrysas3swr1aygay85sy6ayb4-erlang-17.5-javac lrwxr-xr-x 1 woodjk staff 64 Jun 27 00:36 result-11 -> /nix/store/29gn1vapx5mqdka3cixh8flzgnspggwr-erlang-18.3.4.4-odbc lrwxr-xr-x 1 woodjk staff 63 Jun 27 00:36 result-12 -> /nix/store/5b6djrg39bbrfd8k9dfpcsa8vrkygjpd-erlang-16B03-1-odbc lrwxr-xr-x 1 woodjk staff 61 Jun 27 00:36 result-13 -> /nix/store/ddc3ix61gs6nml93wr1yddfawlyxpg0f-erlang-20.0-javac lrwxr-xr-x 1 woodjk staff 56 Jun 27 00:36 result-14 -> /nix/store/b2c6whz71kxaa9jrg7zc25l8s1a1zcr6-elixir-1.3.4 lrwxr-xr-x 1 woodjk staff 61 Jun 27 00:36 result-15 -> /nix/store/dy1g580h1cy06gzh2pv2z99mf0c3kw4j-erlang-16B02-odbc lrwxr-xr-x 1 woodjk staff 70 Jun 27 00:36 result-16 -> /nix/store/nmi33wz63dwld6xnxn7w70cpsbiljs9f-erlang-18.3.4.4-javac-odbc lrwxr-xr-x 1 woodjk staff 60 Jun 27 00:36 result-17 -> /nix/store/asrc5783fmkjrvs1k6r2fbh1p5m6b51x-erlang-19.3-odbc lrwxr-xr-x 1 woodjk staff 66 Jun 27 00:36 result-2 -> /nix/store/5421cdrj4zf5dkqfld4pcba2jfjrk8k4-erlang-19.3-javac-odbc lrwxr-xr-x 1 woodjk staff 65 Jun 27 00:36 result-3 -> /nix/store/hs7jj5niyx13awgi73kqdsb00a62jz8f-erlang-18.3.4.4-javac lrwxr-xr-x 1 woodjk staff 58 Jun 27 00:36 result-4 -> /nix/store/gfks6rlfrg37k20gmw3by6j5pv32lwhi-erlang-16B03-1 lrwxr-xr-x 1 woodjk staff 66 Jun 27 00:36 result-5 -> /nix/store/b39vr4vay0cx9mvlzwb9a28yp3lc9gs6-erlang-17.5-javac-odbc lrwxr-xr-x 1 woodjk staff 61 Jun 27 00:36 result-6 -> /nix/store/55alxr80yb0s0k4cm7a3svw7kh4rwz42-elixir-1.5.0-rc.0 lrwxr-xr-x 1 woodjk staff 60 Jun 27 00:36 result-7 -> /nix/store/dgi75xy4xngpjvn9ijzkabpp1ppwf5w6-erlang-17.5-odbc lrwxr-xr-x 1 woodjk staff 61 Jun 27 00:36 result-8 -> /nix/store/8d2m82zrvq8r247zhx99yj63vmrb8zaw-erlang-19.3-javac lrwxr-xr-x 1 woodjk staff 66 Jun 27 00:36 result-9 -> /nix/store/pmiwscdddsxyvc2la6fiygjh9nqgnr6q-erlang-20.0-javac-odbc

I didn't use the Erlang specific getVersion for two reasons.

Elixir only builds on Erlang 18+. And as you point out, you wrote that function because of R16.

I don't think it would have caused any issues, but using that version would have introduced a circular dependency. I'm not sure if this sort of thing is frowned upon in the Nix world.

I don't mind using the Erlang specific one if my second point is not considered a problem, or if we move the function elsewhere. I just figured it would be nice to use the standard set of functions where possible so that anyone looking at the code in the future would not have to lookup the definition of another function.

If you end up not using the Erlang-specific getVersion, then please delete it in this PR.

IMO the circular dependency is not a problem, since this is a lazy language - it is actually full of circular dependencies (at least at data level).

I think there might be a better solution - to make sure that erlangR16.version is actually 16.3.1 and hide the fact that Erlang/OTP had funny versioning.

Feel free to choose any of the discussed solutions, I do not think this is important enough to stall this PR any longer.

0xABAB · 2017-06-28T23:23:16Z

pkgs/development/interpreters/elixir/generic-builder.nix

+
+      for f in $out/bin/*; do
+       b=$(basename $f)
+        if [ $b == "mix" ]; then continue; fi


Please fix your quoting everywhere.

"$b" = mix is correct and shorter, for example. Also instead of using continue, you can also just use !=

You will have to forgive me, Bash is not my strong suite and this file was just a git mv with the version specific information ripped out.

aside from the "$b" = mix, what other types of quoting needs to be fixed?

I was not aware of that. In that case, don't worry about it.

0xABAB

Overall this looks good, but please fix the quoting and the other concerns raised by the other reviewers.

ankhers · 2017-07-06T02:02:30Z

@0xABAB, or anyone else. Would it be possible to get clarification on the last review?

0xABAB

@LnL7 If you have no further concerns, this can be merged.

LnL7 added the 6.topic: erlang label Jun 19, 2017

ankhers changed the title ~~Generalize building of Elixir interpreter~~ WIP: Generalize building of Elixir interpreter Jun 19, 2017

ankhers force-pushed the generalize-elixir branch from 2480ce9 to 0e45d08 Compare June 23, 2017 00:34

ankhers force-pushed the generalize-elixir branch from 714ece0 to 4c305e8 Compare June 23, 2017 00:55

ankhers force-pushed the generalize-elixir branch from 50290c6 to c2a7621 Compare June 23, 2017 15:22

gleber suggested changes Jun 24, 2017

View reviewed changes

gleber approved these changes Jun 24, 2017

View reviewed changes

gleber reviewed Jun 26, 2017

View reviewed changes

LnL7 reviewed Jun 26, 2017

View reviewed changes

ankhers added 8 commits June 26, 2017 23:13

Generalize building of Elixir interpreter

019afb0

Add erlang that is in scope

838956b

Add in scope rebar version

3595f8e

Remove defaultScope in favour of rec for elixir versions

aa1f059

Also removed superfluous path segments from elixir generic builder

Include Elixir 1.5 rc

909da39

Inherit the elixir versions for beam modules

803797c

Rename attributes from elixir-1_x to elixir_1_x

Make assertion for OTP version in the generic elixir builder

46faaa6

Changed Elixir 1.5 to include the fact it is an rc release

Add the new elixir versions to the top level

dc2f156

Remove additional OTP version information

cc09faa

ankhers force-pushed the generalize-elixir branch from 5ddc8e8 to cc09faa Compare June 27, 2017 03:51

gleber reviewed Jun 27, 2017

View reviewed changes

Remove Erlang specific version retrieval

a19f966

ankhers changed the title ~~WIP: Generalize building of Elixir interpreter~~ Generalize building of Elixir interpreter Jun 27, 2017

0xABAB reviewed Jun 28, 2017

View reviewed changes

0xABAB suggested changes Jun 28, 2017

View reviewed changes

ankhers mentioned this pull request Jul 7, 2017

Cannot search beamPackages #26279

Closed

Fix quoting in Elixir generic builder

6db77cc

0xABAB approved these changes Jul 9, 2017

View reviewed changes

LnL7 approved these changes Jul 9, 2017

View reviewed changes

LnL7 merged commit 5ba05aa into NixOS:master Jul 9, 2017

ankhers mentioned this pull request Aug 27, 2017

BEAM packages support umbrella tracking issue (Erlang, Elixir, LFE, etc.) #11748

Open

ankhers deleted the generalize-elixir branch January 11, 2018 15:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalize building of Elixir interpreter #26716

Generalize building of Elixir interpreter #26716

ankhers commented Jun 19, 2017 •

edited

ankhers commented Jun 19, 2017

gleber commented Jun 19, 2017

ankhers commented Jun 19, 2017

ankhers commented Jun 19, 2017

gleber commented Jun 19, 2017

gleber commented Jun 19, 2017

yurrriq commented Jun 19, 2017

ankhers commented Jun 19, 2017

yurrriq commented Jun 19, 2017

ankhers commented Jun 23, 2017 •

edited

ankhers commented Jun 23, 2017

ankhers commented Jun 23, 2017

ankhers commented Jun 23, 2017

gleber Jun 24, 2017

gleber Jun 24, 2017

gleber Jun 26, 2017 •

edited

LnL7 Jun 26, 2017

LnL7 Jun 26, 2017

ankhers Jun 26, 2017

LnL7 Jun 26, 2017 •

edited

gleber Jun 27, 2017

ankhers Jun 27, 2017 •

edited

gleber Jun 27, 2017

0xABAB Jun 28, 2017 •

edited

ankhers Jun 29, 2017

0xABAB Jul 9, 2017

0xABAB left a comment

ankhers commented Jul 6, 2017

0xABAB left a comment

Generalize building of Elixir interpreter #26716

Generalize building of Elixir interpreter #26716

Conversation

ankhers commented Jun 19, 2017 • edited

Motivation for this change

Things done

ankhers commented Jun 19, 2017

gleber commented Jun 19, 2017

ankhers commented Jun 19, 2017

ankhers commented Jun 19, 2017

gleber commented Jun 19, 2017

gleber commented Jun 19, 2017

yurrriq commented Jun 19, 2017

ankhers commented Jun 19, 2017

yurrriq commented Jun 19, 2017

ankhers commented Jun 23, 2017 • edited

ankhers commented Jun 23, 2017

ankhers commented Jun 23, 2017

ankhers commented Jun 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gleber Jun 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LnL7 Jun 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankhers Jun 27, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xABAB Jun 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xABAB left a comment

Choose a reason for hiding this comment

ankhers commented Jul 6, 2017

0xABAB left a comment

Choose a reason for hiding this comment

ankhers commented Jun 19, 2017 •

edited

ankhers commented Jun 23, 2017 •

edited

gleber Jun 26, 2017 •

edited

LnL7 Jun 26, 2017 •

edited

ankhers Jun 27, 2017 •

edited

0xABAB Jun 28, 2017 •

edited