Deprecate spack:concretization over concretizer:unify (#30038)

* Introduce concretizer:unify option to replace spack:concretization

* Deprecate concretization

* Make spack:concretization overrule concretize:unify for now

* Add environment update logic to move from spack:concretization to spack:concretizer:reuse

* Migrate spack:concretization to spack:concretize:unify in all locations

* For new environments make concretizer:unify explicit, so that defaults can be changed in 0.19
This commit is contained in:
Harmen Stoppels 2022-05-23 22:20:34 +02:00 committed by GitHub
parent ff980a1452
commit f7258e246f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 151 additions and 48 deletions

View File

@ -28,3 +28,9 @@ concretizer:
# instance concretize with target "icelake" while running on "haswell"). # instance concretize with target "icelake" while running on "haswell").
# If "true" only allow targets that are compatible with the host. # If "true" only allow targets that are compatible with the host.
host_compatible: true host_compatible: true
# When "true" concretize root specs of environments together, so that each unique
# package in an environment corresponds to one concrete spec. This ensures
# environments can always be activated. When "false" perform concretization separately
# on each root spec, allowing different versions and variants of the same package in
# an environment.
unify: false

View File

@ -59,7 +59,8 @@ other techniques to minimize the size of the final image:
&& echo " specs:" \ && echo " specs:" \
&& echo " - gromacs+mpi" \ && echo " - gromacs+mpi" \
&& echo " - mpich" \ && echo " - mpich" \
&& echo " concretization: together" \ && echo " concretizer: together" \
&& echo " unify: true" \
&& echo " config:" \ && echo " config:" \
&& echo " install_tree: /opt/software" \ && echo " install_tree: /opt/software" \
&& echo " view: /opt/view") > /opt/spack-environment/spack.yaml && echo " view: /opt/view") > /opt/spack-environment/spack.yaml
@ -245,7 +246,8 @@ software is respectively built and installed:
&& echo " specs:" \ && echo " specs:" \
&& echo " - gromacs+mpi" \ && echo " - gromacs+mpi" \
&& echo " - mpich" \ && echo " - mpich" \
&& echo " concretization: together" \ && echo " concretizer:" \
&& echo " unify: true" \
&& echo " config:" \ && echo " config:" \
&& echo " install_tree: /opt/software" \ && echo " install_tree: /opt/software" \
&& echo " view: /opt/view") > /opt/spack-environment/spack.yaml && echo " view: /opt/view") > /opt/spack-environment/spack.yaml
@ -366,7 +368,8 @@ produces, for instance, the following ``Dockerfile``:
&& echo " externals:" \ && echo " externals:" \
&& echo " - spec: cuda%gcc" \ && echo " - spec: cuda%gcc" \
&& echo " prefix: /usr/local/cuda" \ && echo " prefix: /usr/local/cuda" \
&& echo " concretization: together" \ && echo " concretizer:" \
&& echo " unify: true" \
&& echo " config:" \ && echo " config:" \
&& echo " install_tree: /opt/software" \ && echo " install_tree: /opt/software" \
&& echo " view: /opt/view") > /opt/spack-environment/spack.yaml && echo " view: /opt/view") > /opt/spack-environment/spack.yaml

View File

@ -281,8 +281,8 @@ need to be installed alongside each other. Central installations done
at HPC centers by system administrators or user support groups at HPC centers by system administrators or user support groups
are a common case that fits in this behavior. are a common case that fits in this behavior.
Environments *can also be configured to concretize all Environments *can also be configured to concretize all
the root specs in a self-consistent way* to ensure that the root specs in a unified way* to ensure that
each package in the environment comes with a single configuration. This each package in the environment corresponds to a single concrete spec. This
mode of operation is usually what is required by software developers that mode of operation is usually what is required by software developers that
want to deploy their development environment. want to deploy their development environment.
@ -499,7 +499,7 @@ Spec concretization
Specs can be concretized separately or together, as already Specs can be concretized separately or together, as already
explained in :ref:`environments_concretization`. The behavior active explained in :ref:`environments_concretization`. The behavior active
under any environment is determined by the ``concretization`` property: under any environment is determined by the ``concretizer:unify`` property:
.. code-block:: yaml .. code-block:: yaml
@ -509,10 +509,15 @@ under any environment is determined by the ``concretization`` property:
- netcdf - netcdf
- nco - nco
- py-sphinx - py-sphinx
concretization: together concretizer:
unify: true
which can currently take either one of the two allowed values ``together`` or ``separately`` .. note::
(the default).
The ``concretizer:unify`` config option was introduced in Spack 0.18 to
replace the ``concretization`` property. For reference,
``concretization: separately`` is replaced by ``concretizer:unify:true``,
and ``concretization: together`` is replaced by ``concretizer:unify:false``.
.. admonition:: Re-concretization of user specs .. admonition:: Re-concretization of user specs

