Compare registry values to $INSTDIR before removing keys
#684
Conversation
conda-bot
added
the
cla-signed
marcoesters
changed the title
$INSTDIR before removing keys
|
I like this better than what we have right now, but I wonder if we should pursue #588 after this. That'd mean replacing the logic in NSIS with the logic in This way we make |
| @@ -1185,10 +1187,36 @@ Section "Uninstall" | |||
| # carefully. More info at https://docs.conda.io/projects/conda/en/latest/user-guide/troubleshooting.html#solution | |||
| System::Call 'kernel32::SetEnvironmentVariable(t,t)i("CONDA_DLL_SEARCH_MODIFICATION_ENABLE", "1").r0' | |||
|
|
|||
| # Read variables the uninstaller needs from the registry | |||
There was a problem hiding this comment.
If I understand this correctly, this block will search the registry for two variables:
- INSTALLER_NAME_FULL
- INSTALLER_VERSION
These were previously assumed to be constant and would have used the hardcoded values at install time (${NAME} [used in line 65] and ${VERSION}, respectively).
However we are now saying that this might not be always the case. Could you elaborate under what circumstances this is true? e.g. updating Anaconda in place but not updating the registry keys (or not deleting the old ones before adding the new ones)?
There was a problem hiding this comment.
I guess this logic allows the uninstaller to run for other conda installations if needed, I don't know if that's a bug or a feature 😬
There was a problem hiding this comment.
I guess this logic allows the uninstaller to run for other conda installations if needed, I don't know if that's a bug or a feature 😬
That is already possible. The changes in this PR will, however, conduct checks to make sure the deletion is performed consistently on the file system and the registry.
There is another, admittedly nice, application: if you install the same Miniconda/Miniforge version twice, installation 2 will overwrite the registry keys of number 1. If you then run the uninstaller of number 1, it will delete the keys of number 2.
| ${If} $0 == "$INSTDIR" | ||
| DeleteRegKey SHCTX "Software\Python\PythonCore\${PY_VER}" | ||
| ${EndIf} | ||
| StrCpy $R0 "SOFTWARE\Python\PythonCore" |
There was a problem hiding this comment.
This block is easier to understand, I think. Basically we don't want to assume that the base Python is still the same version. However, is this updated when conda updates the python package in base? I don't remember seeing code for this 🤔 It would still be the same path, so it would work, but maybe the metadata is outdated when it comes to the reported version in the registry.
There was a problem hiding this comment.
This also prevents issues in cross-installer usage, as described in my other comment.
I do not think that conda updates the registry when python is updated, which it should probably do.
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
Description
The current uinstaller relies on values set during compile time to delete registry keys. Changes to the registry after the installation will not be taken into account when performing the uninstallation.
This PR scans the registry during uninstallation and deletes the entries that can be directly tied to the installation through
$INSTDIR.Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?