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

rebar3: add rebar3WithPlugins #119452

Merged
merged 2 commits into from
May 10, 2021
Merged

Conversation

dlesl
Copy link
Contributor

@dlesl dlesl commented Apr 14, 2021

This is an attempt to provide a system for packaging projects built with rebar3. It also aims to make it easier to use nix while working on a project. Before putting any more work into this I would be grateful for any feedback on the general direction!

As a demonstration I have used it to package erlang-ls, as well as for packaging rebar3 itself. The problems it solves are

Providing a rebar3WithPlugins derivation

It was necessary to patch rebar3 to achieve this, explanations of why are in comments in the code. You can use it like this rebar3WithPlugins { plugins = [ beamPackages.pc ]; }.

It would be more useful if we packaged more plugins. Building the erlang_ls derivation complains about a few missing plugins but they don't seem to be essential for the build, still it would be nice to have them.

Fetching/declaring dependencies without using downloadPhase

This is done by means of a small rebar3 plugin, which outputs a nix derivation. An example of usage can be seen in the updateScripts for erlang-ls and rebar3.

Work yet to do

  • Factor out common parts of derivations (maybe merge with buildRebar3)
  • Improve the nix
  • Expand on how this can be useful while working on a project (eg. compiling deps in a separate derivation and providing them via ERL_LIBS)

Update 2021-05-07

For now I've reduced the scope of this PR to just adding rebar3WithPlugins (see discussion in thread)

cth_readable = fetchHex {
pkg = "cth_readable";
version = "1.5.0";
sha256 = "BD81CDFBC9A3088621AA6AC4F20B5CEBC741D1E9C6B555F3F7D0369C9D59A87C";
Copy link
Member

Choose a reason for hiding this comment

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

Please use base64 encoded hashes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are generated by rebar3nix, however if base64 hashes are a requirement we could convert them

patches = [ ./skip-plugins.patch ./global-plugins.patch ];
}));
in stdenv.mkDerivation {
name = "rebar3-with-plugins";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name = "rebar3-with-plugins";
pname = "rebar3-with-plugins";

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Comment on lines 71 to 80
PATH=${
lib.makeBinPath [
common-updater-scripts
coreutils
git
gnused
nix
(rebar3WithPlugins { })
]
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
PATH=${
lib.makeBinPath [
common-updater-scripts
coreutils
git
gnused
nix
(rebar3WithPlugins { })
]
}
PATH=${lib.makeBinPath [ common-updater-scripts coreutils git gnused nix (rebar3WithPlugins { }) ]}

Comment on lines +50 to +63
meta = {
homepage = "https://github.com/rebar/rebar3";
description = "Erlang build tool that makes it easy to compile and test Erlang applications, port drivers and releases";

longDescription = ''
rebar is a self-contained Erlang script, so it's easy to distribute or
even embed directly in a project. Where possible, rebar uses standard
Erlang/OTP conventions for project structures, thus minimizing the amount
of build configuration work. rebar also provides dependency management,
enabling application writers to easily re-use common libraries from a
variety of locations (hex.pm, git, hg, and so on).
'';

platforms = lib.platforms.unix;
maintainers = lib.teams.beam.members;
license = lib.licenses.asl20;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
meta = {
homepage = "https://github.com/rebar/rebar3";
description = "Erlang build tool that makes it easy to compile and test Erlang applications, port drivers and releases";
longDescription = ''
rebar is a self-contained Erlang script, so it's easy to distribute or
even embed directly in a project. Where possible, rebar uses standard
Erlang/OTP conventions for project structures, thus minimizing the amount
of build configuration work. rebar also provides dependency management,
enabling application writers to easily re-use common libraries from a
variety of locations (hex.pm, git, hg, and so on).
'';
platforms = lib.platforms.unix;
maintainers = lib.teams.beam.members;
license = lib.licenses.asl20;
meta = with lib; {
homepage = "https://github.com/rebar/rebar3";
description = "Erlang build tool that makes it easy to compile and test Erlang applications, port drivers and releases";
longDescription = ''
rebar is a self-contained Erlang script, so it's easy to distribute or
even embed directly in a project. Where possible, rebar uses standard
Erlang/OTP conventions for project structures, thus minimizing the amount
of build configuration work. rebar also provides dependency management,
enabling application writers to easily re-use common libraries from a
variety of locations (hex.pm, git, hg, and so on).
'';
platforms = platforms.unix;
maintainers = teams.beam.members;
license = licenses.asl20;

Comment on lines 27 to 28
mkdir -p _checkouts
mkdir -p _build/default/lib/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mkdir -p _checkouts
mkdir -p _build/default/lib/
mkdir -p _checkouts _build/default/lib/

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Comment on lines 32 to 40
PATH=${
lib.makeBinPath [
common-updater-scripts
coreutils
git
nix
(rebar3WithPlugins { })
]
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
PATH=${
lib.makeBinPath [
common-updater-scripts
coreutils
git
nix
(rebar3WithPlugins { })
]
}
PATH=${lib.makeBinPath [ common-updater-scripts coreutils git nix (rebar3WithPlugins { }) ]}

@r-rmcgibbo
Copy link

r-rmcgibbo commented Apr 14, 2021

Result of nixpkgs-review pr 119452 at 8e3c0326 run on aarch64-linux 1

1 package marked as broken and skipped:
  • lfe_1_2
3 packages built successfully:
  • lfe (lfe_1_3)
  • rebar3
  • relxExe

Result of nixpkgs-review pr 119452 at 8e3c0326 run on x86_64-linux 1

1 package marked as broken and skipped:
  • lfe_1_2
3 packages built successfully:
  • lfe (lfe_1_3)
  • rebar3
  • relxExe
5 suggestions:
  • warning: missing-phase-hooks

    buildPhase should probably contain runHook preBuild and runHook postBuild.

    Near pkgs/development/tools/build-managers/rebar3/default.nix:41:5:

       |
    41 |     buildPhase = ''
       |     ^
    
  • warning: missing-phase-hooks

    installPhase should probably contain runHook preInstall and runHook postInstall.

    Near pkgs/development/tools/build-managers/rebar3/default.nix:45:5:

       |
    45 |     installPhase = ''
       |     ^
    
  • warning: unused-argument

    Unused argument: bash.
    Near pkgs/development/tools/build-managers/rebar3/default.nix:2:65:

      |
    2 | , coreutils, git, gnused, nix, tree, writeScript, writeTextDir, bash, p7zip
      |                                                                 ^
    
  • warning: unused-argument

    Unused argument: writeTextDir.
    Near pkgs/development/tools/build-managers/rebar3/default.nix:2:51:

      |
    2 | , coreutils, git, gnused, nix, tree, writeScript, writeTextDir, bash, p7zip
      |                                                   ^
    
  • warning: unused-argument

    Unused argument: writeShellScriptBin.
    Near pkgs/development/tools/build-managers/rebar3/default.nix:3:16:

      |
    3 | , makeWrapper, writeShellScriptBin }:
      |                ^
    

@happysalada
Copy link
Contributor

I have to say, I don't use erlang, so I can't really produce valuable feedback on the general direction. I don't see anything that jumps out to me.
I'm sure you are aware of previous attempts at packing erlang-ls https://github.com/hauleth/nix-elixir/blob/master/lib/erlang-ls.nix
It seems to be just using rebar3relx, maybe you want to replace this as well?

Incidentally, you seem to be interested in nix and erlang, would you be interested in being part of the beam maintainers list? You can safely ignore Elixir related stuff if you're not interested and just focus on erlang stuff. As far as I know you would be the only erlang member active, so I don't think there would be too much extra work involved. That being said, no problem if you have other things to do.

@hauleth you might be interested in this.

@balsoft perhaps you know people in the nix community that use erlang and that this would be relevant to?

@dlesl
Copy link
Contributor Author

dlesl commented Apr 15, 2021

Hi @happysalada, thanks for taking the time to look at this. I wasn't aware of the erlang-ls derivation you linked but I am aware of this one, which works very well. The problem with both of these is that they use something like fetchRebar3Deps, which relies on giving rebar3 internet access in a fixed output derivation. There are some issues with this approach which are discussed here. I have two main concerns, the first is around reproducibility - although rebar3 does lock dependencies, it doesn't lock plugins, so an update to a plugin could stop a derivation from building in the future. The second is around auditability and the ability to track exactly where dependencies are coming from (and cache/mirror them). So my aim with this PR would be not to replace rebar3Relx but rather fetchRebar3Deps.

Thanks for the offer of being part of the beam maintainers list, I will have to read more about what this involves :). It is a shame to hear that I'm the only active erlang user! I think part of the problem might be that erlang is generally used to build bespoke (web) applications rather than packages that people would want to see packaged in nixpkgs, so I think it would be good to focus on improving the experience of developing and deploying such applications using nix.

@ghost
Copy link

ghost commented Apr 20, 2021

Just a quick heads-up that I'm using buildRebar3 in my pleroma packaging: https://git.petabyte.dev/petabyteboy/nixfiles/src/branch/main/pkgs/pleroma/mixnix/mix2nix.nix#L129
I would appreciate it a lot if there would still be be a way to use it like this (let the user take care of obtaining dependencies one-by-one) in the future.

@gleber
Copy link
Contributor

gleber commented Apr 25, 2021

@dlesl I like this new approach!

I was thinking about doing something similar as the second attempt to get Erlang ecosystem in good shape in nixpkgs. Unfortunately my life path went away from Erlang world, so I do not have much motivation to work on it.

Overall, I think this is the right direction! I find it hard to review this PR as it does a lot of things at the same time.

I suggest the following steps:

  • integrate rebar3nix.erl into https://github.com/erlang-nix/rebar3_nix [1]
  • create a separate PR to introduce rebar3_nix plugin into nixpkgs (nixpkgs's standard "init: foo" PR)
  • [not blocking, can be done in parallel] create a PR which replaces rebar-nix-bootstraper script with a call into rebar3 nix bootstrap
  • split this PR into commits, which do:
    • cleans up existing rebar3/default.nix file by moving deps into separate file (as is) and using to iterate over lib.mapAttrsToList
    • introduces rebar3WithPlugins derivation and function (include docs updates)
    • make use of rebar3WithPlugins in https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/beam-modules/build-rebar3.nix as there's some duplication of plugin management logic there with the logic of rebar3WithPlugins
    • introduce updateScript to rebar3
  • create a separate PR to init erlang-ls using rebar3WithPlugins

[1] As of current state of rebar3_nix, please do whatever you think is appropriate there. It needs love.

@dlesl
Copy link
Contributor Author

dlesl commented Apr 25, 2021

@petabyteboy - I didn't explain it very well in the initial post but what I would like to see is that fetching deps individually becomes the standard way of building erlang packages with nix, so this shouldn't affect your usecase. If we get as far as modifying buildRebar3 then it would be great if you could help test to make sure we haven't broken anything for you.

@gleber Thanks for the feedback and pointing me to rebar3_nix. You're right that the PR is a bit unfocused - I'll refactor this PR focus on rebar3WithPlugins as you suggested and address handling deps in a second PR.

Regarding handling deps, I see that rebar3_nix uses the same approach as rebar-nix-bootstrapper so that would be a nice incremental approach to getting rid of the fixed output derivation. However there is another approach I think we should consider which seems to work quite well in my (not very extensive) testing:

  • use a rebar3 which has been unzipped and puts its own libraries at the end of ERL_LIBS (like rebar3WithPlugins {} from this PR). This is necessary to avoid rebar3's own dependencies from taking precedence.
  • set {deps, []} in rebar.config and delete rebar.lock
  • provide the compiled dependencies on ERL_LIBS
  • use rebar3 as usual (eg rebar3 release)

I was thinking we could patch rebar3 to ignore the deps clause and lock file when an env var is set, to enable this with an unmodified rebar.{config,lock}. It might be possible to do it in a plugin too, but AFAICT it would be a bit hacky (overriding and wrapping the builtin providers).

It's hard for me to tell what's best here -- the bootstrapper approach is appealing in that it works with unmodified rebar3. OTOH it seems to me like ERL_LIBS should be the correct way to provide compiled dependencies, and we should work to get better support for that in rebar3 (upstream or by patching).

@gleber
Copy link
Contributor

gleber commented Apr 26, 2021

@dlesl I think the best near-term future is for both approaches to coexist in the rebar3_nix for now, until we incrementally move the whole ecosystem to the target model. I feel like .erl files should not be stored in nixpkgs repo as we do nowadays. I am a big fan of incremental changes.

The bootstrapper approach was used mainly due to an ability to use unmodified rebar3, but it is not the best solution. IIUC our goal is to introduce "ignore dependencies from config and use given deps" and "do not download anything" switches into rebar3 binary. Based on my intuition (no experiments done), the best solutions are (in order):

  1. Incorporate these switches as flags/options into upstream rebar3.
  2. Make use of plugins system to achieve it.
  3. Make use of patches to achieve it.
  4. Make use of binary repackaging to achieve it.
  5. Make use of setting up the environment to trick rebar3 to do what we want it to do.

Some upstream related issues:
erlang/rebar3#958
erlang/rebar3#1281

@dlesl
Copy link
Contributor Author

dlesl commented May 6, 2021

@gleber Thanks for providing some background here. I agree, it seems wrong to have .erl files in the repo, having the plugin in rebar3_nix as a separate project makes sense.

Regarding the proposed solutions, I expect we will have to gradually move backwards from 5 (where we are now). If we can get a good solution working using 2-4 this would help make the argument to the rebar3 maintainers that our rebar3 patches are worth merging and maintaining.

I still plan to update this PR (and make a PR to rebar3_nix), just been a bit short on time lately!

@dlesl dlesl force-pushed the rebar3-automated-packaging branch from 8e3c032 to 842b369 Compare May 7, 2021 11:42
@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label May 7, 2021
@dlesl dlesl changed the title [WIP] rebar3: attempt at new packaging system rebar3: add rebar3WithPlugins May 7, 2021
@ydlr
Copy link
Contributor

ydlr commented May 7, 2021

Should the buildPlugins attribute be removed from the buildRebar3 function as part of this?

@dlesl dlesl force-pushed the rebar3-automated-packaging branch from 842b369 to cd573f8 Compare May 8, 2021 11:48
@dlesl
Copy link
Contributor Author

dlesl commented May 8, 2021

@ydlr rather than remove it I converted it to use rebar3WithPlugins. WDYT? There aren't many derivations which use buildRebar3 in nixpkgs (and none use the compilePorts option) so it's a bit hard to test these changes unfortunately!

Copy link
Contributor

@gleber gleber left a comment

Choose a reason for hiding this comment

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

Great changes! Thank you for pushing it towards the right direction.

Comment on lines 27 to 28
mkdir -p _checkouts
mkdir -p _build/default/lib/
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

patches = [ ./skip-plugins.patch ./global-plugins.patch ];
}));
in stdenv.mkDerivation {
name = "rebar3-with-plugins";
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@dlesl dlesl force-pushed the rebar3-automated-packaging branch from cd573f8 to 64fb916 Compare May 9, 2021 19:52
@dlesl dlesl force-pushed the rebar3-automated-packaging branch from 64fb916 to 3682a84 Compare May 9, 2021 19:57
@ofborg ofborg bot requested a review from gleber May 9, 2021 20:11
@happysalada
Copy link
Contributor

This looks ready to be merged. Does anyone have any objections?
I'll wait a couple of days before merging it, except of course if somebody beats me to it.

@dlesl dlesl mentioned this pull request May 10, 2021
10 tasks
@happysalada happysalada merged commit dcc075c into NixOS:master May 10, 2021
@dlesl dlesl mentioned this pull request May 11, 2021
10 tasks
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.

6 participants