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

Flake #164

Merged
merged 2 commits into from
Jun 20, 2022
Merged

Flake #164

merged 2 commits into from
Jun 20, 2022

Conversation

jmatsushita
Copy link
Contributor

@jmatsushita jmatsushita commented May 21, 2022

Hi @turion

Here's a very simple flake (following up on #162) which uses the experimental haskell-flake-utils approach which is the only tool I know that simulates someone a simple multi package setup with cabal (it doesn't do anything with the cabal.project file, instead it takes an argument where we can specify which packages needs to be built).

It allows to do

nix build .#rhine-gloss
result/bin/rhine-gloss-gears
nix develop
cabal build rhine
#or 
cabal run rhine-gloss-gears

I've tested this on aarch64 and x86_64-linux and the added CI job should test building on linux. Though I expect that nix flake check will fail with:

error: overlay does not take an argument named 'final'

(I've reported this bug to haskell-flake-utils ivanovs-4/haskell-flake-utils#2

It builds using GHC 9.0.2 which is the default compiler in nixpkgs stable at the moment. It's not obvious how to build for multiple GHC versions with this utility

Some things to consider moving forward:

  • Settle on haskell-flake-utils or a more manual approach.
  • Update default.nix and shell.nix with flake-compat for legacy compatibility (as described here)
  • Remove other */shell.nix and */default.nix files.
  • Decide if maintaining stack build makes sense (I couldn't build with the current stack configuration)
  • Update README
  • Add local development tooling (HLS, ghcid, hoogle...) for later
  • Add a gitpod configuration to be able to have a cloud based development setup for later
  • Find a solution for multi compiler builds for later

Happy to keep adding to this PR and discuss as we go or merging smaller blocks of work, as you prefer.

flake.nix Outdated
};

outputs = { self, nixpkgs, flake-utils, haskell-flake-utils, ... }@inputs:
flake-utils.lib.eachSystem [ "x86_64-linux" "aarch64-darwin" ] (system:
Copy link
Owner

Choose a reason for hiding this comment

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

Why exactly these systems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another approach is to use eachDefaultSystem which is more comprehensive, but I went the route of picking the 2 architectures that I was able to test with.

Copy link
Owner

Choose a reason for hiding this comment

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

I'd propose either to use eachDefaultSystem or all systems we can somehow check on CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd propose either to use eachDefaultSystem or all systems we can somehow check on CI.

If that's OK, I would ask that we relax that requirement given that aarch64-darwin (M1 chip) is not available on Github Runners, but that's the machine architecture I'm using.

What I can do is setup a CI matrix for systems that are available in Github that will build x86_64-linux and x86_64-darwin (maybe try i686-linux too).

Note that with this PR haskell-flake-utils changed their API such that now we pass an array of systems instead of using flake-utils.lib.eachSystem (which is now called in haskell-flake-utils).

flake.nix Outdated Show resolved Hide resolved
Copy link
Owner

@turion turion left a comment

Choose a reason for hiding this comment

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

Can you comment in the readme how to use the flake?

flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
flake.nix Show resolved Hide resolved
@turion
Copy link
Owner

turion commented May 24, 2022

https://github.com/turion/rhine/runs/6568785125?check_suite_focus=true fails with error: overlay does not take an argument named 'final'. The nix version used is 2.8.1.

@turion
Copy link
Owner

turion commented May 24, 2022

Here's a very simple flake (following up on #162) which uses the experimental haskell-flake-utils approach which is the only tool I know that simulates someone a simple multi package setup with cabal (it doesn't do anything with the cabal.project file, instead it takes an argument where we can specify which packages needs to be built).

It allows to do

nix build .#rhine-gloss
result/bin/rhine-gloss-gears
nix develop
cabal build rhine
#or 
cabal run rhine-gloss-gears

Awesome!

I've tested this on aarch64 and x86_64-linux and the added CI job should test building on linux. Though I expect that nix flake check will fail with:

error: overlay does not take an argument named 'final'

Indeed it does.

(I've reported this bug to haskell-flake-utils ivanovs-4/haskell-flake-utils#2

And it seems it's fixed upstream yesterday 🎉 can you bump haskell-flake-utils and we try again?

It builds using GHC 9.0.2 which is the default compiler in nixpkgs stable at the moment. It's not obvious how to build for multiple GHC versions with this utility

That's good enough for a start :)

* [ ]  Settle on `haskell-flake-utils` or a more manual approach.

Let's see whether the upstream fix works. If yes, keep it, if no, a more manual approach.

* [ ]  Update default.nix and shell.nix with `flake-compat` for legacy compatibility (as [described here](https://nixos.wiki/wiki/Flakes#Using_flakes_project_from_a_legacy_Nix))

Yes makes sense :)

* [ ]  Remove other `*/shell.nix` and `*/default.nix` files.

I guess not, it contradicts the previous idea.

* [ ]  Decide if maintaining stack build makes sense (I couldn't build with the current stack configuration)

I don't care about stack at all, but maybe @miguel-negrao is motivated to fix it 😀

* [ ]  Update README

Yes.

* [ ]  Add local development tooling (HLS, ghcid, hoogle...)

I don't know how HLS from the flake interacts with the editor. I'd leave these points out for now.

* [ ]  Add a gitpod configuration to be able to have a cloud based development setup

No idea what that is 😅

* [ ]  Find a solution for multi compiler builds

Yes, that can be a separate issue.

Happy to keep adding to this PR and discuss as we go or merging smaller blocks of work, as you prefer.

I prefer smaller blocks. Too big PRs tend to get stuck.

@jmatsushita
Copy link
Contributor Author

Can you comment in the readme how to use the flake?

Sure thing will do.

Saw the upstream fix, and will bump it!

Let's see whether the upstream fix works. If yes, keep it, if no, a more manual approach.

There are other concerns, but I'd like to merge this and submit another PR for discussing what other approaches would entail.

  • Remove other */shell.nix and */default.nix files.

I guess not, it contradicts the previous idea.

I meant the rhine/shell.nix rhine-examples/shell.nix and so on. Unless there's something I'm missing in your workflow, it should be possible to enter the nix develop shell and build all packages together or individually.

I don't know how HLS from the flake interacts with the editor. I'd leave these points out for now.

That's one of the reasons I use flakes and nix :D It works very well, but I agree let's do this in a subsequent PR :)

  • Find a solution for multi compiler builds

Yes, that can be a separate issue.

Should work with this other upstream fix

I prefer smaller blocks. Too big PRs tend to get stuck.

Sounds good!

Thanks a ton for the review, and super excited about your (conceptual) fixes on the terminal PR. Will fix this PR and look at the other one sometime this week 👍

@turion
Copy link
Owner

turion commented May 25, 2022

Let's see whether the upstream fix works. If yes, keep it, if no, a more manual approach.

There are other concerns, but I'd like to merge this and submit another PR for discussing what other approaches would entail.

Great.

  • Remove other */shell.nix and */default.nix files.

I guess not, it contradicts the previous idea.

I meant the rhine/shell.nix rhine-examples/shell.nix and so on. Unless there's something I'm missing in your workflow, it should be possible to enter the nix develop shell and build all packages together or individually.

Oh, I see! Yes, let's remove those.

I don't know how HLS from the flake interacts with the editor. I'd leave these points out for now.

That's one of the reasons I use flakes and nix :D It works very well, but I agree let's do this in a subsequent PR :)

Ok, looking forward to that :)

@jmatsushita
Copy link
Contributor Author

Hi @turion

I think I've addressed all your comments. There are a couple of loose ends.

Regarding systems, Github actions can build x86_64-linux and x86_64-darwin (and maybe i686-linux but not sure how) and I've created a matrix for that in CI, but I'd like aarch64-darwin to be included in the flake since this is needed on my laptop (but not available in Github runners as of yet - except if self-hosted).

Also, unfortunately, despite the fix to overlay does not take an argument named 'final' nix flake check to run tests isn't quite ready (part of the flake things that haven't been quite worked out yet) and will fail with something like:

