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

Fix buildStackProject in nix-build #30072

Merged

Conversation

ElvishJerricco
Copy link
Contributor

@ElvishJerricco ElvishJerricco commented Oct 4, 2017

Motivation for this change

It seems that haskell.lib.buildStackProject never really worked for nix-build, as it wouldn't have even had stack in the PATH. The primary usage of buildStackProject has always been to provide a nix-shell environment that Stack can use for its nix integration, but I see no reason that it can't also be used to perform a Stack build in Nix.

This PR adds stack to the buildInputs, and correctly passes the arguments it expects when used with nix integration. The behavior in nix-shell is unchanged, thus leaving the user interface for stack's nix integration the same.

EDIT: I also removed preferLocalBuild from the resulting derivation. It is unclear to me why that was there in the first place. If we ever create a top level package in all-packages using this, would we not want that cached? I suspect it was that way because nix-build didn't previously matter for these derivations.

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.

Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

I don't know whether this change is good to have or not since I don't this code myself. I'll defer to other people's judgement. One comment I have:

If we ever create a top level package in all-packages using this, would we not want that cached?

We cannot compile Nix packages with stack, because stack cannot ensure a deterministic build outcome. Also, it needs all kinds of crazy network access to download its databases, etc. It might be possible work work around these issues somehow, but personally I just don't see the point because stack is fundamentally a developers tool, i.e. the features it provides are not important or even harmful in the context of Nix.

@peti peti requested a review from domenkozar October 5, 2017 08:37
@peti
Copy link
Member

peti commented Oct 5, 2017

Ping @mboes

@ElvishJerricco
Copy link
Contributor Author

I think that's selling Stack a little short. Certainly it's intention is to be a production tool, not just a dev tool. It is quite often used to build production deployments with it. The whole selling point is that it makes build reproducible. The only significant source of actual nondeterminism that I can think of is the ability to specify a branch for a git-based extra-dep instead of a revision. But that's generally considered bad practice.

I understand that there are significantly more points of failure (in terms of reproducibility) with anything as heavily networked as Stack, but I do believe that nondeterministic builds are the outlier with it. I don't consider it worse than the other common forms of nondeterminism in Nix, like builtins.fetchTarball.

Granted, I'm a bit biased, as I don't even use Stack this way exactly. I do nutty stuff like this:

{ }:

let
  nixpkgs = import ./nixpkgs {};
in nixpkgs.haskell.lib.buildStackProject {
  ghc = nixpkgs.haskellPackages.ghcWithPackages ...; # Not the GHC given by stack!
  ...
}

The nice thing about this is that I set the resolver to ghcXXX, which is a resolver with zero packages included, and Stack will just use the packages selected by my Nix expression since they're effectively baked into the GHC. This makes the nix-build more like a normal Haskell derivation, and I get the nice developer experience with the Stack CLI.

@mboes
Copy link
Contributor

mboes commented Oct 6, 2017

LGTM. Except for the preferLocalBuild thing, which to me looks orthogonal to the other changes, and could be discussed separately. cc @YPares

@ElvishJerricco
Copy link
Contributor Author

@mboes @peti @domenkozar I've removed the preferLocalBuild thing. Will open a separate PR for discussion on that particular topic. Does this look good to go?

@mboes
Copy link
Contributor

mboes commented Oct 21, 2017

LGTM

@vaibhavsagar
Copy link
Member

Can we merge this please?

@domenkozar domenkozar merged commit daf7868 into NixOS:master Nov 1, 2017
@vaibhavsagar
Copy link
Member

Thanks!

@wizzup
Copy link
Contributor

wizzup commented Nov 4, 2017

Is it normal that stack need to download build plan every time I call nix-buildeven there is nothing change and the last build was success?

@ElvishJerricco
Copy link
Contributor Author

@wizzup Yea. Having the ability to use this with nix-build is nice, but it's not super user friendly. We could resolve this by making the STACK_ROOT a derivation of its own, such that it only has to be rebuilt when the stack.yaml changes. In fact, we could even then let nix check parts of that derivation against a hash to help reassure the purity of it.

@wizzup
Copy link
Contributor

wizzup commented Nov 4, 2017

I am fine with dropping in nix-shell and run stack build while making changes.

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.

None yet

6 participants