-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
verilator: set doCheck to true #109493
verilator: set doCheck to true #109493
Conversation
run the basic compiler/simulator checks
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package failed to build and are new build failure:
|
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package failed to build and are new build failure:
|
Thanks! This is something I've wanted to get around to. When the CI looks good we can go ahead. I seem to remember trying to enable this at some point and it looking a little hairy a long time ago, but it might have gotten better. We can also always send patches upstream, too... |
This is my first time trying to dig deeper into |
@thoughtpolice GH says all checks have passed (do we ignore the "neutral checks" that failed?). I don't see any new failure emails from |
No, because it takes a long time and especially in big PRs keeps ofborg busy. Testing locally is always preferred.
You can use virtualbox or vagrant if you like https://github.com/nix-community/nixbox.
That should be the same or similar checks that failed for me. I honestly don't know why they are neutral but the tests need to pass for amd64 linux at least. For darwin we can disable them.
I am just busy with a lot of PRs and didn't have the time to take another look at this. |
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package failed to build and are new build failure:
|
I marked this as stale due to inactivity. → More info |
@yangm2 Hi there, I saw you closed this. Sorry for letting it slide. I was able to look into this when I saw the notification, and also saw that Verilator happily released 5.x recently, so I took the liberty to resurrect your patch to enable the testsuite: see #199068 Sorry for the (super) long wait! |
Awesome, thanks! |
Based on the work from yangm2 in NixOS#109493. Co-authored-by: yangm2 <[email protected]> Signed-off-by: Austin Seipp <[email protected]>
Based on the work from yangm2 in #109493. Co-authored-by: yangm2 <[email protected]> Signed-off-by: Austin Seipp <[email protected]>
Motivation for this change
Follow-on to #109249 enabling doCheck for Verilator
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)