View File

@ -115,7 +115,8 @@ And here's the spack environment built by the pipeline represented as a
spack: spack:
view: false view: false
concretization: separately concretizer:
unify: false
definitions: definitions:
- pkgs: - pkgs:

View File

@ -61,7 +61,7 @@ You can see the packages we added earlier in the ``specs:`` section. If you
ever want to add more packages, you can either use ``spack add`` or manually ever want to add more packages, you can either use ``spack add`` or manually
edit this file. edit this file.
We also need to change the ``concretization:`` option. By default, Spack We also need to change the ``concretizer:unify`` option. By default, Spack
concretizes each spec *separately*, allowing multiple versions of the same concretizes each spec *separately*, allowing multiple versions of the same
package to coexist. Since we want a single consistent environment, we want to package to coexist. Since we want a single consistent environment, we want to
concretize all of the specs *together*. concretize all of the specs *together*.
@ -78,7 +78,8 @@ Here is what your ``spack.yaml`` looks like with this new setting:
# add package specs to the `specs` list # add package specs to the `specs` list
specs: [bash@5, python, py-numpy, py-scipy, py-matplotlib] specs: [bash@5, python, py-numpy, py-scipy, py-matplotlib]
view: true view: true
concretization: together concretizer:
unify: true
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^
Symlink location Symlink location

View File

@ -25,4 +25,5 @@ spack:
- subversion - subversion
# Plotting # Plotting
- graphviz - graphviz
concretization: together concretizer:
unify: true

View File

@ -79,8 +79,9 @@
env_subdir_name = '.spack-env' env_subdir_name = '.spack-env'
#: default spack.yaml file to put in new environments def default_manifest_yaml():
default_manifest_yaml = """\ """default spack.yaml file to put in new environments"""
return """\
# This is a Spack Environment file. # This is a Spack Environment file.
# #
# It describes a set of packages to be installed, along with # It describes a set of packages to be installed, along with
@ -89,7 +90,11 @@
# add package specs to the `specs` list # add package specs to the `specs` list
specs: [] specs: []
view: true view: true
""" concretizer:
unify: {}
""".format('true' if spack.config.get('concretizer:unify') else 'false')
#: regex for validating enviroment names #: regex for validating enviroment names
valid_environment_name_re = r'^\w[\w-]*$' valid_environment_name_re = r'^\w[\w-]*$'
@ -632,11 +637,11 @@ def __init__(self, path, init_file=None, with_view=None, keep_relative=False):
# the init file. # the init file.
with fs.open_if_filename(init_file) as f: with fs.open_if_filename(init_file) as f:
if hasattr(f, 'name') and f.name.endswith('.lock'): if hasattr(f, 'name') and f.name.endswith('.lock'):
self._read_manifest(default_manifest_yaml) self._read_manifest(default_manifest_yaml())
self._read_lockfile(f) self._read_lockfile(f)
self._set_user_specs_from_lockfile() self._set_user_specs_from_lockfile()
else: else:
self._read_manifest(f, raw_yaml=default_manifest_yaml) self._read_manifest(f, raw_yaml=default_manifest_yaml())
# Rewrite relative develop paths when initializing a new # Rewrite relative develop paths when initializing a new
# environment in a different location from the spack.yaml file. # environment in a different location from the spack.yaml file.
@ -700,7 +705,7 @@ def _read(self):
default_manifest = not os.path.exists(self.manifest_path) default_manifest = not os.path.exists(self.manifest_path)
if default_manifest: if default_manifest:
# No manifest, use default yaml # No manifest, use default yaml
self._read_manifest(default_manifest_yaml) self._read_manifest(default_manifest_yaml())
else: else:
with open(self.manifest_path) as f: with open(self.manifest_path) as f:
self._read_manifest(f) self._read_manifest(f)
@ -766,8 +771,11 @@ def _read_manifest(self, f, raw_yaml=None):
self.views = {} self.views = {}
# Retrieve the current concretization strategy # Retrieve the current concretization strategy
configuration = config_dict(self.yaml) configuration = config_dict(self.yaml)
# default concretization to separately
self.concretization = configuration.get('concretization', 'separately') # Let `concretization` overrule `concretize:unify` config for now.
unify = spack.config.get('concretizer:unify')
self.concretization = configuration.get(
'concretization', 'together' if unify else 'separately')
# Retrieve dev-build packages: # Retrieve dev-build packages:
self.dev_specs = configuration.get('develop', {}) self.dev_specs = configuration.get('develop', {})
@ -1869,17 +1877,15 @@ def write(self, regenerate=True):
regenerate (bool): regenerate views and run post-write hooks as regenerate (bool): regenerate views and run post-write hooks as
well as writing if True. well as writing if True.
""" """
# Intercept environment not using the latest schema format and prevent # Warn that environments are not in the latest format.
# them from being modified if not is_latest_format(self.manifest_path):
manifest_exists = os.path.exists(self.manifest_path) ver = '.'.join(str(s) for s in spack.spack_version_info[:2])
if manifest_exists and not is_latest_format(self.manifest_path): msg = ('The environment "{}" is written to disk in a deprecated format. '
msg = ('The environment "{0}" needs to be written to disk, but ' 'Please update it using:\n\n'
'is currently using a deprecated format. Please update it ' '\tspack env update {}\n\n'
'using:\n\n' 'Note that versions of Spack older than {} may not be able to '
'\tspack env update {0}\n\n'
'Note that previous versions of Spack will not be able to '
'use the updated configuration.') 'use the updated configuration.')
raise RuntimeError(msg.format(self.name)) tty.warn(msg.format(self.name, self.name, ver))
# ensure path in var/spack/environments # ensure path in var/spack/environments
fs.mkdirp(self.path) fs.mkdirp(self.path)
@ -2231,14 +2237,16 @@ def _top_level_key(data):
def is_latest_format(manifest): def is_latest_format(manifest):
"""Return True if the manifest file is at the latest schema format, """Return False if the manifest file exists and is not in the latest schema format.
False otherwise.
Args: Args:
manifest (str): manifest file to be analyzed manifest (str): manifest file to be analyzed
""" """
try:
with open(manifest) as f: with open(manifest) as f:
data = syaml.load(f) data = syaml.load(f)
except (OSError, IOError):
return True
top_level_key = _top_level_key(data) top_level_key = _top_level_key(data)
changed = spack.schema.env.update(data[top_level_key]) changed = spack.schema.env.update(data[top_level_key])
return not changed return not changed

