refactor: move windows path logic out of spack.util.path and into llnl.util.path

lib/spack/llnl/util/filesystem.py

@@ -20,10 +20,11 @@
 
 from llnl.util import tty
 from llnl.util.lang import dedupe, memoized
+from llnl.util.path import path_to_os_path, system_path_filter
 from llnl.util.symlink import islink, symlink
 
 from spack.util.executable import CommandNotFoundError, Executable, which
-from spack.util.path import path_to_os_path, system_path_filter
+
 
 is_windows = _platform == "win32"
 

lib/spack/llnl/util/path.py

@@ -0,0 +1,159 @@
+# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+"""Utilities for managing Linux and Windows paths."""
+
+# TODO: look at using pathlib since we now only support Python 3
+
+import os
+import re
+import sys
+from urllib.parse import urlparse
+
+is_windows = sys.platform == "win32"
+
+
+def is_path_url(path):
+    if "\\" in path:
+        return False
+    url_tuple = urlparse(path)
+    return bool(url_tuple.scheme) and len(url_tuple.scheme) > 1
+
+
+def win_exe_ext():
+    return ".exe"
+
+
+def path_to_os_path(*pths):
+    """
+    Takes an arbitrary number of positional parameters
+    converts each arguemnt of type string to use a normalized
+    filepath separator, and returns a list of all values
+    """
+    ret_pths = []
+    for pth in pths:
+        if isinstance(pth, str) and not is_path_url(pth):
+            pth = convert_to_platform_path(pth)
+        ret_pths.append(pth)
+    return ret_pths
+
+
+def sanitize_file_path(pth):
+    """
+    Formats strings to contain only characters that can
+    be used to generate legal file paths.
+
+    Criteria for legal files based on
+    https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations
+
+    Args:
+        pth: string containing path to be created
+            on the host filesystem
+
+    Return:
+        sanitized string that can legally be made into a path
+    """
+    # on unix, splitting path by seperators will remove
+    # instances of illegal characters on join
+    pth_cmpnts = pth.split(os.path.sep)
+
+    if is_windows:
+        drive_match = r"[a-zA-Z]:"
+        is_abs = bool(re.match(drive_match, pth_cmpnts[0]))
+        drive = pth_cmpnts[0] + os.path.sep if is_abs else ""
+        pth_cmpnts = pth_cmpnts[1:] if drive else pth_cmpnts
+        illegal_chars = r'[<>?:"|*\\]'
+    else:
+        drive = "/" if not pth_cmpnts[0] else ""
+        illegal_chars = r"[/]"
+
+    pth = []
+    for cmp in pth_cmpnts:
+        san_cmp = re.sub(illegal_chars, "", cmp)
+        pth.append(san_cmp)
+    return drive + os.path.join(*pth)
+
+
+def system_path_filter(_func=None, arg_slice=None):
+    """
+    Filters function arguments to account for platform path separators.
+    Optional slicing range can be specified to select specific arguments
+
+    This decorator takes all (or a slice) of a method's positional arguments
+    and normalizes usage of filepath separators on a per platform basis.
+
+    Note: **kwargs, urls, and any type that is not a string are ignored
+    so in such cases where path normalization is required, that should be
+    handled by calling path_to_os_path directly as needed.
+
+    Parameters:
+        arg_slice (slice): a slice object specifying the slice of arguments
+            in the decorated method over which filepath separators are
+            normalized
+    """
+    from functools import wraps
+
+    def holder_func(func):
+        @wraps(func)
+        def path_filter_caller(*args, **kwargs):
+            args = list(args)
+            if arg_slice:
+                args[arg_slice] = path_to_os_path(*args[arg_slice])
+            else:
+                args = path_to_os_path(*args)
+            return func(*args, **kwargs)
+
+        return path_filter_caller
+
+    if _func:
+        return holder_func(_func)
+    return holder_func
+
+
+class Path:
+    """
+    Describes the filepath separator types
+    in an enum style
+    with a helper attribute
+    exposing the path type of
+    the current platform.
+    """
+
+    unix = 0
+    windows = 1
+    platform_path = windows if is_windows else unix
+
+
+def format_os_path(path, mode=Path.unix):
+    """
+    Format path to use consistent, platform specific
+    separators. Absolute paths are converted between
+    drive letters and a prepended '/' as per platform
+    requirement.
+
+    Parameters:
+        path (str): the path to be normalized, must be a string
+            or expose the replace method.
+        mode (Path): the path filesperator style to normalize the
+            passed path to. Default is unix style, i.e. '/'
+    """
+    if not path:
+        return path
+    if mode == Path.windows:
+        path = path.replace("/", "\\")
+    else:
+        path = path.replace("\\", "/")
+    return path
+
+
+def convert_to_posix_path(path):
+    return format_os_path(path, mode=Path.unix)
+
+
+def convert_to_windows_path(path):
+    return format_os_path(path, mode=Path.windows)
+
+
+def convert_to_platform_path(path):
+    return format_os_path(path, mode=Path.platform_path)

