I think this approach to the code conflates two things - and this is why I said I don't think we wanthierarchical config : we can can generate arbitrarily confusing outcomes for users when we merge data on a per-key basis..

What I was proposing is that:

• the channel selection is unaltered from today
• we searchseparately for installation defaultsother than channel, along a subset of the same search path, to find the installation default the user has specified. The subset being the sources that areable to specify installation defaults. Which e.g. environment variables cannot.

And writing this has me realise that we also need to provide an escape hatch, for the case where someone has configured defaults that are incompatible with the channel they are overriding. One possible escape hatch is just to document that installation defaults are ignored if the toolchain is already installed (I believe this is the case).

Thank you for writing tests. I think that these tests would be best as unit tests; the UI is not being changed, and we should be able to produce much more compact tests closer to the core. They are ok if you wish to stay with these external layer tests, but I have a very strong preference to see close-to-the-unit tests where possible!

(I haven't read the code in detail, just did a quick skim in a couple of spare minutes I had available; apologies if I've misunderstood something)

@rbtcollins Thanks for your clarification!

What I was proposing is that:

• the channel selection is unaltered from today

It is still the case that once thetoolchain field is set, it will not be overridden again. I believe I did special-case the toolchain selection in the code to ensure this behavior remains unchanged:

After all, the original issue in#3483 is thatrustup currently doesn't even bother tocheck other sources once the toolchain field becomes available. Here I just made it possible to merge other fields.

• we searchseparately for installation defaultsother than channel, along a subset of the same search path, to find the installation default the user has specified. The subset being the sources that areable to specify installation defaults. Which e.g. environment variables cannot.

If I'm not mistaken, all 4 sources can specify a toolchain, but onlyrust-toolchain.toml can specify other fields (this is also stated in#3483), so I believe my patch currently does what you expect it to do already.

And writing this has me realise that we also need to provide an escape hatch, for the case where someone has configured defaults that are incompatible with the channel they are overriding. One possible escape hatch is just to document that installation defaults are ignored if the toolchain is already installed (I believe this is the case).

Would you mind clarifying on this one? In what way they might become incompatible?

Thank you for writing tests. I think that these tests would be best as unit tests; the UI is not being changed, and we should be able to produce much more compact tests closer to the core. They are ok if you wish to stay with these external layer tests, but I have a very strong preference to see close-to-the-unit tests where possible!

I agree. I'll definitely look into that when I have time!
These black box tests are rather for making sure that we are on the same page in terms of what should be implemented in this PR. Now I believe that I have correctly understood your intentions.

And writing this has me realise that we also need to provide an escape hatch, for the case where someone has configured defaults that are incompatible with the channel they are overriding. One possible escape hatch is just to document that installation defaults are ignored if the toolchain is already installed (I believe this is the case).

Would you mind clarifying on this one? In what way they might become incompatible?

Did you mean, e.g. requiringnightly + rls when nightly is already installed withoutrls?

@rbtcollins replied:

Current situation:

1. channel is found first from a list of locations
2. components are specified by first rustup-toolchain.toml IF AND ONLY IF channel is source from rustup-toolchain; and then default-profile
3. targets are specified only by rustup-toolchain.toml IF AND ONLY IF channel is source from rustup-toolchain

.. and (from memory) installed toolchains are never modified

the proposed change here is to drop the IF AND ONLY IF condition in the bottom two cases.

So what happens if the following sequence takes place.

1. rustup installs beta
2. an override is set for beta for a particular path
3. cargo or some other command that triggers implicit toolchain installation is run, finds beta and chooses that, and finds the other components and targets using the changed lookup successfully... but as beta is installed, takes no action.

The user will be filing a nearly identical bug report again.

Checking all components and targets every call is a lot of overhead, as a bunch of file IO is required to do that. I don't think thats a good path forward in the short term.

Now, in the scenario above (which we could test btw) - I strongly suspect that rustup override set ... is one of the commands that does implicit installation today.

cc#1397 (comment)#2686 (comment)

It seems that both the missing components and the missing targets will be installed once an auto-install-invoking command is executed:

So I guess the concern about incompatible config has been addressed.

@rbtcollins is concerned about this change's possibility of making environment isolation in systems like bazel more difficult (ccbazelbuild/rules_rust#2285).

Will#2793 become an option in terms of mitigating this issue? (Thanks for the hint,@djc!)

I've converted this to draft for now - its not in 'ready to review' state.