summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2024-04-23 10:52:15 -0700
committerGitHub <noreply@github.com>2024-04-23 10:52:15 -0700
commitaa0825d642cfa285f5f62761a0e23dc1e511d056 (patch)
treee08f25d73cf41bfd787c19977d087c736009df91
parent978c20f35ad7f1c071ee88dddd00f7a46ba13d61 (diff)
downloadspack-aa0825d642cfa285f5f62761a0e23dc1e511d056.tar.gz
spack-aa0825d642cfa285f5f62761a0e23dc1e511d056.tar.bz2
spack-aa0825d642cfa285f5f62761a0e23dc1e511d056.tar.xz
spack-aa0825d642cfa285f5f62761a0e23dc1e511d056.zip
Refactor to improve `spec format` speed (#43712)
When looking at where we spend our time in solver setup, I noticed a fair bit of time is spent in `Spec.format()`, and `Spec.format()` is a pretty old, slow, convoluted method. This PR does a number of things: - [x] Consolidate most of what was being done manually with a character loop and several regexes into a single regex. - [x] Precompile regexes where we keep them - [x] Remove the `transform=` argument to `Spec.format()` which was only used in one place in the code (modules) to uppercase env var names, but added a lot of complexity - [x] Avoid escaping and colorizing specs unless necessary - [x] Refactor a lot of the colorization logic to avoid unnecessary object construction - [x] Add type hints and remove some spots in the code where we were using nonexistent arguments to `format()`. - [x] Add trivial cases to `__str__` in `VariantMap` and `VersionList` to avoid sorting - [x] Avoid calling `isinstance()` in the main loop of `Spec.format()` - [x] Don't bother constructing a `string` representation for the result of `_prev_version` as it is only used for comparisons. In my timings (on all the specs formatted in a solve of `hdf5`), this is over 2.67x faster than the original `format()`, and it seems to reduce setup time by around a second (for `hdf5`).
-rw-r--r--lib/spack/llnl/util/tty/__init__.py17
-rw-r--r--lib/spack/llnl/util/tty/color.py111
-rw-r--r--lib/spack/spack/cmd/info.py17
-rw-r--r--lib/spack/spack/modules/common.py23
-rw-r--r--lib/spack/spack/spec.py263
-rw-r--r--lib/spack/spack/test/bindist.py4
-rw-r--r--lib/spack/spack/test/config.py2
-rw-r--r--lib/spack/spack/test/modules/lmod.py3
-rw-r--r--lib/spack/spack/test/modules/tcl.py3
-rw-r--r--lib/spack/spack/test/spec_semantics.py38
-rw-r--r--lib/spack/spack/variant.py3
-rw-r--r--lib/spack/spack/version/version_types.py20
12 files changed, 230 insertions, 274 deletions
diff --git a/lib/spack/llnl/util/tty/__init__.py b/lib/spack/llnl/util/tty/__init__.py
index 3f3ba1d462..807e50a159 100644
--- a/lib/spack/llnl/util/tty/__init__.py
+++ b/lib/spack/llnl/util/tty/__init__.py
@@ -12,7 +12,7 @@ import textwrap
import traceback
from datetime import datetime
from sys import platform as _platform
-from typing import NoReturn
+from typing import Any, NoReturn
if _platform != "win32":
import fcntl
@@ -158,21 +158,22 @@ def get_timestamp(force=False):
return ""
-def msg(message, *args, **kwargs):
+def msg(message: Any, *args: Any, newline: bool = True) -> None:
if not msg_enabled():
return
if isinstance(message, Exception):
- message = "%s: %s" % (message.__class__.__name__, str(message))
+ message = f"{message.__class__.__name__}: {message}"
+ else:
+ message = str(message)
- newline = kwargs.get("newline", True)
st_text = ""
if _stacktrace:
st_text = process_stacktrace(2)
- if newline:
- cprint("@*b{%s==>} %s%s" % (st_text, get_timestamp(), cescape(_output_filter(message))))
- else:
- cwrite("@*b{%s==>} %s%s" % (st_text, get_timestamp(), cescape(_output_filter(message))))
+
+ nl = "\n" if newline else ""
+ cwrite(f"@*b{{{st_text}==>}} {get_timestamp()}{cescape(_output_filter(message))}{nl}")
+
for arg in args:
print(indent + _output_filter(str(arg)))
diff --git a/lib/spack/llnl/util/tty/color.py b/lib/spack/llnl/util/tty/color.py
index bef7af8e58..a471505d34 100644
--- a/lib/spack/llnl/util/tty/color.py
+++ b/lib/spack/llnl/util/tty/color.py
@@ -62,6 +62,7 @@ To output an @, use '@@'. To output a } inside braces, use '}}'.
import re
import sys
from contextlib import contextmanager
+from typing import Optional
class ColorParseError(Exception):
@@ -95,7 +96,7 @@ colors = {
} # white
# Regex to be used for color formatting
-color_re = r"@(?:@|\.|([*_])?([a-zA-Z])?(?:{((?:[^}]|}})*)})?)"
+COLOR_RE = re.compile(r"@(?:(@)|(\.)|([*_])?([a-zA-Z])?(?:{((?:[^}]|}})*)})?)")
# Mapping from color arguments to values for tty.set_color
color_when_values = {"always": True, "auto": None, "never": False}
@@ -203,77 +204,64 @@ def color_when(value):
set_color_when(old_value)
-class match_to_ansi:
- def __init__(self, color=True, enclose=False, zsh=False):
- self.color = _color_when_value(color)
- self.enclose = enclose
- self.zsh = zsh
-
- def escape(self, s):
- """Returns a TTY escape sequence for a color"""
- if self.color:
- if self.zsh:
- result = rf"\e[0;{s}m"
- else:
- result = f"\033[{s}m"
-
- if self.enclose:
- result = rf"\[{result}\]"
-
- return result
+def _escape(s: str, color: bool, enclose: bool, zsh: bool) -> str:
+ """Returns a TTY escape sequence for a color"""
+ if color:
+ if zsh:
+ result = rf"\e[0;{s}m"
else:
- return ""
+ result = f"\033[{s}m"
- def __call__(self, match):
- """Convert a match object generated by ``color_re`` into an ansi
- color code. This can be used as a handler in ``re.sub``.
- """
- style, color, text = match.groups()
- m = match.group(0)
-
- if m == "@@":
- return "@"
- elif m == "@.":
- return self.escape(0)
- elif m == "@":
- raise ColorParseError("Incomplete color format: '%s' in %s" % (m, match.string))
-
- string = styles[style]
- if color:
- if color not in colors:
- raise ColorParseError(
- "Invalid color specifier: '%s' in '%s'" % (color, match.string)
- )
- string += ";" + str(colors[color])
-
- colored_text = ""
- if text:
- colored_text = text + self.escape(0)
+ if enclose:
+ result = rf"\[{result}\]"
- return self.escape(string) + colored_text
+ return result
+ else:
+ return ""
-def colorize(string, **kwargs):
+def colorize(
+ string: str, color: Optional[bool] = None, enclose: bool = False, zsh: bool = False
+) -> str:
"""Replace all color expressions in a string with ANSI control codes.
Args:
- string (str): The string to replace
+ string: The string to replace
Returns:
- str: The filtered string
+ The filtered string
Keyword Arguments:
- color (bool): If False, output will be plain text without control
- codes, for output to non-console devices.
- enclose (bool): If True, enclose ansi color sequences with
+ color: If False, output will be plain text without control codes, for output to
+ non-console devices (default: automatically choose color or not)
+ enclose: If True, enclose ansi color sequences with
square brackets to prevent misestimation of terminal width.
- zsh (bool): If True, use zsh ansi codes instead of bash ones (for variables like PS1)
+ zsh: If True, use zsh ansi codes instead of bash ones (for variables like PS1)
"""
- color = _color_when_value(kwargs.get("color", get_color_when()))
- zsh = kwargs.get("zsh", False)
- string = re.sub(color_re, match_to_ansi(color, kwargs.get("enclose")), string, zsh)
- string = string.replace("}}", "}")
- return string
+ color = color if color is not None else get_color_when()
+
+ def match_to_ansi(match):
+ """Convert a match object generated by ``COLOR_RE`` into an ansi
+ color code. This can be used as a handler in ``re.sub``.
+ """
+ escaped_at, dot, style, color_code, text = match.groups()
+
+ if escaped_at:
+ return "@"
+ elif dot:
+ return _escape(0, color, enclose, zsh)
+ elif not (style or color_code):
+ raise ColorParseError(
+ f"Incomplete color format: '{match.group(0)}' in '{match.string}'"
+ )
+
+ ansi_code = _escape(f"{styles[style]};{colors.get(color_code, '')}", color, enclose, zsh)
+ if text:
+ return f"{ansi_code}{text}{_escape(0, color, enclose, zsh)}"
+ else:
+ return ansi_code
+
+ return COLOR_RE.sub(match_to_ansi, string).replace("}}", "}")
def clen(string):
@@ -305,7 +293,7 @@ def cprint(string, stream=None, color=None):
cwrite(string + "\n", stream, color)
-def cescape(string):
+def cescape(string: str) -> str:
"""Escapes special characters needed for color codes.
Replaces the following symbols with their equivalent literal forms:
@@ -321,10 +309,7 @@ def cescape(string):
Returns:
(str): the string with color codes escaped
"""
- string = str(string)
- string = string.replace("@", "@@")
- string = string.replace("}", "}}")
- return string
+ return string.replace("@", "@@").replace("}", "}}")
class ColorStream:
diff --git a/lib/spack/spack/cmd/info.py b/lib/spack/spack/cmd/info.py
index b007c60516..3075107d07 100644
--- a/lib/spack/spack/cmd/info.py
+++ b/lib/spack/spack/cmd/info.py
@@ -263,8 +263,8 @@ def _fmt_name_and_default(variant):
return color.colorize(f"@c{{{variant.name}}} @C{{[{_fmt_value(variant.default)}]}}")
-def _fmt_when(when, indent):
- return color.colorize(f"{indent * ' '}@B{{when}} {color.cescape(when)}")
+def _fmt_when(when: "spack.spec.Spec", indent: int):
+ return color.colorize(f"{indent * ' '}@B{{when}} {color.cescape(str(when))}")
def _fmt_variant_description(variant, width, indent):
@@ -441,7 +441,7 @@ def print_versions(pkg, args):
return "No URL"
url = get_url(preferred) if pkg.has_code else ""
- line = version(" {0}".format(pad(preferred))) + color.cescape(url)
+ line = version(" {0}".format(pad(preferred))) + color.cescape(str(url))
color.cwrite(line)
print()
@@ -464,7 +464,7 @@ def print_versions(pkg, args):
continue
for v, url in vers:
- line = version(" {0}".format(pad(v))) + color.cescape(url)
+ line = version(" {0}".format(pad(v))) + color.cescape(str(url))
color.cprint(line)
@@ -475,10 +475,7 @@ def print_virtuals(pkg, args):
color.cprint(section_title("Virtual Packages: "))
if pkg.provided:
for when, specs in reversed(sorted(pkg.provided.items())):
- line = " %s provides %s" % (
- when.colorized(),
- ", ".join(s.colorized() for s in specs),
- )
+ line = " %s provides %s" % (when.cformat(), ", ".join(s.cformat() for s in specs))
print(line)
else:
@@ -497,7 +494,9 @@ def print_licenses(pkg, args):
pad = padder(pkg.licenses, 4)
for when_spec in pkg.licenses:
license_identifier = pkg.licenses[when_spec]
- line = license(" {0}".format(pad(license_identifier))) + color.cescape(when_spec)
+ line = license(" {0}".format(pad(license_identifier))) + color.cescape(
+ str(when_spec)
+ )
color.cprint(line)
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 20d75f61c1..5e46ca4c6c 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -83,6 +83,17 @@ _valid_tokens = (
)
+_FORMAT_STRING_RE = re.compile(r"({[^}]*})")
+
+
+def _format_env_var_name(spec, var_name_fmt):
+ """Format the variable name, but uppercase any formatted fields."""
+ fmt_parts = _FORMAT_STRING_RE.split(var_name_fmt)
+ return "".join(
+ spec.format(part).upper() if _FORMAT_STRING_RE.match(part) else part for part in fmt_parts
+ )
+
+
def _check_tokens_are_valid(format_string, message):
"""Checks that the tokens used in the format string are valid in
the context of module file and environment variable naming.
@@ -737,20 +748,12 @@ class BaseContext(tengine.Context):
exclude = self.conf.exclude_env_vars
# We may have tokens to substitute in environment commands
-
- # Prepare a suitable transformation dictionary for the names
- # of the environment variables. This means turn the valid
- # tokens uppercase.
- transform = {}
- for token in _valid_tokens:
- transform[token] = lambda s, string: str.upper(string)
-
for x in env:
# Ensure all the tokens are valid in this context
msg = "some tokens cannot be expanded in an environment variable name"
+
_check_tokens_are_valid(x.name, message=msg)
- # Transform them
- x.name = self.spec.format(x.name, transform=transform)
+ x.name = _format_env_var_name(self.spec, x.name)
if self.modification_needs_formatting(x):
try:
# Not every command has a value
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 301510336d..7d8266400e 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -51,7 +51,6 @@ line is a spec for a particular installation of the mpileaks package.
import collections
import collections.abc
import enum
-import io
import itertools
import os
import pathlib
@@ -59,7 +58,7 @@ import platform
import re
import socket
import warnings
-from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union
+from typing import Any, Callable, Dict, List, Match, Optional, Set, Tuple, Union
import llnl.path
import llnl.string
@@ -121,36 +120,44 @@ __all__ = [
"SpecDeprecatedError",
]
+
+SPEC_FORMAT_RE = re.compile(
+ r"(?:" # this is one big or, with matches ordered by priority
+ # OPTION 1: escaped character (needs to be first to catch opening \{)
+ # Note that an unterminated \ at the end of a string is left untouched
+ r"(?:\\(.))"
+ r"|" # or
+ # OPTION 2: an actual format string
+ r"{" # non-escaped open brace {
+ r"([%@/]|arch=)?" # optional sigil (to print sigil in color)
+ r"(?:\^([^}\.]+)\.)?" # optional ^depname. (to get attr from dependency)
+ # after the sigil or depname, we can have a hash expression or another attribute
+ r"(?:" # one of
+ r"(hash\b)(?:\:(\d+))?" # hash followed by :<optional length>
+ r"|" # or
+ r"([^}]*)" # another attribute to format
+ r")" # end one of
+ r"(})?" # finish format string with non-escaped close brace }, or missing if not present
+ r"|"
+ # OPTION 3: mismatched close brace (option 2 would consume a matched open brace)
+ r"(})" # brace
+ r")",
+ re.IGNORECASE,
+)
+
#: Valid pattern for an identifier in Spack
IDENTIFIER_RE = r"\w[\w-]*"
+# Coloring of specs when using color output. Fields are printed with
+# different colors to enhance readability.
+# See llnl.util.tty.color for descriptions of the color codes.
COMPILER_COLOR = "@g" #: color for highlighting compilers
VERSION_COLOR = "@c" #: color for highlighting versions
ARCHITECTURE_COLOR = "@m" #: color for highlighting architectures
-ENABLED_VARIANT_COLOR = "@B" #: color for highlighting enabled variants
-DISABLED_VARIANT_COLOR = "r" #: color for highlighting disabled varaints
-DEPENDENCY_COLOR = "@." #: color for highlighting dependencies
+VARIANT_COLOR = "@B" #: color for highlighting variants
HASH_COLOR = "@K" #: color for highlighting package hashes
-#: This map determines the coloring of specs when using color output.
-#: We make the fields different colors to enhance readability.
-#: See llnl.util.tty.color for descriptions of the color codes.
-COLOR_FORMATS = {
- "%": COMPILER_COLOR,
- "@": VERSION_COLOR,
- "=": ARCHITECTURE_COLOR,
- "+": ENABLED_VARIANT_COLOR,
- "~": DISABLED_VARIANT_COLOR,
- "^": DEPENDENCY_COLOR,
- "#": HASH_COLOR,
-}
-
-#: Regex used for splitting by spec field separators.
-#: These need to be escaped to avoid metacharacters in
-#: ``COLOR_FORMATS.keys()``.
-_SEPARATORS = "[\\%s]" % "\\".join(COLOR_FORMATS.keys())
-
#: Default format for Spec.format(). This format can be round-tripped, so that:
#: Spec(Spec("string").format()) == Spec("string)"
DEFAULT_FORMAT = (
@@ -193,26 +200,7 @@ class InstallStatus(enum.Enum):
missing = "@r{[-]} "
-def colorize_spec(spec):
- """Returns a spec colorized according to the colors specified in
- COLOR_FORMATS."""
-
- class insert_color:
- def __init__(self):
- self.last = None
-
- def __call__(self, match):
- # ignore compiler versions (color same as compiler)
- sep = match.group(0)
- if self.last == "%" and sep == "@":
- return clr.cescape(sep)
- self.last = sep
-
- return "%s%s" % (COLOR_FORMATS[sep], clr.cescape(sep))
-
- return clr.colorize(re.sub(_SEPARATORS, insert_color(), str(spec)) + "@.")
-
-
+# regexes used in spec formatting
OLD_STYLE_FMT_RE = re.compile(r"\${[A-Z]+}")
@@ -4295,10 +4283,7 @@ class Spec:
yield deps
- def colorized(self):
- return colorize_spec(self)
-
- def format(self, format_string=DEFAULT_FORMAT, **kwargs):
+ def format(self, format_string: str = DEFAULT_FORMAT, color: Optional[bool] = False) -> str:
r"""Prints out particular pieces of a spec, depending on what is
in the format string.
@@ -4361,79 +4346,65 @@ class Spec:
literal ``\`` character.
Args:
- format_string (str): string containing the format to be expanded
-
- Keyword Args:
- color (bool): True if returned string is colored
- transform (dict): maps full-string formats to a callable \
- that accepts a string and returns another one
-
+ format_string: string containing the format to be expanded
+ color: True for colorized result; False for no color; None for auto color.
"""
ensure_modern_format_string(format_string)
- color = kwargs.get("color", False)
- transform = kwargs.get("transform", {})
-
- out = io.StringIO()
-
- def write(s, c=None):
- f = clr.cescape(s)
- if c is not None:
- f = COLOR_FORMATS[c] + f + "@."
- clr.cwrite(f, stream=out, color=color)
-
- def write_attribute(spec, attribute, color):
- attribute = attribute.lower()
-
- sig = ""
- if attribute.startswith(("@", "%", "/")):
- # color sigils that are inside braces
- sig = attribute[0]
- attribute = attribute[1:]
- elif attribute.startswith("arch="):
- sig = " arch=" # include space as separator
- attribute = attribute[5:]
- current = spec
- if attribute.startswith("^"):
- attribute = attribute[1:]
- dep, attribute = attribute.split(".", 1)
- current = self[dep]
+ def safe_color(sigil: str, string: str, color_fmt: Optional[str]) -> str:
+ # avoid colorizing if there is no color or the string is empty
+ if (color is False) or not color_fmt or not string:
+ return sigil + string
+ # escape and add the sigil here to avoid multiple concatenations
+ if sigil == "@":
+ sigil = "@@"
+ return clr.colorize(f"{color_fmt}{sigil}{clr.cescape(string)}@.", color=color)
+
+ def format_attribute(match_object: Match) -> str:
+ (esc, sig, dep, hash, hash_len, attribute, close_brace, unmatched_close_brace) = (
+ match_object.groups()
+ )
+ if esc:
+ return esc
+ elif unmatched_close_brace:
+ raise SpecFormatStringError(f"Unmatched close brace: '{format_string}'")
+ elif not close_brace:
+ raise SpecFormatStringError(f"Missing close brace: '{format_string}'")
+
+ current = self if dep is None else self[dep]
+
+ # Hash attributes can return early.
+ # NOTE: we currently treat abstract_hash like an attribute and ignore
+ # any length associated with it. We may want to change that.
+ if hash:
+ if sig and sig != "/":
+ raise SpecFormatSigilError(sig, "DAG hashes", hash)
+ try:
+ length = int(hash_len) if hash_len else None
+ except ValueError:
+ raise SpecFormatStringError(f"Invalid hash length: '{hash_len}'")
+ return safe_color(sig or "", current.dag_hash(length), HASH_COLOR)
if attribute == "":
raise SpecFormatStringError("Format string attributes must be non-empty")
+ attribute = attribute.lower()
parts = attribute.split(".")
assert parts
# check that the sigil is valid for the attribute.
- if sig == "@" and parts[-1] not in ("versions", "version"):
+ if not sig:
+ sig = ""
+ elif sig == "@" and parts[-1] not in ("versions", "version"):
raise SpecFormatSigilError(sig, "versions", attribute)
elif sig == "%" and attribute not in ("compiler", "compiler.name"):
raise SpecFormatSigilError(sig, "compilers", attribute)
- elif sig == "/" and not re.match(r"(abstract_)?hash(:\d+)?$", attribute):
+ elif sig == "/" and attribute != "abstract_hash":
raise SpecFormatSigilError(sig, "DAG hashes", attribute)
- elif sig == " arch=" and attribute not in ("architecture", "arch"):
- raise SpecFormatSigilError(sig, "the architecture", attribute)
-
- # find the morph function for our attribute
- morph = transform.get(attribute, lambda s, x: x)
-
- # Special cases for non-spec attributes and hashes.
- # These must be the only non-dep component of the format attribute
- if attribute == "spack_root":
- write(morph(spec, spack.paths.spack_root))
- return
- elif attribute == "spack_install":
- write(morph(spec, spack.store.STORE.layout.root))
- return
- elif re.match(r"hash(:\d)?", attribute):
- col = "#"
- if ":" in attribute:
- _, length = attribute.split(":")
- write(sig + morph(spec, current.dag_hash(int(length))), col)
- else:
- write(sig + morph(spec, current.dag_hash()), col)
- return
+ elif sig == "arch=":
+ if attribute not in ("architecture", "arch"):
+ raise SpecFormatSigilError(sig, "the architecture", attribute)
+ sig = " arch=" # include space as separator
# Iterate over components using getattr to get next element
for idx, part in enumerate(parts):
@@ -4442,7 +4413,7 @@ class Spec:
if part.startswith("_"):
raise SpecFormatStringError("Attempted to format private attribute")
else:
- if isinstance(current, vt.VariantMap):
+ if part == "variants" and isinstance(current, vt.VariantMap):
# subscript instead of getattr for variant names
current = current[part]
else:
@@ -4466,62 +4437,31 @@ class Spec:
raise SpecFormatStringError(m)
if isinstance(current, vn.VersionList):
if current == vn.any_version:
- # We don't print empty version lists
- return
+ # don't print empty version lists
+ return ""
if callable(current):
raise SpecFormatStringError("Attempted to format callable object")
+
if current is None:
- # We're not printing anything
- return
+ # not printing anything
+ return ""
# Set color codes for various attributes
- col = None
+ color = None
if "variants" in parts:
- col = "+"
+ color = VARIANT_COLOR
elif "architecture" in parts:
- col = "="
+ color = ARCHITECTURE_COLOR
elif "compiler" in parts or "compiler_flags" in parts:
- col = "%"
+ color = COMPILER_COLOR
elif "version" in parts or "versions" in parts:
- col = "@"
-
- # Finally, write the output
- write(sig + morph(spec, str(current)), col)
-
- attribute = ""
- in_attribute = False
- escape = False
-
- for c in format_string:
- if escape:
- out.write(c)
- escape = False
- elif c == "\\":
- escape = True
- elif in_attribute:
- if c == "}":
- write_attribute(self, attribute, color)
- attribute = ""
- in_attribute = False
- else:
- attribute += c
- else:
- if c == "}":
- raise SpecFormatStringError(
- "Encountered closing } before opening { in %s" % format_string
- )
- elif c == "{":
- in_attribute = True
- else:
- out.write(c)
- if in_attribute:
- raise SpecFormatStringError(
- "Format string terminated while reading attribute." "Missing terminating }."
- )
+ color = VERSION_COLOR
- formatted_spec = out.getvalue()
- return formatted_spec.strip()
+ # return colored output
+ return safe_color(sig, str(current), color)
+
+ return SPEC_FORMAT_RE.sub(format_attribute, format_string).strip()
def cformat(self, *args, **kwargs):
"""Same as format, but color defaults to auto instead of False."""
@@ -4529,6 +4469,16 @@ class Spec:
kwargs.setdefault("color", None)
return self.format(*args, **kwargs)
+ @property
+ def spack_root(self):
+ """Special field for using ``{spack_root}`` in Spec.format()."""
+ return spack.paths.spack_root
+
+ @property
+ def spack_install(self):
+ """Special field for using ``{spack_install}`` in Spec.format()."""
+ return spack.store.STORE.layout.root
+
def format_path(
# self, format_string: str, _path_ctor: Optional[pathlib.PurePath] = None
self,
@@ -4554,14 +4504,21 @@ class Spec:
path_ctor = _path_ctor or pathlib.PurePath
format_string_as_path = path_ctor(format_string)
- if format_string_as_path.is_absolute():
+ if format_string_as_path.is_absolute() or (
+ # Paths that begin with a single "\" on windows are relative, but we still
+ # want to preserve the initial "\\" to be consistent with PureWindowsPath.
+ # Ensure that this '\' is not passed to polite_filename() so it's not converted to '_'
+ (os.name == "nt" or path_ctor == pathlib.PureWindowsPath)
+ and format_string_as_path.parts[0] == "\\"
+ ):
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
+ fs.polite_filename(self.format(part)) for part in input_path_components
]
return str(path_ctor(*output_path_components))
diff --git a/lib/spack/spack/test/bindist.py b/lib/spack/spack/test/bindist.py
index 8afd69e3c5..b30955641f 100644
--- a/lib/spack/spack/test/bindist.py
+++ b/lib/spack/spack/test/bindist.py
@@ -390,11 +390,11 @@ def test_built_spec_cache(mirror_dir):
assert any([r["spec"] == s for r in results])
-def fake_dag_hash(spec):
+def fake_dag_hash(spec, length=None):
# Generate an arbitrary hash that is intended to be different than
# whatever a Spec reported before (to test actions that trigger when
# the hash changes)
- return "tal4c7h4z0gqmixb1eqa92mjoybxn5l6"
+ return "tal4c7h4z0gqmixb1eqa92mjoybxn5l6"[:length]
@pytest.mark.usefixtures(
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index 76052eeb38..64fe403daa 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -1276,7 +1276,7 @@ def test_user_config_path_is_default_when_env_var_is_empty(working_env):
def test_default_install_tree(monkeypatch, default_config):
s = spack.spec.Spec("nonexistent@x.y.z %none@a.b.c arch=foo-bar-baz")
- monkeypatch.setattr(s, "dag_hash", lambda: "abc123")
+ monkeypatch.setattr(s, "dag_hash", lambda length: "abc123")
_, _, projections = spack.store.parse_install_tree(spack.config.get("config"))
assert s.format(projections["all"]) == "foo-bar-baz/none-a.b.c/nonexistent-x.y.z-abc123"
diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py
index 9038d8c9f6..5128518caa 100644
--- a/lib/spack/spack/test/modules/lmod.py
+++ b/lib/spack/spack/test/modules/lmod.py
@@ -146,9 +146,6 @@ class TestLmod:
assert len([x for x in content if "depends_on(" in x]) == 5
- @pytest.mark.skipif(
- str(archspec.cpu.host().family) != "x86_64", reason="test data is specific for x86_64"
- )
def test_alter_environment(self, modulefile_content, module_configuration):
"""Tests modifications to run-time environment."""
diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py
index 6630065373..76e854ced9 100644
--- a/lib/spack/spack/test/modules/tcl.py
+++ b/lib/spack/spack/test/modules/tcl.py
@@ -114,9 +114,6 @@ class TestTcl:
assert len([x for x in content if "prereq" in x]) == 5
- @pytest.mark.skipif(
- str(archspec.cpu.host().family) != "x86_64", reason="test data is specific for x86_64"
- )
def test_alter_environment(self, modulefile_content, module_configuration):
"""Tests modifications to run-time environment."""
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 2f4e7f43a4..171c194dc0 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -703,22 +703,25 @@ class TestSpecSemantics:
actual = spec.format(named_str)
assert expected == actual
- def test_spec_formatting_escapes(self, default_mock_concretization):
- spec = default_mock_concretization("multivalue-variant cflags=-O2")
-
- sigil_mismatches = [
+ @pytest.mark.parametrize(
+ "fmt_str",
+ [
"{@name}",
"{@version.concrete}",
"{%compiler.version}",
"{/hashd}",
"{arch=architecture.os}",
- ]
+ ],
+ )
+ def test_spec_formatting_sigil_mismatches(self, default_mock_concretization, fmt_str):
+ spec = default_mock_concretization("multivalue-variant cflags=-O2")
- for fmt_str in sigil_mismatches:
- with pytest.raises(SpecFormatSigilError):
- spec.format(fmt_str)
+ with pytest.raises(SpecFormatSigilError):
+ spec.format(fmt_str)
- bad_formats = [
+ @pytest.mark.parametrize(
+ "fmt_str",
+ [
r"{}",
r"name}",
r"\{name}",
@@ -728,11 +731,12 @@ class TestSpecSemantics:
r"{dag_hash}",
r"{foo}",
r"{+variants.debug}",
- ]
-
- for fmt_str in bad_formats:
- with pytest.raises(SpecFormatStringError):
- spec.format(fmt_str)
+ ],
+ )
+ def test_spec_formatting_bad_formats(self, default_mock_concretization, fmt_str):
+ spec = default_mock_concretization("multivalue-variant cflags=-O2")
+ with pytest.raises(SpecFormatStringError):
+ spec.format(fmt_str)
def test_combination_of_wildcard_or_none(self):
# Test that using 'none' and another value raises
@@ -1138,12 +1142,12 @@ def _check_spec_format_path(spec_str, format_str, expected, path_ctor=None):
r"\\hostname\sharename\{name}\{version}",
r"\\hostname\sharename\git-test\git.foo_bar",
),
- # Windows doesn't attribute any significance to a leading
- # "/" so it is discarded
+ # leading '/' is preserved on windows but converted to '\'
+ # note that it's still not "absolute" -- absolute windows paths start with a drive.
(
"git-test@git.foo/bar",
r"/installroot/{name}/{version}",
- r"installroot\git-test\git.foo_bar",
+ r"\installroot\git-test\git.foo_bar",
),
],
)
diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py
index 07aafe8945..bb4138116f 100644
--- a/lib/spack/spack/variant.py
+++ b/lib/spack/spack/variant.py
@@ -638,6 +638,9 @@ class VariantMap(lang.HashableMap):
return clone
def __str__(self):
+ if not self:
+ return ""
+
# print keys in order
sorted_keys = sorted(self.keys())
diff --git a/lib/spack/spack/version/version_types.py b/lib/spack/spack/version/version_types.py
index b772cd9d90..f35192192d 100644
--- a/lib/spack/spack/version/version_types.py
+++ b/lib/spack/spack/version/version_types.py
@@ -146,13 +146,11 @@ class StandardVersion(ConcreteVersion):
@staticmethod
def typemin():
- return StandardVersion("", ((), (ALPHA,)), ("",))
+ return _STANDARD_VERSION_TYPEMIN
@staticmethod
def typemax():
- return StandardVersion(
- "infinity", ((VersionStrComponent(len(infinity_versions)),), (FINAL,)), ("",)
- )
+ return _STANDARD_VERSION_TYPEMAX
def __bool__(self):
return True
@@ -390,6 +388,13 @@ class StandardVersion(ConcreteVersion):
return self[:index]
+_STANDARD_VERSION_TYPEMIN = StandardVersion("", ((), (ALPHA,)), ("",))
+
+_STANDARD_VERSION_TYPEMAX = StandardVersion(
+ "infinity", ((VersionStrComponent(len(infinity_versions)),), (FINAL,)), ("",)
+)
+
+
class GitVersion(ConcreteVersion):
"""Class to represent versions interpreted from git refs.
@@ -1019,6 +1024,9 @@ class VersionList:
return hash(tuple(self.versions))
def __str__(self):
+ if not self.versions:
+ return ""
+
return ",".join(
f"={v}" if isinstance(v, StandardVersion) else str(v) for v in self.versions
)
@@ -1127,7 +1135,9 @@ def _prev_version(v: StandardVersion) -> StandardVersion:
components[1::2] = separators[: len(release)]
if prerelease_type != FINAL:
components.extend((PRERELEASE_TO_STRING[prerelease_type], *prerelease[1:]))
- return StandardVersion("".join(str(c) for c in components), (release, prerelease), separators)
+
+ # this is only used for comparison functions, so don't bother making a string
+ return StandardVersion(None, (release, prerelease), separators)
def Version(string: Union[str, int]) -> Union[GitVersion, StandardVersion]: