-
Couldn't load subscription status.
- Fork 5
Project Nix Flakes #44
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
Conversation
flake.nix
Outdated
| [ | ||
| # Haskell stuff | ||
| hs-pkgs.cabal-install | ||
| hs-pkgs.haskell-language-server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please remove the HLS requirement here? It is not necessary for building the project and adds quite some overhead for non-HLS users.
It is also no restriction to HLS users, as they still can add the dependency in their private overlays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to do two dev shells? "minimal" and "full" or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about fourmolu then? I presume the same reasoning could apply there.
Also I like the idea of having a minimal and full devshell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is fourmolu a heavy dependency? I don't think most people can exactly match the mandatory fourmolu CI check off the top of their heads. I'd rather see people run fourmolu before opening a PR than waste CI run time. But I could be persuaded to make it optional, I think.
flake.nix
Outdated
| flake-utils.lib.eachDefaultSystem (system: | ||
| let | ||
| # What version of the GHC compiler to use for protocols | ||
| compiler-version = "ghc910"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We recently had the issue that the natnormalise plugin was missing 9.10.2 support, but nixpkgs already updated the ghc910 reference to point to 9.10.2. This was causing the flake to break after updating the lock file, which we needed to get some updates for other packages. We only could fix this by explicitly requiring to ghc9101.
The issue is fixed now, but I'm wondering whether it makes sense to pin the minor version as well by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not apposed to it, but I feel like pinning minor versions has to start with clash-compiler. Since otherwise we'll be creating workarounds in this and other flakes to get it to include clash-compiler properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's pretty uncommon that a minor GHC update breaks Clash, and that it occurs infrequently enough that we can deal with it post hoc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I changed my mind. For one thing, both 9.8 and 9.10 had Clash break on minor updates of GHC. We actually used to track a specific minor version, but changed that in clash-lang/clash-compiler#2810. I'd like to track minor versions in clash-compiler again, and packages like clash-cores can track at a coarser granularity.
However, we used to have files like overlay-ghc962.nix which became overlay-ghc96.nix. I don't want the actual file names to mention the minor version because it makes git history hard to work with (constant renames, and was it just a rename or was anything changed as well?). The simplest solution seems a symlink from 962 to 96, but a solution in Nix itself has my preference. Currently, the same variable ${compilerVersion} is used both to select a GHC and select a file to load the overlay from. The latter needs to be changed somehow.
For this PR, it seems out of scope, though.
[rant]
I think it was a bad idea to concatenate the numbers in version numbers like Nix does here. We're at ghc910 and ghc912 now. Those are still unique, but only thanks to GHC skipping odd numbers, it's dumb luck. But if we ever get to a GHC 9.22.1 release, is ghc922 GHC 9.22 or GHC 9.2.2?
[/rant]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible to strip the minor version of compilerVersion, then we can have it specify the minor version (for selecting the right ghc) and then strip the last character for when it needs to load the correct overlay.
stripMinor = ghcVer: builtins.substring 0 (builtins.stringLength ghcVer - 1) ghcVer;
This only needs to be done inside of clash-compiler, since the flakes in clash-cores and other clash-* projects simply follow whatever clash-compiler does.
@DigitalBrains1 if you're on board, I can make PRs tomorrow pinning clash compiler and clash-* projects down to a minor version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go for it :-)
Pins down the GHC version down to the minor version. As discussed in a PR in clash-cores (clash-lang/clash-cores#44 (comment)). Each supported GHC version is now postfixed with a minor version number. However the overlays are not. The last digit of the version number gets stripped when Nix imports an overlay. This way we can have one set of GHC version numbers pin down the right GHC version *and* import the right overlay. I `nix build .#clash-ghc` with all three versions succesfully.
|
I am a little conflicted on whether or not to delete the old-style Nix files. I didn't touch them at all during this PR, but I have no clue if they actually still work or not. If they don't work, then this seems like a great time to remove them. |
Creates a flake.nix, so that people can easily include clash-cores into their own projects without having dependency issues. It also exposes two developer shells, a minimal and full shell to develop clash-cores.
Yeetus deletus! |
Pins down the GHC version down to the minor version. As discussed in a PR in clash-cores (clash-lang/clash-cores#44 (comment)). Each supported GHC version is now postfixed with a minor version number. However the overlays are not. The last digit of the version number gets stripped when Nix imports an overlay. This way we can have one set of GHC version numbers pin down the right GHC version *and* import the right overlay. I `nix build .#clash-ghc` with all three versions succesfully.
|
Closing this due to me having created a new PR (#46) which achieves the same goal, but better and addresses the issues mentioned here. |
Pins down the GHC version down to the minor version. As discussed in a PR in clash-cores (clash-lang/clash-cores#44 (comment)). Each supported GHC version is now postfixed with a minor version number. However the overlays are not. The last digit of the version number gets stripped when Nix imports an overlay. This way we can have one set of GHC version numbers pin down the right GHC version *and* import the right overlay. I `nix build .#clash-ghc` with all three versions succesfully. (cherry picked from commit 762f7f0) # Conflicts: # flake.nix
Pins down the GHC version down to the minor version. As discussed in a PR in clash-cores (clash-lang/clash-cores#44 (comment)). Each supported GHC version is now postfixed with a minor version number. However the overlays are not. The last digit of the version number gets stripped when Nix imports an overlay. This way we can have one set of GHC version numbers pin down the right GHC version *and* import the right overlay. I `nix build .#clash-ghc` with all three versions succesfully.
Introduces a Nix flake for clash-cores, making it easy to include clash-cores in projects. Aside from clash-cores itself, it also exposes an overlay which makes it easy to add clash-cores to your local project flake without getting weird version conflicts.
THIS REQUIRES A PR IN PROTOCOLS TO BE MERGED FIRST!clash-lang/clash-protocols#162