-
Notifications
You must be signed in to change notification settings - Fork 247
Add evalSystem
and evalPackages
project args
#1546
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
This adds a way to specify the `evalSystem` explicitly when calling the `project` functions. Currently if we want to make a `flake` that supports multiple systems we have few options: * Require builders for all the supported systems (even just for `nix flake show`). * Pass `--impure` so that haskell.nix can see `builtins.currentSystem` to set up `pkgs.evalPackages` to use that. Unfortunately this prevents nix from caching some of the work it does and often results in it recalculating for each supported system when it would otherwise be cached and take no time at all. * Add an overlay to replace `evalPackages`. This works, but it is not straight forward. * Materialize the nix files for the project. This change allows `evalSystem = "x86_64-linux";` to be passed telling `haskell.nix` to run `cabal` and `nix-tools` on that system. The user will have to have a builder for that system, but does not need to have builders for the others (unless building outputs for them).
I wonder if it would make sense to ask the user to provide |
I have removed I removed I left |
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.
Maybe this was a bad idea, I don't know. It seems like we now don't have evalPackages
in the places where we need them?
@@ -202,7 +199,7 @@ in { | |||
cleanGit = import ./clean-git.nix { | |||
inherit lib cleanSourceWith; | |||
git = gitMinimal; | |||
inherit (pkgs.evalPackages.buildPackages) runCommand; | |||
inherit (pkgs.buildPackages.buildPackages) runCommand; |
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.
Are these changes actually okay? Won't this again mean that you can't eval without a build machine of the right system, because it will try and clean the source on a build machine?
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.
Same for the other places where this switch happens.
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.
I am not sure about cleanGit
. It should be ok on CI where .git
does not exist, but it may fail for local evals. I'll do some tests.
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.
Yeah, I guess it should be fairly obvious if you try building a darwin derivation when you don't have a darwin machine or something. If it works then I'm happy! It just seems suspicious - presumably it means that the usage of evalPackages
was actually unncessary in each of these places to begin with?
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.
I think there is a situation when not using flakes (so cleanGit
would have work to do) and cross compiling where this would have been useful so I have included a note in the change log.
else if pkgs.buildPackages.system == config.evalSystem | ||
then pkgs.buildPackages | ||
else | ||
import pkgs.path { |
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.
could also throw in this case and require the user to specify?
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.
I've changed the default to use buildPackages
. I also removed the default to make sure nothing in haskell.nix
itself (including the CI) depends on the default. The import
code here should now only get used if evalSystem
was explicitly passed in (for instance if the user passed evalSystem = builtins.currentSystem;
).
overlays/tools.nix
Outdated
// { name = final.haskell-nix.toolPackageName.${name} or name; })) | ||
.getComponent "exe:${final.haskell-nix.packageToolName.${name} or name}"; | ||
package = final.haskell-nix.hackage-package ( | ||
(if builtins.isList projectModule then projectModule else [projectModule]) |
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.
Can we not do this? it just makes it hard to know what types things have
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.
I have changed hackage-tool
so it only takes a single module. If there is a way to compose nix modules we could change the others too (hackage-package
, hackage-project
, cabalProject
, etc.). Alternatively we could also split out variants that take a list and have wrappers for those that take a single module. Let me know if you think that would be nicer.
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.
I think we suffer a lot from not having types, so I guess the heuristic I end up going for a lot is that an argument named X
should always have the same type. So I would just use a list of modules everywhere just so it's super consistent, even if that's a bit more annoying in places where you probably only want one?
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.
I think it might make sense to move cabalProject
and stackProject
code from overlays/haskell.nix
to modules/cabal-project.nix
and overlays/stack-project.nix
. I think then we might be able to simplify the way we eval the project modules. However I started trying that and it seems like a much bigger change. So for now I have changed hackage-tool
to accept only a list.
bors try |
tryBuild failed: |
bors try |
tryBuild succeeded: |
evalSystem
project argumentevalSystem
and evalPackages
project args
I believe this commit broke the evaluation of our
I've tracked it down to this commit by pinning our edit: Here's a trace:
where ghc8107Tools = final.haskell-nix.tools "ghc8107" {
cabal-fmt = "latest";
cabal-edit = "latest";
}; |
Should be fixed by #1570 |
Thanks! I can confirm that it's fixed now. |
This adds a way to specify the
evalSystem
orevalPackages
explicitly when calling theproject
functions.Currently if we want to make a
flake
that supports multiple systems we have few options:Require builders for all the supported systems (even just for
nix flake show
).Pass
--impure
so that haskell.nix can seebuiltins.currentSystem
to set uppkgs.evalPackages
to use that. Unfortunately this prevents nix from caching some of the work it does and often results in it recalculating for each supported system when it would otherwise be cached and take no time at all.Add an overlay to replace
evalPackages
. This works, but it is not straight forward.Materialize the nix files for the project.
This change allows
evalSystem = "x86_64-linux";
to be passed tellinghaskell.nix
to runcabal
andnix-tools
on that system. The user will have to have a builder for that system, but does not need to have builders for the others (unless building outputs for them).