bdk icon indicating copy to clipboard operation
bdk copied to clipboard
verified publisher icon bitcoindevkit

ci: move to Nix

Open storopoli opened this issue 2 years ago • 7 comments

Description

This would make all the tests possible to run locally. It enhances developer experience and facilitates onboarding of new contributors.

(Updated MSRV to 1.63)

Additions:

  • Crate auditing capabilities using rustsec/advisory-db.
  • Local and CI caching of all artifacts from running cargo {check,build,test} in the whole workspace. Superseds .github/workflows/audit.yml.
  • Pre-commit checks for everything that we enforce (gpg-signed/conventional commits, and some goodies like typos, so docs: fix spelling errors #1086 kind of things are not necessary anymore)
  • Instructions and rationale in NIX.md

Closes #1162. Superseds #1122, #1156, #1165.

Pinneds Dependencies:

TODO:

  • [x] Fix llvm deps.

  • [x] Fix openssl errors.

  • [x] Fix dependencies on MacOS.

  • [x] Fix WASM.

  • [x] Fix MSRV. Add a custom CargoMSRV.lock? (Or maybe a CargoMSRV.toml with the pinned MSRV dependencies and then cargo generate-lockfile with Rust MSRV?) Depends on solving fix: build crane-utils using a different toolchain ipetkov/crane#422

  • [x] move all the logic from cont_integration.yml to flake.nix (all the cargo update -p) From the Fedimint suggestion we'll use crane. This would allow caching of a lot of things Then the user would call nix buid -L .#MSRV --keep-failed and so on. This would also eliminate all the multiple runs-on in cont_integration.yml to a single one where each step would be a name and run the nix build command.

  • [x] Add DeterminateSystems/magic-nix-cache-action to cache stuff in CI.

  • [x] Update CONTRIBUTING with instructions. Use fedimint instructions for inspiration.

  • [x] nix develop:

    • [x] default: Rust latest
    • [x] MSRV
    • [x] WASM

  • [x] nix flake check:

    • [x] clippy

    • [x] audit

    • [x] rustfmt

    • [x] Rust latest test

      • [x] --all-features
      • [x] --no-default-features

    • [x] MSRV test

      • [x] --all-features
      • [x] --no-default-features

    • [x] WASM (--target wasm32-unknown-unknown) test

      • [x] -p bdk --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown,dev-getrandom-wasm
      • [x] -p esplora --target wasm32-unknown-unknown --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown,async

    • [x] cachix/pre-commit-hooks.nix

      • [x] signed commits
      • [x] conventional commits with commitizen
      • [x] typos
      • [x] rustfmt

  • [ ] ~Add a CI to update Cargo.lock~ (is this possible?)

  • [x] Add a CI to update flake.lock

  • [x] Delete .github/workflows/audit.yml

  • [x] Port Code Coverage to Nix

  • [x] Nixify the Nightly Docs CI

  • [ ] Add numtide/devshell

  • [x] Clean up commit messages (this is a mess of squash fixup and ammends 😂)

Nix Commands

  • nix flake show: show all possible commands from the flake.

  • nix develop: creates a devShell with all the things you need to develop installed. Also handles ENV vars. Currently is bash, git, ripgrep, bitcoind (pinned), electrs (pinned), openssl, pkg-config, libiconv, and latest stable Rust with all goodies. It also handles specific MacOS deps: Apple XCode SDKs (for bitcoind and electrsd crates). Open to suggestions on what to include.

    • nix develop .#MSRV: same but with Rust MSRV.

  • nix flake check:

    • checks for typos, gpg-signed commits, conventional commits, rustfmt, nixpkgs-fmt (.nix files).
    • runs clippy in all workspace with --all-features --all-targets -- -D warnings.
    • runs cargo check in all workspace (latest stable Rust).
    • checks dependencies for security advisory using rustsec/advisory-db.
    • test everything!!! (see below the individual checks in .#checks.${system}.{CHECK}

  • nix build -L .: runs cargo build in all workspace with latest stable Rust.

    • nix build -L .#stable: runs cargo build in all workspace with latest stable Rust.
    • nix build -L .#MSRV: runs cargo build in all workspace with MSRV stable Rust.
    • PLACEHOLDER: ...

  • nix build -L .#checks.${system}.{CHECK}: runs a specific CHECK. In my case system = aarch64-darwin then it is nix build .#checks.aarch64-darwin.CHECK.

    • pre-commit-check: checks for typos, conventional commits, nixpkgs-fmt (.nix files).
    • clippy: runs clippy in all workspace with --all-features --all-targets -- -D warnings.
    • fmt: runs cargo fmt with --all -- --check --config format_code_in_doc_comments=true in all workspace with latest Rust.
    • audit: checks dependencies for security advisory using rustsec/advisory-db.
    • latest: cargo build in whole workspace using latest Rust.
    • latestAll: cargo test --all-features in whole workspace using latest Rust.
    • latestNoDefault: cargo test --no-default-features in whole workspace using latest Rust.
    • latestNoStdBdk: cargo test -p bdk --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown using latest Rust.
    • latestNoStdChain: cargo test -p bdk_chain --no-default-features --features bitcoin/no-std,miniscript/no-std,hashbrown using latest Rust.
    • latestNoStdEsplora: cargo test -p bdk_esplora --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown using latest Rust.
    • MSRV: cargo build in whole workspace using MSRV Rust.
    • MRSVAll: cargo test --all-features in whole workspace using MSRV Rust.
    • MSRVNoDefault: cargo test --no-default-features in whole workspace using MSRV Rust.
    • MSRVNoStdBdk: cargo test -p bdk --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown using MSRV Rust.
    • MRSVNoStdChain: cargo test -p bdk_chain --no-default-features --features bitcoin/no-std,miniscript/no-std,hashbrown using MSRV Rust.
    • MSRVNoStdEsplora: cargo test -p bdk_esplora --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown using MSRV Rust.

Notes to the reviewers

  1. We are adding automatic pre-commit checks with checks for typos, gpg-signed commits, conventional commits, nixpkgs-fmt (.nix files). This is done automatically if the user has Nix and devshell installed. If not, it will be checked on CI, or with a nix flake check. I fixed a bunch of typos and added the .typos.toml to whitelist somethings like hashes, addresses, etc that were being flagged as false positives.

  2. I am bumping hashbrown to 0.11.2 since it is compatible with MSRV of 1.63

  3. I am adding in the building cache/tests a crate name and version. This does not interact with the name or versioning in any of bdk's crates Cargo.toml To avoid this nasty warning in Nix:

    trace: warning: crane will use a placeholder value since `name` cannot be found in /nix/store/ja4yr1m9ba72gy3pbr2vg540zjdbzp33-source/Cargo.toml
to silence this warning consider one of the following:
- setting `pname = "...";` in the derivation arguments explicitly
- setting `package.name = "..."` or `workspace.package.name = "..."` in the root Cargo.toml
- explicitly looking up the values from a different Cargo.toml via
  `craneLib.crateNameFromCargoToml { cargoToml = ./path/to/Cargo.toml; }`

  4. We are using legacyPackages instead of packages in the ci build calls because legacyPackages allows nested sets, e.g.:

    legacyPackages = ci = {
  latest = {
    all = ...;
    noDefault = ...;
    ...
  };
  MSRV = { ... };
  WASM = { ... };
  codeCoverage = ...;
};

    It makes a nice grouping of all CI stuff under the same call:

    nix build -L .#ci.latest.{CHECK}

  5. We are moving from mozilla/grcov to taiki-e/cargo-llvm-cov. Why? 3 reasons:

    1. Mozilla's grcov is a big thing, it does coverage for a lot of things C/C++, JS, Java, and Rust. cargo-llvm-cov uses Rust's native coverage tools via LLVM.
    2. crane (craneLib.cargoLlvmCov) supports natively cargo-llvm-cov which will be much easier to make it work and maintain
    3. Our friends at fedimint also use cargo-llvm-cov with crane so it makes easier to collaborate in improvements and issues.

Potential issues:

  • We had to remove Cargo.lock from the .gitignore. Nix (crane) needs it for deterministic stuff. From the crane FAQ:

    First consider if there is a release of this project available with a lock file as it may be simpler and more consistent to use the exact dependencies published by the project itself. Projects published on crates.io always come with a lock file and nixpkgs has a fetchCrate fetcher which pulls straight from crates.io.

    Also Rust changed their Cargo.lock commit policy a couple months ago:

    For years, the Cargo team has encouraged Rust developers to commit their Cargo.lock file for packages with binaries but not libraries. We now recommend people do what is best for their project. To help people make a decision, we do include some considerations and suggest committing Cargo.lock as a starting point in their decision making. To align with that starting point, cargo new will no longer ignore Cargo.lock for libraries as of nightly-2023-08-24. Regardless of what decision projects make, we encourage regular testing against their latest dependencies.

  • Mismatch versions between the executables under the *_EXEC env vars for bitcoind/electrsd crates and the version the crate thinks to have.

  • Centralizes CI maintainability to people that have Nix experience.

  • We are removing the auto-download feature of bitcoind and electrsd in the bitcoind_rpc and esplora crates. I added an explanation in the repo and crates' README.mds. This also simplifies a little bit the MSRV pinning of deps.

Changelog notice

Checklists

All Submissions:

  • [x] I've signed all my commits
  • [x] I followed the contribution guidelines
  • [x] I ran cargo fmt and cargo clippy before committing

New Features:

  • [x] I've added tests for the new feature
  • [x] I've added docs for the new feature

Bugfixes:

  • [ ] This pull request breaks the existing API
  • [x] I've added tests to reproduce the issue which are now passing
  • [x] I'm linking the issue being fixed by this PR

storopoli avatar Jan 06 '24 08:01 storopoli

How does one update the CargoMSRV.lock file?

After the version of bip39 in the root Cargo.toml file changed to 2.0, I encounter an error when running nix flake -L check --keep-failed

It appears to involve the 1.2.0 version that's in CargoMSRV.lock.

I tried changing the version to 2.0.0 in the root Cargo.toml

I ran the command nix flake update but that didn't update the CargoMSRV.lock file.

I tried removing this lock file, and that didn't work either.

I'm relatively new to nix. Perhaps instructions on handling such version updates in Cargo.toml that involve CargoMSRV.lock would be helpful in the NIX.md file.

Here is the code I started with, rebased to include recent commits to master

casey-bowman avatar Jan 09 '24 07:01 casey-bowman

It is not that trivial.

  1. flake.nix: in the MSRV devshell change cargoArtifacts = null; (so that it won't trigger a broken compile when you enter this shell)
  2. enter the MSRV devshell with nix develop .#MSRV
  3. rm Cargo.lock
  4. cargo update (then you might need to also pin some deps)
  5. once the thing builds you can mv Cargo.lock CargoMSRV.lock
  6. reset the change in cargoArtifacts in flake.nix
  7. reset the removed Cargo.lock
  8. try nix flake -L check --keep-failed
  9. commit the new CargoMSRV.lock

I am planning to fix this and rebase today. This is the workflow that'll be doing.

storopoli avatar Jan 09 '24 07:01 storopoli

Thanks, @storopoli !

casey-bowman avatar Jan 09 '24 07:01 casey-bowman

Last night I ran nix flake -L check --keep-failed on commit 2105f99

It ran successfully, except for one set of errors in the middle. I am attaching those errors, from somewhere in the middle of the console output, and a warning at the end of the console output showing which systems were not tested.

The check omitted these incompatible systems: aarch64-darwin, aarch64-linux, x86_64-darwin

The errors were all of the form

crates-audit> error: couldn't check if the package is yanked: not found: No such crate in crates.io index: addr2line

errors.txt

Here are the details of my x86-64 system -

Operating System - NixOS 24.05 (Uakari) Hardware Model - Purism Librem 13 v2 Processor - Intel Core i5-6200U x 4 Memory - 31.3 GB

It's amazing what ground that flake covers!

casey-bowman avatar Jan 09 '24 17:01 casey-bowman

these are http errors in the cargo-audit check. I am running these fine in my local machine and also in GitHub CI. Maybe your machine is offline or you are blocking crates.io?

storopoli avatar Jan 10 '24 14:01 storopoli

these are http errors in the cargo-audit check. I am running these fine in my local machine and also in GitHub CI. Maybe your machine is offline or you are blocking crates.io?

nix build etc. runs in a sandbox and network calls are forbidden there.

These errors might be normal/to be ignored. The advisory-db is passed as a locked input, and not being able to check for yanked crates is just a limitation of running it in Nix, I guess.

BTW. In Fedimint we run the audit by updating the input to the latest version and then doing the update, to always get the audit against current advisories. https://github.com/fedimint/fedimint/blob/59a88ce18c1721acec0db89c6bb97d9ea18af0d9/.github/workflows/ci-nix.yml#L169

dpc avatar Jan 10 '24 16:01 dpc

BTW. In Fedimint we run the audit by updating the input to the latest version and then doing the update, to always get the audit against current advisories.

Thanks for the tip I added this as well to our CI yml file

storopoli avatar Jan 11 '24 10:01 storopoli

Closing in favor of #1320. Cc @yanganto.

storopoli avatar Mar 25 '24 13:03 storopoli