Fix regression that placed the pre-uninstall script in the package cache. (#1205)

* Fix location of pre-uninstall script for EXE installers

* Move options parsing before pre-uninstall scripts

* Use global variable for uninstallation options

* Update tests

* Add news

Author: marcoesters

constructor/nsis/main.nsi.tmpl

@@ -1823,47 +1823,51 @@ Section "Uninstall"
         System::Call 'kernel32::SetEnvironmentVariable(t,t)i("INSTALLER_UNATTENDED", "0")'
     ${EndIf}
 
-    ${If} ${FileExists} "$INSTDIR\pkgs\pre_uninstall.bat"
-        ${Print} "Running pre-uninstall script..."
-        # `type` is used to simulate a `tee`-like output in cmd.exe
-        push '"$INSTDIR\pkgs\pre_uninstall.bat" > "${STEP_LOG}" 2>&1 & SET ERR=!ERRORLEVEL! & type "${STEP_LOG}" & EXIT /B !ERR!'
-        push "Failed to run pre-uninstall scripts."
-        push 'WithLog'
-        call un.AbortRetryNSExecWait
-    ${EndIf}
-    ${If} ${FileExists} "$INSTDIR\.nonadmin"
-        !insertmacro AddRemovePath "remove" "un."
-    ${EndIf}
-
 {%- if uninstall_with_conda_exe %}
-    # Parse arguments
-    StrCpy $R0 ""
+    # Parse uninstall options
+    var /global UninstOptions
+    StrCpy $UninstOptions ""
 
     ${If} $UninstRemoveConfigFiles_User_State == ${BST_CHECKED}
     ${If} $UninstRemoveConfigFiles_System_State == ${BST_CHECKED}
-        StrCpy $R0 "$R0 --remove-config-files=all"
+        StrCpy $UninstOptions "$UninstOptions --remove-config-files=all"
         System::Call 'kernel32::SetEnvironmentVariable(t,t)i("UNINSTALLER_REMOVE_CONFIG_FILES", "all")'
     ${Else}
-        StrCpy $R0 "$R0 --remove-config-files=user"
+        StrCpy $UninstOptions "$UninstOptions  --remove-config-files=user"
         System::Call 'kernel32::SetEnvironmentVariable(t,t)i("UNINSTALLER_REMOVE_CONFIG_FILES", "user")'
     ${EndIf}
     ${ElseIf} $UninstRemoveConfigFiles_System_State == ${BST_CHECKED}
-        StrCpy $R0 "$R0 --remove-config-files=system"
+        StrCpy $UninstOptions "$UninstOptions --remove-config-files=system"
         System::Call 'kernel32::SetEnvironmentVariable(t,t)i("UNINSTALLER_REMOVE_CONFIG_FILES", "system")'
     ${EndIf}
 
     ${If} $UninstRemoveUserData_State == ${BST_CHECKED}
-        StrCpy $R0 "$R0 --remove-user-data"
+        StrCpy $UninstOptions "$UninstOptions  --remove-user-data"
         System::Call 'kernel32::SetEnvironmentVariable(t,t)i("UNINSTALLER_REMOVE_USER_DATA", "1")'
     ${EndIf}
 
     ${If} $UninstRemoveCaches_State == ${BST_CHECKED}
-        StrCpy $R0 "$R0 --remove-caches"
+        StrCpy $UninstOptions "$UninstOptions  --remove-caches"
         System::Call 'kernel32::SetEnvironmentVariable(t,t)i("UNINSTALLER_REMOVE_CACHES", "1")'
     ${EndIf}
+{%- endif %}
+
+    ${If} ${FileExists} "$INSTDIR\pre_uninstall.bat"
+        ${Print} "Running pre-uninstall script..."
+        # `type` is used to simulate a `tee`-like output in cmd.exe
+        push '"$INSTDIR\pre_uninstall.bat" > "${STEP_LOG}" 2>&1 & SET ERR=!ERRORLEVEL! & type "${STEP_LOG}" & EXIT /B !ERR!'
+        push "Failed to run pre-uninstall scripts."
+        push 'WithLog'
+        call un.AbortRetryNSExecWait
+    ${EndIf}
+    ${If} ${FileExists} "$INSTDIR\.nonadmin"
+        !insertmacro AddRemovePath "remove" "un."
+    ${EndIf}
+
+{%- if uninstall_with_conda_exe %}
 
     ${Print} "Removing files and folders..."
-    push '"$INSTDIR\_conda.exe" constructor uninstall $R0 --prefix "$INSTDIR" {{ CONDA_LOG_ARG }}'
+    push '"$INSTDIR\_conda.exe" constructor uninstall $UninstOptions --prefix "$INSTDIR" {{ CONDA_LOG_ARG }}'
     push 'Failed to remove files and folders. Please see the log for more information.'
     push 'WithLog'
     SetDetailsPrint listonly

news/1205-pre-uninstall-location

@@ -0,0 +1,19 @@
+### Enhancements
+
+* <news item>
+
+### Bug fixes
+
+* Fix regression that placed the pre-uninstall script in the package cache. (#1205)
+
+### Deprecations
+
+* <news item>
+
+### Docs
+
+* <news item>
+
+### Other
+
+* <news item>

tests/test_examples.py

@@ -1352,18 +1352,6 @@ def test_uninstallation_standalone(
     input_path = tmp_path / "input"
     shutil.copytree(str(recipe_path), str(input_path))
 
-    # Create extra files to delete in pre-uninstall script
-    user_files_dir = tmp_path / "user_files"
-    for file in ("cache", "data", "config_user", "config_system"):
-        (user_files_dir / file).touch()
-
-    construct_yaml_file = input_path / "construct.yaml"
-    with construct_yaml_file.open() as file:
-        construct_yaml = yaml.load(file)
-    construct_yaml["script_env_variables"] = {"USER_FILES": str(user_files_dir)}
-    with construct_yaml_file.open(mode="w") as file:
-        yaml.dump(construct_yaml, file)
-
     installer, install_dir = next(create_installer(input_path, tmp_path))
     monkeypatch.setenv("USERPROFILE", str(tmp_path))
     _run_installer(
@@ -1404,16 +1392,35 @@ def test_uninstallation_standalone(
     if remove_caches:
         uninstall_options.append("/RemoveCaches=1")
 
+    # Create extra files to delete in pre-uninstall script
+    user_files_dir = tmp_path / "user_files"
+    user_files_dir.mkdir()
+    user_files: dict[str, bool] = {
+        "cache": remove_caches,
+        "data": remove_user_data,
+        "config_user": remove_user_rcs,
+        "config_system": remove_system_rcs,
+    }
+    for file in user_files:
+        (user_files_dir / file).touch()
+        assert (user_files_dir / file).exists()
+    construct_yaml_file = input_path / "construct.yaml"
+    with construct_yaml_file.open() as file:
+        construct_yaml = yaml.load(file)
+    construct_yaml["script_env_variables"] = {"USER_FILES": str(user_files_dir)}
+    with construct_yaml_file.open(mode="w") as file:
+        yaml.dump(construct_yaml, file)
+
     try:
         _run_uninstaller_exe(install_dir, check=True, options=uninstall_options)
         assert dot_conda_dir.exists() != remove_user_data
         assert pkg_cache.exists() != remove_caches
         assert system_rc.exists() != remove_system_rcs
         assert user_rc.exists() != remove_user_rcs
-        assert (user_files_dir / "data").exists() != remove_user_data
-        assert (user_files_dir / "cache").exists() != remove_caches
-        assert (user_files_dir / "config_system").exists() != remove_system_rcs
-        assert (user_files_dir / "config_user").exists() != remove_user_rcs
+        # The post-install script only touches the files inside user_files_dir
+        assert user_files_dir.exists()
+        for file, removed in user_files.items():
+            assert (user_files_dir / file).exists() != removed
     finally:
         if system_rc.parent.exists():
             shutil.rmtree(system_rc.parent)