-
Notifications
You must be signed in to change notification settings - Fork 2
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
rust-toolchain.toml
support
#56
Comments
Absolutely. Thanks for the feature request. |
@Ch4s3r - I've been playing around with this and I'm a little unclear about the recommended workflow for the What's unclear is if rustup is supposed to install these for you and if so, what command should be run to make that happen. In my testing, if I enter the directory that contains That all said, I can make the buildpack do this, but I'm just trying to find somewhere that documents the expected behavior around this command so what I add into the buildpack is stable going forward. What is your understanding of the workflow here? |
Yes it's quite unexpected that There is an issue about moving away from installing/downloading toolchains via |
Thanks for those links. I've subscribed to them, so I'll follow along with progress there. In the meantime, I will add in a call to |
Sounds like an ok solution for the time being. |
This PR will optionally install Rust as specified in a 'rust-toolchain.toml' file. If a 'rust-toolchain' file is present, that file is used first. If not present, then 'rust-toolchain.toml' will be used. If both are present then 'rust-toolchain' is used. When a 'rust-toolchain' or 'rust-toolchain.toml' is specified, then the buildpack does not use 'BP_RUST_PROFILE' or 'BP_RUST_TOOLCHAIN'. The two are mutually exclusive. The 'BP_RUST_TARGET' is still installed, as this is necessary if you are running on the Tiny stack. Resolves #56 Signed-off-by: Daniel Mikusa <[email protected]>
I've got a PR queued up for this. When it get's merged, I will cut a release and you can test it out. It's pretty simple though. If the file exists, the buildpack will just run |
Would it be better to have the env variables priority in case one exists? Otherwise it would be hard to override on the cli. Not sure if this is possible easily. |
My thought was that if the file is present, then you know what you're doing and the buildpack should use the values from the file. I could flip it around and make the env variables override the config file, but off the top of my head, I think it would have to be all or nothing so all env variables or all config file. My thought is that it would get messy trying to run the config file and then add the env variable changes on top of that. I can play around with it though if you feel like there's a use case for it. My thought was that if you have the file and want something different, you would just edit the file and rebuild (as opposed to setting env variables). Does that seem OK, or can you see a case where setting the env variables would be a lot more convenient? |
Only suggested it because that's how it's done in spring boot, where env variables override |
As the rust-toolchain file is anyway rather small compared to an |
This PR will optionally install Rust as specified in a 'rust-toolchain.toml' file. If a 'rust-toolchain' file is present, that file is used first. If not present, then 'rust-toolchain.toml' will be used. If both are present then 'rust-toolchain' is used. When a 'rust-toolchain' or 'rust-toolchain.toml' is included, then the buildpack will run 'rustup' and install Rust as specified in the file. If 'BP_RUST_PROFILE' or 'BP_RUST_TOOLCHAIN' are also set (non-default values), the buildpack will install them as well. If 'rust-toolchain' or 'rust-toolchain.toml' are not present then the buildpack will install 'BP_RUST_PROFILE' or 'BP_RUST_TOOLCHAIN'. The additional target as set in 'BP_RUST_TARGET' is still installed, as this is necessary if you are running on the Tiny stack. Resolves #56 Signed-off-by: Daniel Mikusa <[email protected]>
This PR will optionally install Rust as specified in a 'rust-toolchain.toml' file. If a 'rust-toolchain' file is present, that file is used first. If not present, then 'rust-toolchain.toml' will be used. If both are present then 'rust-toolchain' is used. When a 'rust-toolchain' or 'rust-toolchain.toml' is included, then the buildpack will run 'rustup' and install Rust as specified in the file. If 'BP_RUST_PROFILE' or 'BP_RUST_TOOLCHAIN' are also set (non-default values), the buildpack will install them as well. If 'rust-toolchain' or 'rust-toolchain.toml' are not present then the buildpack will install 'BP_RUST_PROFILE' or 'BP_RUST_TOOLCHAIN'. The additional target as set in 'BP_RUST_TARGET' is still installed, as this is necessary if you are running on the Tiny stack. Resolves #56 Signed-off-by: Daniel Mikusa <[email protected]>
That's a fair point. In other buildpacks, we typically (always?) have the env variables be the most specific configuration option and allow them to override other configuration methods. I played around with it and think I have it working a little better. It could help if you are trying to adjust the build but not adjust the source control commit.
The only downside is if you have the same values set in envs as in the config file, it'll run |
This PR will optionally install Rust as specified in a 'rust-toolchain.toml' file. If a 'rust-toolchain' file is present, that file is used first. If not present, then 'rust-toolchain.toml' will be used. If both are present then 'rust-toolchain' is used. When a 'rust-toolchain' or 'rust-toolchain.toml' is included, then the buildpack will run 'rustup' and install Rust as specified in the file. If 'BP_RUST_PROFILE' or 'BP_RUST_TOOLCHAIN' are also set (non-default values), the buildpack will install them as well. If 'rust-toolchain' or 'rust-toolchain.toml' are not present then the buildpack will install 'BP_RUST_PROFILE' or 'BP_RUST_TOOLCHAIN'. The additional target as set in 'BP_RUST_TARGET' is still installed, as this is necessary if you are running on the Tiny stack. Resolves #56 Signed-off-by: Daniel Mikusa <[email protected]>
FYI, I just cut 1.4.0 which has this support. |
Would it be possible to support for the rust-toolchain.toml?
This would be quite convenient, as you don't need to specify the options for the toolchain also when running pack.
I have one in my repo already so everybody who checks out that repo does not need to bother with setting the correct settings for their toolchain and also the CI runs the tests with these settings then.
For deploying it would also be awesome to have these settings taken from this standardized file instead of specifying them again in a
project.toml
file.My
project.toml
looks like this:rust-toolchain.toml
Yes they diverge, but that's not the point of this ticket 😉
The text was updated successfully, but these errors were encountered: