Skip to content

Commit

Permalink
Features: Improve Spec format strings (#10556)
Browse files Browse the repository at this point in the history
* Update spec format to simpler syntax, maintain backwards compatibility
* Switch to new spec.format method throughout internals
* update package files for new format strings
* documentation and minor code cleanup. removed nonsensical variant sigils
  • Loading branch information
becker33 authored and tgamblin committed Apr 18, 2019
1 parent 49334f0 commit f242f5f
Show file tree
Hide file tree
Showing 42 changed files with 568 additions and 180 deletions.
4 changes: 2 additions & 2 deletions lib/spack/docs/config_yaml.rst
Expand Up @@ -39,7 +39,7 @@ default path uses the full 32 characters.
Secondly, it is
also possible to modify the entire installation scheme. By default
Spack uses
``${ARCHITECTURE}/${COMPILERNAME}-${COMPILERVER}/${PACKAGE}-${VERSION}-${HASH}``
``{architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}``
where the tokens that are available for use in this directive are the
same as those understood by the ``Spec.format`` method. Using this parameter it
is possible to use a different package layout or reduce the depth of
Expand All @@ -48,7 +48,7 @@ the installation paths. For example
.. code-block:: yaml
config:
install_path_scheme: '${PACKAGE}/${VERSION}/${HASH:7}'
install_path_scheme: '{name}/{version}/{hash:7}'
would install packages into sub-directories using only the package
name, version and a hash length of 7 characters.
Expand Down
4 changes: 2 additions & 2 deletions lib/spack/docs/configuration.rst
Expand Up @@ -459,7 +459,7 @@ account all scopes. For example, to see the fully merged
install_tree: $spack/opt/spack
template_dirs:
- $spack/templates
directory_layout: ${ARCHITECTURE}/${COMPILERNAME}-${COMPILERVER}/${PACKAGE}-${VERSION}-${HASH}
directory_layout: {architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}
module_roots:
tcl: $spack/share/spack/modules
lmod: $spack/share/spack/lmod
Expand Down Expand Up @@ -510,7 +510,7 @@ down the problem:
./my-scope/config.yaml:2 install_tree: /path/to/some/tree
/home/myuser/spack/etc/spack/defaults/config.yaml:23 template_dirs:
/home/myuser/spack/etc/spack/defaults/config.yaml:24 - $spack/templates
/home/myuser/spack/etc/spack/defaults/config.yaml:28 directory_layout: ${ARCHITECTURE}/${COMPILERNAME}-${COMPILERVER}/${PACKAGE}-${VERSION}-${HASH}
/home/myuser/spack/etc/spack/defaults/config.yaml:28 directory_layout: {architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}
/home/myuser/spack/etc/spack/defaults/config.yaml:32 module_roots:
/home/myuser/spack/etc/spack/defaults/config.yaml:33 tcl: $spack/share/spack/modules
/home/myuser/spack/etc/spack/defaults/config.yaml:34 lmod: $spack/share/spack/lmod
Expand Down
4 changes: 2 additions & 2 deletions lib/spack/docs/module_file_support.rst
Expand Up @@ -535,10 +535,10 @@ most likely via the ``+blas`` variant specification.
modules:
tcl:
naming_scheme: '${PACKAGE}/${VERSION}-${COMPILERNAME}-${COMPILERVER}'
naming_scheme: '{name}/{version}-{compiler.name}-{compiler.version}'

This comment has been minimized.

Copy link
@alalazo

alalazo Apr 19, 2019

Member

@tgamblin @becker33 @scheibelp I like that #10556 reverted the syntax used in modules.yaml (and other configuration files) to the state it was before #1848, but I suspect this will hit many users that are working with a "moving" develop branch in production. In fact we had the first request for help on this matter on Slack today (modules channel).

From my personal point of view this is not a problem as we are still in version 0.X and in any case develop should never be considered stable. Anyhow, since I got from previous discussions that using a moving develop branch is a common case in our community I thought it's worth pointing this issue out in case you want to take actions (better error messages or a PR that adds back the old behavior and emits a deprecation warning)

This comment has been minimized.

Copy link
@becker33

becker33 May 2, 2019

Author Member

I had a small bug in what I wrote to handle backwards compatibility in #10556, it should be resolved now.

all:
conflict:
- '${PACKAGE}'
- '{name}'
- 'intel/14.0.1'
will create module files that will conflict with ``intel/14.0.1`` and with the
Expand Down
33 changes: 17 additions & 16 deletions lib/spack/docs/tutorial_modules.rst
Expand Up @@ -646,14 +646,14 @@ modules that refer to different flavors of the same library/application:
modules:
tcl:
hash_length: 0
naming_scheme: '${PACKAGE}/${VERSION}-${COMPILERNAME}-${COMPILERVER}'
naming_scheme: '{name}/{version}-{compiler.name}-{compiler.version}'
whitelist:
- gcc
blacklist:
- '%gcc@5.4.0'
all:
conflict:
- '${PACKAGE}'
- '{name}'
suffixes:
'^openblas': openblas
'^netlib-lapack': netlib
Expand Down Expand Up @@ -713,22 +713,22 @@ is installed. You can achieve this with Spack by adding an
modules:
tcl:
hash_length: 0
naming_scheme: '${PACKAGE}/${VERSION}-${COMPILERNAME}-${COMPILERVER}'
naming_scheme: '{name}/{version}-{compiler.name}-{compiler.version}'
whitelist:
- gcc
blacklist:
- '%gcc@5.4.0'
all:
conflict:
- '${PACKAGE}'
- '{name}'
suffixes:
'^openblas': openblas
'^netlib-lapack': netlib
filter:
environment_blacklist: ['CPATH', 'LIBRARY_PATH']
environment:
set:
'${PACKAGE}_ROOT': '${PREFIX}'
'{name}_ROOT': '{prefix}'
netlib-scalapack:
suffixes:
'^openmpi': openmpi
Expand All @@ -737,8 +737,9 @@ is installed. You can achieve this with Spack by adding an
Under the hood Spack uses the :meth:`~spack.spec.Spec.format` API to substitute
tokens in either environment variable names or values. There are two caveats though:

- The set of allowed tokens in variable names is restricted to ``PACKAGE``,
``VERSION``, ``COMPILER``, ``COMPILERNAME``, ``COMPILERVER``, ``ARCHITECTURE``
- The set of allowed tokens in variable names is restricted to
``name``, ``version``, ``compiler``, ``compiler.name``,
``compiler.version``, ``architecture``
- Any token expanded in a variable name is made uppercase, but other than that
case sensitivity is preserved

Expand Down Expand Up @@ -784,22 +785,22 @@ etc. in the ``gcc`` module file and apply other custom modifications to the
modules:
tcl:
hash_length: 0
naming_scheme: '${PACKAGE}/${VERSION}-${COMPILERNAME}-${COMPILERVER}'
naming_scheme: '{name}/{version}-{compiler.name}-{compiler.version}'
whitelist:
- gcc
blacklist:
- '%gcc@5.4.0'
all:
conflict:
- '${PACKAGE}'
- '{name}'
suffixes:
'^openblas': openblas
'^netlib-lapack': netlib
filter:
environment_blacklist: ['CPATH', 'LIBRARY_PATH']
environment:
set:
'${PACKAGE}_ROOT': '${PREFIX}'
'{name}_ROOT': '{prefix}'
gcc:
environment:
set:
Expand Down Expand Up @@ -896,22 +897,22 @@ directive and assigning it the value ``direct``:
tcl:
verbose: True
hash_length: 0
naming_scheme: '${PACKAGE}/${VERSION}-${COMPILERNAME}-${COMPILERVER}'
naming_scheme: '{name}/{version}-{compiler.name}-{compiler.version}'
whitelist:
- gcc
blacklist:
- '%gcc@5.4.0'
all:
conflict:
- '${PACKAGE}'
- '{name}'
suffixes:
'^openblas': openblas
'^netlib-lapack': netlib
filter:
environment_blacklist: ['CPATH', 'LIBRARY_PATH']
environment:
set:
'${PACKAGE}_ROOT': '${PREFIX}'
'{name}_ROOT': '{prefix}'
gcc:
environment:
set:
Expand Down Expand Up @@ -1089,7 +1090,7 @@ After these modifications your configuration file should look like:
environment_blacklist: ['CPATH', 'LIBRARY_PATH']
environment:
set:
'${PACKAGE}_ROOT': '${PREFIX}'
'{name}_ROOT': '{prefix}'
gcc:
environment:
set:
Expand Down Expand Up @@ -1298,7 +1299,7 @@ Coming back to our example, let's add ``lapack`` to the hierarchy and remove any
environment_blacklist: ['CPATH', 'LIBRARY_PATH']
environment:
set:
'${PACKAGE}_ROOT': '${PREFIX}'
'{name}_ROOT': '{prefix}'
gcc:
environment:
set:
Expand Down Expand Up @@ -1534,7 +1535,7 @@ it's ``netlib-scalapack``:
environment_blacklist: ['CPATH', 'LIBRARY_PATH']
environment:
set:
'${PACKAGE}_ROOT': '${PREFIX}'
'{name}_ROOT': '{prefix}'
gcc:
environment:
set:
Expand Down
6 changes: 3 additions & 3 deletions lib/spack/docs/workflows.rst
Expand Up @@ -541,9 +541,9 @@ spec format strings, as shown in the example below.
.. code-block:: yaml
projections:
zlib: ${PACKAGE}-${VERSION}
^mpi: ${PACKAGE}-${VERSION}/${DEP:mpi:PACKAGE}-${DEP:mpi:VERSION}-${COMPILERNAME}-${COMPILERVER}
all: ${PACKAGE}-${VERSION}/${COMPILERNAME}-${COMPILERVER}
zlib: {name}-{version}
^mpi: {name}-{version}/{^mpi.name}-{^mpi.version}-{compiler.name}-{compiler.version}
all: {name}-{version}/{compiler.name}-{compiler.version}
The entries in the projections configuration file must all be either
specs or the keyword ``all``. For each spec, the projection used will
Expand Down
28 changes: 14 additions & 14 deletions lib/spack/spack/architecture.py
Expand Up @@ -336,7 +336,7 @@ def __init__(self, plat=None, os=None, target=None):
self.platform = plat
if plat and os:
os = self.platform.operating_system(os)
self.platform_os = os
self.os = os
if plat and target:
target = self.platform.target(target)
self.target = target
Expand All @@ -349,16 +349,16 @@ def __init__(self, plat=None, os=None, target=None):
def concrete(self):
return all((self.platform is not None,
isinstance(self.platform, Platform),
self.platform_os is not None,
isinstance(self.platform_os, OperatingSystem),
self.os is not None,
isinstance(self.os, OperatingSystem),
self.target is not None, isinstance(self.target, Target)))

def __str__(self):
if self.platform or self.platform_os or self.target:
if self.platform or self.os or self.target:
if self.platform.name == 'darwin':
os_name = self.platform_os.name if self.platform_os else "None"
os_name = self.os.name if self.os else "None"
else:
os_name = str(self.platform_os)
os_name = str(self.os)

return (str(self.platform) + "-" +
os_name + "-" + str(self.target))
Expand All @@ -371,7 +371,7 @@ def __contains__(self, string):
# TODO: make this unnecessary: don't include an empty arch on *every* spec.
def __nonzero__(self):
return (self.platform is not None or
self.platform_os is not None or
self.os is not None or
self.target is not None)
__bool__ = __nonzero__

Expand All @@ -380,21 +380,21 @@ def _cmp_key(self):
platform = self.platform.name
else:
platform = self.platform
if isinstance(self.platform_os, OperatingSystem):
platform_os = self.platform_os.name
if isinstance(self.os, OperatingSystem):
os = self.os.name
else:
platform_os = self.platform_os
os = self.os
if isinstance(self.target, Target):
target = self.target.name
else:
target = self.target
return (platform, platform_os, target)
return (platform, os, target)

def to_dict(self):
str_or_none = lambda v: str(v) if v else None
d = syaml_dict([
('platform', str_or_none(self.platform)),
('platform_os', str_or_none(self.platform_os)),
('platform_os', str_or_none(self.os)),
('target', str_or_none(self.target))])
return syaml_dict([('arch', d)])

Expand Down Expand Up @@ -430,13 +430,13 @@ def arch_for_spec(arch_spec):
assert(arch_spec.concrete)

arch_plat = get_platform(arch_spec.platform)
if not (arch_plat.operating_system(arch_spec.platform_os) and
if not (arch_plat.operating_system(arch_spec.os) and
arch_plat.target(arch_spec.target)):
raise ValueError(
"Can't recreate arch for spec %s on current arch %s; "
"spec architecture is too different" % (arch_spec, sys_type()))

return Arch(arch_plat, arch_spec.platform_os, arch_spec.target)
return Arch(arch_plat, arch_spec.os, arch_spec.target)


@memoized
Expand Down
2 changes: 1 addition & 1 deletion lib/spack/spack/build_environment.py
Expand Up @@ -381,7 +381,7 @@ def set_build_environment_variables(pkg, env, dirty):
if spack.config.get('config:debug'):
env.set(SPACK_DEBUG, 'TRUE')
env.set(SPACK_SHORT_SPEC, pkg.spec.short_spec)
env.set(SPACK_DEBUG_LOG_ID, pkg.spec.format('${PACKAGE}-${HASH:7}'))
env.set(SPACK_DEBUG_LOG_ID, pkg.spec.format('{name}-{hash:7}'))
env.set(SPACK_DEBUG_LOG_DIR, spack.main.spack_working_dir)

# Find ccache binary and hand it to build environment
Expand Down
17 changes: 9 additions & 8 deletions lib/spack/spack/cmd/__init__.py
Expand Up @@ -184,10 +184,11 @@ def disambiguate_spec(spec, env):
tty.die("Spec '%s' matches no installed packages." % spec)

elif len(matching_specs) > 1:
format_string = '{name}{@version}{%compiler}{arch=architecture}'
args = ["%s matches multiple packages." % spec,
"Matching packages:"]
args += [colorize(" @K{%s} " % s.dag_hash(7)) +
s.cformat('$_$@$%@$=') for s in matching_specs]
s.cformat(format_string) for s in matching_specs]
args += ["Use a more specific spec."]
tty.die(*args)

Expand Down Expand Up @@ -263,15 +264,15 @@ def get_arg(name, default=None):
hashes = True
hlen = None

nfmt = '{fullpackage}' if namespace else '{package}'
nfmt = '{namespace}{name}' if namespace else '{name}'
ffmt = ''
if full_compiler or flags:
ffmt += '$%'
ffmt += '{%compiler.name}'
if full_compiler:
ffmt += '@'
ffmt += '+'
vfmt = '$+' if variants else ''
format_string = '$%s$@%s%s' % (nfmt, ffmt, vfmt)
ffmt += '{@compiler.version}'
ffmt += ' {compiler_flags}'
vfmt = '{variants}' if variants else ''
format_string = nfmt + '{@version}' + ffmt + vfmt

# Make a dict with specs keyed by architecture and compiler.
index = index_by(specs, ('architecture', 'compiler'))
Expand Down Expand Up @@ -329,7 +330,7 @@ def fmt(s):
if hashes:
string += gray_hash(s, hlen) + ' '
string += s.cformat(
'$%s$@%s' % (nfmt, vfmt), transform=transform)
nfmt + '{@version}' + vfmt, transform=transform)
return string

