path and remote_file_cache: support windows paths (#49437)

Windows paths with drives were being interpreted as network protocols
in canonicalize_path (which was expanded to handle more general URLs
in #48784).

This fixes that and adds some tests for it.

lib/spack/spack/test/util/path.py

@@ -134,3 +134,18 @@ def test_path_debug_padded_filter(debug, monkeypatch):
     monkeypatch.setattr(tty, "_debug", debug)
     with spack.config.override("config:install_tree", {"padded_length": 128}):
         assert expected == sup.debug_padded_filter(string)
+
+
+@pytest.mark.parametrize(
+    "path,expected",
+    [
+        ("/home/spack/path/to/file.txt", "/home/spack/path/to/file.txt"),
+        ("file:///home/another/config.yaml", "/home/another/config.yaml"),
+        ("path/to.txt", os.path.join(os.environ["SPACK_ROOT"], "path", "to.txt")),
+        (r"C:\Files (x86)\Windows\10", r"C:\Files (x86)\Windows\10"),
+        (r"E:/spack stage", "E:\\spack stage"),
+    ],
+)
+def test_canonicalize_file(path, expected):
+    """Confirm canonicalize path handles local files and file URLs."""
+    assert sup.canonicalize_path(path) == os.path.normpath(expected)

lib/spack/spack/test/util/remote_file_cache.py

@@ -29,11 +29,20 @@ def test_rfc_local_path_bad_scheme(path, err):
 
 
 @pytest.mark.parametrize(
-    "path", ["/a/b/c/d/e/config.py", "file:///this/is/a/file/url/include.yaml"]
+    "path,expected",
+    [
+        ("/a/b/c/d/e/config.py", "/a/b/c/d/e/config.py"),
+        ("file:///this/is/a/file/url/include.yaml", "/this/is/a/file/url/include.yaml"),
+        (
+            "relative/packages.txt",
+            os.path.join(os.environ["SPACK_ROOT"], "relative", "packages.txt"),
+        ),
+        (r"C:\Files (x86)\Windows\10", r"C:\Files (x86)\Windows\10"),
+        (r"D:/spack stage", "D:\\spack stage"),
+    ],
 )
-def test_rfc_local_path_file(path):
-    actual = path.split("://")[1] if ":" in path else path
-    assert rfc_util.local_path(path, "") == os.path.normpath(actual)
+def test_rfc_local_file(path, expected):
+    assert rfc_util.local_path(path, "") == os.path.normpath(expected)
 
 
 def test_rfc_remote_local_path_no_dest():

lib/spack/spack/util/path.py

@@ -9,6 +9,7 @@
 import contextlib
 import getpass
 import os
+import pathlib
 import re
 import subprocess
 import sys
@@ -245,6 +246,7 @@ def canonicalize_path(path: str, default_wd: Optional[str] = None) -> str:
 
     Arguments:
         path: path being converted as needed
+        default_wd: optional working directory/root for non-yaml string paths
 
     Returns: An absolute path or non-file URL with path variable substitution
     """
@@ -260,6 +262,14 @@ def canonicalize_path(path: str, default_wd: Optional[str] = None) -> str:
 
     path = substitute_path_variables(path)
 
+    # Ensure properly process a Windows path
+    win_path = pathlib.PureWindowsPath(path)
+    if win_path.drive:
+        # Assume only absolute paths are supported with a Windows drive
+        # (though DOS does allow drive-relative paths).
+        return os.path.normpath(str(win_path))
+
+    # Now process linux-like paths and remote URLs
     url = urllib.parse.urlparse(path)
     url_path = urllib.request.url2pathname(url.path)
     if url.scheme:
@@ -270,15 +280,18 @@ def canonicalize_path(path: str, default_wd: Optional[str] = None) -> str:
         # Drop the URL scheme from the local path
         path = url_path
 
-    if not os.path.isabs(path):
-        if filename:
-            path = os.path.join(filename, path)
-        else:
-            base = default_wd or os.getcwd()
-            path = os.path.join(base, path)
-            tty.debug(f"Using working directory {base} as base for abspath")
+    if os.path.isabs(path):
+        return os.path.normpath(path)
 
-    return os.path.normpath(path)
+    # Have a relative path so prepend the appropriate dir to make it absolute
+    if filename:
+        # Prepend the directory of the syaml path
+        return os.path.normpath(os.path.join(filename, path))
+
+    # Prepend the default, if provided, or current working directory.
+    base = default_wd or os.getcwd()
+    tty.debug(f"Using working directory {base} as base for abspath")
+    return os.path.normpath(os.path.join(base, path))
 
 
 def longest_prefix_re(string, capture=True):

lib/spack/spack/util/remote_file_cache.py

@@ -4,6 +4,7 @@
 
 import hashlib
 import os.path
+import pathlib
 import shutil
 import tempfile
 import urllib.parse
@@ -68,7 +69,7 @@ def local_path(raw_path: str, sha256: str, make_dest: Optional[Callable[[], str]
         sha256: the expected sha256 for the file
         make_dest: function to create a stage for remote files, if needed (e.g., `mkdtemp`)
 
-    Returns: resolved, normalized local path or None
+    Returns: resolved, normalized local path
 
     Raises:
         ValueError: missing or mismatched arguments, unsupported URL scheme
@@ -76,17 +77,25 @@ def local_path(raw_path: str, sha256: str, make_dest: Optional[Callable[[], str]
     if not raw_path:
         raise ValueError("path argument is required to cache remote files")
 
+    file_schemes = ["", "file"]
+
     # Allow paths (and URLs) to contain spack config/environment variables,
     # etc.
     path = canonicalize_path(raw_path)
+
+    # Save off the Windows drive of the canonicalized path (since now absolute)
+    # to ensure recognized by URL parsing as a valid file "scheme".
+    win_path = pathlib.PureWindowsPath(path)
+    if win_path.drive:
+        file_schemes.append(win_path.drive.lower().strip(":"))
+
     url = urllib.parse.urlparse(path)
 
-    # Path isn't remote so return absolute, normalized path with substitutions.
-    if url.scheme in ["", "file"]:
-        return path
+    # Path isn't remote so return normalized, absolute path with substitutions.
+    if url.scheme in file_schemes:
+        return os.path.normpath(path)
 
-    # If scheme is not valid, path is not a url
-    # of a type Spack is generally aware
+    # If scheme is not valid, path is not a supported url.
     if validate_scheme(url.scheme):
         # Fetch files from supported URL schemes.
         if url.scheme in ("http", "https", "ftp"):