Rework alias linking to ensure links are only created within bin directory.

Fixes #258

Author: zooba

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

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(),
     )