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

Introduce a basic justfile #2018

Merged
merged 1 commit into from Sep 3, 2023
Merged

Conversation

tcharding
Copy link
Member

Introduce usage of just by adding a basic justfile.

If this goes in we can add various script invocations to it plus other useful things that often get red CI runs (eg, checking no-std).

@tcharding
Copy link
Member Author

Props to @dpc for the idea.

@apoelstra
Copy link
Member

LGTM. I think that just format should actually run cargo fmt --check ... but probably no matter what I will never call that command because I won't be sure whether it'll be destructive or not.

apoelstra
apoelstra previously approved these changes Aug 22, 2023
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 9f92480

justfile Outdated

# Run the formatter
format:
cargo +nightly fmt --all
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it from Fedimint or do you actually expect people to run nightly formatter?

Copy link
Member

Choose a reason for hiding this comment

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

@dpc we have to run the nightly formatter because the stable one butchers the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, OK. In Fedimint the nix dev shell sets up toolchain with stable compiler, but nightly rustfmt. But motivation was mostly some sweet unstable formatting options.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, same here. Pretty nice that Nix can do this automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can achieve the same thing in guix @stevenroose?

Copy link
Member

Choose a reason for hiding this comment

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

@tcharding it's doubtful because "nightly" is not a well-defined thing. When you use nightly compilers in Nix then you'll get different results on different invocatons, something that Nix allows because it's "pragmatic" while Guix does not because it takes reproducibility seriously.

Copy link
Contributor

@dpc dpc Aug 24, 2023

Choose a reason for hiding this comment

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

@apoelstra

cough, cough

I will have you know that Nix takes reproducibility seriously, and there's nothing "ill-defined" about nightly.

In Fedimint rustfmt comes from nightly toolchain via fenix project input that is locked in flake.lock which includes checking its cryptographic checksum . When we decide it's time to update toolchains, we nix flake lock --update-input fenix and check-in newly pinned versions.

Fedimint artifacts produced via Nix are byte-reproducible, which we did absolutely nothing in particular to achieve.

You can verify it by going to some master branch build, taking the git commit (here: 87d68d8356c1d56c27c91de4fc70ee0952b07b13) running:

nix build 'github:fedimint/fedimint?rev=87d68d8356c1d56c27c91de4fc70ee0952b07b13#fedimint-pkgs' && sha256sum result/bin/*

waiting approximately forever, and then comparing the checksums with ones printed by the CI:

image

But yes, AFAICT Nix is more pragmatic. We can run Steam, systemd and use configuration language with the usual number of parenthesis.

sanket1729
sanket1729 previously approved these changes Aug 23, 2023
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

utACK 9f92480. Don't know much of just and I don't plan to use this, but if people find this handy we should go ahead and merge this.

@tcharding
Copy link
Member Author

Updated format command to use --check. No other changes.

@yancyribbens
Copy link
Contributor

yancyribbens commented Aug 23, 2023

Don't know much of just and I don't plan to use this, but if people find this handy we should go ahead and merge this.

It looks like a Makefile where for example just format when typed will now run cargo +nightly fmt --all --check which is cool if I understand correctly. Would it be a good idea to add something to the README for these commands?

@tcharding
Copy link
Member Author

Added a brief mention to the README.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 9d53469

Introduce usage of `just` by adding a basic `justfile`.
Copy link
Collaborator

@RCasatta RCasatta left a comment

Choose a reason for hiding this comment

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

ACK eccd3fe

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK eccd3fe

@apoelstra apoelstra merged commit 8e573f4 into rust-bitcoin:master Sep 3, 2023
29 checks passed
@tcharding tcharding deleted the 08-22-just branch September 6, 2023 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants