From f7258e246fdf836a69c3dfba403e42f780f679db Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Mon, 23 May 2022 22:20:34 +0200 Subject: 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 --- lib/spack/docs/containers.rst | 9 ++++-- lib/spack/docs/environments.rst | 17 ++++++---- lib/spack/docs/pipelines.rst | 3 +- lib/spack/docs/replace_conda_homebrew.rst | 5 +-- lib/spack/docs/spack.yaml | 3 +- lib/spack/spack/environment/environment.py | 52 +++++++++++++++++------------- lib/spack/spack/schema/__init__.py | 6 +++- lib/spack/spack/schema/concretizer.py | 8 +++++ lib/spack/spack/schema/env.py | 49 ++++++++++++++++++++++++++-- lib/spack/spack/test/cmd/config.py | 2 +- lib/spack/spack/test/cmd/env.py | 25 ++++++++++++-- 11 files changed, 138 insertions(+), 41 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/containers.rst b/lib/spack/docs/containers.rst index 72d7a82d9d..8ef54e8693 100644 --- a/lib/spack/docs/containers.rst +++ b/lib/spack/docs/containers.rst @@ -59,7 +59,8 @@ other techniques to minimize the size of the final image: && echo " specs:" \ && echo " - gromacs+mpi" \ && echo " - mpich" \ - && echo " concretization: together" \ + && echo " concretizer: together" \ + && echo " unify: true" \ && echo " config:" \ && echo " install_tree: /opt/software" \ && echo " view: /opt/view") > /opt/spack-environment/spack.yaml @@ -245,7 +246,8 @@ software is respectively built and installed: && echo " specs:" \ && echo " - gromacs+mpi" \ && echo " - mpich" \ - && echo " concretization: together" \ + && echo " concretizer:" \ + && echo " unify: true" \ && echo " config:" \ && echo " install_tree: /opt/software" \ && echo " view: /opt/view") > /opt/spack-environment/spack.yaml @@ -366,7 +368,8 @@ produces, for instance, the following ``Dockerfile``: && echo " externals:" \ && echo " - spec: cuda%gcc" \ && echo " prefix: /usr/local/cuda" \ - && echo " concretization: together" \ + && echo " concretizer:" \ + && echo " unify: true" \ && echo " config:" \ && echo " install_tree: /opt/software" \ && echo " view: /opt/view") > /opt/spack-environment/spack.yaml diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index 7744dc87e6..fca70d8af9 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -281,8 +281,8 @@ need to be installed alongside each other. Central installations done at HPC centers by system administrators or user support groups are a common case that fits in this behavior. Environments *can also be configured to concretize all -the root specs in a self-consistent way* to ensure that -each package in the environment comes with a single configuration. This +the root specs in a unified way* to ensure that +each package in the environment corresponds to a single concrete spec. This mode of operation is usually what is required by software developers that want to deploy their development environment. @@ -499,7 +499,7 @@ Spec concretization Specs can be concretized separately or together, as already 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 @@ -509,10 +509,15 @@ under any environment is determined by the ``concretization`` property: - netcdf - nco - py-sphinx - concretization: together + concretizer: + unify: true -which can currently take either one of the two allowed values ``together`` or ``separately`` -(the default). +.. note:: + + 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 diff --git a/lib/spack/docs/pipelines.rst b/lib/spack/docs/pipelines.rst index e2891deed0..bd695c29ba 100644 --- a/lib/spack/docs/pipelines.rst +++ b/lib/spack/docs/pipelines.rst @@ -115,7 +115,8 @@ And here's the spack environment built by the pipeline represented as a spack: view: false - concretization: separately + concretizer: + unify: false definitions: - pkgs: diff --git a/lib/spack/docs/replace_conda_homebrew.rst b/lib/spack/docs/replace_conda_homebrew.rst index f2f81a4799..3f640e35cf 100644 --- a/lib/spack/docs/replace_conda_homebrew.rst +++ b/lib/spack/docs/replace_conda_homebrew.rst @@ -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 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 package to coexist. Since we want a single consistent environment, we want to 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 specs: [bash@5, python, py-numpy, py-scipy, py-matplotlib] view: true - concretization: together + concretizer: + unify: true ^^^^^^^^^^^^^^^^ Symlink location diff --git a/lib/spack/docs/spack.yaml b/lib/spack/docs/spack.yaml index 75e3069175..04754d13eb 100644 --- a/lib/spack/docs/spack.yaml +++ b/lib/spack/docs/spack.yaml @@ -25,4 +25,5 @@ spack: - subversion # Plotting - graphviz - concretization: together + concretizer: + unify: true diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 8c71deb561..2536186b63 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -79,8 +79,9 @@ lockfile_name = 'spack.lock' env_subdir_name = '.spack-env' -#: default spack.yaml file to put in new environments -default_manifest_yaml = """\ +def default_manifest_yaml(): + """default spack.yaml file to put in new environments""" + return """\ # This is a Spack Environment file. # # It describes a set of packages to be installed, along with @@ -89,7 +90,11 @@ spack: # add package specs to the `specs` list specs: [] view: true -""" + concretizer: + unify: {} +""".format('true' if spack.config.get('concretizer:unify') else 'false') + + #: regex for validating enviroment names valid_environment_name_re = r'^\w[\w-]*$' @@ -632,11 +637,11 @@ class Environment(object): # the init file. with fs.open_if_filename(init_file) as f: 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._set_user_specs_from_lockfile() 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 # environment in a different location from the spack.yaml file. @@ -700,7 +705,7 @@ class Environment(object): default_manifest = not os.path.exists(self.manifest_path) if default_manifest: # No manifest, use default yaml - self._read_manifest(default_manifest_yaml) + self._read_manifest(default_manifest_yaml()) else: with open(self.manifest_path) as f: self._read_manifest(f) @@ -766,8 +771,11 @@ class Environment(object): self.views = {} # Retrieve the current concretization strategy 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: self.dev_specs = configuration.get('develop', {}) @@ -1869,17 +1877,15 @@ class Environment(object): regenerate (bool): regenerate views and run post-write hooks as well as writing if True. """ - # Intercept environment not using the latest schema format and prevent - # them from being modified - manifest_exists = os.path.exists(self.manifest_path) - if manifest_exists and not is_latest_format(self.manifest_path): - msg = ('The environment "{0}" needs to be written to disk, but ' - 'is currently using a deprecated format. Please update it ' - 'using:\n\n' - '\tspack env update {0}\n\n' - 'Note that previous versions of Spack will not be able to ' + # Warn that environments are not in the latest format. + if not is_latest_format(self.manifest_path): + ver = '.'.join(str(s) for s in spack.spack_version_info[:2]) + msg = ('The environment "{}" is written to disk in a deprecated format. ' + 'Please update it using:\n\n' + '\tspack env update {}\n\n' + 'Note that versions of Spack older than {} may not be able to ' '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 fs.mkdirp(self.path) @@ -2231,14 +2237,16 @@ def _top_level_key(data): def is_latest_format(manifest): - """Return True if the manifest file is at the latest schema format, - False otherwise. + """Return False if the manifest file exists and is not in the latest schema format. Args: manifest (str): manifest file to be analyzed """ - with open(manifest) as f: - data = syaml.load(f) + try: + with open(manifest) as f: + data = syaml.load(f) + except (OSError, IOError): + return True top_level_key = _top_level_key(data) changed = spack.schema.env.update(data[top_level_key]) return not changed diff --git a/lib/spack/spack/schema/__init__.py b/lib/spack/spack/schema/__init__.py index 60a28bfb4a..45514d4a7c 100644 --- a/lib/spack/spack/schema/__init__.py +++ b/lib/spack/spack/schema/__init__.py @@ -4,6 +4,8 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) """This module contains jsonschema files for all of Spack's YAML formats.""" +import warnings + import six import llnl.util.lang @@ -49,10 +51,12 @@ def _make_validator(): msg = msg_str_or_func.format(properties=deprecated_properties) else: msg = msg_str_or_func(instance, deprecated_properties) + if msg is None: + return is_error = deprecated['error'] if not is_error: - llnl.util.tty.warn(msg) + warnings.warn(msg) else: import jsonschema yield jsonschema.ValidationError(msg) diff --git a/lib/spack/spack/schema/concretizer.py b/lib/spack/spack/schema/concretizer.py index 9138ab7685..46d0e9126b 100644 --- a/lib/spack/spack/schema/concretizer.py +++ b/lib/spack/spack/schema/concretizer.py @@ -25,6 +25,14 @@ properties = { } } }, + 'unify': { + 'type': 'boolean' + # Todo: add when_possible. + # 'oneOf': [ + # {'type': 'boolean'}, + # {'type': 'string', 'enum': ['when_possible']} + # ] + } } } } diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 65ef1a76a8..85a33cb188 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -16,6 +16,24 @@ import spack.schema.merged import spack.schema.packages 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 keys = ('spack', 'env') @@ -61,6 +79,11 @@ schema = { 'type': 'object', 'default': {}, 'additionalProperties': False, + 'deprecatedProperties': { + 'properties': ['concretization'], + 'message': deprecate_concretization, + 'error': False + }, 'properties': union_dicts( # merged configuration scope schemas spack.schema.merged.properties, @@ -169,11 +192,33 @@ def update(data): Returns: True if data was changed, False otherwise """ + updated = False if 'include' in data: msg = ("included configuration files should be updated manually" " [files={0}]") warnings.warn(msg.format(', '.join(data['include']))) if 'packages' in data: - return spack.schema.packages.update(data['packages']) - return False + updated |= spack.schema.packages.update(data['packages']) + + # 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 diff --git a/lib/spack/spack/test/cmd/config.py b/lib/spack/spack/test/cmd/config.py index 4b63994618..99aa005f6e 100644 --- a/lib/spack/spack/test/cmd/config.py +++ b/lib/spack/spack/test/cmd/config.py @@ -486,7 +486,7 @@ def test_config_remove_from_env(mutable_empty_config, mutable_mock_env_path): config('rm', 'config:dirty') output = config('get') - expected = ev.default_manifest_yaml + expected = ev.default_manifest_yaml() expected += """ config: {} """ diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 48f0092c09..350dbe7ec1 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2251,7 +2251,7 @@ def test_env_write_only_non_default(): with open(e.manifest_path, 'r') as f: yaml = f.read() - assert yaml == ev.default_manifest_yaml + assert yaml == ev.default_manifest_yaml() @pytest.mark.regression('20526') @@ -2358,6 +2358,26 @@ def test_old_format_cant_be_updated_implicitly(packages_yaml_v015): 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') def test_can_update_attributes_with_override(tmpdir): spack_yaml = """ @@ -2391,7 +2411,8 @@ spack: modules: - libelf/3.18.1 - concretization: together + concretizer: + unify: false """ abspath = tmpdir.join('spack.yaml') abspath.write(spack_yaml) -- cgit v1.2.3-60-g2f50