Skip to content

Add callStackToNix function #116

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

Merged
merged 6 commits into from
May 17, 2019

Conversation

eamsden
Copy link
Contributor

@eamsden eamsden commented May 6, 2019

Add a callStackToNix function which can be pointed at a source directory with a package set and generate an input to mkStackPkgSet.

This still needs better filtering so that it is only rebuild when stack.yaml or a package definition changes.

@rvl
Copy link
Contributor

rvl commented May 7, 2019

Excellent, thanks!
Would you also be able to create a test case for callStackToNix?

@eamsden
Copy link
Contributor Author

eamsden commented May 7, 2019

@rvl I can do that, sure. I am also trying to think about filtering. On our monorepo this routinely generates the Warning, dumping very large path message. I think it needs to filter everything except stack.yaml, cabal files, and package.yaml files (since hpack infers module lists from haskell sources.) Figuring out exactly how to do that robustly is the main thing keeping me from taking the draft tag off of this PR.

@angerman
Copy link
Collaborator

angerman commented May 7, 2019

@eamsden

Warning, dumping very large path

this is something you'll find with haskell.nix right now all the time. So I wouldn't worry too much that this is specific to your repository.

@eamsden
Copy link
Contributor Author

eamsden commented May 7, 2019

@angerman ok I'll try to put together a test case and then take the draft tag off. That will probably happen tomorrow though. It's late here and right now the critical path for me is getting ARM cross-compilation working.

@hamishmack
Copy link
Collaborator

hamishmack commented May 7, 2019

I have been using something similar but for plan-to-nix here. It might be a useful example. In particular it filters out only the files needed (.project, .cabal and package.yaml files). Runs hpack on the package.yaml files. Runs cabal new-configure and plan-to-nix. Then it combines the resulting .nix files in with the original source (so that the .nix files have valid references to it).