lib/spack/spack/binary_distribution.py

@@ -42,6 +42,7 @@
 import spack.store
 import spack.util.file_cache as file_cache
 import spack.util.gpg
+import spack.util.path
 import spack.util.spack_json as sjson
 import spack.util.spack_yaml as syaml
 import spack.util.url as url_util

lib/spack/spack/build_environment.py

@@ -64,7 +64,6 @@
 import spack.store
 import spack.subprocess_context
 import spack.user_environment
-import spack.util.path
 import spack.util.pattern
 from spack.error import NoHeadersError, NoLibrariesError
 from spack.installer import InstallError

lib/spack/spack/build_systems/cmake.py

@@ -15,7 +15,6 @@
 import spack.build_environment
 import spack.builder
 import spack.package_base
-import spack.util.path
 from spack.directives import build_system, depends_on, variant
 from spack.multimethod import when
 

lib/spack/spack/cmd/make_installer.py

@@ -6,10 +6,11 @@
 import posixpath
 import sys
 
+from llnl.util.path import convert_to_posix_path
+
 import spack.paths
 import spack.util.executable
 from spack.spec import Spec
-from spack.util.path import convert_to_posix_path
 
 description = "generate Windows installer"
 section = "admin"

lib/spack/spack/compiler.py

@@ -24,7 +24,7 @@
 import spack.util.module_cmd
 import spack.version
 from spack.util.environment import filter_system_paths
-from spack.util.path import system_path_filter
+from llnl.util.path import system_path_filter
 
 __all__ = ["Compiler"]
 

lib/spack/spack/install_test.py

@@ -14,6 +14,7 @@
 import spack.error
 import spack.paths
 import spack.util.prefix
+import spack.util.path
 import spack.util.spack_json as sjson
 from spack.spec import Spec
 

lib/spack/spack/main.py

@@ -46,7 +46,6 @@
 import spack.util.debug
 import spack.util.environment
 import spack.util.git
-import spack.util.path
 from spack.error import SpackError
 
 #: names of profile statistics

lib/spack/spack/package_base.py

@@ -31,6 +31,7 @@
 
 import llnl.util.filesystem as fsys
 import llnl.util.tty as tty
+import llnl.util.path
 from llnl.util.lang import classproperty, memoized, nullcontext
 from llnl.util.link_tree import LinkTree
 
@@ -204,9 +205,9 @@ def __init__(cls, name, bases, attr_dict):
             def platform_executables(cls):
                 def to_windows_exe(exe):
                     if exe.endswith("$"):
-                        exe = exe.replace("$", "%s$" % spack.util.path.win_exe_ext())
+                        exe = exe.replace("$", "%s$" % llnl.util.path.win_exe_ext())
                     else:
-                        exe += spack.util.path.win_exe_ext()
+                        exe += llnl.util.path.win_exe_ext()
                     return exe
 
                 plat_exe = []

lib/spack/spack/repo.py

@@ -30,6 +30,7 @@
 
 import llnl.util.filesystem as fs
 import llnl.util.lang
+import llnl.util.path
 import llnl.util.tty as tty
 from llnl.util.filesystem import working_dir
 
