Remove validation for custom toolchains when reading rust-toolchain.toml#4250
Conversation
There was a problem hiding this comment.
Makes sense to me. Will wait for @rami3l to have a look.
I guess alternatively we could show a warning, and then store a setting that we've shown the warning, or something? (Or, store the date when we've last shown the warning and only show it again after x days?)
1b80189 to
0c3d6eb
Compare
|
@wesleywiser Thanks a lot for your comprehension and your quick patch!
Sorry for the back and forth, but I would still like to see an automated test for this scenario. Is it possible to trick rustup into believing the existence of that toolchain by installing e.g. |
e0a8a6b to
21716f8
Compare
|
Thanks @rami3l for that suggestion! I've added a test that covers this 🙂 |
|
@wesleywiser About the message that @djc has mentioned, I think it will probably be worthwhile to add a warning clarifying that we are not doing any checks, as demonstrated in the new test. That said, it will definitely not be a blocker anyway. Thanks again for your contribution! @djc I'm afraid a "last emitted warning" check might not be enough now that we are considering some workstation with multiple repos/toolchain combinations... |
21716f8 to
1f446ea
Compare
This validation was introduced in e1306b3 However, it breaks a number of scenarios that previously worked without issue such as rust-lang#4248. As requested by rustup maintainers in that thread, I'm taking out the extra validation added in that commit, leaving the rest of the code as-is. Testing confirms this fixes rust-lang#4248.
1f446ea to
0585f41
Compare
This validation was introduced in e1306b3 However, it breaks a number of scenarios that previously worked without issue such as #4248. As requested by rustup maintainers in that thread, I'm taking out the extra validation added in that commit, leaving the rest of the code as-is.
Testing confirms this fixes #4248.