Make the removal of components consistent

[FranciscoTGouveia]

Currently, the behavior of rustup component remove depends on the order of the provided components.

If an invalid component is listed first, the command fails immediately, and no components are removed.
However, if valid components are listed before an invalid one, those valid components are removed before the command returns an error.

This behavior is thus inconsistent, as it is dependent on the argument ordering.
From a usability perspective, I would argue that it is more intuitive for the command to attempt to remove all valid components and report any invalid ones afterward, regardless of their position.

That said, I am interested in hearing others' thoughts on this.

---

Examples of said inconsistency:

$ rustup component remove asd rustc-dev llvm-tools
error: toolchain 'stable-x86_64-unknown-linux-gnu' does not contain component 'asd' for target 'x86_64-unknown-linux-gnu'

$ rustup component remove rustc-dev llvm-tools asd
info: removing component rustc-dev
info: removing component llvm-tools
error: toolchain 'stable-x86_64-unknown-linux-gnu' does not contain component 'asd' for target 'x86_64-unknown-linux-gnu'

[djc]

From a usability perspective, I would argue that it is more intuitive for the command to attempt to remove all valid components and report any invalid ones afterward, regardless of their position.

Sounds good to me!

[rami3l]

@FranciscoTGouveia I'm surprised that the second case works at all 😓 what will apt/apk/dnf say about this? Shouldn't they deny the whole transaction IIRC?

[FranciscoTGouveia]

I’ve just tested this with apt, and it does indeed reject the entire transaction, returning an error for each invalid package.

[djc]

I think there's also plenty of precedent for best effort deletion (that is, delete everything that is valid). For example, I think that's what git branch -D does.

The question is if there's "serious harm" that might be done by deleting some but not of all of the components. It doesn't feel like that's the case here?

[FranciscoTGouveia]

I believe that any defined behavior would be an improvement over the current one, and I do not see this as a breaking change since either option would make the command's behavior (more) consistent.

That said, I would favor attempting to remove all specified components and then reporting any that could not be removed.
Additionally, it might be worthwhile to go a step further by detecting minor typos (i.e., suggesting the closest matching component name).

What are your thoughts on this approach?

CC @ChrisDenton

[Inconnu08]

Would it make sense to split this behavior into two phases, as in resolve/validate first, then remove, so the behavior becomes deterministic? That would also make it easy to support either transactional or best-effort semantics cleanly.

If we go with best-effort, I’m happy to take a stab at implementing it by collecting valid/invalid components first and reporting failures after removal.

[rami3l]

Would it make sense to split this behavior into two phases, as in resolve/validate first, then remove, so the behavior becomes deterministic?

@Inconnu08 Certainly, but I believe that's not the point: the debate here is really whether we should do best-effort removals or reject the transaction. It has nothing to do with upfront analysis, since either approach can be implemented with or without it.

[rami3l]

@FranciscoTGouveia Given your analysis I think it's okay to just do best-effort removals as @djc has suggested.

The question is if there's "serious harm" that might be done by deleting some but not of all of the components. It doesn't feel like that's the case here?

@djc It doesn't seem to be a problem now; in a traditional package manager, it will be when you delete foo's dependency without deleting foo itself (say foo depends on bar, and when uninstalling both, you mistyped uninstall goo bar). But even at that time we can go with the upfront analysis to ensure the invariant of all dependencies being installed just like @Inconnu08 has indicated, so I think either approach is acceptable to me as long as we don't introduce a lot of inconsistencies within ourselves.