@@ -563,7 +564,7 @@ def __init__(self, package_checker, namespace, cache):
         self.checker = package_checker
         self.packages_path = self.checker.packages_path
         if sys.platform == "win32":
-            self.packages_path = spack.util.path.convert_to_posix_path(self.packages_path)
+            self.packages_path = llnl.util.path.convert_to_posix_path(self.packages_path)
         self.namespace = namespace
 
         self.indexers = {}

lib/spack/spack/spec.py

@@ -62,6 +62,7 @@
 import llnl.util.string
 import llnl.util.filesystem as fs
 import llnl.util.lang as lang
+import llnl.util.path as pth
 import llnl.util.tty as tty
 import llnl.util.tty.color as clr
 
@@ -83,7 +84,6 @@
 import spack.util.executable
 import spack.util.hash
 import spack.util.module_cmd as md
-import spack.util.path as pth
 import spack.util.prefix
 import spack.util.spack_json as sjson
 import spack.util.spack_yaml as syaml

lib/spack/spack/stage.py

@@ -28,6 +28,7 @@
     partition_path,
     remove_linked_tree,
 )
+from llnl.util.path import sanitize_file_path
 
 import spack.caches
 import spack.config
@@ -378,7 +379,7 @@ def expected_archive_files(self):
         expanded = True
         if isinstance(self.default_fetcher, fs.URLFetchStrategy):
             expanded = self.default_fetcher.expand_archive
-            clean_url = os.path.basename(sup.sanitize_file_path(self.default_fetcher.url))
+            clean_url = os.path.basename(sanitize_file_path(self.default_fetcher.url))
             fnames.append(clean_url)
 
         if self.mirror_paths:

lib/spack/spack/test/build_environment.py

@@ -11,6 +11,7 @@
 import pytest
 
 from llnl.util.filesystem import HeaderList, LibraryList
+from llnl.util.path import Path, convert_to_platform_path
 
 import spack.build_environment
 import spack.config
@@ -25,7 +26,6 @@
 from spack.paths import build_env_path
 from spack.util.environment import EnvironmentModifications
 from spack.util.executable import Executable
-from spack.util.path import Path, convert_to_platform_path
 
 
 def os_pathsep_join(path, *pths):

lib/spack/spack/test/cmd/bootstrap.py

@@ -7,13 +7,14 @@
 
 import pytest
 
+from llnl.util.path import convert_to_posix_path
+
 import spack.bootstrap
 import spack.bootstrap.core
 import spack.config
 import spack.environment as ev
 import spack.main
 import spack.mirror
-from spack.util.path import convert_to_posix_path
 
 _bootstrap = spack.main.SpackCommand("bootstrap")
 

lib/spack/spack/test/cmd/develop.py

@@ -12,6 +12,7 @@
 
 import spack.environment as ev
 import spack.spec
+import spack.util.path
 from spack.main import SpackCommand
 
 develop = SpackCommand("develop")

lib/spack/spack/test/cmd/external.py

@@ -8,6 +8,7 @@
 
 import pytest
 
+import llnl.util.path
 from llnl.util.filesystem import getuid, touch
 
 import spack
@@ -35,7 +36,7 @@ def _platform_executables(monkeypatch):
     def _win_exe_ext():
         return ".bat"
 
-    monkeypatch.setattr(spack.util.path, "win_exe_ext", _win_exe_ext)
+    monkeypatch.setattr(llnl.util.path, "win_exe_ext", _win_exe_ext)
 
 
 def define_plat_exe(exe):

lib/spack/spack/test/directory_layout.py

@@ -11,6 +11,8 @@
 
 import pytest
 
+from llnl.util.path import path_to_os_path
+
 import spack.paths
 import spack.repo
 from spack.directory_layout import (
@@ -18,7 +20,6 @@
     InvalidDirectoryLayoutParametersError,
 )
 from spack.spec import Spec
-from spack.util.path import path_to_os_path
 
 # number of packages to test (to reduce test time)
 max_packages = 10

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

