Skip to content
Merged
23 changes: 19 additions & 4 deletions constructor/nsis/main.nsi.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ ${Using:StrFunc} StrStr
var /global INIT_CONDA
var /global REG_PY

var /global NO_REGISTRY

var /global INSTDIR_JUSTME
var /global INSTALLER_VERSION
var /global INSTALLER_NAME_FULL
Expand Down Expand Up @@ -262,8 +264,7 @@ FunctionEnd

Function InitializeVariables
StrCpy $CheckPathLength "{{ 1 if check_path_length else 0 }}"
StrCpy $ARGV_NoRegistry "0"
StrCpy $ARGV_KeepPkgCache "{{ 1 if keep_pkgs else 0 }}"
StrCpy $NO_REGISTRY "0"

# Package cache option
StrCpy $Ana_ClearPkgCache_State {{ '${BST_UNCHECKED}' if keep_pkgs else '${BST_CHECKED}' }}
Expand Down Expand Up @@ -425,9 +426,23 @@ FunctionEnd

ClearErrors
${GetOptions} $ARGV "/KeepPkgCache=" $ARGV_KeepPkgCache
${IfNot} ${Errors}
${If} $ARGV_KeepPkgCache = "1"
StrCpy $Ana_ClearPkgCache_State ${BST_UNCHECKED}
${ElseIf} $ARGV_KeepPkgCache = "0"
StrCpy $Ana_ClearPkgCache_State ${BST_CHECKED}
${EndIf}
${EndIf}

ClearErrors
${GetOptions} $ARGV "/NoRegistry=" $ARGV_NoRegistry
${IfNot} ${Errors}
${If} $ARGV_NoRegistry = "1"
StrCpy $NO_REGISTRY "1"
${ElseIf} $ARGV_NoRegistry = "0"
StrCpy $NO_REGISTRY "0"
${EndIf}
${EndIf}

ClearErrors
${GetOptions} $ARGV "/NoScripts=" $ARGV_NoScripts
Expand Down Expand Up @@ -1055,7 +1070,7 @@ Function OnDirectoryLeave
; With windows 10, we can enable support for long path, for earlier
; version, suggest user to use shorter installation path
${If} ${AtLeastWin10}
${AndIfNot} $ARGV_NoRegistry = "1"
${AndIfNot} $NO_REGISTRY = "1"
; If we have admin right, we enable long path on windows
${If} ${UAC_IsAdmin}
WriteRegDWORD HKLM "SYSTEM\CurrentControlSet\Control\FileSystem" "LongPathsEnabled" 1
Expand Down Expand Up @@ -1619,7 +1634,7 @@ Section "Install"
${EndIf}
{%- endif %}

${If} $ARGV_NoRegistry == "0"
${If} $NO_REGISTRY == "0"
# Registry uninstall info
WriteRegStr SHCTX "${UNINSTREG}" "DisplayName" "${UNINSTALL_NAME}"
WriteRegStr SHCTX "${UNINSTREG}" "DisplayVersion" "${VERSION}"
Expand Down
19 changes: 19 additions & 0 deletions news/1132-fix-cli-args
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
### Enhancements

* <news item>

### Bug fixes

* EXE: Fixed an issue for silent installers where command-line argument `/KeepPkgCache` was ignored and `/NoRegistry` would reset the default value. (#1132)

### Deprecations

* <news item>

### Docs

* <news item>

### Other

* <news item>
83 changes: 81 additions & 2 deletions tests/test_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
if sys.platform == "darwin":
from constructor.osxpkg import calculate_install_dir
elif sys.platform.startswith("win"):
import winreg

import ntsecuritycon as con
import win32security

Expand All @@ -60,6 +62,49 @@
KEEP_ARTIFACTS_PATH = None


def _is_program_installed(partial_name: str) -> bool:
"""
Checks if a program is listed in the Windows 'Installed apps' menu.
We search by looking for a partial name to avoid having to account for Python version and arch.
Returns True if a match is found, otherwise False.
"""

if not sys.platform.startswith("win"):
return False

# For its current purpose HKEY_CURRENT_USER is sufficient,
# but additional registry locations could be added later.
UNINSTALL_PATHS = [
(winreg.HKEY_CURRENT_USER, r"SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall"),
]

partial_name = partial_name.lower()

for hive, path in UNINSTALL_PATHS:
try:
reg_key = winreg.OpenKey(hive, path)
except FileNotFoundError:
continue

subkey_count = winreg.QueryInfoKey(reg_key)[0]

for i in range(subkey_count):
try:
subkey_name = winreg.EnumKey(reg_key, i)
subkey = winreg.OpenKey(reg_key, subkey_name)

display_name, _ = winreg.QueryValueEx(subkey, "DisplayName")

if partial_name in display_name.lower():
return True

except (FileNotFoundError, OSError, TypeError):
# Some keys may lack DisplayName or have unexpected value types
continue

return False


def _execute(
cmd: Iterable[str], installer_input=None, check=True, timeout=420, **env_vars
) -> subprocess.CompletedProcess:
Expand Down Expand Up @@ -1038,8 +1083,6 @@ def test_initialization(tmp_path, request, monkeypatch, method):
)
if installer.suffix == ".exe":
try:
import winreg

paths = []
for root, keyname in (
(winreg.HKEY_CURRENT_USER, r"Environment"),
Expand Down Expand Up @@ -1423,3 +1466,39 @@ def test_regressions(tmp_path, request):
check_subprocess=True,
uninstall=True,
)


@pytest.mark.parametrize("no_registry", (0, 1))
@pytest.mark.skipif(not ON_CI, reason="CI only")
@pytest.mark.skipif(not sys.platform.startswith("win"), reason="Windows only")
def test_not_in_installed_menu_list_(tmp_path, request, no_registry):
"""Verify the app is in the Installed Apps Menu (or not), based on the CLI arg '/NoRegistry'.
If NoRegistry=0, we expect to find the installer in the Menu, otherwise not.
"""
input_path = _example_path("extra_files") # The specific example we use here is not important
options = ["/InstallationType=JustMe", f"/NoRegistry={no_registry}"]
for installer, install_dir in create_installer(input_path, tmp_path):
_run_installer(
input_path,
installer,
install_dir,
request=request,
check_subprocess=True,
uninstall=False,
options=options,
)

# Use the installer file name for the registry search
installer_file_name_parts = Path(installer).name.split("-")
name = installer_file_name_parts[0]
version = installer_file_name_parts[1]
partial_name = f"{name} {version}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems brittle. Instead of manually parsing the construct.yaml file, I suggest using the UninstallString value and see if it starts with $INSTDIR\Uninstall-:

WriteRegStr SHCTX "${UNINSTREG}" "UninstallString" \
"$\"$INSTDIR\Uninstall-${NAME}.exe$\""

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, what about using the variable installer we get from create_installer? If I'm not mistaken that should be the path to the exe and it is named as <name>-<version>-<build number>-<platform>-<arch>.exe. If we split installer.name on - we have name from [0] and version from [1], let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the installer uses the build number (it's part of the version string), but that could work, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduced it to

    installer_file_name_parts = Path(installer).name.split("-")
    name = installer_file_name_parts[0]
    version = installer_file_name_parts[1]
    partial_name = f"{name} {version}"


is_in_installed_apps_menu = _is_program_installed(partial_name)
_run_uninstaller_exe(install_dir)

# If no_registry=0 we expect is_in_installed_apps_menu=True
# If no_registry=1 we expect is_in_installed_apps_menu=False
assert is_in_installed_apps_menu == (no_registry == 0), (
f"Unable to find program '{partial_name}' in the 'Installed apps' menu"
)
Loading