-
-
Notifications
You must be signed in to change notification settings - Fork 43
chore(nix): nix package #106
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
Conversation
2818fa3 to
b09b3a7
Compare
|
Do you know why the deps.nix files has a bunch of dependencies we don't have like rustler and explorer? |
There's some nix code in deps.nix for handling rustler and some overrides. These aren't actually being used in this case but are indeed in this file. If you focus solely on the Or here's a view from the repl: |
b09b3a7 to
96be34b
Compare
|
Gotcha, okay. I tested locally on NixOS, seemed to start up. Need to actually test it tho still (I'm about to board an airplane). Is this implementation able to be upstreamed to nixpkgs? When it is put into nixpkgs, do you recommend removing our local package here? |
|
This is pretty close to what would be in nixpkgs, but likely we'd switch back to mixFodDeps there with hashes. A nixpkgs package can't import deps.nix from the repo, since that would be IFD (import from derivation). (The source is a derivation, and then we'd be trying to import from within the source). I think deps_nix is great for in-project packages though which is why I suggested it here. There are benefits for keeping a package in repo here, in that users can easily access and build from any commit in the repo. But there's the downside of keeping deps.nix (or hashes) up to date with the mix.exs/mix.lock. If they drift, the nix package will fail to build, so ideally they're updated together. It's up to you and the other maintainers whether it stays though. :) I'd be interested in if this package works for you. In my testing with neovim it only generates errors |
|
I will port this to nixpkgs as an unstable release once I see it working fully. :) Then move to stable versioning once y'all do. |
96be34b to
86f6f23
Compare
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.
do you recommend renaming this file from expert.nix to package.nix? Or does that not really matter. I think we named it package.nix in Next LS
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.
It doesn't really matter.
|
I'm getting the same error as #59, but it can start and load. |
|
@adamcstephens in another PR, if you have time could you add a CI check for the nix build? |
I opted to use deps_nix as it simplifies hash management, and allows re-use of dependencies as they're individual fixed output derivations (FOD). Unfortunately, it makes this first diff quite large.
I can go back to straight hashes if this feels too invasive for the non-nix users, but it shouldn't affect them outside an extra dep. I also added to all apps even though it seems it may not be necessary for forge and expert_credo?
I've seen issues with builds failing during the release stage, but it's likely a timing issue as re-build works.
cc @mhanberg
This is an alternative to #96
fixes #95