From 624ffb6ac682022d7aeafb73b3ee90e4ef408d38 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 19 Feb 2026 13:00:41 +0000 Subject: [PATCH 1/5] Rework alias linking to ensure links are only created within bin directory. Fixes #258 --- src/manage/aliasutils.py | 96 +++++++++++++++++++++------------------- tests/test_alias.py | 5 ++- 2 files changed, 53 insertions(+), 48 deletions(-) diff --git a/src/manage/aliasutils.py b/src/manage/aliasutils.py index dcf42ab..8a8f64c 100644 --- a/src/manage/aliasutils.py +++ b/src/manage/aliasutils.py @@ -129,17 +129,58 @@ def _create_alias( return existing_bytes = b'' - try: - with open(p, 'rb') as f: - existing_bytes = f.read(len(launcher_bytes) + 1) - except FileNotFoundError: - pass - except OSError: - LOGGER.debug("Failed to read existing alias launcher.") + if getattr(cmd, "force", False): + # Only expect InstallCommand to have .force + unlink(p) + else: + try: + with open(p, 'rb') as f: + existing_bytes = f.read(len(launcher_bytes) + 1) + except FileNotFoundError: + pass + except OSError: + LOGGER.debug("Failed to read existing alias launcher.") launcher_remap = cmd.scratch.setdefault("aliasutils.create_alias.launcher_remap", {}) - if not allow_link or not _link: - # If links are disallowed, always replace the target with a copy. + + if existing_bytes != launcher_bytes and allow_link and _link: + # Try to find an existing launcher we can hard-link + launcher2 = launcher_remap.get(launcher.name) + if not launcher2: + # None known, so search existing files + try: + LOGGER.debug("Searching %s for suitable launcher to link", cmd.global_dir) + for p2 in cmd.global_dir.glob("*.exe"): + try: + with open(p2, 'rb') as f: + existing_bytes2 = f.read(len(launcher_bytes) + 1) + except OSError: + LOGGER.debug("Failed to check %s contents", p2, exc_info=True) + else: + if existing_bytes2 == launcher_bytes: + launcher2 = p2 + break + else: + LOGGER.debug("No existing launcher available") + except Exception: + LOGGER.debug("Failed to find existing launcher", exc_info=True) + + if launcher2: + # We know that the target either doesn't exist or needs replacing + unlink(p) + try: + _link(launcher2, p) + existing_bytes = launcher_bytes + launcher_remap[launcher.name] = launcher2 + LOGGER.debug("Created %s as hard link to %s", p.name, launcher2.name) + except FileNotFoundError: + raise + except OSError: + LOGGER.debug("Failed to create hard link to %s", launcher2.name) + launcher2 = None + + # Recheck - existing_bytes will have been updated if we successfully linked + if existing_bytes != launcher_bytes: unlink(p) try: p.write_bytes(launcher_bytes) @@ -148,43 +189,6 @@ def _create_alias( except OSError: LOGGER.error("Failed to create global command %s.", name) LOGGER.debug("TRACEBACK", exc_info=True) - elif existing_bytes == launcher_bytes: - # Valid existing launcher, so save its path in case we need it later - # for a hard link. - launcher_remap.setdefault(launcher.name, p) - else: - # Links are allowed and we need to create one, so try to make a link, - # falling back to a link to another existing alias (that we've checked - # already during this run), and then falling back to a copy. - # This handles the case where our links are on a different volume to the - # install (so hard links don't work), but limits us to only a single - # copy (each) of the redirector(s), thus saving space. - unlink(p) - try: - _link(launcher, p) - LOGGER.debug("Created %s as hard link to %s", p.name, launcher.name) - except OSError as ex: - if ex.winerror != 17: - # Report errors other than cross-drive links - LOGGER.debug("Failed to create hard link for command.", exc_info=True) - launcher2 = launcher_remap.get(launcher.name) - if launcher2: - try: - _link(launcher2, p) - LOGGER.debug("Created %s as hard link to %s", p.name, launcher2.name) - except FileNotFoundError: - raise - except OSError: - LOGGER.debug("Failed to create hard link to fallback launcher") - launcher2 = None - if not launcher2: - try: - p.write_bytes(launcher_bytes) - LOGGER.debug("Created %s as copy of %s", p.name, launcher.name) - launcher_remap[launcher.name] = p - except OSError: - LOGGER.error("Failed to create global command %s.", name) - LOGGER.debug("TRACEBACK", exc_info=True) p_target = p.with_name(p.name + ".__target__") do_update = True diff --git a/tests/test_alias.py b/tests/test_alias.py index 17ebe5e..334533f 100644 --- a/tests/test_alias.py +++ b/tests/test_alias.py @@ -17,6 +17,7 @@ class Cmd: launcher_exe = "launcher.txt" launcherw_exe = "launcherw.txt" default_platform = "-64" + force = False def __init__(self, platform=None): self.scratch = {} @@ -184,7 +185,8 @@ def fake_link(x, y): ) assert_log( "Create %s linking to %s", - "Failed to create hard link.+", + "Searching %s for suitable launcher to link", + "No existing launcher available", "Created %s as copy of %s", assert_log.end_of_log(), ) @@ -218,7 +220,6 @@ def fake_link(x, y): ) assert_log( "Create %s linking to %s", - "Failed to create hard link.+", ("Created %s as hard link to %s", ("test.exe", "actual_launcher.txt")), assert_log.end_of_log(), ) From 768deb8ddfbda288a16f3496e061c8cc17dee641 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 19 Feb 2026 16:09:21 +0000 Subject: [PATCH 2/5] Slightly improved logic, significantly improve logging --- src/manage/aliasutils.py | 34 ++++++++++++++++++------------- src/manage/pathutils.py | 43 +++++++++++++++++++++++++++++++--------- src/manage/startutils.py | 4 ++-- tests/test_pathutils.py | 21 +++++++++++++++++++- 4 files changed, 76 insertions(+), 26 deletions(-) diff --git a/src/manage/aliasutils.py b/src/manage/aliasutils.py index 8a8f64c..ddf0311 100644 --- a/src/manage/aliasutils.py +++ b/src/manage/aliasutils.py @@ -3,7 +3,7 @@ from .exceptions import FilesInUseError, NoLauncherTemplateError from .fsutils import atomic_unlink, ensure_tree, unlink from .logging import LOGGER -from .pathutils import Path +from .pathutils import Path, relative_to from .tagutils import install_matches_any _EXE = ".exe".casefold() @@ -105,16 +105,18 @@ def _create_alias( if windowed: launcher = cmd.launcherw_exe or launcher + chosen_by = "default" if plat: - LOGGER.debug("Checking for launcher for platform -%s", plat) launcher = _if_exists(launcher, f"-{plat}") + chosen_by = "platform tag" if not launcher.is_file(): - LOGGER.debug("Checking for launcher for default platform %s", cmd.default_platform) launcher = _if_exists(launcher, cmd.default_platform) + chosen_by = "default platform" if not launcher.is_file(): - LOGGER.debug("Checking for launcher for -64") launcher = _if_exists(launcher, "-64") - LOGGER.debug("Create %s linking to %s using %s", name, target, launcher) + chosen_by = "fallback default" + LOGGER.debug("Create %s for %s using %s, chosen by %s", name, + relative_to(target, cmd.install_dir), launcher, chosen_by) if not launcher or not launcher.is_file(): raise NoLauncherTemplateError() @@ -128,8 +130,9 @@ def _create_alias( LOGGER.debug("Failed to read %s", launcher, exc_info=True) return + force = getattr(cmd, "force", False) existing_bytes = b'' - if getattr(cmd, "force", False): + if force: # Only expect InstallCommand to have .force unlink(p) else: @@ -146,8 +149,10 @@ def _create_alias( if existing_bytes != launcher_bytes and allow_link and _link: # Try to find an existing launcher we can hard-link launcher2 = launcher_remap.get(launcher.name) - if not launcher2: - # None known, so search existing files + if (not launcher2 or not launcher2.is_file()) and not force: + # None known, so search existing files. Or, user is forcing us, so + # we only want to use an existing launcher if we've cached it this + # session. try: LOGGER.debug("Searching %s for suitable launcher to link", cmd.global_dir) for p2 in cmd.global_dir.glob("*.exe"): @@ -165,10 +170,11 @@ def _create_alias( except Exception: LOGGER.debug("Failed to find existing launcher", exc_info=True) - if launcher2: + if launcher2 and launcher2.is_file(): # We know that the target either doesn't exist or needs replacing unlink(p) try: + LOGGER.debug("Creating %s as hard link to %s", p, launcher2) _link(launcher2, p) existing_bytes = launcher_bytes launcher_remap[launcher.name] = launcher2 @@ -256,13 +262,13 @@ def _readlines(path): return -def _scan_one(install, root): +def _scan_one(cmd, install, root): # Scan d for dist-info directories with entry_points.txt dist_info = [d for d in root.glob("*.dist-info") if d.is_dir()] entrypoints = [f for f in [d / "entry_points.txt" for d in dist_info] if f.is_file()] if len(entrypoints): LOGGER.debug("Found %i entry_points.txt files in %i dist-info in %s", - len(entrypoints), len(dist_info), root) + len(entrypoints), len(dist_info), relative_to(root, cmd.install_dir)) # Filter down to [console_scripts] and [gui_scripts] for ep in entrypoints: @@ -281,10 +287,10 @@ def _scan_one(install, root): mod=mod, func=func, **alias) -def _scan(install, prefix, dirs): +def _scan(cmd, install, prefix, dirs): for dirname in dirs or (): root = prefix / dirname - yield from _scan_one(install, root) + yield from _scan_one(cmd, install, root) def calculate_aliases(cmd, install, *, _scan=_scan): @@ -326,7 +332,7 @@ def calculate_aliases(cmd, install, *, _scan=_scan): site_dirs = s.get("dirs", ()) break - for ai in _scan(install, prefix, site_dirs): + for ai in _scan(cmd, install, prefix, site_dirs): if ai.windowed and default_alias_w: yield ai.replace(target=default_alias_w.target) elif not ai.windowed and default_alias: diff --git a/src/manage/pathutils.py b/src/manage/pathutils.py index 4576573..1a89089 100644 --- a/src/manage/pathutils.py +++ b/src/manage/pathutils.py @@ -7,6 +7,10 @@ import os +def _eq(x, y): + return x == y or x.casefold() == y.casefold() + + class PurePath: def __init__(self, *parts): total = "" @@ -14,7 +18,7 @@ def __init__(self, *parts): try: p = p.__fspath__().replace("/", "\\") except AttributeError: - p = str(p).replace("/", "\\") + p = os.fsdecode(p).replace("/", "\\") p = p.replace("\\\\", "\\") if p == ".": continue @@ -24,8 +28,11 @@ def __init__(self, *parts): total += "\\" + p else: total += p - self._parent, _, self.name = total.rpartition("\\") - self._p = total.rstrip("\\") + drive, root, tail = os.path.splitroot(total) + parent, _, name = tail.rpartition("\\") + self._parent = drive + root + parent + self.name = name + self._p = drive + root + tail.rstrip("\\") def __fspath__(self): return self._p @@ -36,6 +43,9 @@ def __repr__(self): def __str__(self): return self._p + def __bytes__(self): + return os.fsencode(self) + def __hash__(self): return hash(self._p.casefold()) @@ -86,13 +96,13 @@ def __truediv__(self, other): def __eq__(self, other): if isinstance(other, PurePath): - return self._p.casefold() == other._p.casefold() - return self._p.casefold() == str(other).casefold() + return _eq(self._p, other._p) + return _eq(self._p == str(other)) def __ne__(self, other): if isinstance(other, PurePath): - return self._p.casefold() != other._p.casefold() - return self._p.casefold() != str(other).casefold() + return not _eq(self._p, other._p) + return not _eq(self._p, str(other)) def with_name(self, name): return type(self)(os.path.join(self._parent, name)) @@ -105,7 +115,7 @@ def with_suffix(self, suffix): def relative_to(self, base): base = PurePath(base).parts parts = self.parts - if not all(x.casefold() == y.casefold() for x, y in zip(base, parts)): + if not all(_eq(x, y) for x, y in zip(base, parts)): raise ValueError("path not relative to base") return type(self)("\\".join(parts[len(base):])) @@ -128,7 +138,7 @@ def match(self, pattern, full_match=False): m = m.casefold() if "*" not in p: - return m.casefold() == p + return m == p or m.casefold() == p must_start_with = True for bit in p.split("*"): @@ -219,3 +229,18 @@ def write_bytes(self, data): def write_text(self, text, encoding="utf-8", errors="strict"): with open(self._p, "w", encoding=encoding, errors=errors) as f: f.write(text) + + +def relative_to(path, root): + if not root: + return path + parts_1 = list(PurePath(path).parts) + parts_2 = list(PurePath(root).parts) + while parts_1 and parts_2 and _eq(parts_1[0], parts_2[0]): + parts_1.pop(0) + parts_2.pop(0) + if parts_1 and not parts_2: + if isinstance(path, PurePath): + return type(path)(*parts_1) + return type(path)(PurePath(*parts_1)) + return path diff --git a/src/manage/startutils.py b/src/manage/startutils.py index 76349c2..20efcb5 100644 --- a/src/manage/startutils.py +++ b/src/manage/startutils.py @@ -2,7 +2,7 @@ from .fsutils import rmtree, unlink from .logging import LOGGER -from .pathutils import Path +from .pathutils import Path, relative_to from .tagutils import install_matches_any @@ -36,7 +36,7 @@ def _make(root, prefix, item, allow_warn=True): lnk = root / (n + ".lnk") target = _unprefix(item["Target"], prefix) - LOGGER.debug("Creating shortcut %s to %s", lnk, target) + LOGGER.debug("Creating shortcut %s to %s", relative_to(lnk, root), target) try: lnk.relative_to(root) except ValueError: diff --git a/tests/test_pathutils.py b/tests/test_pathutils.py index 54ebc41..42e9ee3 100644 --- a/tests/test_pathutils.py +++ b/tests/test_pathutils.py @@ -1,6 +1,6 @@ import pytest -from manage.pathutils import Path, PurePath +from manage.pathutils import Path, PurePath, relative_to def test_path_match(): p = Path("python3.12.exe") @@ -30,3 +30,22 @@ def test_path_stem(): p = Path(".exe") assert p.stem == "" assert p.suffix == ".exe" + + +def test_path_relative_to(): + p = Path(r"C:\A\B\C\python.exe") + actual = relative_to(p, r"C:\A\B\C") + assert isinstance(actual, Path) + assert str(actual) == "python.exe" + actual = relative_to(p, "C:\\") + assert isinstance(actual, Path) + assert str(actual) == r"A\B\C\python.exe" + actual = relative_to(str(p), r"C:\A\B") + assert isinstance(actual, str) + assert actual == r"C\python.exe" + actual = relative_to(bytes(p), r"C:\A\B") + assert isinstance(actual, bytes) + assert actual == rb"C\python.exe" + + assert relative_to(p, r"C:\A\B\C\D") is p + assert relative_to(p, None) is p From c1bfec8bfb19f94cc1d4e70f8a0c377dd517154f Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 19 Feb 2026 17:21:51 +0000 Subject: [PATCH 3/5] Allow install_dir to be optional for logging --- src/manage/aliasutils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/manage/aliasutils.py b/src/manage/aliasutils.py index ddf0311..fd15cdc 100644 --- a/src/manage/aliasutils.py +++ b/src/manage/aliasutils.py @@ -116,7 +116,8 @@ def _create_alias( launcher = _if_exists(launcher, "-64") chosen_by = "fallback default" LOGGER.debug("Create %s for %s using %s, chosen by %s", name, - relative_to(target, cmd.install_dir), launcher, chosen_by) + relative_to(target, getattr(cmd, "install_dir", None)), + launcher, chosen_by) if not launcher or not launcher.is_file(): raise NoLauncherTemplateError() @@ -268,7 +269,8 @@ def _scan_one(cmd, install, root): entrypoints = [f for f in [d / "entry_points.txt" for d in dist_info] if f.is_file()] if len(entrypoints): LOGGER.debug("Found %i entry_points.txt files in %i dist-info in %s", - len(entrypoints), len(dist_info), relative_to(root, cmd.install_dir)) + len(entrypoints), len(dist_info), + relative_to(root, getattr(cmd, "install_dir", None))) # Filter down to [console_scripts] and [gui_scripts] for ep in entrypoints: From 46c08545c8deac2cf6d700835ca3b21085a9b782 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 19 Feb 2026 17:31:40 +0000 Subject: [PATCH 4/5] Remove unnecessary message and fix tests --- src/manage/aliasutils.py | 1 - tests/test_alias.py | 13 +++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/manage/aliasutils.py b/src/manage/aliasutils.py index fd15cdc..76c4701 100644 --- a/src/manage/aliasutils.py +++ b/src/manage/aliasutils.py @@ -175,7 +175,6 @@ def _create_alias( # We know that the target either doesn't exist or needs replacing unlink(p) try: - LOGGER.debug("Creating %s as hard link to %s", p, launcher2) _link(launcher2, p) existing_bytes = launcher_bytes launcher_remap[launcher.name] = launcher2 diff --git a/tests/test_alias.py b/tests/test_alias.py index 334533f..b1da40d 100644 --- a/tests/test_alias.py +++ b/tests/test_alias.py @@ -129,10 +129,7 @@ def test_write_alias_launcher_missing(fake_config, assert_log, tmp_path): target=tmp_path / "target.exe", ) assert_log( - "Checking for launcher.*", - "Checking for launcher.*", - "Checking for launcher.*", - "Create %s linking to %s", + "Create %s for %s using %s, chosen by %s", assert_log.end_of_log(), ) @@ -161,7 +158,7 @@ def read_bytes(): target=tmp_path / "target.exe", ) assert_log( - "Create %s linking to %s", + "Create %s for %s", "Failed to read launcher template at %s\\.", "Failed to read %s", assert_log.end_of_log(), @@ -184,7 +181,7 @@ def fake_link(x, y): _link=fake_link ) assert_log( - "Create %s linking to %s", + "Create %s for %s", "Searching %s for suitable launcher to link", "No existing launcher available", "Created %s as copy of %s", @@ -219,7 +216,7 @@ def fake_link(x, y): _link=fake_link ) assert_log( - "Create %s linking to %s", + "Create %s for %s", ("Created %s as hard link to %s", ("test.exe", "actual_launcher.txt")), assert_log.end_of_log(), ) @@ -241,7 +238,7 @@ def test_write_alias_launcher_no_linking(fake_config, assert_log, tmp_path): _link=None ) assert_log( - "Create %s linking to %s", + "Create %s for %s", ("Created %s as copy of %s", ("test.exe", "launcher.txt")), assert_log.end_of_log(), ) From 2b619dc0bc5268f9220c15cc25c2f26640ad70ed Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 19 Feb 2026 19:36:56 +0000 Subject: [PATCH 5/5] Update src/manage/pathutils.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/manage/pathutils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manage/pathutils.py b/src/manage/pathutils.py index 1a89089..f2798e8 100644 --- a/src/manage/pathutils.py +++ b/src/manage/pathutils.py @@ -97,7 +97,7 @@ def __truediv__(self, other): def __eq__(self, other): if isinstance(other, PurePath): return _eq(self._p, other._p) - return _eq(self._p == str(other)) + return _eq(self._p, str(other)) def __ne__(self, other): if isinstance(other, PurePath):