if not flags and not full_compiler:
Expand Down
2 changes: 1 addition & 1 deletion lib/spack/spack/cmd/arch.py
Expand Up @@ -32,7 +32,7 @@ def arch(parser, args):
if args.platform:
print(arch.platform)
elif args.operating_system:
print(arch.platform_os)
print(arch.os)
elif args.target:
print(arch.target)
else:
Expand Down
3 changes: 2 additions & 1 deletion lib/spack/spack/cmd/dependencies.py
Expand Up @@ -42,7 +42,8 @@ def dependencies(parser, args):
env = ev.get_env(args, 'dependencies')
spec = spack.cmd.disambiguate_spec(specs[0], env)

tty.msg("Dependencies of %s" % spec.format('$_$@$%@$/', color=True))
format_string = '{name}{@version}{%compiler}{/hash:7}'
tty.msg("Dependencies of %s" % spec.format(format_string, color=True))
deps = spack.store.db.installed_relatives(
spec, 'children', args.transitive)
if deps:
Expand Down
3 changes: 2 additions & 1 deletion lib/spack/spack/cmd/dependents.py
Expand Up @@ -85,7 +85,8 @@ def dependents(parser, args):
env = ev.get_env(args, 'dependents')
spec = spack.cmd.disambiguate_spec(specs[0], env)

tty.msg("Dependents of %s" % spec.cformat('$_$@$%@$/'))
format_string = '{name}{@version}{%compiler}{/hash:7}'
tty.msg("Dependents of %s" % spec.cformat(format_string))
deps = spack.store.db.installed_relatives(
spec, 'parents', args.transitive)
if deps:
Expand Down
4 changes: 2 additions & 2 deletions lib/spack/spack/cmd/mirror.py
Expand Up @@ -162,7 +162,7 @@ def mirror_create(args):
# If nothing is passed, use all packages.
if not specs:
specs = [Spec(n) for n in spack.repo.all_package_names()]
specs.sort(key=lambda s: s.format("$_$@").lower())
specs.sort(key=lambda s: s.format("{name}{@version}").lower())

# If the user asked for dependencies, traverse spec DAG get them.
if args.dependencies:
Expand Down Expand Up @@ -204,7 +204,7 @@ def mirror_create(args):
" %-4d failed to fetch." % e)
if error:
tty.error("Failed downloads:")
colify(s.cformat("$_$@") for s in error)
colify(s.cformat("{name}{@version}") for s in error)


def mirror(parser, args):
Expand Down
4 changes: 3 additions & 1 deletion lib/spack/spack/cmd/modules/__init__.py
Expand Up @@ -344,7 +344,9 @@ def modules_cmd(parser, args, module_type, callbacks=callbacks):
except MultipleSpecsMatch:
msg = "the constraint '{query}' matches multiple packages:\n"
for s in specs:
msg += '\t' + s.cformat(format_string='$/ $_$@$+$%@+$+$=') + '\n'
spec_fmt = '{hash:7} {name}{@version}{%compiler}'
spec_fmt += '{compiler_flags}{variants}{arch=architecture}'
msg += '\t' + s.cformat(spec_fmt) + '\n'
tty.error(msg.format(query=args.constraint))
tty.die('In this context exactly **one** match is needed: please specify your constraints better.') # NOQA: ignore=E501

Expand Down

0 comments on commit f242f5f

Please sign in to comment.