From 4aec6d1d126936700f757ae80291526e59a47383 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Mon, 10 Jun 2024 12:29:13 -0400 Subject: [PATCH] Perform proper case-insensitive matching in command_cache on Windows (#5477) Closes #5476 Closes #5469 --- docs/platform-issues.rst | 25 ++++++++++++++ news/5477.rst | 23 +++++++++++++ pyproject.toml | 3 ++ tests/test_commands_cache.py | 27 +++++++++++++-- xonsh/commands_cache.py | 66 ++++++++++++------------------------ 5 files changed, 97 insertions(+), 47 deletions(-) create mode 100644 news/5477.rst diff --git a/docs/platform-issues.rst b/docs/platform-issues.rst index fd253f443..d2fe7ccf1 100644 --- a/docs/platform-issues.rst +++ b/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. diff --git a/news/5477.rst b/news/5477.rst new file mode 100644 index 000000000..86e1fc72c --- /dev/null +++ b/news/5477.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* Minor cleanup of ``commands_cache``, unifying behavior across platforms. + +**Deprecated:** + +* + +**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:** + +* diff --git a/pyproject.toml b/pyproject.toml index 20cadeaf8..6e0e13975 100644 --- a/pyproject.toml +++ b/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__"} diff --git a/tests/test_commands_cache.py b/tests/test_commands_cache.py index 61428922f..a586b9b02 100644 --- a/tests/test_commands_cache.py +++ b/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" diff --git a/xonsh/commands_cache.py b/xonsh/commands_cache.py index b86a85c1c..07cc046c1 100644 --- a/xonsh/commands_cache.py +++ b/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]