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

nix/haskell-packages: replace IFD with importing generated files #1220

Merged
merged 2 commits into from
Feb 19, 2020

Conversation

basvandijk
Copy link
Contributor

@basvandijk basvandijk commented Feb 19, 2020

https://dfinity.atlassian.net/browse/INF-1002

Replace IFD calls to callCabal2nix with importing generated default.nix files.

The generated default.nix files are stored in the repository and contain instructions on how to regenerate them.

The generation of the default.nix files is done using the haskellSrc2nixWithDoc function which generates a directory containing a default.nix which is the result of running cabal2nix
with the extraCabal2nixOptions on the provided src. A header is then added to the default.nix which contains instructions on how to regenerate that file.

Finally the new jobs are added which check that the stored default.nix matches the expected default.nix. So whenever the .cabal files are updated and don't match the stored default.nix anymore CI will complain.

@basvandijk basvandijk force-pushed the basvandijk/get-rid-of-IFD branch 3 times, most recently from 3e5a156 to 6c5b5ea Compare February 19, 2020 14:18
@basvandijk basvandijk marked this pull request as ready for review February 19, 2020 14:19
Copy link
Contributor

@nmattia nmattia left a comment

Choose a reason for hiding this comment

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

Damn I hate the Nix haskell infrastructure

Copy link
Collaborator

@nomeata nomeata left a comment

Choose a reason for hiding this comment

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

Nice solution!

I wonder if it would be nicer to keep the generated files all in

./nix/generated/

(e.g. ./nix/generated/winter.nix), have one command to update all generated files, and only one check, and not require an essentially empty winter/ directory. But probably overkill.

Maybe add a section in Building.md about Updating Haskell dependencies?

@basvandijk
Copy link
Contributor Author

I wonder if it would be nicer to keep the generated files all in

I opted for keeping the .cabal, default.nix and generate.nix in the same directory so folks don't have to search for these files.

Maybe add a section in Building.md about Updating Haskell dependencies?

Good idea. Done.

@basvandijk basvandijk added the automerge-squash When ready, merge (using squash) label Feb 19, 2020
Replace IFD calls to callCabal2nix with importing generated
`default.nix` files.

The generated `default.nix` files are stored in the repository and
contain instructions on how to regenerate them.

The generation of the `default.nix` files is done using the
`haskellSrc2nixWithDoc` function which generates a directory
containing a `default.nix` which is the result of running `cabal2nix`
with the `extraCabal2nixOptions` on the provided `src`. A header is
then added to the `default.nix` which contains instructions on how to
regenerate that file.

Finally the new jobs are added which check that the stored
`default.nix` matches the expected `default.nix`. So whenever the
`.cabal` files are updated and don't match the stored `default.nix`
anymore CI will complain.
@basvandijk basvandijk merged commit b90fbc5 into master Feb 19, 2020
@mergify mergify bot deleted the basvandijk/get-rid-of-IFD branch February 19, 2020 20:20
@mergify mergify bot removed the automerge-squash When ready, merge (using squash) label Feb 19, 2020
@nomeata
Copy link
Collaborator

nomeata commented Feb 19, 2020

Ugh, now I get this. Any idea?

$ nix-build -A ic-stub  --option extra-substituters https://nix.dfinity.systems --option trusted-public-keys cache.dfinity.systems-1:IcOn/2SVyPGOi8i3hKhQOlyiSQotiOBKwTFmyPX5YNw=
these derivations will be built:
  /nix/store/fc8szv58sak7f8rnsg39slhy1kn8d9ph-ic-stub-0.1.0.0.drv
building '/nix/store/fc8szv58sak7f8rnsg39slhy1kn8d9ph-ic-stub-0.1.0.0.drv'...
setupCompilerEnvironmentPhase
Build with /nix/store/5rk546bp91dqb700kxikw9gnm5xkxh5n-ghc-8.6.5.
unpacking sources
unpacking source archive /nix/store/hsnww9bcwgnzggm4b4l0d78q7pbiycfq-ic-stub
source root is ic-stub
patching sources
compileBuildDriverPhase
setupCompileFlags: -package-db=/tmp/nix-build-ic-stub-0.1.0.0.drv-0/setup-package.conf.d -j4 -threaded
Loaded package environment from /tmp/nix-build-ic-stub-0.1.0.0.drv-0/ic-stub/.ghc.environment.x86_64-linux-8.6.5
[1 of 1] Compiling Main             ( /nix/store/4mdp8nhyfddh7bllbi7xszz7k9955n79-Setup.hs, /tmp/nix-build-ic-stub-0.1.0.0.drv-0/Main.o )

/nix/store/4mdp8nhyfddh7bllbi7xszz7k9955n79-Setup.hs:1:1: error:
    Could not load module ‘Distribution.Simple’
    It is a member of the hidden package ‘Cabal-2.4.0.1’.
    You can run ‘:set -package Cabal’ to expose it.
    (Note: this unloads all the modules in the current scope.)
    Use -v to see a list of the files searched for.
  |
1 | import Distribution.Simple
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^

builder for '/nix/store/fc8szv58sak7f8rnsg39slhy1kn8d9ph-ic-stub-0.1.0.0.drv' failed with exit code 1
error: build of '/nix/store/fc8szv58sak7f8rnsg39slhy1kn8d9ph-ic-stub-0.1.0.0.drv' failed

@nomeata
Copy link
Collaborator

nomeata commented Feb 19, 2020

Ah, it somehow broke the proper use of gitSource

@nomeata
Copy link
Collaborator

nomeata commented Feb 19, 2020

              src_subst = "./.";

is of course wrong. Will refactor…

nomeata added a commit that referenced this pull request Feb 20, 2020
the reiterates on #1220. The motivation for the refactoring was that the
generated files were not using `gitSource`, so I would get cache misses
and dirty files in the repo. Fixing this was easier if all generated nix
files are in one place, so I did the following changes:

 * All generated nix files are in `nix/generated`. This de-pollutes the
   normal working area with generated files, and keeps them out from
   sources (more precise derivations).
 * The local generated files now use `gitSource`.
 * The generator is now `nix/generate.nix`.
 * I moved complex logic from `nix/default.nix` to `nix/generate.nix`,
   this means everything is in one place, instead of spread over several
   nix files.
 * There is now only a single check to check if the generated files are
   upto date, and a single command to update the files.
 * This check is no longer marked `allowSubstitutes = false`. The check
    isn’t cheap (it requires `cabal2nix`) and we want the check to happen
    on builders, not hydra itself, so that the cache has `cabal2nix`.
 * The check is now included in `all-systems-go`.
mergify bot pushed a commit that referenced this pull request Feb 20, 2020
the reiterates on #1220. The motivation for the refactoring was that the
generated files were not using `gitSource`, so I would get cache misses
and dirty files in the repo. Fixing this was easier if all generated nix
files are in one place, so I did the following changes:

 * All generated nix files are in `nix/generated`. This de-pollutes the
   normal working area with generated files, and keeps them out from
   sources (more precise derivations).
 * The local generated files now use `gitSource`.
 * The generator is now `nix/generate.nix`.
 * I moved complex logic from `nix/default.nix` to `nix/generate.nix`,
   this means everything is in one place, instead of spread over several
   nix files.
 * There is now only a single check to check if the generated files are
   upto date, and a single command to update the files.
 * This check is no longer marked `allowSubstitutes = false`. The check
    isn’t cheap (it requires `cabal2nix`) and we want the check to happen
    on builders, not hydra itself, so that the cache has `cabal2nix`.
 * The check is now included in `all-systems-go`.
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.

3 participants