View File

@ -4,6 +4,8 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT) # SPDX-License-Identifier: (Apache-2.0 OR MIT)
"""This module contains jsonschema files for all of Spack's YAML formats.""" """This module contains jsonschema files for all of Spack's YAML formats."""
import warnings
import six import six
import llnl.util.lang import llnl.util.lang
@ -49,10 +51,12 @@ def _deprecated_properties(validator, deprecated, instance, schema):
msg = msg_str_or_func.format(properties=deprecated_properties) msg = msg_str_or_func.format(properties=deprecated_properties)
else: else:
msg = msg_str_or_func(instance, deprecated_properties) msg = msg_str_or_func(instance, deprecated_properties)
if msg is None:
return
is_error = deprecated['error'] is_error = deprecated['error']
if not is_error: if not is_error:
llnl.util.tty.warn(msg) warnings.warn(msg)
else: else:
import jsonschema import jsonschema
yield jsonschema.ValidationError(msg) yield jsonschema.ValidationError(msg)

View File

@ -25,6 +25,14 @@
} }
} }
}, },
'unify': {
'type': 'boolean'
# Todo: add when_possible.
# 'oneOf': [
# {'type': 'boolean'},
# {'type': 'string', 'enum': ['when_possible']}
# ]
}
} }
} }
} }

View File