error: a 'aarch64-linux' with features {} is required to build '/nix/store/ci2j3y5cc4mhm5hpl3zkq05ix6429094-cabal2nix-rhine.drv', but I am a 'aarch64-darwin' with features {benchmark, big-parallel, nixos-test}

This is a known issue and the alternatives are:

Happy to open another issue to figure that one out :) For now I've added a nix develop -c cabal test all command to run tests... And then realised there were no tests yet 🤣 (it's ok though, no tests is a passing test suite).

Also I've tested this PR together with #166 and running stack build (with nix enabled system wide on my machine) seems to work well. Awesome @miguel-negrao !

Cheers,

Jun

@turion
Copy link
Owner

turion commented May 30, 2022

Awesome :) two systems built is already great!

Let's leave out nix flake check for the moment. The tests (when they're added) can be run on the cabal jobs for the time being.

flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
shell.nix Outdated Show resolved Hide resolved
shell.nix Outdated Show resolved Hide resolved
default.nix Show resolved Hide resolved
default.nix Outdated
rhine-examples
;
}
(import (
Copy link
Owner

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 way to detect in a default.nix whether it is called from nix build or nix shell. This could be used to remove shell.nix completely. But I can't find the option.

Copy link
Contributor Author

@jmatsushita jmatsushita Jun 18, 2022

Choose a reason for hiding this comment

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

If we're ok with requiring a recent version of nix (from 22.05 it's the default, and before this release it just requires setting experimental-features = nix-command flakes in the nix.conf), then we actually need neither default.nix nor shell.nix which are only here to support legacy nix.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, that's also ok. Let's drop support for legacy nix.

Copy link
Owner

Choose a reason for hiding this comment

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

Or rather, I'll leave the choice up to you. Whatever you're more comfortable with.

Copy link
Owner

@turion turion left a comment

Choose a reason for hiding this comment

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

Looking great :) just some small syntax things.

@jmatsushita jmatsushita requested a review from turion June 18, 2022 10:36
Uses haskell-flake-utils (experimental) to provide build and devShell with nix flakes:
```
nix build .#rhine-gloss
result/bin/rhine-gloss-gears
```

```
nix develop
cabal build rhine
```
flake.nix Outdated Show resolved Hide resolved
Copy link
Owner

@turion turion left a comment

Choose a reason for hiding this comment

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

Actually this is fine with default.nix and shell.nix for now. I'll squash and merge. Thank you very much!

@turion turion merged commit 35625e7 into turion:master Jun 20, 2022
@jmatsushita jmatsushita deleted the flake branch June 20, 2022 17:59
@jmatsushita
Copy link
Contributor Author

That's great. Will open a new PR for adding haskell language server and look into the multi compiler setup. 👍

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.

2 participants