Various fixes for Nix compatibility #538
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
This lets you run all the tests with `cargo test --all`. It also causes all crates to share a lockfile, for better or worse. We cannot also bring 'embedded' as a workspace because it doesn't actually build on desktop CPUs.
Also fmt embedded, since in an earlier iteration of this PR, it was a new workspace. (Total diff: 2 lines.)
0.5.0 doesn't work.
apoelstra
force-pushed
the
2023-04--fix-bitcoind
branch
from
6ad80e8 to
f50cbbb
Compare
Merged
sanket1729
added a commit
that referenced
this pull request
May 9, 2023
a6405f8 fuzz: replace README.md (Andrew Poelstra) 7b04860 fuzz: delete old ./travis-fuzz.sh (Andrew Poelstra) 8e8dcb4 fuzz: remove afl support, run clippy and fmt on fuzz targets (Andrew Poelstra) 14ceccd fuzz: remove existing CI; run fuzz/generate-files.sh to make new one (Andrew Poelstra) 6fea17f fuzz: copy *.sh files from rust-bitcoin; tweak generate-files.sh (Andrew Poelstra) Pull request description: Followup to #538. Copies the new fuzzing scripts from rust-bitcoin and cleans some things up. Confirmed that I can run all these fuzztests in my local nix-based CI. ACKs for top commit: sanket1729: utACK a6405f8 Tree-SHA512: f19fa7a62e244aaebf3ac0a0bcfe1686392eca3679d71edd8d6504888bf1a5004a7527fa46e8ee6296be4a27dfc20408a8ea258569f6337a73f41118d751a451
gruve-p
pushed a commit
to Groestlcoin/rust-miniscript
that referenced
this pull request
Aug 28, 2023
f50cbbb set honggfuzz dependency to 0.5.55 (Andrew Poelstra) 73bcffc cargo fmt new workspaces (bitcoin-tests, fuzz) (Andrew Poelstra) cd0abcd bitcoind-tests: find the bitcoind executable no matter where we are running from (Andrew Poelstra) bf98e2a bitcoind-tests: allow overriding the BITCOIND_EXE variable (Andrew Poelstra) 54d7657 promote bitcoind-tests and fuzz to workspaces (Andrew Poelstra) Pull request description: This brings the fuzztests and bitcoind-tests into workspaces along with the root crate. This means they are covered by `cargo test --all` and `cargo fmt --all` without any special action, and also means that `crate2nix` can test them independently without my manually messing around with directories. I run `cargo +nightly fmt` to bring the two new workspaces into proper formatting -- there were no changes that I'd consider controversial. It also allows the `bitcoin-tests` directory to be run with a custom `BITCOIND_EXE` variable, and computes the default version more flexibly so that you can run the tests from any directory. Finally I set the minimum honggfuzz version to 0.5.55, since the dep currently was set to 0.5.0, which definitely did not work. With these changes, everything in this repo should work with a lockfile generated by `cargo +nightly update -Z minimal-versions`, and I am able to run both the bitcoind and fuzz tests (for five minutes per unit locally). I'll push some code to my [local-nix-ci repo](https://github.com/apoelstra/local-nix-ci) tomorrow to demonstrate whan I'm doing. This PR does **not** do more dramatic rearrangemeent of the fuzztests ... though when rust-bitcoin/rust-bitcoin#1732 gets merged I will open a similar PR here which does that. ACKs for top commit: sanket1729: ACK f50cbbb. Tree-SHA512: a234f9f09ad059b051b2336738a1f8fb4d807af4523380037c8121173cfd05ab0deb567b92abfae3fc794d9ad91966f43d7396623a453df242bc5392a1bcb489
gruve-p
pushed a commit
to Groestlcoin/rust-miniscript
that referenced
this pull request
Aug 28, 2023
a6405f8 fuzz: replace README.md (Andrew Poelstra) 7b04860 fuzz: delete old ./travis-fuzz.sh (Andrew Poelstra) 8e8dcb4 fuzz: remove afl support, run clippy and fmt on fuzz targets (Andrew Poelstra) 14ceccd fuzz: remove existing CI; run fuzz/generate-files.sh to make new one (Andrew Poelstra) 6fea17f fuzz: copy *.sh files from rust-bitcoin; tweak generate-files.sh (Andrew Poelstra) Pull request description: Followup to rust-bitcoin#538. Copies the new fuzzing scripts from rust-bitcoin and cleans some things up. Confirmed that I can run all these fuzztests in my local nix-based CI. ACKs for top commit: sanket1729: utACK a6405f8 Tree-SHA512: f19fa7a62e244aaebf3ac0a0bcfe1686392eca3679d71edd8d6504888bf1a5004a7527fa46e8ee6296be4a27dfc20408a8ea258569f6337a73f41118d751a451
apoelstra
added a commit
to ElementsProject/elements-miniscript
that referenced
this pull request
Aug 9, 2024
fb73306 avoid setting {BITCOIND,ELEMENTSD}_EXE in setup (Riccardo Casatta) Pull request description: The logic of setting the env var inside the setup prevents user of other OS like nixos to run the tests (because the included binary are not working on nixos) and also to run other specific versions. Setting those env vars is meant to be done in the dev environment not in the code, so this remove the setting of the env vars in the code. With this users with `bitcoind` and `elementsd` in their PATH will work too. The CI script will set the variables with the included binary only if they are not already set. This allows the CI to run while allowing other users to override with specific versions and run it locally. Note this is different from what has been done upstream rust-bitcoin/rust-miniscript#538 but I think it's better ACKs for top commit: apoelstra: ACK fb73306; the integration tests work for me in Nix; though after fully enabling my local CI there are a bunch of clippy and other issues everywhere :) Tree-SHA512: 766a50733db49abb21697733a061d0dfa3ff16f1dfc7562ac943c21d9ffea9924110ec723b390133e3057205612aaa2d0104de80b0fd1c68a6b0581010668668
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.
This brings the fuzztests and bitcoind-tests into workspaces along with the root crate. This means they are covered by
cargo test --allandcargo fmt --allwithout any special action, and also means thatcrate2nixcan test them independently without my manually messing around with directories. I runcargo +nightly fmtto bring the two new workspaces into proper formatting -- there were no changes that I'd consider controversial.It also allows the
bitcoin-testsdirectory to be run with a customBITCOIND_EXEvariable, and computes the default version more flexibly so that you can run the tests from any directory.Finally I set the minimum honggfuzz version to 0.5.55, since the dep currently was set to 0.5.0, which definitely did not work.
With these changes, everything in this repo should work with a lockfile generated by
cargo +nightly update -Z minimal-versions, and I am able to run both the bitcoind and fuzz tests (for five minutes per unit locally). I'll push some code to my local-nix-ci repo tomorrow to demonstrate whan I'm doing.This PR does not do more dramatic rearrangemeent of the fuzztests ... though when rust-bitcoin/rust-bitcoin#1732 gets merged I will open a similar PR here which does that.