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

Generate/write nix derivations for all the haskell code #2331

Merged
merged 130 commits into from
Oct 18, 2022

Conversation

akshaymankar
Copy link
Member

@akshaymankar akshaymankar commented Apr 28, 2022

Major changes:

  1. Instead of pinning things in cabal.project, we pin them in nix/haskell-pins.nix. This allows us to share built artefacts among the team and helps us avoid problems of compiling things with newer C libraries because cabal doesn't invalidate the built artefacts in the cabal-store.
  2. Images are built using nixpkgs' dockerTools.streamLayeredImage. This allows us to build minimal images without needing a docker daemon to be running. This also helps us cache most of our images in the nix cache.
  3. Every time any cabal file is changed or a new package is added, we must run make regen-local-nix-derivations. This will update various nix derivations we have for our project. This is protected by make check-local-nix-derivations which runs as a dependency of make lint-all in CI.

Usual development workflow shouldn't change if you were already using nix provided development environment. In case you face any problems deleting one or all of these directories from the root of the project might fix your issues:

  • .env
  • dist-newstyle

@akshaymankar akshaymankar temporarily deployed to cachix April 28, 2022 11:50 Inactive
@smatting
Copy link
Contributor

The build.nix at the root of the repo

This seems to be missing from the PR

@akshaymankar akshaymankar force-pushed the akshaymankar/nix-build-attempt-2 branch from 83ecc9a to b69ef65 Compare April 28, 2022 12:49
@akshaymankar akshaymankar temporarily deployed to cachix April 28, 2022 12:49 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix April 28, 2022 14:09 Inactive
@akshaymankar
Copy link
Member Author

The build.nix at the root of the repo

This seems to be missing from the PR

Added it now.

@akshaymankar akshaymankar temporarily deployed to cachix April 28, 2022 15:49 Inactive
build.nix Outdated Show resolved Hide resolved
@akshaymankar akshaymankar changed the title Generate/write derivations for all the haskell code Generate/write nix derivations for all the haskell code Apr 28, 2022
@akshaymankar akshaymankar marked this pull request as draft May 2, 2022 09:10
@akshaymankar akshaymankar temporarily deployed to cachix May 2, 2022 09:54 Inactive
build.nix Outdated Show resolved Hide resolved
Comment on lines 7 to 8
cabalFiles=$(find "$ROOT_DIR" -name '*.cabal' \
| grep -v dist-newstyle)
Copy link
Member Author

Choose a reason for hiding this comment

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

Use find -not -name

Copy link
Contributor

Choose a reason for hiding this comment

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

Might have to use -not -path instead, since I think dist-newstyle doesn't appear in the final path component ("name"). Alternatively, you could type \( -type d -name dist-newstyle -prune \) -o ... to avoid walking those directories at all, if you are just worried about directories with that specific name and not something like "indist-newstyler.cabal".

Copy link
Member Author

Choose a reason for hiding this comment

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

\( -type d -name dist-newstyle -prune \) -o ... doesn't seem to work and -not -path requires wildcards anyway. I think I will just leave it like this and hope nobody creates indist-newstyler.cabal or anything like that!

build.nix Outdated Show resolved Hide resolved
@stephen-smith
Copy link
Contributor

How much of this is generated vs. written? I naturally recoil at treating anything generated as source code and checking it in, though I certainly understand it's the path of least resistence in some cases.

@akshaymankar
Copy link
Member Author

How much of this is generated vs. written? I naturally recoil at treating anything generated as source code and checking it in, though I certainly understand it's the path of least resistence in some cases.

All the new files in the PR except pins.yaml, build.nix and the 3 shell scripts in hack/ are generated. For all the default.nix files, we could use callCabal2nix function, but a file with list of those calls to the function for each package would either need to be generated or manually written. For all the pinned things, I think it is possible to write something with callCabal2nix and fetchGit, but it will be a little effort to do this. That said, I think this will also make things harder to debug, so I would still prefer just committing these files and have CI ensure that these files are not out of date.

@pcapriotti
Copy link
Contributor

A few questions, sorry if they are dumb, but I still struggle with nix things.

  • Why are you generating all these nix files? Can't cabal2nix or whatever just read the cabal file dynamically and create the corresponding derivation?
  • Are we dropping stackage now? Is this necessary? Couldn't we be using the same pins as before and avoid the trouble of finding a consistent choice of versions? This might become an issue in the future.
  • Relatedly, why do we need the hackage portion of pins.yaml? Can't we get versions from the existing yaml files (i.e. cabal.project.freeze).

@akshaymankar
Copy link
Member Author

Why are you generating all these nix files? Can't cabal2nix or whatever just read the cabal file dynamically and create the corresponding derivation?

There exists a callCabal2nix but it only works for local paths. I think we can write something similar to it to avoid generating the files. We will still have to generate a file which calls this new function with the right parameters and creates the overrides.

Are we dropping stackage now? Is this necessary? Couldn't we be using the same pins as before and avoid the trouble of finding a consistent choice of versions? This might become an issue in the future.

The pkgs.haskell.packages.ghc8107 already follows some stackage pin, but it refers to a nightly snapshot. The overrides are similar to our overrides written in the stack.yaml.

Relatedly, why do we need the hackage portion of pins.yaml? Can't we get versions from the existing yaml files (i.e. cabal.project.freeze).

We want to not rely on stack.yaml so I chose to not read it. The cabal.project.freeze file is not a YAML file and I didn't feel like writing a parser and it contains more versions than we want to pin as we want to follow some stackage snapshot. So, for this approach to work we have to delete the cabal.project.freeze file along with the source-repository-package sections in the cabal.project file.

We could also try to get rid of the pins.yaml file if we can get a function similar to callCabal2nix. If we do that we can write all our pins in the build.nix file.

@@ -0,0 +1,18 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put this in a less hacky location, and add a toplevel comment to all shellscripts, explaining what they do and how /when they're supossed to be called?

Copy link
Member Author

@akshaymankar akshaymankar Oct 13, 2022

Choose a reason for hiding this comment

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

What a less hacky location?

build.nix Outdated Show resolved Hide resolved
dev-packages.nix Outdated Show resolved Hide resolved
@smatting smatting force-pushed the akshaymankar/nix-build-attempt-2 branch from 841292d to 4b8adaa Compare August 19, 2022 10:41
@smatting smatting temporarily deployed to cachix August 19, 2022 10:42 Inactive
@smatting smatting temporarily deployed to cachix August 19, 2022 10:42 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 13, 2022 09:45 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 13, 2022 10:33 Inactive
fast used to point to default when used with cabal anyway.
@akshaymankar akshaymankar temporarily deployed to cachix October 17, 2022 13:10 Inactive
@akshaymankar akshaymankar marked this pull request as ready for review October 17, 2022 13:11
@akshaymankar akshaymankar temporarily deployed to cachix October 17, 2022 14:55 Inactive
It helps with not having to run cabal2nix over and over again in CI.
@akshaymankar akshaymankar temporarily deployed to cachix October 18, 2022 08:36 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 18, 2022 09:08 Inactive
This reverts commit 660b12e.

Doesn't really give us anything
@akshaymankar akshaymankar temporarily deployed to cachix October 18, 2022 09:55 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 18, 2022 09:56 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 18, 2022 10:13 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 18, 2022 10:31 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 18, 2022 10:47 Inactive
Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

Thanks for all the comments! I'm curious to see how well this nixified approach will work out going forward.

@akshaymankar akshaymankar merged commit da6bd68 into develop Oct 18, 2022
@akshaymankar akshaymankar deleted the akshaymankar/nix-build-attempt-2 branch October 18, 2022 12:42
akshaymankar added a commit that referenced this pull request Oct 19, 2022
* Makefile: Avoid building haddocks while building production images

* Add changelog for nix builds, forgotten in #2331
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-ok-to-test Not approved for running tests in CI, this label is ignored if ok-to-test also exists on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants