-
Notifications
You must be signed in to change notification settings - Fork 2k
chore: update nix flake #10485
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
chore: update nix flake #10485
Conversation
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.
unfamiliar with this or how to review
but this changes seem appropriate from what a can decipher
anyone familiar with nix @grandizzy @zerosnacks @yash-atreya ?
@mattsse ah didn't notice that PR yup, can merge it first and i'll update this PR after |
I can also update it based on this #9260 and add CI for the flake |
flake.nix
Outdated
solc_0_8_27 | ||
# default for foundry's tests | ||
(solc.mkDefault pkgs solc_0_8_27) |
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.
The latest solc is now 0.8.30
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.
unstable unfortunately doesn't have 0.8.30 yet but i've just updated it to use solc directly from nixpkgs instead of needing on overlay per @beeb suggestion
Hey! I think there are some interesting points in this PR as well as #10316. Ideally we would merge them. Which one should we use to merge those ideas @zerosnacks ? |
Co-authored-by: sveitser <[email protected]>
@beeb Thanks!
Let's use this one to aggregate the proposals as this one has momentum right now I've copied the proposed CI flow with some minor modifications and pushed it to this branch with the original author included as co-author @shaunkh would you mind integrating changes you see fit from https://github.com/foundry-rs/foundry/blob/27279a03744124b42e166b2d4d8f4fdd3ffbd7bb/flake.nix? |
Will review shortly |
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 looking good! Small question
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.
LGTM! Thanks
@zerosnacks pushed changes, lmk what you think |
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.
lgtm 👍
I haven't been able to test the CI workflow locally w/ act
but will manually test after merging, looks good to me
# Environment variables | ||
RUST_SRC_PATH = "${toolchain}/lib/rustlib/src/rust/library"; | ||
LD_LIBRARY_PATH = lib.makeLibraryPath [ pkgs.libusb1 ]; | ||
CFLAGS = "-DJEMALLOC_STRERROR_R_RETURNS_CHAR_WITH_GNU_SOURCE"; |
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.
Ref for reviewers: https://github.com/foundry-rs/foundry/pull/9848/files, it is a fix for building tikv-jemalloc-sys
Any progress on this? @zerosnacks @mattsse |
Hi @beeb thanks for the ping - will follow up shortly, no blockers on my end |
Merging, manually reviewed again together with @grandizzy |
Hi, it unfortunately looks like the CI step to compile is failing: https://github.com/foundry-rs/foundry/actions/runs/15585038634/job/43889031628 It looks like we it is using Rust
Additionally it would be an improvement if the CI would fetch and build prior to creating a PR to bump the I'll leave the PR merged for now, I'll disable the weekly action however as it is currently failing until a fix has been merged |
@zerosnacks |
The nix flake dev shell needed some updating.