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.

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

Generalize building of LFE #27295

Merged
merged 4 commits into from
Jul 16, 2017
Merged

Generalize building of LFE #27295

merged 4 commits into from
Jul 16, 2017

Conversation

ankhers
Copy link
Contributor

@ankhers ankhers commented Jul 11, 2017

Motivation for this change

This is based on #17240

This only includes LFE generalization. It only includes a derivation for 1.2. I am holding off on 1.3 for now as there is apparently an issue with it.

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 via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • 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 @gleber @yurrriq

@mention-bot
Copy link

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

@@ -58,7 +58,7 @@ rec {
# `beam.packages.erlangR19.elixir`.
inherit (packages.erlang) elixir elixir_1_5_rc elixir_1_4 elixir_1_3;

lfe = packages.erlang.lfe;
inherit (packages.erlang) lfe lfe_1_2;
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to re-inherit it in pkgs/top-level/all-packages.nix if you want it to be available at top level (I think it should be there)

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

@yurrriq
Copy link
Member

yurrriq commented Jul 11, 2017

We use PropEr for testing, for now anyway. Is there a reason you removed it? The tests won't be able to pass without it.

@yurrriq
Copy link
Member

yurrriq commented Jul 11, 2017

Also, perhaps noteworthy is the Makefile will change dramatically in 1.3, such that the manual installPhase will likely be obviated. Perhaps we could pass that (or a more generic/inclusive @args) as an optional parameter to the generic builder.

@yurrriq
Copy link
Member

yurrriq commented Jul 11, 2017

With that said, thanks for getting this started (and avoiding 1.3 for now).

@ankhers
Copy link
Contributor Author

ankhers commented Jul 11, 2017

@yurrriq I skipped the tests because the Erlang and Elixir builds are not running their tests. I also figured, with it being an official release, someone would have run the tests already.

If you believe the tests should be run, I have no issues with putting them back in.

@yurrriq
Copy link
Member

yurrriq commented Jul 11, 2017

I'm fine with not running tests then. Thanks for the explanation.

@LnL7
Copy link
Member

LnL7 commented Jul 11, 2017

@ankhers I would encourage to enable the tests if it's not to much work. It's a nice sanity check to verify that the build doesn't have any broken libraries or missing runtime dependencies.

@ankhers
Copy link
Contributor Author

ankhers commented Jul 11, 2017

I have just re-enabled the test suite. I have just noticed though that LFE will not build on R20. It looks like a rebar3 issue though.

14:16:39 {generalize_lfe} nixpkgs$ nix-build -A beam.packages.erlangR20.lfe .
these derivations will be built:
  /nix/store/9w46lxad4w5cm07cc7m3vqrn6zd66zp2-rebar3-3.3.2.drv
  /nix/store/5i7zjx56n67l6rjpjxpyzypp0fvci9zk-proper-1.1.1-beta.drv
  /nix/store/jbli7q5y64s5dwks52cppmygbqmf8fkl-lfe-1.2.1.drv
building path(s) ‘/nix/store/7ypmwa83rb6r89cp2bnibma1c4v656bb-rebar3-3.3.2’
unpacking sources
unpacking source archive /nix/store/rlrfqafz205ky56w6m20cklx829ivvp4-3.3.2.tar.gz
source root is rebar3-3.3.2
setting SOURCE_DATE_EPOCH to timestamp 1476486763 of file rebar3-3.3.2/test/rebar_xref_SUITE.erl
patching sources
applying patch /nix/store/lhxihvmq59av57bjwz8p4qnvylc5wc80-hermetic-bootstrap.patch
patching file bootstrap
applying patch /nix/store/8gs107jn4sl18wca5cjh71ks9w9bpmga-hermetic-rebar3.patch
patching file src/rebar3.erl
patching file src/rebar_hermeticity.erl
patching file src/rebar_pkg_resource.erl
patching file src/rebar_prv_update.erl
postPatch
Bootstrapping Hex Registry for Rebar
/nix/store/r9n8rfy7bmdksh5mha83vz7lxypkviby-erlang-20.0/lib/erlang/lib
configuring
no configure script, doing nothing
building
/private/var/folders/lp/n8j6lzmj5dj34zgzb4zxw35c0000gn/T/nix-build-rebar3-3.3.2.drv-0/rebar3-3.3.2/_build/default/lib/erlware_commons/src/ec_semver_parser.erl:15: Warning: export_all flag enabled - all functions will be exported

builder for ‘/nix/store/9w46lxad4w5cm07cc7m3vqrn6zd66zp2-rebar3-3.3.2.drv’ failed with exit code 1
cannot build derivation ‘/nix/store/jbli7q5y64s5dwks52cppmygbqmf8fkl-lfe-1.2.1.drv’: 1 dependencies couldn't be built
error: build of ‘/nix/store/jbli7q5y64s5dwks52cppmygbqmf8fkl-lfe-1.2.1.drv’ failed

@ankhers
Copy link
Contributor Author

ankhers commented Jul 11, 2017

@yurrriq As per the Makefile change, I think it would be better to hold off on changing it in the current generic builder. Once the 1.3 release is fixed and we actually need to accommodate different build phases per version, we can add it then.

@yurrriq
Copy link
Member

yurrriq commented Jul 11, 2017

Sounds good to me. I can take a crack and getting the tests running and passing, if you like.

@ankhers
Copy link
Contributor Author

ankhers commented Jul 12, 2017

The tests are already running and passing. But R20 is causing an issue with build rebar3.

@yurrriq
Copy link
Member

yurrriq commented Jul 12, 2017

Ah cool. Yeah, we ran into some issues with 1.2 on OTP 20 too. Can we somehow mark it unsupported, i.e. disallow LFE 1.2 builds on OTP 20? The issues will be resolved in 1.3.

@ankhers
Copy link
Contributor Author

ankhers commented Jul 12, 2017 via email

@ankhers
Copy link
Contributor Author

ankhers commented Jul 15, 2017

Is there anything else that should be done for this PR to be merged?

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

Successfully merging this pull request may close these issues.

None yet

5 participants