yosys: use external abc and simplify derivation #79007
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some consumers of abc-verifier require certain abc versions. For that reason, expose the exact rev via a passtrhru.
|
yosys is designed to be used with a specific pinned version of ABC, so this isn't a good idea unless the ABC package is going to be kept at that same version. Edit: Ah, I see there's already a check for that in place. Never mind then! |
|
I didn't see anything else using it (except people might invoke abc directly).
The PR introduces a check for compatibility, and fails if the rev doesn't match.
We could also do an overrideAttrs to explicitly hardcode abc's src in yosys, but I'm not sure that's necessary.
|
emilazy
approved these changes
Feb 1, 2020
| # checkPhase defaults to VERBOSE=y, which gets passed down to abc, | ||
| # which then does $(VERBOSE)gcc, which then complains about not | ||
| # being able to find ygcc. Life is pain. | ||
| checkFlags = [ " " ]; |
There was a problem hiding this comment.
Are you sure this is no longer needed? Looks good to me other than that!
There was a problem hiding this comment.
Well, we don't build abc in here anymore, but pass it in externally. So we don't need to worry about what was being passed to the vendored abc build. Right? ;-)
There was a problem hiding this comment.
Right, sorry, seems I haven't yet woken up today >.<
anna328p
pushed a commit
to anna328p/nixpkgs
that referenced
this pull request
Feb 2, 2020
yosys: use external abc and simplify derivation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
The yosys derivation currently is very hard to override and more complex than it needs to be.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)cc @heijligen @piegames