Perform proper case-insensitive matching in command_cache on Windows (#5477)

Closes #5476
Closes #5469

docs/platform-issues.rst

@@ -241,3 +241,28 @@ You can add aliases to your ``~/.xonshrc`` to have it always
 available when xonsh starts.
 
 
+Working Directory on PATH
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Windows users, particularly those coming from the ``cmd.exe`` shell,
+might be accustomed to being able to run executables from the current
+directory by simply typing the program name.
+
+Since version 0.16, ``xonsh`` follows the more secure and modern
+approach of not including the current working directory in the search
+path, similar to Powershell and popular Unix shells. To invoke commands
+in the current directory on any platform, include the current directory
+explicitly:
+
+.. code-block:: xonshcon
+
+    >>> ./my-program
+
+Although not recommended, to restore the behavior found in the
+``cmd.exe`` shell, simply append ``.`` to the ``PATH``:
+
+.. code-block:: xonshcon
+
+    >>> $PATH.append('.')
+
+Add that to ``~/.xonshrc`` to enable that as the default behavior.

news/5477.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* Minor cleanup of ``commands_cache``, unifying behavior across platforms.
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* No longer is ``.`` implied for running commands on Windows. Instead the behavior is the same across platforms. Windows users will need to prefix ``./`` or ``.\`` to run commands from the current directory (#5476).
+
+**Fixed:**
+
+* Commands on Windows now honor the case as they appear on the file system (#5469).
+
+**Security:**
+
+* <news item>

pyproject.toml

@@ -15,6 +15,9 @@ authors = [{ name = "Anthony Scopatz" }, { email = "scopatz@gmail.com" }]
 maintainers = [{ name = "Anthony Scopatz" }, { email = "scopatz@gmail.com" }]
 license = { text = "BSD 2-Clause License" }
 requires-python = ">=3.9"
+dependencies = [
+    "case-insensitive-dictionary; platform_system=='Windows'",
+]
 
 [tool.setuptools.dynamic]
 version = {attr = "xonsh.__version__"}

tests/test_commands_cache.py

@@ -157,7 +157,6 @@ def test_commands_cache_predictor_default(args, xession, tmp_path):
 
 
 class Test_is_only_functional_alias:
-    @skip_if_on_windows
     def test_cd(self, xession):
         xession.aliases["cd"] = lambda args: os.chdir(args[0])
         xession.env["PATH"] = []
@@ -204,7 +203,31 @@ def test_update_cache(xession, tmp_path):
     assert file2.samefile(cached[basename][0])
 
 
-@skip_if_on_windows
+@pytest.fixture
+def faux_binary(tmp_path):
+    """
+    A fake binary in the temp path.
+
+    Uses mixed case so tests may make assertions about it.
+    """
+    binary = tmp_path / "RunMe.exe"
+    binary.touch()
+    binary.chmod(0o755)
+    return binary
+
+
+def test_find_binary_retains_case(faux_binary):
+    cache = CommandsCache({"PATH": []})
+    loc = cache.locate_binary(str(faux_binary))
+    assert faux_binary.name in loc
+
+
+def test_exes_in_cwd_are_not_matched(faux_binary, monkeypatch):
+    monkeypatch.chdir(faux_binary.parent)
+    cache = CommandsCache({"PATH": []})
+    assert cache.locate_binary(faux_binary.name) is None
+
+
 def test_nixos_coreutils(tmp_path):
     """On NixOS the core tools are the symlinks to one universal ``coreutils`` binary file."""
     path = tmp_path / "core"

xonsh/commands_cache.py

@@ -18,6 +18,11 @@ from xonsh.lazyasd import lazyobject
 from xonsh.platform import ON_POSIX, ON_WINDOWS, pathbasename
 from xonsh.tools import executables_in
 
+if ON_WINDOWS:
+    from case_insensitive_dict import CaseInsensitiveDict as CacheDict
+else:
+    CacheDict = dict
+
 
 class _Commands(tp.NamedTuple):
     mtime: float
@@ -80,12 +85,7 @@ class CommandsCache(cabc.Mapping):
 
     def iter_commands(self):
         """Wrapper for handling windows path behaviour"""
-        for cmd, (path, is_alias) in self.all_commands.items():
-            if ON_WINDOWS and path is not None:
-                # All command keys are stored in uppercase on Windows.
-                # This ensures the original command name is returned.
-                cmd = pathbasename(path)
-            yield cmd, (path, is_alias)
+        return self.all_commands.items()
 
     def __len__(self):
         return len(self.all_commands)
@@ -99,15 +99,16 @@ class CommandsCache(cabc.Mapping):
         return len(self._cmds_cache) == 0
 
     def get_possible_names(self, name):
-        """Generates the possible `PATHEXT` extension variants of a given executable
-        name on Windows as a list, conserving the ordering in `PATHEXT`.
-        Returns a list as `name` being the only item in it on other platforms."""
-        if ON_WINDOWS:
-            pathext = [""] + self.env.get("PATHEXT", [])
-            name = name.upper()
-            return [name + ext for ext in pathext]
-        else:
-            return [name]
+        """Expand name to all possible variants based on `PATHEXT`.
+
+        PATHEXT is a Windows convention containing extensions to be
+        considered when searching for an executable file.
+
+        Conserves order of any extensions found and gives precedence
+        to the bare name.
+        """
+        extensions = [""] + self.env.get("PATHEXT", [])
+        return [name + ext for ext in extensions]
 
     @staticmethod
     def remove_dups(paths):
@@ -159,11 +160,10 @@ class CommandsCache(cabc.Mapping):
         # entries at the back.
         paths = tuple(reversed(tuple(self.remove_dups(env.get("PATH") or []))))
         if any(self._check_changes(paths)):
-            all_cmds = {}
+            all_cmds = CacheDict()
             for cmd, path in self._iter_binaries(paths):
-                key = cmd.upper() if ON_WINDOWS else cmd
                 # None     -> not in aliases
-                all_cmds[key] = (path, None)
+                all_cmds[cmd] = (path, None)
 
             # aliases override cmds
             for cmd in self.aliases:
@@ -178,9 +178,8 @@ class CommandsCache(cabc.Mapping):
                     # (path, False) -> has same named alias
                     all_cmds[override_key] = (all_cmds[override_key][0], False)
                 else:
-                    key = cmd.upper() if ON_WINDOWS else cmd
                     # True -> pure alias
-                    all_cmds[key] = (cmd, True)
+                    all_cmds[cmd] = (cmd, True)
             self._cmds_cache = all_cmds
         return self._cmds_cache
 
@@ -218,13 +217,9 @@ class CommandsCache(cabc.Mapping):
 
     def cached_name(self, name):
         """Returns the name that would appear in the cache, if it exists."""
-        if name is None:
-            return None
         cached = pathbasename(name) if os.pathsep in name else name
-        if ON_WINDOWS:
-            keys = self.get_possible_names(cached)
-            cached = next((k for k in keys if k in self._cmds_cache), None)
-        return cached
+        keys = self.get_possible_names(cached)
+        return next((k for k in keys if k in self._cmds_cache), name)
 
     def lazyin(self, key):
         """Checks if the value is in the current cache without the potential to
@@ -262,7 +257,6 @@ class CommandsCache(cabc.Mapping):
             Force return of binary path even if alias of ``name`` exists
             (default ``False``)
         """
-        # make sure the cache is up to date by accessing the property
         self.update_cache()
         return self.lazy_locate_binary(name, ignore_alias)
 
@@ -278,12 +272,6 @@ class CommandsCache(cabc.Mapping):
             (default ``False``)
         """
         possibilities = self.get_possible_names(name)
-        if ON_WINDOWS:
-            # Windows users expect to be able to execute files in the same
-            # directory without `./`
-            local_bin = next((fn for fn in possibilities if os.path.isfile(fn)), None)
-            if local_bin:
-                return os.path.abspath(local_bin)
         cached = next((cmd for cmd in possibilities if cmd in self._cmds_cache), None)
         if cached:
             (path, alias) = self._cmds_cache[cached]
@@ -329,18 +317,6 @@ class CommandsCache(cabc.Mapping):
         """
         name = self.cached_name(cmd0)
         predictors = self.threadable_predictors
-        if ON_WINDOWS:
-            # On all names (keys) are stored in upper case so instead
-            # we get the original cmd or alias name
-            path, _ = self.lazyget(name, (None, None))
-            if path is None:
-                return predict_true
-            else:
-                name = pathbasename(path)
-            if name not in predictors:
-                pre, ext = os.path.splitext(name)
-                if pre in predictors:
-                    predictors[name] = predictors[pre]
         if name not in predictors:
             predictors[name] = self.default_predictor(name, cmd0)
         predictor = predictors[name]