@@ -0,0 +1,17 @@
+# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+import llnl.util.path as lup
+
+
+def test_sanitze_file_path(tmpdir):
+    """Test filtering illegal characters out of potential file paths"""
+    # *nix illegal files characters are '/' and none others
+    illegal_file_path = str(tmpdir) + "//" + "abcdefghi.txt"
+    if is_windows:
+        # Windows has a larger set of illegal characters
+        illegal_file_path = os.path.join(tmpdir, 'a<b>cd?e:f"g|h*i.txt')
+    real_path = lup.sanitize_file_path(illegal_file_path)
+    assert real_path == os.path.join(str(tmpdir), "abcdefghi.txt")

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

@@ -32,17 +32,6 @@
 ]
 
 
-def test_sanitze_file_path(tmpdir):
-    """Test filtering illegal characters out of potential file paths"""
-    # *nix illegal files characters are '/' and none others
-    illegal_file_path = str(tmpdir) + "//" + "abcdefghi.txt"
-    if is_windows:
-        # Windows has a larger set of illegal characters
-        illegal_file_path = os.path.join(tmpdir, 'a<b>cd?e:f"g|h*i.txt')
-    real_path = sup.sanitize_file_path(illegal_file_path)
-    assert real_path == os.path.join(str(tmpdir), "abcdefghi.txt")
-
-
 # This class pertains to path string padding manipulation specifically
 # which is used for binary caching. This functionality is not supported
 # on Windows as of yet.

lib/spack/spack/util/environment.py

@@ -18,12 +18,12 @@
 
 import llnl.util.tty as tty
 from llnl.util.lang import dedupe
+from llnl.util.path import path_to_os_path, system_path_filter
 
 import spack.config
 import spack.platforms
 import spack.spec
 import spack.util.executable as executable
-from spack.util.path import path_to_os_path, system_path_filter
 
 is_windows = sys.platform == "win32"
 

lib/spack/spack/util/executable.py

@@ -10,9 +10,9 @@
 import sys
 
 import llnl.util.tty as tty
+from llnl.util.path import Path, format_os_path, path_to_os_path, system_path_filter
 
 import spack.error
-from spack.util.path import Path, format_os_path, path_to_os_path, system_path_filter
 
 __all__ = ["Executable", "which", "ProcessError"]
 

lib/spack/spack/util/path.py

@@ -15,7 +15,6 @@
 import sys
 import tempfile
 from datetime import date
-from urllib.parse import urlparse
 
 import llnl.util.tty as tty
 from llnl.util.lang import memoized
@@ -94,17 +93,6 @@ def replacements():
 SPACK_PATH_PADDING_CHARS = "__spack_path_placeholder__"
 
 
-def is_path_url(path):
-    if "\\" in path:
-        return False
-    url_tuple = urlparse(path)
-    return bool(url_tuple.scheme) and len(url_tuple.scheme) > 1
-
-
-def win_exe_ext():
-    return ".exe"
-
-
 def find_sourceforge_suffix(path):
     """find and match sourceforge filepath components
     Return match object"""
@@ -114,92 +102,6 @@ def find_sourceforge_suffix(path):
     return path, ""
 
 
