From 1e580d7fe44dc267b6a487257485630e6bdefa80 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 21:27:08 +0100 Subject: [PATCH 1/5] Pass through list of installs to warn about to shortcut implementations. Fixes #85 --- src/manage/arputils.py | 11 +++++---- src/manage/install_command.py | 19 ++++++++------- src/manage/pep514utils.py | 45 ++++++++++++++++++++++++----------- src/manage/startutils.py | 7 +++--- 4 files changed, 53 insertions(+), 29 deletions(-) diff --git a/src/manage/arputils.py b/src/manage/arputils.py index 6996bee..63ade07 100644 --- a/src/manage/arputils.py +++ b/src/manage/arputils.py @@ -5,6 +5,8 @@ from .fsutils import rglob from .logging import LOGGER from .pathutils import Path +from .tagutils import install_matches_any + def _root(): return winreg.CreateKey( @@ -59,7 +61,7 @@ def _set_value(key, name, value): winreg.SetValueEx(key, name, None, winreg.REG_SZ, str(value)) -def _make(key, item, shortcut): +def _make(key, item, shortcut, *, allow_warn=True): prefix = Path(item["prefix"]) for value, from_dict, value_name, relative_to in [ @@ -122,13 +124,14 @@ def _iter_keys(key): return -def create_one(install, shortcut): +def create_one(install, shortcut, warn_for=[]): + allow_warn = install_matches_any(install, warn_for) with _root() as root: install_id = f"pymanager-{install['id']}" LOGGER.debug("Creating ARP entry for %s", install_id) try: with winreg.CreateKey(root, install_id) as key: - _make(key, install, shortcut) + _make(key, install, shortcut, allow_warn=allow_warn) except OSError: LOGGER.debug("Failed to create entry for %s", install_id) LOGGER.debug("TRACEBACK:", exc_info=True) @@ -136,7 +139,7 @@ def create_one(install, shortcut): raise -def cleanup(preserve_installs): +def cleanup(preserve_installs, warn_for=[]): keep = {f"pymanager-{i['id']}".casefold() for i in preserve_installs} to_delete = [] with _root() as root: diff --git a/src/manage/install_command.py b/src/manage/install_command.py index bda3c1e..6adeb18 100644 --- a/src/manage/install_command.py +++ b/src/manage/install_command.py @@ -242,7 +242,10 @@ def _write_alias(cmd, install, alias, target): launcher = _if_exists(launcher, "-64") LOGGER.debug("Create %s linking to %s using %s", alias["name"], target, launcher) if not launcher or not launcher.is_file(): - LOGGER.warn("Skipping %s alias because the launcher template was not found.", alias["name"]) + if install_matches_any(install, getattr(cmd, "tags", None)): + LOGGER.warn("Skipping %s alias because the launcher template was not found.", alias["name"]) + else: + LOGGER.debug("Skipping %s alias because the launcher template was not found.", alias["name"]) return p.write_bytes(launcher.read_bytes()) p.with_name(p.name + ".__target__").write_text(str(target), encoding="utf-8") @@ -250,33 +253,33 @@ def _write_alias(cmd, install, alias, target): def _create_shortcut_pep514(cmd, install, shortcut): from .pep514utils import update_registry - update_registry(cmd.pep514_root, install, shortcut) + update_registry(cmd.pep514_root, install, shortcut, cmd.tags) def _cleanup_shortcut_pep514(cmd, install_shortcut_pairs): from .pep514utils import cleanup_registry - cleanup_registry(cmd.pep514_root, {s["Key"] for i, s in install_shortcut_pairs}) + cleanup_registry(cmd.pep514_root, {s["Key"] for i, s in install_shortcut_pairs}, cmd.tags) def _create_start_shortcut(cmd, install, shortcut): from .startutils import create_one - create_one(cmd.start_folder, install, shortcut) + create_one(cmd.start_folder, install, shortcut, cmd.tags) def _cleanup_start_shortcut(cmd, install_shortcut_pairs): from .startutils import cleanup - cleanup(cmd.start_folder, [s for i, s in install_shortcut_pairs]) + cleanup(cmd.start_folder, [s for i, s in install_shortcut_pairs], cmd.tags) def _create_arp_entry(cmd, install, shortcut): # ARP = Add/Remove Programs from .arputils import create_one - create_one(install, shortcut) + create_one(install, shortcut, cmd.tags) def _cleanup_arp_entries(cmd, install_shortcut_pairs): from .arputils import cleanup - cleanup([i for i, s in install_shortcut_pairs]) + cleanup([i for i, s in install_shortcut_pairs], cmd.tags) SHORTCUT_HANDLERS = { @@ -359,7 +362,7 @@ def print_cli_shortcuts(cmd): names = get_install_alias_names(aliases, windowed=True) LOGGER.debug("%s will be launched by %s", i["display-name"], ", ".join(names)) - if tags and not install_matches_any(i, cmd.tags): + if not install_matches_any(i, tags): continue names = get_install_alias_names(aliases, windowed=False) diff --git a/src/manage/pep514utils.py b/src/manage/pep514utils.py index ca2401c..ab011b4 100644 --- a/src/manage/pep514utils.py +++ b/src/manage/pep514utils.py @@ -4,6 +4,7 @@ from .logging import LOGGER from .pathutils import Path +from .tagutils import install_matches_any from .verutils import Version @@ -136,7 +137,7 @@ def _update_reg_values(key, data, install, exclude=set()): winreg.SetValueEx(key, k, None, v_kind, v) -def _is_tag_managed(company_key, tag_name, *, creating=False): +def _is_tag_managed(company_key, tag_name, *, creating=False, allow_warn=True): try: tag = winreg.OpenKey(company_key, tag_name) except FileNotFoundError: @@ -188,12 +189,15 @@ def _is_tag_managed(company_key, tag_name, *, creating=False): new_name = f"{orig_name}.{i}" # Raises standard PermissionError (5) if new_name exists reg_rename_key(tag.handle, orig_name, new_name) - LOGGER.warn("An existing registry key for %s was renamed to %s " - "because it appeared to be invalid. If this is " - "correct, the registry key can be safely deleted. " - "To avoid this in future, ensure that the " - "InstallPath key refers to a valid path.", - tag_name, new_name) + if allow_warn: + LOGGER.warn("An existing registry key for %s was renamed to %s " + "because it appeared to be invalid. If this is " + "correct, the registry key can be safely deleted. " + "To avoid this in future, ensure that the " + "InstallPath key refers to a valid path.", + tag_name, new_name) + else: + LOGGER.debug("Renamed %s to %s", tag_name, new_name) break except FileNotFoundError: LOGGER.debug("Original key disappeared, so we will claim it") @@ -207,8 +211,12 @@ def _is_tag_managed(company_key, tag_name, *, creating=False): orig_name, new_name, exc_info=True) raise else: - LOGGER.warn("Attempted to clean up invalid registry key %s but " - "failed after too many attempts.", tag_name) + if allow_warn: + LOGGER.warn("Attempted to clean up invalid registry key %s but " + "failed after too many attempts.", tag_name) + else: + LOGGER.debug("Attempted to clean up invalid registry key %s but " + "failed after too many attempts.", tag_name) return False return True @@ -226,31 +234,40 @@ def _split_root(root_name): return hive, name -def update_registry(root_name, install, data): +def update_registry(root_name, install, data, warn_for=[]): hive, name = _split_root(root_name) with winreg.CreateKey(hive, name) as root: - if _is_tag_managed(root, data["Key"], creating=True): + allow_warn = install_matches_any(install, warn_for) + if _is_tag_managed(root, data["Key"], creating=True, allow_warn=allow_warn): with winreg.CreateKey(root, data["Key"]) as tag: LOGGER.debug("Creating/updating %s\\%s", root_name, data["Key"]) winreg.SetValueEx(tag, "ManagedByPyManager", None, winreg.REG_DWORD, 1) _update_reg_values(tag, data, install, {"kind", "Key", "ManagedByPyManager"}) - else: + elif allow_warn: LOGGER.warn("An existing runtime is registered at %s in the registry, " "and so the new one has not been registered.", data["Key"]) LOGGER.info("This may prevent some other applications from detecting " "the new installation, although 'py -V:...' will work. " "To register the new installation, remove the existing " "runtime and then run 'py install --refresh'") + else: + LOGGER.debug("An existing runtime is registered at %s and so the new " + "install has not been registered.", data["Key"]) -def cleanup_registry(root_name, keep): +def cleanup_registry(root_name, keep, warn_for=[]): hive, name = _split_root(root_name) with _reg_open(hive, name, writable=True) as root: for company_name in _iter_keys(root): any_left = False with winreg.OpenKey(root, company_name, access=winreg.KEY_ALL_ACCESS) as company: for tag_name in _iter_keys(company): - if f"{company_name}\\{tag_name}" in keep or not _is_tag_managed(company, tag_name): + # Calculate whether to show warnings or not + install = {"company": company_name, "tag": tag_name} + allow_warn = install_matches_any(install, warn_for) + + if (f"{company_name}\\{tag_name}" in keep + or not _is_tag_managed(company, tag_name, allow_warn=allow_warn)): any_left = True else: _reg_rmtree(company, tag_name) diff --git a/src/manage/startutils.py b/src/manage/startutils.py index f46e013..e9da0e0 100644 --- a/src/manage/startutils.py +++ b/src/manage/startutils.py @@ -3,6 +3,7 @@ from .fsutils import rmtree, unlink from .logging import LOGGER from .pathutils import Path +from .tagutils import install_matches_any def _unprefix(p, prefix): @@ -136,12 +137,12 @@ def _get_to_keep(keep, root, item): pass -def create_one(root, install, shortcut): +def create_one(root, install, shortcut, warn_for=[]): root = Path(_native.shortcut_get_start_programs()) / root - _make(root, install["prefix"], shortcut) + _make(root, install["prefix"], shortcut, allow_warn=install_matches_any(install, warn_for)) -def cleanup(root, preserve): +def cleanup(root, preserve, warn_for=[]): root = Path(_native.shortcut_get_start_programs()) / root if not root.is_dir(): From 1e266894bd3008e43a4dcdcd82c37f96654014de Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 22:06:34 +0100 Subject: [PATCH 2/5] Missed warning --- src/manage/startutils.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/manage/startutils.py b/src/manage/startutils.py index e9da0e0..2ca0b9f 100644 --- a/src/manage/startutils.py +++ b/src/manage/startutils.py @@ -26,7 +26,7 @@ def _unprefix(p, prefix): return p -def _make(root, prefix, item): +def _make(root, prefix, item, allow_warn=True): n = item["Name"] try: _make_directory(root, n, prefix, item["Items"]) @@ -40,7 +40,10 @@ def _make(root, prefix, item): try: lnk.relative_to(root) except ValueError: - LOGGER.warn("Package attempted to create shortcut outside of its directory") + if allow_warn: + LOGGER.warn("Package attempted to create shortcut outside of its directory") + else: + LOGGER.debug("Package attempted to create shortcut outside of its directory") LOGGER.debug("Path: %s", lnk) LOGGER.debug("Directory: %s", root) return None From 296786f4559f56afbbd3858669cf394f83fbdc34 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 22:54:35 +0100 Subject: [PATCH 3/5] Add warning test --- tests/conftest.py | 4 ++-- tests/test_pep514utils.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index eaf3b56..4cc1e74 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -60,10 +60,10 @@ def quiet_log(): class LogCaptureHandler(list): - def skip_until(self, pattern, args=()): + def skip_until(self, pattern, args=None): return ('until', pattern, args) - def not_logged(self, pattern, args=()): + def not_logged(self, pattern, args=None): return ('not', pattern, args) def __call__(self, *cmp): diff --git a/tests/test_pep514utils.py b/tests/test_pep514utils.py index 994b151..214cf99 100644 --- a/tests/test_pep514utils.py +++ b/tests/test_pep514utils.py @@ -2,6 +2,7 @@ import winreg from manage import pep514utils +from manage import tagutils def test_is_tag_managed(registry, tmp_path): registry.setup(Company={ @@ -23,3 +24,37 @@ def test_is_tag_managed(registry, tmp_path): assert pep514utils._is_tag_managed(registry.key, r"Company\3.0", creating=True) with winreg.OpenKey(registry.key, r"Company\3.0.2"): pass + + +def test_is_tag_managed_warning_suppressed(registry, tmp_path, assert_log): + registry.setup(Company={ + "3.0.0": {"": "Just in the way here"}, + "3.0.1": {"": "Also in the way here"}, + }) + pep514utils.update_registry( + rf"HKEY_CURRENT_USER\{registry.root}", + dict(company="Company", tag="3.0.0"), + dict(kind="pep514", Key="Company\\3.0.0", InstallPath=dict(_="dir")), + warn_for=[tagutils.tag_or_range(r"Company\3.0.1")], + ) + assert_log( + "Registry key %s appears invalid.+", + assert_log.not_logged("An existing runtime is registered at %s"), + ) + + +def test_is_tag_managed_warning(registry, tmp_path, assert_log): + registry.setup(Company={ + "3.0.0": {"": "Just in the way here"}, + "3.0.1": {"": "Also in the way here"}, + }) + pep514utils.update_registry( + rf"HKEY_CURRENT_USER\{registry.root}", + dict(company="Company", tag="3.0.0"), + dict(kind="pep514", Key="Company\\3.0.0", InstallPath=dict(_="dir")), + warn_for=[tagutils.tag_or_range(r"Company\3.0.0")], + ) + assert_log( + "Registry key %s appears invalid.+", + assert_log.skip_until("An existing registry key for %s"), + ) From 61557b64330fa488f0b2f4a8abe94dc49c6dfff3 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 16:39:33 +0100 Subject: [PATCH 4/5] Fix uninstall and add uninstall test --- .github/workflows/build.yml | 7 +++++++ ci/release.yml | 8 ++++++++ src/manage/uninstall_command.py | 14 +++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f5571ef..9e4fa52 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -149,6 +149,13 @@ jobs: - name: 'Launch default runtime' run: pymanager exec -m site + env: + PYMANAGER_DEBUG: true + + - name: 'Uninstall runtime' + run: pymanager uninstall -y default + env: + PYMANAGER_DEBUG: true - name: 'Emulate first launch' run: | diff --git a/ci/release.yml b/ci/release.yml index 1542c62..c476cda 100644 --- a/ci/release.yml +++ b/ci/release.yml @@ -252,6 +252,14 @@ stages: - powershell: | pymanager exec -m site displayName: 'Launch default runtime' + env: + PYMANAGER_DEBUG: true + + - powershell: | + pymanager uninstall -y default + displayName: 'Uninstall runtime' + env: + PYMANAGER_DEBUG: true - powershell: | $i = (mkdir -force test_installs) diff --git a/src/manage/uninstall_command.py b/src/manage/uninstall_command.py index da117b7..5773437 100644 --- a/src/manage/uninstall_command.py +++ b/src/manage/uninstall_command.py @@ -26,6 +26,8 @@ def execute(cmd): cmd.virtual_env = None installed = list(cmd.get_installs()) + self.tags = [] + if cmd.purge: if cmd.ask_yn("Uninstall all runtimes?"): for i in installed: @@ -61,16 +63,18 @@ def execute(cmd): to_uninstall = [] if not cmd.by_id: for tag in cmd.args: - if tag.casefold() == "default".casefold(): - tag = cmd.default_tag try: - t_or_r = tag_or_range(tag) + if tag.casefold() == "default".casefold(): + self.tags.append(tag_or_range(cmd.default_tag)) + else: + self.tags.append(tag_or_range(tag)) except ValueError as ex: LOGGER.warn("%s", ex) - continue + + for tag in self.tags: candidates = get_matching_install_tags( installed, - t_or_r, + tag, default_platform=cmd.default_platform, ) if not candidates: From 1007e3bc5c6ef03db4c0f3feb13ed7d28d15bbe9 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 19:58:15 +0100 Subject: [PATCH 5/5] Fix selfs --- src/manage/uninstall_command.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/manage/uninstall_command.py b/src/manage/uninstall_command.py index 5773437..9574ef6 100644 --- a/src/manage/uninstall_command.py +++ b/src/manage/uninstall_command.py @@ -26,7 +26,7 @@ def execute(cmd): cmd.virtual_env = None installed = list(cmd.get_installs()) - self.tags = [] + cmd.tags = [] if cmd.purge: if cmd.ask_yn("Uninstall all runtimes?"): @@ -65,13 +65,13 @@ def execute(cmd): for tag in cmd.args: try: if tag.casefold() == "default".casefold(): - self.tags.append(tag_or_range(cmd.default_tag)) + cmd.tags.append(tag_or_range(cmd.default_tag)) else: - self.tags.append(tag_or_range(tag)) + cmd.tags.append(tag_or_range(tag)) except ValueError as ex: LOGGER.warn("%s", ex) - for tag in self.tags: + for tag in cmd.tags: candidates = get_matching_install_tags( installed, tag,