Make updates better and more resilient #245
Conversation
Put a check in-place to unset the global default toolchain if it is no longer installed Set the global default to the installed toolchain if it is not set Add full toolchain selection resolution to the update operation resolve update parameters Fix the use command and toolchain selection routine to consider a global default set to a toolchain that is not installed as no selection at all Add check for the physical presence of a toolchain to proxy so that it prevents circularity errors and provides an actionable message
|
TODO: update the main branch with these fixes |
Add test cases Control verbosity of uninstall operation on macOS and other platforms
|
@swift-ci test macOS |
| var installedToolchains = startingConfig.listInstalledToolchains(selector: selector) | ||
| // This is in the unusual case that the inUse toolchain is not listed in the installed toolchains | ||
| if let inUse = startingConfig.inUse, selector.matches(toolchain: inUse) && !startingConfig.installedToolchains.contains(inUse) { | ||
| installedToolchains.append(inUse) |
There was a problem hiding this comment.
are we going to allow swiftly then to be able to uninstall toolchains it didn't have a hand in installing?
There was a problem hiding this comment.
If the toolchain is in the inUse property of the config.json then it probably did have a hand in installing the toolchain at some point in time. This includes it in the list of installed toolchains for the purposes of the uninstall command. Below swiftly will expunge it from the config, and try once again to remove it from disk.
| try await Use.execute(version, globalDefault: false, &config) | ||
| } | ||
|
|
||
| // We always update the global default toolchain if there is none set. This could | ||
| // be the only toolchain that is installed, which makes it the only choice. | ||
| if config.inUse == nil { |
There was a problem hiding this comment.
do we only want to do this when the user passes --use to install? In other words if --use was omitted does this still make sense?
There was a problem hiding this comment.
I think that it will be good to try and set a default global toolchain whenever we can to help users avoid cases where there's no default toolchain to fallback. It's an extra annoyance that most people probably don't care to have.
"Give me some kind of a swift toolchain please. I know that I have at least one installed in there."
| // Check to ensure that the global default in use toolchain matches one of the installed toolchains, and return | ||
| // no selected toolchain if it doesn't. | ||
| guard let defaultInUse = config.inUse, config.installedToolchains.contains(defaultInUse) else { | ||
| return (nil, .globalDefault) |
There was a problem hiding this comment.
Just to confirm: You shouldn't be able to select a toolchain that wasn't installed by swiftly?
There was a problem hiding this comment.
This should be a rare case, caused by things like improper handling of the update operation that this PR should solve. Another potential case is race conditions. This is pure hardening of the code, and also giving the user a way back to a nominal state by using a toolchain that is in the list that swiftly installed.
No description provided.