-
Notifications
You must be signed in to change notification settings - Fork 551
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
flake.nix: add openroad-release attrubute #6004
base: master
Are you sure you want to change the base?
Conversation
Commits need to be signed with 'git commit -s' Please see the details link by DCO for how to fix it. @donn I have no opinion on this as it is just for OL usage. |
clang-tidy review says "All clean, LGTM! 👍" |
e9e7b61
to
d80af7c
Compare
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.
Overall looks okay. We don't use this flake though, it is purely convenience for developing in Nix environments (i.e. me working on OpenROAD), for the OpenLanes we build OpenROAD out-of-tree, with submodules factored out and built standalone (+ some sed
s to support that): https://github.com/efabless/openlane2/blob/1bfa43d34dcab6f859bb699b182948eda91884e2/nix/openroad.nix
The reason is having this entire repository be a flake is a download in the order of hundreds of megabytes during Nix evaluation time, which slows things down. Additionally, a recursive fetchFromGitHub
on this repo is a download of a couple gigabytes in addition to that. Nevertheless, this is strictly more useful in some scenarios than the status quo so I'd still merge it with those minor tweaks.
fetchSubmodules = true; | ||
sha256 = "sha256-Ye9XJcoUxtg031eazT4qrexvyN0jZHd8/kmvAr/lPzk="; | ||
}; | ||
}); | ||
default = all.openroad; |
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 default should be openroad-release
because it'd actually work with nix run
/etc… @widlarizer thoughts?
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 didn't change the default because I didn't want to disrupt anybody's flow, but I agree that it would make sense. In any case it's still possible to use all nix commands by adding #openroad-release
.
Signed-off-by: Federico Beffa <[email protected]>
d80af7c
to
3373c3f
Compare
The current
flake.nix
openroad
attribute can't be imported into other flakes for consumption nor used to installopenroad
. This is due to a limitation innix
that doesn't fetch submodules (with the exception of self with '.?submodules=1'). Due to this limitation, peoples usingnix
wanting to installopenroad
have tonix
to build or installopenroad
from the git clone using a special undocumented flag.Step 2. copies the git source code in the nix store ending up with two copies of a large repo on disk.
To remedy this we propose to add a second attribute
openroad-release
usingfetchFromGitHub
which is capable to retrieve submodules and pointing to a git commit (tag) known to work. (I tried to compile HEAD with thenix
flake without success.)@donn