summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/spack/llnl/util/filesystem.py31
-rw-r--r--lib/spack/spack/binary_distribution.py12
-rw-r--r--lib/spack/spack/cmd/ci.py4
-rw-r--r--lib/spack/spack/cmd/install.py3
-rw-r--r--lib/spack/spack/directory_layout.py2
-rw-r--r--lib/spack/spack/filesystem_view.py6
-rw-r--r--lib/spack/spack/install_test.py2
-rw-r--r--lib/spack/spack/modules/common.py2
-rw-r--r--lib/spack/spack/modules/lmod.py9
-rw-r--r--lib/spack/spack/package_base.py7
-rw-r--r--lib/spack/spack/parser.py13
-rw-r--r--lib/spack/spack/spec.py41
-rw-r--r--lib/spack/spack/stage.py2
-rw-r--r--lib/spack/spack/test/spec_semantics.py80
-rw-r--r--lib/spack/spack/test/spec_syntax.py8
-rw-r--r--lib/spack/spack/test/versions.py19
16 files changed, 214 insertions, 27 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index 8f4217049d..47c66248b5 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -156,6 +156,37 @@ if sys.version_info < (3, 7, 4):
shutil.copystat = copystat
+def polite_path(components: Iterable[str]):
+ """
+ Given a list of strings which are intended to be path components,
+ generate a path, and format each component to avoid generating extra
+ path entries.
+
+ For example all "/", "\", and ":" characters will be replaced with
+ "_". Other characters like "=" will also be replaced.
+ """
+ return os.path.join(*[polite_filename(x) for x in components])
+
+
+@memoized
+def _polite_antipattern():
+ # A regex of all the characters we don't want in a filename
+ return re.compile(r"[^A-Za-z0-9_.-]")
+
+
+def polite_filename(filename: str) -> str:
+ """
+ Replace generally problematic filename characters with underscores.
+
+ This differs from sanitize_filename in that it is more aggressive in
+ changing characters in the name. For example it removes "=" which can
+ confuse path parsing in external tools.
+ """
+ # This character set applies for both Windows and Linux. It does not
+ # account for reserved filenames in Windows.
+ return _polite_antipattern().sub("_", filename)
+
+
def getuid():
if sys.platform == "win32":
import ctypes
diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py
index 5559e89820..7484fee097 100644
--- a/lib/spack/spack/binary_distribution.py
+++ b/lib/spack/spack/binary_distribution.py
@@ -797,11 +797,7 @@ def tarball_directory_name(spec):
Return name of the tarball directory according to the convention
<os>-<architecture>/<compiler>/<package>-<version>/
"""
- return os.path.join(
- str(spec.architecture),
- f"{spec.compiler.name}-{spec.compiler.version}",
- f"{spec.name}-{spec.version}",
- )
+ return spec.format_path("{architecture}/{compiler.name}-{compiler.version}/{name}-{version}")
def tarball_name(spec, ext):
@@ -809,10 +805,10 @@ def tarball_name(spec, ext):
Return the name of the tarfile according to the convention
<os>-<architecture>-<package>-<dag_hash><ext>
"""
- return (
- f"{spec.architecture}-{spec.compiler.name}-{spec.compiler.version}-"
- f"{spec.name}-{spec.version}-{spec.dag_hash()}{ext}"
+ spec_formatted = spec.format_path(
+ "{architecture}-{compiler.name}-{compiler.version}-{name}-{version}-{hash}"
)
+ return f"{spec_formatted}{ext}"
def tarball_path_name(spec, ext):
diff --git a/lib/spack/spack/cmd/ci.py b/lib/spack/spack/cmd/ci.py
index b30483218a..cf2ee11c04 100644
--- a/lib/spack/spack/cmd/ci.py
+++ b/lib/spack/spack/cmd/ci.py
@@ -579,7 +579,9 @@ def ci_rebuild(args):
"SPACK_COLOR=always",
"SPACK_INSTALL_FLAGS={}".format(args_to_string(deps_install_args)),
"-j$(nproc)",
- "install-deps/{}".format(job_spec.format("{name}-{version}-{hash}")),
+ "install-deps/{}".format(
+ ev.depfile.MakefileSpec(job_spec).safe_format("{name}-{version}-{hash}")
+ ),
],
spack_cmd + ["install"] + root_install_args,
]
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index 2f49732094..b74f982755 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -240,8 +240,7 @@ def default_log_file(spec):
"""Computes the default filename for the log file and creates
the corresponding directory if not present
"""
- fmt = "test-{x.name}-{x.version}-{hash}.xml"
- basename = fmt.format(x=spec, hash=spec.dag_hash())
+ basename = spec.format_path("test-{name}-{version}-{hash}.xml")
dirname = fs.os.path.join(spack.paths.reports_path, "junit")
fs.mkdirp(dirname)
return fs.os.path.join(dirname, basename)
diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index 46bb6c8557..c0741a037c 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -104,7 +104,7 @@ class DirectoryLayout:
_check_concrete(spec)
projection = spack.projections.get_projection(self.projections, spec)
- path = spec.format(projection)
+ path = spec.format_path(projection)
return str(Path(path))
def write_spec(self, spec, path):
diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py
index f0e79afd7d..e6631fecbf 100644
--- a/lib/spack/spack/filesystem_view.py
+++ b/lib/spack/spack/filesystem_view.py
@@ -500,7 +500,7 @@ class YamlFilesystemView(FilesystemView):
proj = spack.projections.get_projection(self.projections, locator_spec)
if proj:
- return os.path.join(self._root, locator_spec.format(proj))
+ return os.path.join(self._root, locator_spec.format_path(proj))
return self._root
def get_all_specs(self):
@@ -776,7 +776,7 @@ class SimpleFilesystemView(FilesystemView):
spec = spec.package.extendee_spec
p = spack.projections.get_projection(self.projections, spec)
- return spec.format(p) if p else ""
+ return spec.format_path(p) if p else ""
def get_projection_for_spec(self, spec):
"""
@@ -791,7 +791,7 @@ class SimpleFilesystemView(FilesystemView):
proj = spack.projections.get_projection(self.projections, spec)
if proj:
- return os.path.join(self._root, spec.format(proj))
+ return os.path.join(self._root, spec.format_path(proj))
return self._root
diff --git a/lib/spack/spack/install_test.py b/lib/spack/spack/install_test.py
index 0d8fa782b6..662a1536c4 100644
--- a/lib/spack/spack/install_test.py
+++ b/lib/spack/spack/install_test.py
@@ -1039,7 +1039,7 @@ class TestSuite:
Returns:
str: the install test package identifier
"""
- return spec.format("{name}-{version}-{hash:7}")
+ return spec.format_path("{name}-{version}-{hash:7}")
@classmethod
def test_log_name(cls, spec):
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 4a3485c864..4b60f52bf4 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -586,7 +586,7 @@ class BaseFileLayout:
if not projection:
projection = self.conf.default_projections["all"]
- name = self.spec.format(projection)
+ name = self.spec.format_path(projection)
# Not everybody is working on linux...
parts = name.split("/")
name = os.path.join(*parts)
diff --git a/lib/spack/spack/modules/lmod.py b/lib/spack/spack/modules/lmod.py
index 5c001c9ead..d81e07e0bf 100644
--- a/lib/spack/spack/modules/lmod.py
+++ b/lib/spack/spack/modules/lmod.py
@@ -9,6 +9,7 @@ import os.path
import posixpath
from typing import Any, Dict, List
+import llnl.util.filesystem as fs
import llnl.util.lang as lang
import spack.compilers
@@ -283,8 +284,10 @@ class LmodFileLayout(BaseFileLayout):
Returns:
str: part of the path associated with the service
"""
+
# General format for the path part
- path_part_fmt = os.path.join("{token.name}", "{token.version}")
+ def path_part_fmt(token):
+ return fs.polite_path([f"{token.name}", f"{token.version}"])
# If we are dealing with a core compiler, return 'Core'
core_compilers = self.conf.core_compilers
@@ -296,13 +299,13 @@ class LmodFileLayout(BaseFileLayout):
# CompilerSpec does not have a hash, as we are not allowed to
# use different flavors of the same compiler
if name == "compiler":
- return path_part_fmt.format(token=value)
+ return path_part_fmt(token=value)
# In case the hierarchy token refers to a virtual provider
# we need to append a hash to the version to distinguish
# among flavors of the same library (e.g. openblas~openmp vs.
# openblas+openmp)
- path = path_part_fmt.format(token=value)
+ path = path_part_fmt(token=value)
path = "-".join([path, value.dag_hash(length=7)])
return path
diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index 940c12c11a..81cc9b8d61 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -991,13 +991,14 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
return None
def _make_resource_stage(self, root_stage, resource):
+ pretty_resource_name = fsys.polite_filename(f"{resource.name}-{self.version}")
return ResourceStage(
resource.fetcher,
root=root_stage,
resource=resource,
name=self._resource_stage(resource),
mirror_paths=spack.mirror.mirror_archive_paths(
- resource.fetcher, os.path.join(self.name, f"{resource.name}-{self.version}")
+ resource.fetcher, os.path.join(self.name, pretty_resource_name)
),
path=self.path,
)
@@ -1008,8 +1009,10 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
def _make_root_stage(self, fetcher):
# Construct a mirror path (TODO: get this out of package.py)
+ format_string = "{name}-{version}"
+ pretty_name = self.spec.format_path(format_string)
mirror_paths = spack.mirror.mirror_archive_paths(
- fetcher, os.path.join(self.name, f"{self.name}-{self.version}"), self.spec
+ fetcher, os.path.join(self.name, pretty_name), self.spec
)
# Construct a path where the stage should build..
s = self.spec
diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py
index 5e46ddb1b1..7e3532e948 100644
--- a/lib/spack/spack/parser.py
+++ b/lib/spack/spack/parser.py
@@ -76,7 +76,9 @@ IS_WINDOWS = sys.platform == "win32"
IDENTIFIER = r"(?:[a-zA-Z_0-9][a-zA-Z_0-9\-]*)"
DOTTED_IDENTIFIER = rf"(?:{IDENTIFIER}(?:\.{IDENTIFIER})+)"
GIT_HASH = r"(?:[A-Fa-f0-9]{40})"
-GIT_VERSION = rf"(?:(?:git\.(?:{DOTTED_IDENTIFIER}|{IDENTIFIER}))|(?:{GIT_HASH}))"
+#: Git refs include branch names, and can contain "." and "/"
+GIT_REF = r"(?:[a-zA-Z_0-9][a-zA-Z_0-9./\-]*)"
+GIT_VERSION_PATTERN = rf"(?:(?:git\.(?:{GIT_REF}))|(?:{GIT_HASH}))"
NAME = r"[a-zA-Z_0-9][a-zA-Z_0-9\-.]*"
@@ -127,7 +129,8 @@ class TokenType(TokenBase):
# Dependency
DEPENDENCY = r"(?:\^)"
# Version
- VERSION_HASH_PAIR = rf"(?:@(?:{GIT_VERSION})=(?:{VERSION}))"
+ VERSION_HASH_PAIR = rf"(?:@(?:{GIT_VERSION_PATTERN})=(?:{VERSION}))"
+ GIT_VERSION = rf"@(?:{GIT_VERSION_PATTERN})"
VERSION = rf"(?:@\s*(?:{VERSION_LIST}))"
# Variants
PROPAGATED_BOOL_VARIANT = rf"(?:(?:\+\+|~~|--)\s*{NAME})"
@@ -358,8 +361,10 @@ class SpecNodeParser:
compiler_name.strip(), compiler_version
)
self.has_compiler = True
- elif self.ctx.accept(TokenType.VERSION) or self.ctx.accept(
- TokenType.VERSION_HASH_PAIR
+ elif (
+ self.ctx.accept(TokenType.VERSION_HASH_PAIR)
+ or self.ctx.accept(TokenType.GIT_VERSION)
+ or self.ctx.accept(TokenType.VERSION)
):
if self.has_version:
raise spack.spec.MultipleVersionError(
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 08576419f0..07b3e56c7d 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -54,6 +54,7 @@ import enum
import io
import itertools
import os
+import pathlib
import platform
import re
import socket
@@ -4453,6 +4454,42 @@ class Spec:
kwargs.setdefault("color", None)
return self.format(*args, **kwargs)
+ def format_path(
+ # self, format_string: str, _path_ctor: Optional[pathlib.PurePath] = None
+ self,
+ format_string: str,
+ _path_ctor: Optional[Callable[[Any], pathlib.PurePath]] = None,
+ ) -> str:
+ """Given a `format_string` that is intended as a path, generate a string
+ like from `Spec.format`, but eliminate extra path separators introduced by
+ formatting of Spec properties.
+
+ Path separators explicitly added to the string are preserved, so for example
+ "{name}/{version}" would generate a directory based on the Spec's name, and
+ a subdirectory based on its version; this function guarantees though that
+ the resulting string would only have two directories (i.e. that if under
+ normal circumstances that `str(Spec.version)` would contain a path
+ separator, it would not in this case).
+ """
+ format_component_with_sep = r"\{[^}]*[/\\][^}]*}"
+ if re.search(format_component_with_sep, format_string):
+ raise SpecFormatPathError(
+ f"Invalid path format string: cannot contain {{/...}}\n\t{format_string}"
+ )
+
+ path_ctor = _path_ctor or pathlib.PurePath
+ format_string_as_path = path_ctor(format_string)
+ if format_string_as_path.is_absolute():
+ output_path_components = [format_string_as_path.parts[0]]
+ input_path_components = list(format_string_as_path.parts[1:])
+ else:
+ output_path_components = []
+ input_path_components = list(format_string_as_path.parts)
+ output_path_components += [
+ fs.polite_filename(self.format(x)) for x in input_path_components
+ ]
+ return str(path_ctor(*output_path_components))
+
def __str__(self):
sorted_nodes = [self] + sorted(
self.traverse(root=False), key=lambda x: x.name or x.abstract_hash
@@ -5379,6 +5416,10 @@ class SpecFormatStringError(spack.error.SpecError):
"""Called for errors in Spec format strings."""
+class SpecFormatPathError(spack.error.SpecError):
+ """Called for errors in Spec path-format strings."""
+
+
class SpecFormatSigilError(SpecFormatStringError):
"""Called for mismatched sigils and attributes in format strings"""
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 90fb193a63..d53ec5fee8 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -58,7 +58,7 @@ def compute_stage_name(spec):
"""Determine stage name given a spec"""
default_stage_structure = stage_prefix + "{name}-{version}-{hash}"
stage_name_structure = spack.config.get("config:stage_name", default=default_stage_structure)
- return spec.format(format_string=stage_name_structure)
+ return spec.format_path(format_string=stage_name_structure)
def create_stage_root(path: str) -> None:
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 662ea5ef0e..579ba4486c 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -3,6 +3,8 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import pathlib
+
import pytest
import spack.directives
@@ -1005,6 +1007,84 @@ class TestSpecSemantics:
assert new_spec.compiler_flags["cxxflags"] == ["-O1"]
+@pytest.mark.parametrize(
+ "spec_str,format_str,expected",
+ [
+ ("zlib@git.foo/bar", "{name}-{version}", str(pathlib.Path("zlib-git.foo_bar"))),
+ ("zlib@git.foo/bar", "{name}-{version}-{/hash}", None),
+ ("zlib@git.foo/bar", "{name}/{version}", str(pathlib.Path("zlib", "git.foo_bar"))),
+ (
+ "zlib@{0}=1.0%gcc".format("a" * 40),
+ "{name}/{version}/{compiler}",
+ str(pathlib.Path("zlib", "{0}_1.0".format("a" * 40), "gcc")),
+ ),
+ (
+ "zlib@git.foo/bar=1.0%gcc",
+ "{name}/{version}/{compiler}",
+ str(pathlib.Path("zlib", "git.foo_bar_1.0", "gcc")),
+ ),
+ ],
+)
+def test_spec_format_path(spec_str, format_str, expected):
+ _check_spec_format_path(spec_str, format_str, expected)
+
+
+def _check_spec_format_path(spec_str, format_str, expected, path_ctor=None):
+ spec = Spec(spec_str)
+ if not expected:
+ with pytest.raises((spack.spec.SpecFormatPathError, spack.spec.SpecFormatStringError)):
+ spec.format_path(format_str, _path_ctor=path_ctor)
+ else:
+ formatted = spec.format_path(format_str, _path_ctor=path_ctor)
+ assert formatted == expected
+
+
+@pytest.mark.parametrize(
+ "spec_str,format_str,expected",
+ [
+ (
+ "zlib@git.foo/bar",
+ r"C:\\installroot\{name}\{version}",
+ r"C:\installroot\zlib\git.foo_bar",
+ ),
+ (
+ "zlib@git.foo/bar",
+ r"\\hostname\sharename\{name}\{version}",
+ r"\\hostname\sharename\zlib\git.foo_bar",
+ ),
+ # Windows doesn't attribute any significance to a leading
+ # "/" so it is discarded
+ ("zlib@git.foo/bar", r"/installroot/{name}/{version}", r"installroot\zlib\git.foo_bar"),
+ ],
+)
+def test_spec_format_path_windows(spec_str, format_str, expected):
+ _check_spec_format_path(spec_str, format_str, expected, path_ctor=pathlib.PureWindowsPath)
+
+
+@pytest.mark.parametrize(
+ "spec_str,format_str,expected",
+ [
+ ("zlib@git.foo/bar", r"/installroot/{name}/{version}", "/installroot/zlib/git.foo_bar"),
+ ("zlib@git.foo/bar", r"//installroot/{name}/{version}", "//installroot/zlib/git.foo_bar"),
+ # This is likely unintentional on Linux: Firstly, "\" is not a
+ # path separator for POSIX, so this is treated as a single path
+ # component (containing literal "\" characters); secondly,
+ # Spec.format treats "\" as an escape character, so is
+ # discarded (unless directly following another "\")
+ (
+ "zlib@git.foo/bar",
+ r"C:\\installroot\package-{name}-{version}",
+ r"C__installrootpackage-zlib-git.foo_bar",
+ ),
+ # "\" is not a POSIX separator, and Spec.format treats "\{" as a literal
+ # "{", which means that the resulting format string is invalid
+ ("zlib@git.foo/bar", r"package\{name}\{version}", None),
+ ],
+)
+def test_spec_format_path_posix(spec_str, format_str, expected):
+ _check_spec_format_path(spec_str, format_str, expected, path_ctor=pathlib.PurePosixPath)
+
+
@pytest.mark.regression("3887")
@pytest.mark.parametrize("spec_str", ["py-extension2", "extension1", "perl-extension"])
def test_is_extension_after_round_trip_to_dict(config, mock_packages, spec_str):
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index b79b829f96..d731fcd31c 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -517,6 +517,14 @@ def specfile_for(default_mock_concretization):
[Token(TokenType.VERSION, value="@:0.4"), Token(TokenType.COMPILER, value="% nvhpc")],
"@:0.4%nvhpc",
),
+ (
+ "zlib@git.foo/bar",
+ [
+ Token(TokenType.UNQUALIFIED_PACKAGE_NAME, "zlib"),
+ Token(TokenType.GIT_VERSION, "@git.foo/bar"),
+ ],
+ "zlib@git.foo/bar",
+ ),
],
)
def test_parse_single_spec(spec_str, tokens, expected_roundtrip):
diff --git a/lib/spack/spack/test/versions.py b/lib/spack/spack/test/versions.py
index 1dcf28cd71..50fcc19798 100644
--- a/lib/spack/spack/test/versions.py
+++ b/lib/spack/spack/test/versions.py
@@ -675,6 +675,25 @@ def test_git_ref_comparisons(mock_git_version_info, install_mockery, mock_packag
assert str(spec_branch.version) == "git.1.x=1.2"
+def test_git_branch_with_slash():
+ class MockLookup(object):
+ def get(self, ref):
+ assert ref == "feature/bar"
+ return "1.2", 0
+
+ v = spack.version.from_string("git.feature/bar")
+ assert isinstance(v, GitVersion)
+ v.attach_lookup(MockLookup())
+
+ # Create a version range
+ test_number_version = spack.version.from_string("1.2")
+ v.satisfies(test_number_version)
+
+ serialized = VersionList([v]).to_dict()
+ v_deserialized = VersionList.from_dict(serialized)
+ assert v_deserialized[0].ref == "feature/bar"
+
+
@pytest.mark.parametrize(
"string,git",
[
generated by cgit v1.2.3-70-g09d2 (git 2.46.0) at 2024-11-17 23:54:55 +0000