-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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: 3.6.1 -> 3.9.0 #54115
rebar3: 3.6.1 -> 3.9.0 #54115
Conversation
cc @sjmackenzie for review |
Anyone? @gleber @couchemar @sjmackenzie @the-kenny |
|
Are |
Could be something related to the hermetic patch. I'll take a look later. |
Ok, these errors were caused by not updating hex cache, which is completely useless when building with nix-build. My workaround with unpacking contents of hex packages to |
Unfortunately hashes used by hex are different from what could be used by nix. They concatenate the source tarball together with some metadata, which is unknown in advance [1]. It is still possible to use hex-style hash in a fixed-output derivation, but via a hack so dirty I don't even want to talk about. So I had to resort to fixed-output derivations containing downloaded sources. Similar approach is used by Rust people, however this practice is disputed [2] [1] https://github.com/hexpm/hex_core/blob/bfd012f7200091cac2901850b2f7a828f7bf6301/src/hex_tarball.erl#L244 |
|
@Mic92 Sorry for the delay. Both hex2nix and relxExe build now and nox doesn't report any errors. |
hermeticRebar3 = true; | ||
}; | ||
rebar3-open = callPackage ../tools/build-managers/rebar3 { }; | ||
rebar3 = callPackage ../tools/build-managers/rebar3 { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need the hermeticRebar3
option. It defaults to true, which means rebar3-open
and rebar3
are now the same things. And users of rebar3-open
will be unable to download packages from hex because it is hermetic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After going through it a little further, I see that you removed the hermetic patch, which means (I think) it now goes against how Nix is expecting things. It is free to download anything that it wants instead of you explicitly saying what the applications dependencies are. I would like someone with more knowledge than myself to say whether this is acceptable or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
In short, removal of the hermetic patch isn't against Nix's way of doing things: nix-build runs with network isolation (unless it's a fixed-output derivation) therefore rebar3 won't be able to download anything during the process of build.
Fetching dependencies is a different story. It was moved to a separate step. Using rebar3 get-deps
produces fixed output (rebar3 itself uses sha256 to freeze the dependencies), so this is no worse than using fetchurl
.
I described my reasoning more in depth here: #53834
|
||
downloadPhase = '' | ||
cp ${src} . | ||
HOME='.' DEBUG=1 ${rebar3}/bin/rebar3 get-deps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth, this will only work on rebar3-open
because it is not hermetic. How does this really differ from the current fetchHex
(aside from it possibly getting non-hex deps)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current way of dealing erlang dependencies is not viable. Not only hermetic patch breaks rebar3 for everyone, but it also heavily relies on undocumented rebar3 features and file locations.
With the current approach in order to update a package one needs to
- Ask @gleber to update hex index cache repo (note that hex index is not actually needed. it's only used to avert rebar3 errors)
- Wait until index hash repo is merged and the derivation is updated in nixpkgs repo
- Ping someone to run hex2nix and update hex packages in this repo.
- Wait again until the new PR is reviewed and merged
- Manually convert lock file contents to the Nix expression
- Pray that nothing doesn't have non-trivial build steps
- Pray especially hard that no one uses rebar3
override
feature
With the new approach every package can be maintained completely independently.
Since this is still open, would you mind bumping to 3.9.0? |
Remove hermetic patch (make it compatible with the upstream) (Mostly) eliminate the need for hex package registry
@ankhers Done, also squashed commits to reduce noise |
I'm good with this. My only other suggestion would be to drop |
Someone may use it. Is there a good way to deprecate a package? |
Based on a conversation in IRC, there doesn't seem to be a way to deprecate things. We may just need to change some docs the refer to it. Lets keep it for now and we can clear it out in a different PR. |
If possible, I would like to update docs through a separate PR after the rest of planned changes are proven viable and implemented. After that both old |
@ankhers May I have this one approved so I can move on to the next step? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be good.
I can't actually merge. Will need to wait for someone else to do that. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/state-of-the-beam-ecosystem-in-nix/4202/2 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/state-of-the-beam-ecosystem-in-nix/4202/2 |
1 similar comment
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/state-of-the-beam-ecosystem-in-nix/4202/2 |
Motivation for this change
Things done
Updated rebar3 package
Removed incompatible hermetic patch (nix-build runs with network isolation, hence the hermetic patch is not needed anymore)
Started eliminating the need of hex pac
Tested using sandboxing (nix.useSandbox on NixOS, or option
sandbox
innix.conf
on non-NixOS)Built on platform(s)
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/
)Determined the impact on package closure size (by running
nix path-info -S
before and after)Assured whether relevant documentation is up to date
Fits CONTRIBUTING.md.