-
Notifications
You must be signed in to change notification settings - Fork 109
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
shell.nix: Add nix shell #519
base: master
Are you sure you want to change the base?
Conversation
We want to use a dual-toolchain setup to work around the exact issue you've been encountering: As a result, I believe it is fair to have the Nix expression use a nightly toolchain by default. We can introduce a parameter which switches toolchains. However, we should avoid changing the Apart from that, I have a similar set of concerns w.r.t. to using oxalica's overlay here: tock/tock#3727 (comment) @jrvanwhy Can you confirm that my above statements about the parallel use of stable & nightly toolchains are indeed correct? |
For futher background: When a contributor runs
Thinking of them as "release" and "development" toolchains isn't accurate -- they're both there for development purposes.
I think the Just to confirm: the purpose of |
That's right. We could add another Nix expression for that purpose (canonically called |
I think we have three options here:
To elaborate on option 1: We could create a script (I'll call it One behavior the existing implementation has that I would like us to keep is that on systems that use |
I updated to use the new Tock shell.nix (adds back tockloader, which I incorrectly removed, and uses fenix rust overlay). Using rustup is antithetical to the determinism of using a nix environment. It also doesn't work right with paths in the nix store. I got Nix to provide both toolchains and hacked up a way to switch between them. You cannot access shell env variables in the Makefile, so I wrote in pseudocode in the Makefile. Running the commands manually works fine. How does this approach look? jrvanwhy suggested using a bash script to switch toolchains, but I don't want to add another file for something this minor. |
I don't particularly like this, to be honest. It's weird to have both toolchains in the build inputs (which one ends up in your default path?), and the path rewriting through shell aliases, although clever, seems very brittle. How about we just bite the bullet and always use Nightly in Nix environments? We could add a switch to the Nix expression's argument list that uses stable instead, where developers just have to know that this toolchains can't execute the nightly-dependent Miri tests. The CI & all other infrastructure for this repo doesn't use Nix anyways, nightly Rust should support a strict superset of the features of stable, and I'm not aware of anyone using the Nix expression to realize "production" builds of any Tock components with a stable toolchain. In fact, Tock itself doesn't compile on a stable toolchain (yet). |
Specifies a Nix shell that creates a deterministic development environment. This is based on the Tock Nix shell but is modified to combine the required dependencies between the dual stable and nightly toolchains. This `shell.nix` is meant for developing, so it uses the nightly Rust version in order to be able to use Miri. Co-authored-by: Anthony Tarbinian <[email protected]>
a56ccd5
to
c0392d2
Compare
The nightly |
# Use nightly development toolchain because Miri is not supported | ||
# by the MSRV (Minimum Supported Rust Version) toolchain. | ||
nightlyToolchain = nixpkgs.fenix.fromToolchainName { | ||
name = nightly.channel; | ||
sha256 = "sha256-R/ONZzJaWQr0pl5RoXFIbnxIE3m6oJWy/rr2W0wXQHQ="; | ||
}; |
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 appreciate your efforts in making this work by combining the two toolchains, and while I think that this is a decent approach generally, to me this really points to a deficiency in how we pick nightly toolchain versions for libtock-rs
. For the Tock kernel, we have a script that searches for a Nightly that has all of the required components (including Miri). I think we should rather choose a proper nightly this way, and avoid mix- and matching Rust components. I'm not even sure whether Miri is portable across rustc versions (e.g., in the features that it recognizes).
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 removed the dual toolchain installation from before. Only the nightly derivation is generated, with the references on lines 41-42 being to the pure text of the file. This Nix logic has the (virtually) same effect as applying the diff in my original post before doing the toolchain derivation generation.
The problem you foresaw with some components being unavailable was not an issue. make test
works, but I agree that incompatible nightlies are a risk in the future.
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'm sorry, I'm having trouble matching what you write with the current Nix expression of this PR. Lines lines 51-57 still provide you a toolchain that is efffectively mix-and-match between stable and binary, and what you get is dependent on which components these two (stable + nightly) toolchains happen to provide, right?
And lines 58-65 then also throw in some components (namely, rust-std
) taken from whatever the fenix overlay defines as the latest toolchain, which only happens to coincide with the pinned nightly toolchain by the correctness of the condition in lines 35? That seems very brittle.
I have added a working nix shell based off the Tock file (tock/tock#3727). I removed Tockloader, but did not bother pruning other deps that may not be in use.
Using the Tock nix shell does not work to develop this repo because it uses a stable Rust toolchain that gets messed up by the parsing that was there.
I am able to run
make tests
properly now.However,
miri
does not work on stable Rust toolchains. I am successfully able to runcargo miri test
as it is written in the Makefile if I apply this diff.This is the same Rust nightly channel that Tock is using right now. I don't understand the justification for the dual toolchain from this commit 0ae3974.
Is it alright for me to submit this toolchain diff?
CC maintainers: @lschuermann @alevy @bradjc