Skip to content

Conversation

@jaschutte
Copy link
Contributor

Instead of only supporting the 'default' ghc version of clash-compiler,
it now supports the same set of ghc versions as clash-compiler itself.

More about why this is a problem can be found at (clash-lang/clash-compiler#2987).

It also addresses the Cabal-Nix bug where Setup.hs improperly gets flags set. This problem also persists in clash-cores and has been worked around with (https://github.com/clash-lang/clash-cores/blob/main/clash-cores.cabal#L85). However considering it's an issue with Nix specifically, I feel it's more appropriate to patch it within the flake itself.

@jaschutte jaschutte requested a review from diegodiv August 8, 2025 12:29
@jaschutte jaschutte force-pushed the nix-multi-ghc-support branch from 230711a to f788434 Compare August 8, 2025 12:41
@jaschutte jaschutte force-pushed the nix-multi-ghc-support branch from f788434 to b055cc9 Compare August 11, 2025 09:05
@jaschutte
Copy link
Contributor Author

Me and Diego discussed this PR some more in PMs. Nix build works as intended but when running nix develop followed by cabal build does not make use of the circuit-notation package the Nix shell exposes. It manually compiles circuit-notation. This is due to manual source inclusion of circuit-notation inside of cabal.project. Although it's not a big deal now, it might become an issue later on and it's unwanted regardless.

I am not aware of a way to resolve this within Nix, it is caused by Cabal prioritizing sources mentioned in cabal.project over the sources Nix gives it. So the only way to fix this would be to remove it from project.cabal but I assume that will break regular non-Nix compilation, so that's not an option.

I think it's best to have this behavior documented in the README and that's the best we can do, opinions on this @DigitalBrains1 ?

@jaschutte jaschutte force-pushed the nix-multi-ghc-support branch from b055cc9 to 5d87598 Compare August 11, 2025 10:06
@DigitalBrains1
Copy link
Member

I am not aware of a better way to do this than "If you use Nix, remove cabal.project and please make sure not to commit that removal". So yeah, documentation rather than code.

@jaschutte jaschutte force-pushed the nix-multi-ghc-support branch from 5d87598 to 1acc9e5 Compare August 11, 2025 11:40
@jaschutte
Copy link
Contributor Author

I am not aware of a better way to do this than "If you use Nix, remove cabal.project and please make sure not to commit that removal". So yeah, documentation rather than code.

I've added it as a comment above the devShell itself in the flake. I wanted to originally add it to the README but I couldn't find an appropriate place to mention it. (The entire README feels more a guide on Df than a README for the repository in my opinion anyways)

@jaschutte jaschutte force-pushed the nix-multi-ghc-support branch from 1acc9e5 to 8b076fc Compare August 11, 2025 11:46
Copy link

@diegodiv diegodiv left a comment

Choose a reason for hiding this comment

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

It's basically good for me, and I left two suggestions for improvement.

@jaschutte jaschutte force-pushed the nix-multi-ghc-support branch 5 times, most recently from 5c9d2c6 to 5de4704 Compare August 12, 2025 11:43
Instead of only supporting the 'default' ghc version of clash-compiler,
it now supports the same set of ghc versions as clash-compiler itself.

More about why this is a problem can be found at
(clash-lang/clash-compiler#2987)

It also addresses the Cabal-Nix bug where Setup.hs improperly gets flags
set. This problem also persists in clash-cores and has been worked
around with
(https://github.com/clash-lang/clash-cores/blob/main/clash-cores.cabal#L85).
@jaschutte jaschutte force-pushed the nix-multi-ghc-support branch from 5de4704 to e08ca82 Compare August 13, 2025 08:56
Copy link

@diegodiv diegodiv left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@jaschutte jaschutte merged commit 522e7c6 into main Aug 21, 2025
9 checks passed
@jaschutte jaschutte deleted the nix-multi-ghc-support branch August 21, 2025 10:55
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 21, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 22, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 22, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 27, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 27, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 27, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 28, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 29, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in:
clash-lang/clash-protocols#164
jaschutte added a commit to clash-lang/clash-cores that referenced this pull request Aug 29, 2025
Restructures everything Nix related to be more similar to how
clash-protocols and clash-circuits have it setup.

This means it now uses flakes, removes all the 'old style' Nix files and
applies the GHC patches mentioned in clash-cores.cabal prior to this
commit.

All of this is very similar to how it is done in:
clash-lang/clash-protocols#164
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