search

numtide / devshell

Per project developer environments
https://numtide.github.io/devshell/
MIT License
1.25k stars 89 forks source link

rust: add option for environment variables #187

Closed happysalada closed 2 years ago

happysalada commented 2 years ago

I've put the rust environment variables behind a flag, just in case some people might not want it.

I've removed the RUST_SRC_PKGS as I don't think anyone would be using the rust coming from nixpkgs as it is consistently 1 or 2 versions behind stable. Most people using rust would be using one of the multiple overlays.

I'll post once more when I have tested.

happysalada commented 2 years ago

I just tested it. The good news is that it works. The less good news is that the user needs to override the variables by appending, this is a little bit of a gotcha that I didn't like. I couldn't find how to make the variables of this module be evaluated after the user defined ones. (to clarify, if a user wants to define RUSTFLAGS for example, they will need to add eval = "\"their values $RUSTFLAGS\""; which is not easy to guess.

Last note, I'm getting the sense that you want this to be easy to consume. I'm trying to think of the best way to make the rust toolchain available, but even in the nixpkgs wiki, the recommendation is to go through outside overlays. This shell could potentially choose an overlay and simplify setup with some options, not sure this is the way you want to take it though. I personally use https://github.com/oxalica/rust-overlay but I couldn't really tell you if it's really the best overlay out there because I haven't tested the other ones.

zimbatm commented 2 years ago

I just tested it. The good news is that it works. The less good news is that the user needs to override the variables by appending, this is a little bit of a gotcha that I didn't like. I couldn't find how to make the variables of this module be evaluated after the user defined ones. (to clarify, if a user wants to define RUSTFLAGS for example, they will need to add eval = "\"their values $RUSTFLAGS\""; which is not easy to guess.

That's fine. It's not ideal but that's a limitation of the env semantic.

Last note, I'm getting the sense that you want this to be easy to consume. I'm trying to think of the best way to make the rust toolchain available, but even in the nixpkgs wiki, the recommendation is to go through outside overlays. This shell could potentially choose an overlay and simplify setup with some options, not sure this is the way you want to take it though. I personally use https://github.com/oxalica/rust-overlay but I couldn't really tell you if it's really the best overlay out there because I haven't tested the other ones.

You should be able to use oxalica or fenix, and the package set shape would be the same. I think it's reasonable to ask the user to do this outside of the TOML in a preparation step.

happysalada commented 2 years ago

the oxalica overlay is just a single attribute that needs overrides. The only way I thought to keep it compatible was to provide a toolchain attribute that is a list of needed packages. When using that overlay, it would just be a single package. When using fenix, it would be one or two. Let me know if you think we can/should improve the api.

happysalada commented 2 years ago

I gave it some more thought. If you prefer, we could just leave the settings that you had and put them behind a flag useDefaultToolchain or something like that. It would be true by default, so that people who don't know would just have it enabled. For people using overlays, you could expect they are more knowledgeable and they would need to disable that flag. I couldn't find a solution that works for all types of users.

happysalada commented 2 years ago

hey, just a friendly ping regarding this. (no worries if you are busy).

The question is

happy with both solutions, and/or with other ideas you might have.

zimbatm commented 2 years ago

Ok that seems like a good compromise. If you can prepare the changes and ping me when it's done, that should be good.

happysalada commented 2 years ago

sorry, it was an either or scenario. Do you mean that you like the second choice better ? The first choice is the one implemented in this PR (changing to a new api to fit overlays as well).

Second choice would be keep the previous api but add a variable to disable the default tools for people using overlays.

zimbatm commented 2 years ago

oops, I would rather have the old api and a flag on top that disables pulling those tools

happysalada commented 2 years ago

all should be good now, let me know if you need anything else.

zimbatm commented 2 years ago

thanks!