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

feat: Add nix/flake support #3743

Merged
merged 3 commits into from
May 2, 2023
Merged

feat: Add nix/flake support #3743

merged 3 commits into from
May 2, 2023

Conversation

chives101
Copy link
Contributor

@chives101 chives101 commented Apr 13, 2023

Motivation

This PR enables power users to easily and reliably build the project from source code.

Expectation

A user with the nix package manager installed can build the project with the following command:

nix build github:mimblewimble/grin

and expect the resulting binaries in ./result/bin.

Testing

If flake hasn't been enabled, use nix build --extra-experimental-features 'nix-command flakes' in place of nix build.

A test run for this PR can be achieved using my forked repo with the following command:

nix build github:chives101/grin/flake and expect a binary at ./result/bin/grin.

Or by entering the project folder and do nix build after applying the PR.

Dev notes

This PR is the result of a simple adaptation of the code from this blog post:

https://www.tweag.io/blog/2022-09-22-rust-nix/

@chives101 chives101 force-pushed the flake branch 2 times, most recently from 1c5f73e to dc3cd2f Compare April 13, 2023 12:12
@chives101 chives101 force-pushed the flake branch 2 times, most recently from 9fe7301 to bbbf949 Compare April 13, 2023 12:55
@chives101 chives101 changed the title Add nix/flake support feat: Add nix/flake support Apr 13, 2023
@chives101 chives101 force-pushed the flake branch 3 times, most recently from b792b32 to d0dbe76 Compare April 14, 2023 09:41
@phyro
Copy link
Member

phyro commented Apr 14, 2023

@chives101 I seem to have a rather strange encoding issue when I run it with ./result/bin/grin --testnet. It works if I do cargo build and ./target/debug/grin --testnet so it seems it's something with the build.

here

@quentinlesceller
Copy link
Member

@phyro iirc this might be something related with libncurses.

@chives101
Copy link
Contributor Author

@phyro Is it tested with the latest repo? I did a force push that updated the nixpkgs dependency which was pretty outdated.

I just tested the repo, works fine for me. BTW, you can do a local test by entering the project folder and do nix build after applying the PR.

@chives101
Copy link
Contributor Author

@phyro I did a fresh build on debian bullseye, I couldn't reproduce your issue though.

@chives101 chives101 force-pushed the flake branch 2 times, most recently from 4380c16 to 55bc203 Compare April 17, 2023 08:05
@pkariz
Copy link

pkariz commented Apr 19, 2023

on nixos 22.11 it worked for me although it's weird that @phyro is getting these characters. I recall having the same problem but i've fixed it by installing newer ncurses or some other package. Will try to reproduce the problem.

Example:

nix build github:mimblewimble/grin

./result/bin/grin --help
result is the default symlink output for nix build
@chives101
Copy link
Contributor Author

@pkariz It is weird, because flake is supposed to be hermetic. Guess it's not that airtight after all.

@phyro
Copy link
Member

phyro commented May 2, 2023

@chives101 apologies that I didn't get back to you. I tried it again with the latest changes and I get the same. It seems @pkariz can't test this right now. My suggestion is to take the incremetal improvement appoach and merge it in. The current solution seems to work at least for those that are on nixos and some others (debian above) and this itself is valuable. It's also my observation that quite a few people in the Rust community like Nix so we'll likely get more people to chime in and patch it over time.

I'll wait a bit in case someone disagrees with merging, otherwise I'll go ahead and merge it.

@phyro phyro merged commit de94f95 into mimblewimble:master May 2, 2023
@phyro
Copy link
Member

phyro commented May 2, 2023

Thanks for your contribution @chives101 ! I'm not around my PC to test the grin-wallet PR again after the changes until next week, but will do that then. 👍

@chives101
Copy link
Contributor Author

@phyro It's fine, it's nothing. I'm pretty sure there's more I can do :D

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.

4 participants