Handle Corrupt Install Script Installs #2235
Conversation
The dotnet install script has a bug where it wont do the install if the folder is not empty. Sometimes it will leave a partially done install, like if killed, in which case we get to a bad state. This fixes that by cleaning up the directory, and or checking if dotnet.exe exists, whether it works. dotnet --list-runtimes is a fast way to do this since it should only invoke the host and not need to invoke the SDK. doing just 'dotnet' will not work as that does not return 0. Also, gets rid of the 'lock' exception since thats from the proper-lockfile lib which we don't use anymore. Resolves: dotnet#2059 First scrapped attempt: dotnet#2106
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts:337
- Consider adding an assertion that verifies the presence of the dotnet executable after reinstall to ensure that the cleanup and reinstall logic works as expected.
const _ = await installRuntime('9.0', 'runtime');
vscode-dotnet-runtime-library/src/Acquisition/AcquisitionInvoker.ts
Outdated
Show resolved
Hide resolved
wipe dir only wipes the files, and the script looks at the folders
The Install Multiple versions tests fails bc the deprecated sdk code installs everything into the same folder, so this new cleanup logic will wipe the older folder. The uninstall logic fails because the original sdk code does not work with respect to finding a local sdk when a global sdk is on the machine (it finds a 7.0.4xx sdk), this code is not used by anything anymore so it is likely not worth investigating.
There was a problem hiding this comment.
I'm not really clear as to why your dotnet --list-runtimes check really tests whether the installation is valid or not. Any part could be corrupted, right? And --list-runtimes skips a lot of that, so shouldn't we install or not install based entirely on user input, i.e., user says it's broken --> reinstall?
That's a great point. I've changed it to validate that the install includes a runtime that is correct and matches the correct version/arch. w.r.t user input, this is the runtime that devkit runs on, so it doesnt require any user interaction. We try to get all of this done early on in the process so CDK and C# don't load slowly, so we wouldn't want to add interaction here with how its currently designed. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
vscode-dotnet-sdk-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The dotnet install script has a bug where it wont do the install if the folder is not empty. Sometimes it will leave a partially done install, like if killed, in which case we get to a bad state.
This fixes that by cleaning up the directory, and or checking if dotnet.exe exists, whether it works. dotnet --list-runtimes is a fast way to do this since it should only invoke the host and not need to invoke the SDK. doing just 'dotnet' will not work as that does not return 0.
Also, gets rid of the 'lock' exception since thats from the proper-lockfile lib which we don't use anymore.
Resolves: #2059
First scrapped attempt: #2106