search

clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.4k stars 147 forks source link

clash-cores: Add missing `TryDomain` for CRCs in the catalog #2742

Closed rowanG077 closed 6 days ago

DigitalBrains1 commented 6 days ago

We just ran into this, clash-cores:doctests did not work with multiple hidden, which we tripped over in nix-shell.

We should add clash-cores:doctests to CI (using Cabal).

And I wonder why the nix-build CI job runs, to name one, clash-prelude:unittests, but not clash-cores:doctests.

rowanG077 commented 6 days ago

I know that motivated this PR :)

DigitalBrains1 commented 6 days ago

You're basically our chef Nix; do you have the time to see why our nix-build CI job doesn't seem to run for clash-cores:doctests even though it runs for, for instance, clash-prelude:doctests? I'm just testing locally on an Ubuntu machine, invoking nix-build, but that's what I'm seeing if I deliberately introduce failing tests.

rowanG077 commented 6 days ago

nix-build builds clash-ghc which means clash-cores is not in the dependency chain. So clash-cores doesn't get build at all, See:

https://github.com/clash-lang/clash-compiler/blob/b972a79f2ceeb4f9784f3dfdf44f08fbf06cdb19/flake.nix#L102-L103

But it is for nix-shell since the dev shell explicitly brings the inputs from clash-testsuite into scope which includes clash-cores:

https://github.com/clash-lang/clash-compiler/blob/b972a79f2ceeb4f9784f3dfdf44f08fbf06cdb19/nix/devshell.nix#L6-L21

DigitalBrains1 commented 6 days ago

Thanks! We'll need to decide whether that is the behaviour we are looking for.

What would be the deciding factor for me is: would building and testing under Nix have a fair chance of finding mistakes that would not be found by the tests we do with Cabal? If yes, let's test more under Nix. Otherwise, it might not be a good ratio between CPU time for the test and the odds of it finding actual issues.

rowanG077 commented 6 days ago

I''m happy with nix build. You can specify the target you want to build explicitly if you want. nix build .#clash-cores for example will build clash-cores. It's just that we currently only build the default package which is set as clash-ghc.

For the shell I'm less happy. It requires the build of allmost packages inside the repo which takes ages. It makes little sense that if I want to develop the prelude I need to first compile prelude before I get into the shell for example. Ideally I would want it to just build the dependencies. And you can then build any of the packages in clash-compiler inside the shell.

DigitalBrains1 commented 6 days ago

I''m happy with nix build. You can specify the target you want to build explicitly if you want. nix build .#clash-cores for example will build clash-cores. It's just that we currently only build the default package which is set as clash-ghc.

It sounds like you're reasoning about local usage, whereas I'm reasoning purely about CI. I'm just wondering: what behaviour do we want in CI? I can't "specify the target" in CI, it just runs what it runs and that's it.