-def path_to_os_path(*pths):
-    """
-    Takes an arbitrary number of positional parameters
-    converts each arguemnt of type string to use a normalized
-    filepath separator, and returns a list of all values
-    """
-    ret_pths = []
-    for pth in pths:
-        if isinstance(pth, str) and not is_path_url(pth):
-            pth = convert_to_platform_path(pth)
-        ret_pths.append(pth)
-    return ret_pths
-
-
-def sanitize_file_path(pth):
-    """
-    Formats strings to contain only characters that can
-    be used to generate legal file paths.
-
-    Criteria for legal files based on
-    https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations
-
-    Args:
-        pth: string containing path to be created
-            on the host filesystem
-
-    Return:
-        sanitized string that can legally be made into a path
-    """
-    # on unix, splitting path by seperators will remove
-    # instances of illegal characters on join
-    pth_cmpnts = pth.split(os.path.sep)
-
-    if is_windows:
-        drive_match = r"[a-zA-Z]:"
-        is_abs = bool(re.match(drive_match, pth_cmpnts[0]))
-        drive = pth_cmpnts[0] + os.path.sep if is_abs else ""
-        pth_cmpnts = pth_cmpnts[1:] if drive else pth_cmpnts
-        illegal_chars = r'[<>?:"|*\\]'
-    else:
-        drive = "/" if not pth_cmpnts[0] else ""
-        illegal_chars = r"[/]"
-
-    pth = []
-    for cmp in pth_cmpnts:
-        san_cmp = re.sub(illegal_chars, "", cmp)
-        pth.append(san_cmp)
-    return drive + os.path.join(*pth)
-
-
-def system_path_filter(_func=None, arg_slice=None):
-    """
-    Filters function arguments to account for platform path separators.
-    Optional slicing range can be specified to select specific arguments
-
-    This decorator takes all (or a slice) of a method's positional arguments
-    and normalizes usage of filepath separators on a per platform basis.
-
-    Note: **kwargs, urls, and any type that is not a string are ignored
-    so in such cases where path normalization is required, that should be
-    handled by calling path_to_os_path directly as needed.
-
-    Parameters:
-        arg_slice (slice): a slice object specifying the slice of arguments
-            in the decorated method over which filepath separators are
-            normalized
-    """
-    from functools import wraps
-
-    def holder_func(func):
-        @wraps(func)
-        def path_filter_caller(*args, **kwargs):
-            args = list(args)
-            if arg_slice:
-                args[arg_slice] = path_to_os_path(*args[arg_slice])
-            else:
-                args = path_to_os_path(*args)
-            return func(*args, **kwargs)
-
-        return path_filter_caller
-
-    if _func:
-        return holder_func(_func)
-    return holder_func
-
-
 @memoized
 def get_system_path_max():
     # Choose a conservative default
@@ -221,54 +123,6 @@ def get_system_path_max():
     return sys_max_path_length
 
 
-class Path:
-    """
-    Describes the filepath separator types
-    in an enum style
-    with a helper attribute
-    exposing the path type of
-    the current platform.
-    """
-
-    unix = 0
-    windows = 1
-    platform_path = windows if is_windows else unix
-
-
-def format_os_path(path, mode=Path.unix):
-    """
-    Format path to use consistent, platform specific
-    separators. Absolute paths are converted between
-    drive letters and a prepended '/' as per platform
-    requirement.
-
-    Parameters:
-        path (str): the path to be normalized, must be a string
-            or expose the replace method.
-        mode (Path): the path filesperator style to normalize the
-            passed path to. Default is unix style, i.e. '/'
-    """
-    if not path:
-        return path
-    if mode == Path.windows:
-        path = path.replace("/", "\\")
-    else:
-        path = path.replace("\\", "/")
-    return path
-
-
-def convert_to_posix_path(path):
-    return format_os_path(path, mode=Path.unix)
-
-
-def convert_to_windows_path(path):
-    return format_os_path(path, mode=Path.windows)
-
-
-def convert_to_platform_path(path):
-    return format_os_path(path, mode=Path.platform_path)
-
-
 def substitute_config_variables(path):
     """Substitute placeholders into paths.
 

lib/spack/spack/util/url.py

@@ -15,7 +15,7 @@
 import urllib.parse
 import urllib.request
 
-from spack.util.path import convert_to_posix_path
+from llnl.util.path import convert_to_posix_path
 
 
 def validate_scheme(scheme):

lib/spack/spack/util/web.py

@@ -23,6 +23,7 @@
 import llnl.util.lang
 import llnl.util.tty as tty
 from llnl.util.filesystem import mkdirp, rename, working_dir
+from llnl.util.path import convert_to_posix_path
 
 import spack
 import spack.config
@@ -36,7 +37,6 @@
 import spack.util.url as url_util
 from spack.util.compression import ALLOWED_ARCHIVE_TYPES
 from spack.util.executable import CommandNotFoundError, which
-from spack.util.path import convert_to_posix_path
 
 
 def _urlopen():