{ pkgs, runCommand, nix-tools, cabal-install
, hackageIndex, ghc, hpack
} :
{src} :
let
  cabalFiles =
    builtins.filterSource (path: type:
      type == "directory" ||
      pkgs.lib.any (i: (pkgs.lib.hasSuffix i path)) [ ".project" ".cabal" "package.yaml" ])
      src;
  plan = runCommand "plan" {
    buildInputs = [ ghc hpack ];
  } ''
    tmp=$(mktemp -d)
    cd $tmp
    cp -r ${cabalFiles}/* .
    chmod +w -R .
    find . -name package.yaml -exec hpack "{}" \;
    HOME=${hackageIndex} ${cabal-install}/bin/cabal new-configure
    HOME=$out ${nix-tools}/bin/plan-to-nix --plan-json dist-newstyle/cache/plan.json -o nix-plan
    cp -r nix-plan $out
  '';
in
  runCommand "plan-and-src" {} ''
    mkdir $out
    cp -r ${src}/* $out
    ln -sf ${plan} $out/nix-plan
  ''

@eamsden
Copy link
Contributor Author

eamsden commented May 7, 2019

@hamishmack hpack depends on the .hs files in the source tree to infer some module lists. So it seems like your source filter would result in incorrect module lists unless one is following a convention of setting both exposed-modules and other-modules explciitly in every package.yaml file.

@hamishmack
Copy link
Collaborator

hpack depends on the .hs files in the source tree to infer some module lists

As far as I know plan-to-nix does not depend on the module lists, so I think it should be ok. The .cabal files generated by hpack are only in the $tmp dir and are not included in the output of the derivation.

@angerman
Copy link
Collaborator

angerman commented May 7, 2019

hpack depends on the .hs files in the source tree to infer some module lists

As far as I know plan-to-nix does not depend on the module lists, so I think it should be ok. The .cabal files generated by hpack are only in the $tmp dir and are not included in the output of the derivation.

I'm not sure if @eamsden was referring to the initial invocation of hpack to generate the initial .cabal file in the temporary directory that would serve as a basis for the new-configure invocation, but @hamishmack is correct that there is no module listing or anything left in the resulting nix expressions. plan-to-nix should generate the correct cabal-generator fields[1] though, such that it will later call hpack on the source accordingly.

--
[1]:

cabal-generator = mkOption {
type = nullOr str;
default = null;
};

@angerman
Copy link
Collaborator

@eamsden any update on this?

@rvl
Copy link
Contributor

rvl commented May 13, 2019

@eamsden If you don't have time to create an IFD test case then I can do it.

Also I'm curious to know, will this work in a sandboxed build when the stack.yaml has extra-deps which are fetched from git commits?

@eamsden
Copy link
Contributor Author

eamsden commented May 14, 2019

@rvl I'm busy trying to get cross-compilation to work, so if you are able to make a test case that would be great! I imagine there might be issues with fetching from git. I haven't yet tried it with that. It depends on how stack-to-nix works, that is, if it tries to fetch files from git during its run.

I can think of a couple of ways to help with that, but again, I'm pushing hard on cross compilation.

@eamsden
Copy link
Contributor Author

eamsden commented May 15, 2019

I'm building a test case based on the stack-simple test case right now. Once that's done I'll take the draft tag off of this PR. @rvl @angerman

@eamsden eamsden marked this pull request as ready for review May 15, 2019 17:40
@eamsden
Copy link
Contributor Author

eamsden commented May 15, 2019

@rvl @angerman could one of you review this please? :)

@angerman
Copy link
Collaborator

I'll try to give this a try today.

Copy link
Collaborator

@angerman angerman left a comment

Choose a reason for hiding this comment

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

Apart from the things I don't understand, this looks good to me!

pkgsNix = pkgs.stdenv.mkDerivation {
name = "pkgs-nix";
inherit src;
nativeBuildInputs = [ nix-tools pkgs.nix-prefetch-git pkgs.rsync ];
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need pkgs.rsync

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rsync handles copying hidden files and directories (like .plan.nix and .stack.nix) better than cp. I couldn't get cp to do that reliably. Maybe there is a flag I am missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh. I guess it is no longer used since I'm calling stack-to-nic directly into $out. I'll get rid of it.

default.nix Outdated
@@ -52,7 +52,12 @@ let
override = "stackage";
});

packages = self: ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

I keep wondering if we can't simplify this. Do we really need to bring in the callPackage logic for the buildPackages?

Copy link
Contributor Author

@eamsden eamsden May 16, 2019

Choose a reason for hiding this comment

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

I needed the callPackages stuff in buildPackages as callStackToNix needs to be built with buildPackages, otherwise if you do a cross-compile you wind up building nix-tools for the host platform rather than the build platform. This actually escaped my notice at first because I had binfmt emulation for armv7l set up on my build machine, but popped up when I tried an Aarch64 cross compile.

Copy link
Collaborator

@angerman angerman May 17, 2019

Choose a reason for hiding this comment

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

Can't we just pull nix-tools from pkgs.buildPackages.callPackage ./nix-tools { inherit fetchExternal; };

@angerman
Copy link
Collaborator

angerman commented May 17, 2019

In the end I think it all comes down to the UX/UI we expect the consumer to use, right now it's mostly like this:

haskell.callPackage ./some/path

where /some/path/default.nix would look like:

{ mkStackPkgSet, callStackToNix, ... }:
let
  pkgSet = mkStackPkgSet {
    stack-pkgs = callStackToNix { src = ./.; };
    pkg-def-extras = [];
    modules = [];
  };

Most of the code in haskell.nix is pure nix code and as such (aside from asking build/host/target platform related information) independent on where it is evaluated. I would even argue we want to evaluate it in the nixpkgs that are feed into haskel.nix, such that it can make informed decisions about the platforms.

However for the nix-tools and other helper tooling, we likely want that to be built for and on the build machine and as such be pulled from the pkg.buildPackages set. I'm not much a fan of indirection, especially the heavy layering in nixpkgs. Thus I think what we really want is:

    nix-tools = pkgs.buildPackages.callPackage ./nix-tools
      { inherit fetchExternal; inherit (self) mkCabalProjectPkgSet; };

or am I mistaken?

@angerman angerman force-pushed the eamsden-call-stack-to-nix branch from dfb1f57 to 4135f9d Compare May 17, 2019 07:47
@angerman angerman merged commit 3907718 into input-output-hk:master May 17, 2019
@rvl
Copy link
Contributor

rvl commented May 17, 2019

@eamsden thanks for noticing that nix-tools needs to be built with pkgs.buildPackages.

@angerman Sorry I got to the PR review party a bit late, but I think the corrections you have made w.r.t. buildPackages are almost spot on. However I prefer individual dependencies in the argument list (i.e. mkDerivation and nix-prefetch-git, rather than the entire pkgs).

@eamsden
Copy link
Contributor Author

eamsden commented May 18, 2019

@angerman: you broke it :(

If you just call the nix tools package from pkgs.buildPackage it still tries to build it as cross-compiled, because of the way the haskell.nix packages pass around stdenv etc. That was why I added the buildPackages attribute to the haskell package set the way I did, to make sure that the whole scope was properly set up for buildPackages.

Right now if I try to build a package set that uses callStackToNix as a cross-compiled build, with your changes in place, it will still try to build nix-tools as a cross-compiled build and then run them on the build machine. I know the way I had it set up was not as clean but it was the result of a lot of trying and failing to get nix-tools to build on the right platform.

@rvl rvl mentioned this pull request Jun 15, 2019
andreabedini pushed a commit to andreabedini/haskell.nix that referenced this pull request Sep 14, 2022
andreabedini pushed a commit to andreabedini/haskell.nix that referenced this pull request Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants