Skip to content
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: add flake.nix including developer shell #641

Merged
merged 7 commits into from
Oct 10, 2024
Merged

Conversation

aldur
Copy link
Collaborator

@aldur aldur commented Oct 9, 2024

Description

As someone who was configuring how to build this project for the first time, I thought it would be useful to have an easy way to install all build dependencies at once. I gave this a shot with nix.

The result is that if you have nix installed and configured, you can run nix develop to enter a shell having all dependencies installed. Well, all except Smithy, who doesn't have a nix package yet.

Closes: NA

Changes

  • Add flake.nix for development shells.

Testing Information

  • Install nix
  • Make sure you have flakes enabled (the installer linked above takes care of it, else see here)
  • Run nix develop

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@cylewitruk
Copy link
Member

Could you also update the repo readme with information on its (optional) use in the development workflow?

@cylewitruk
Copy link
Member

cylewitruk commented Oct 10, 2024

Lgtm, but I'll tag @djordon and @matteojug for their opinions as well.

@aldur It might be worth taking an hour and writing a package for Smithy? I've never done it, but it doesn't look too involved?

Have you tried the main make commands with the nix shell, do they all work (after installing Smithy manually)?

@AshtonStephens
Copy link
Collaborator

Hey! Smithy shouldn't be used here anymore because we switched to using rust annotations to generate the Emily API template. Where are we needing this?

Copy link
Contributor

@djordon djordon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I don't think we use Smithy anymore so I wouldn't worry about adding it, but I'd wait for @AshtonStephens to verify.

@aldur
Copy link
Collaborator Author

aldur commented Oct 10, 2024

Oh, I wish I had read your comments before 7b8075c. I can make another commit and remove smithy from the README as well?

@AshtonStephens
Copy link
Collaborator

Yeah I'm sorry, I should have updated the readme at the same time as I removed it. Though, smithy is used to generate some of the CLIs that we pull in as crates - I'd assume there's not dependency on smithy for those but I'm wondering if you somehow got smithy related errors

@aldur
Copy link
Collaborator Author

aldur commented Oct 10, 2024

Yeah I'm sorry, I should have updated the readme at the same time as I removed it. Though, smithy is used to generate some of the CLIs that we pull in as crates - I'd assume there's not dependency on smithy for those but I'm wondering if you somehow got smithy related errors

Oh no I didn't (and was wondering why :)), but I still wanted to provide a full package.

@aldur
Copy link
Collaborator Author

aldur commented Oct 10, 2024

Have you tried the main make commands with the nix shell, do they all work (after installing Smithy manually)?

Yep, they all run. Three integration tests fail, but that does not seem related to my changes:

running 3 tests
test validation::minimal_push_check ... FAILED
test validation::op_csv_disabled ... FAILED
test validation::tx_validation_from_mempool ... FAILED

failures:

---- validation::minimal_push_check stdout ----
thread 'validation::minimal_push_check' panicked at sbtc/src/testing/regtest.rs:113:18:
called `Result::unwrap()` on an `Err` value: JsonRpc(Transport(SocketError(Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- validation::op_csv_disabled stdout ----
thread 'validation::op_csv_disabled' panicked at sbtc/src/testing/regtest.rs:113:18:
called `Result::unwrap()` on an `Err` value: JsonRpc(Transport(SocketError(Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })))

---- validation::tx_validation_from_mempool stdout ----
thread 'validation::tx_validation_from_mempool' panicked at sbtc/src/testing/regtest.rs:113:18:
called `Result::unwrap()` on an `Err` value: JsonRpc(Transport(SocketError(Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })))


failures:
    validation::minimal_push_check
    validation::op_csv_disabled
    validation::tx_validation_from_mempool

test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

...but on the fact that I was not running Docker and the required services 🤦 fixing that.

Confirmed, they all run.

@cylewitruk
Copy link
Member

cylewitruk commented Oct 10, 2024

but that does not seem related to my changes

Yeah, you didn't change anything there so that's unrelated to you.

The PR author gets the honors of clicking "Squash and merge" in this repo :)

@cylewitruk cylewitruk added the enhancement New feature or request label Oct 10, 2024
@aldur aldur merged commit 2059739 into main Oct 10, 2024
4 checks passed
@aldur aldur deleted the chore/nix_dev_shell branch October 10, 2024 14:00
@cylewitruk
Copy link
Member

merged commit 2059739 into main

Wow, not every day you get to see a fully-numeric commit hash (prefix) :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants