-
Notifications
You must be signed in to change notification settings - Fork 588
chore: Update and improve nix devshell #1366
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
base: master
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1 @@ | |||
| use flake | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain what this file does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I explained it in the commit message:
Automatically enter nix shell via nix-direnv
nix-direnv loads the nix devshell as soon as you enter the project
directory (after allowing it once).
[nix-direnv](https://github.com/nix-community/nix-direnv)
Basically it allows you to continue to use your system shell, but loads all the dependencies declared in the nix flake into your path. And does that automatically once you enter the prost repo directory and unload the nix shell once you exit.
I think it is super convenient. And it doesn't intrude on anybody who doesn't want to use nix-direnv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but that doesn't have a place in this repo. I understand the flake files are useful for some people, but this is too niche.
| Cargo.lock | ||
|
|
||
| .DS_Store | ||
| .direnv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this file needs to be ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a directory created by nix-direnv and holds some caches. It's not recommended to be tracked by direnv.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but that doesn't have a place in this repo. I understand the flake files are useful for some people, but this is too niche.
flake.lock
Outdated
| "owner": "NixOS", | ||
| "repo": "nixpkgs", | ||
| "rev": "47e29c20abef74c45322eca25ca1550cdf5c3b50", | ||
| "rev": "91c9a64ce2a84e648d0cf9671274bb9c2fb9ba60", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone else also updated the nix files in #1307 . They have too many different changes in one PR, therefore it is not merged.
They had more changes to the lock file. Does it make sense to do that in this PR?
I have no knowledge about nix, so it is difficult for me to understand what happens here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice hint! Thanks. I cherry picked @lalvesl commits as it is a lot nicer :)
Previously nix only had one input (nixpkgs). So updating the lock file resulted in a small diff. By including more inputs, updating these inputs will of course result in a bigger lock file and a bigger diff.
I'll add more info and explanations to the pr description soon.
…other develop rust_minimum_version for tests
- Add support for MSRV tokio with `nix develop .#rust_minimum_version`; - Update readme, with this nix command.
Flake lock file updates:
• Updated input 'fenix':
'github:nix-community/fenix/70bb04a7de606a75ba0a2ee9d47b99802780b35d?narHash=sha256-0Xq5tZbvgZvxbbxv6kRHFuZE4Tq2za016NXh32nX0%2BQ%3D' (2025-07-10)
→ 'github:nix-community/fenix/2d8176c02f7be6d13578d24d5fd5049f1b46a4c5?narHash=sha256-FzUCFwXNjLnnZmVqYj/FjlBhUpat59SExflEaIGT62s%3D' (2025-11-27)
• Updated input 'fenix/nixpkgs':
'github:nixos/nixpkgs/9807714d6944a957c2e036f84b0ff8caf9930bc0?narHash=sha256-LwWRsENAZJKUdD3SpLluwDmdXY9F45ZEgCb0X%2BxgOL0%3D' (2025-07-08)
→ 'github:nixos/nixpkgs/5ae3b07d8d6527c42f17c876e404993199144b6a?narHash=sha256-6eeL1YPcY1MV3DDStIDIdy/zZCDKgHdkCmsrLJFiZf0%3D' (2025-11-24)
• Updated input 'fenix/rust-analyzer-src':
'github:rust-lang/rust-analyzer/6e3abe164b9036048dce1a3aa65a7e7e5200c0d3?narHash=sha256-USpVUdiWXDfPoh%2BagbvoBQaBhg3ZdKZgHXo/HikMfVo%3D' (2025-07-09)
→ 'github:rust-lang/rust-analyzer/71ddf07c1c75046df3bb496cf824de5c053d99ad?narHash=sha256-LfgFqvPz3C80VjaffSjy8lLyRWfbThhB7gE7IWXHjYU%3D' (2025-11-26)
• Updated input 'nixpkgs':
'github:NixOS/nixpkgs/47e29c20abef74c45322eca25ca1550cdf5c3b50?narHash=sha256-fQ4k/hyQiH9RRPznztsA9kbcDajvwV1sRm01el6Sr3c%3D' (2024-12-31)
→ 'github:NixOS/nixpkgs/5c46f3bd98147c8d82366df95bbef2cab3a967ea?narHash=sha256-nXv6xb7cq%2BXpjBYIjWEGTLCqQetxJu6zvVlrqHMsCOA%3D' (2025-11-26)
nix-direnv loads the nix devshell as soon as you enter the project directory (after allowing it once). [nix-direnv](https://github.com/nix-community/nix-direnv)
Required for them to not be picked up by `cargo test`
caspermeijn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand correctly that the changes to flake.nix is just a rewrite using new language features?
I agree with the updated dependencies and updated readme. Just some nitpicks left.
|
|
||
| This will drop you into a shell with all dependencies configured to build the entire project. | ||
|
|
||
| If you want to use the minimum supported Rust version as required by Tokio [see MSRV](#msrv), run: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not true. Prost has its own MSRV, but applies the same policy. So this statement is incorrect.
| devShells."rust_minimum_version" = | ||
| let | ||
| rustpkgs = (fenix.packages.${system}.fromToolchainName { | ||
| name = "1.82"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you take this version number from Cargo.toml? Then it never has to be updated again.
| @@ -0,0 +1 @@ | |||
| use flake | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but that doesn't have a place in this repo. I understand the flake files are useful for some people, but this is too niche.
| Cargo.lock | ||
|
|
||
| .DS_Store | ||
| .direnv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but that doesn't have a place in this repo. I understand the flake files are useful for some people, but this is too niche.
Nix Development Environment Update
nix develop .#rust_minimum_versionnow sets up a development shell with the appropriate Minimum Supported Rust Version (MSRV) required by Tokio.