@ -16,6 +16,24 @@
import spack.schema.packages import spack.schema.packages
import spack.schema.projections import spack.schema.projections
warned_about_concretization = False
def deprecate_concretization(instance, props):
global warned_about_concretization
if warned_about_concretization:
return None
# Deprecate `spack:concretization` in favor of `spack:concretizer:unify`.
concretization_to_unify = {'together': 'true', 'separately': 'false'}
concretization = instance['concretization']
unify = concretization_to_unify[concretization]
return (
'concretization:{} is deprecated and will be removed in Spack 0.19 in favor of '
'the new concretizer:unify:{} config option.'.format(concretization, unify)
)
#: legal first keys in the schema #: legal first keys in the schema
keys = ('spack', 'env') keys = ('spack', 'env')
@ -61,6 +79,11 @@
'type': 'object', 'type': 'object',
'default': {}, 'default': {},
'additionalProperties': False, 'additionalProperties': False,
'deprecatedProperties': {
'properties': ['concretization'],
'message': deprecate_concretization,
'error': False
},
'properties': union_dicts( 'properties': union_dicts(
# merged configuration scope schemas # merged configuration scope schemas
spack.schema.merged.properties, spack.schema.merged.properties,
@ -169,11 +192,33 @@ def update(data):
Returns: Returns:
True if data was changed, False otherwise True if data was changed, False otherwise
""" """
updated = False
if 'include' in data: if 'include' in data:
msg = ("included configuration files should be updated manually" msg = ("included configuration files should be updated manually"
" [files={0}]") " [files={0}]")
warnings.warn(msg.format(', '.join(data['include']))) warnings.warn(msg.format(', '.join(data['include'])))
if 'packages' in data: if 'packages' in data:
return spack.schema.packages.update(data['packages']) updated |= spack.schema.packages.update(data['packages'])
return False
# Spack 0.19 drops support for `spack:concretization` in favor of
# `spack:concretizer:unify`. Here we provide an upgrade path that changes the former
# into the latter, or warns when there's an ambiguity. Note that Spack 0.17 is not
# forward compatible with `spack:concretizer:unify`.
if 'concretization' in data:
has_unify = 'unify' in data.get('concretizer', {})
to_unify = {'together': True, 'separately': False}
unify = to_unify[data['concretization']]
if has_unify and data['concretizer']['unify'] != unify:
warnings.warn(
'The following configuration conflicts: '
'`spack:concretization:{}` and `spack:concretizer:unify:{}`'
'. Please update manually.'.format(
data['concretization'], data['concretizer']['unify']))
else:
data.update({'concretizer': {'unify': unify}})
data.pop('concretization')
updated = True
return updated

View File

@ -486,7 +486,7 @@ def test_config_remove_from_env(mutable_empty_config, mutable_mock_env_path):
config('rm', 'config:dirty') config('rm', 'config:dirty')
output = config('get') output = config('get')
expected = ev.default_manifest_yaml expected = ev.default_manifest_yaml()
expected += """ config: {} expected += """ config: {}
""" """

View File

@ -2251,7 +2251,7 @@ def test_env_write_only_non_default():
with open(e.manifest_path, 'r') as f: with open(e.manifest_path, 'r') as f:
yaml = f.read() yaml = f.read()
assert yaml == ev.default_manifest_yaml assert yaml == ev.default_manifest_yaml()
@pytest.mark.regression('20526') @pytest.mark.regression('20526')
@ -2358,6 +2358,26 @@ def test_old_format_cant_be_updated_implicitly(packages_yaml_v015):
add('hdf5') add('hdf5')
@pytest.mark.parametrize('concretization,unify', [
('together', 'true'),
('separately', 'false')
])
def test_update_concretization_to_concretizer_unify(concretization, unify, tmpdir):
spack_yaml = """\
spack:
concretization: {}
""".format(concretization)
tmpdir.join('spack.yaml').write(spack_yaml)
# Update the environment
env('update', '-y', str(tmpdir))
with open(str(tmpdir.join('spack.yaml'))) as f:
assert f.read() == """\
spack:
concretizer:
unify: {}
""".format(unify)
@pytest.mark.regression('18147') @pytest.mark.regression('18147')
def test_can_update_attributes_with_override(tmpdir): def test_can_update_attributes_with_override(tmpdir):
spack_yaml = """ spack_yaml = """
@ -2391,7 +2411,8 @@ def test_newline_in_commented_sequence_is_not_an_issue(tmpdir):
modules: modules:
- libelf/3.18.1 - libelf/3.18.1
concretization: together concretizer:
unify: false
""" """
abspath = tmpdir.join('spack.yaml') abspath = tmpdir.join('spack.yaml')
abspath.write(spack_yaml) abspath.write(spack_yaml)

View File

@ -1,9 +1,9 @@
spack: spack:
view: false view: false
concretization: separately
concretizer: concretizer:
reuse: false reuse: false
unify: false
config: config:
install_tree: install_tree:

View File

@ -1,9 +1,9 @@
spack: spack:
view: false view: false
concretization: separately
concretizer: concretizer:
reuse: false reuse: false
unify: false
config: config:
install_tree: install_tree:

View File

@ -1,9 +1,9 @@
spack: spack:
view: false view: false
concretization: separately
concretizer: concretizer:
reuse: false reuse: false
unify: false
config: config:
concretizer: clingo concretizer: clingo

View File

@ -1,9 +1,9 @@
spack: spack:
view: false view: false
concretization: separately
concretizer: concretizer:
reuse: false reuse: false
unify: false
config: config:
concretizer: clingo concretizer: clingo

View File

@ -1,9 +1,9 @@
spack: spack:
view: false view: false
concretization: separately
concretizer: concretizer:
reuse: false reuse: false
unify: false
config: config:
concretizer: clingo concretizer: clingo

View File

@ -1,9 +1,9 @@
spack: spack:
concretization: separately
view: false view: false
concretizer: concretizer:
reuse: false reuse: false
unify: false
config: config:
concretizer: clingo concretizer: clingo

View File

@ -1,9 +1,9 @@
spack: spack:
view: false view: false
concretization: separately
concretizer: concretizer:
reuse: false reuse: false
unify: false
config: config:
install_tree: install_tree: