Forbid using env: as a top level environment attribute (#38199)
				
					
				
			* Remove "env" from environment schema * Remove spack.env.schema.keys * Remove spack.environment.config_dict
This commit is contained in:
		 Massimiliano Culpo
					Massimiliano Culpo
				
			
				
					committed by
					
						 GitHub
						GitHub
					
				
			
			
				
	
			
			
			 GitHub
						GitHub
					
				
			
						parent
						
							63cad5d338
						
					
				
				
					commit
					ddfc43be96
				
			| @@ -751,7 +751,7 @@ def generate_gitlab_ci_yaml( | ||||
|             env.concretize() | ||||
|             env.write() | ||||
| 
 | ||||
|     yaml_root = ev.config_dict(env.manifest) | ||||
|     yaml_root = env.manifest[ev.TOP_LEVEL_KEY] | ||||
| 
 | ||||
|     # Get the joined "ci" config with all of the current scopes resolved | ||||
|     ci_config = cfg.get("ci") | ||||
|   | ||||
| @@ -228,7 +228,7 @@ def ci_reindex(args): | ||||
|     Use the active, gitlab-enabled environment to rebuild the buildcache | ||||
|     index for the associated mirror.""" | ||||
|     env = spack.cmd.require_active_env(cmd_name="ci rebuild-index") | ||||
|     yaml_root = ev.config_dict(env.manifest) | ||||
|     yaml_root = env.manifest[ev.TOP_LEVEL_KEY] | ||||
| 
 | ||||
|     if "mirrors" not in yaml_root or len(yaml_root["mirrors"].values()) < 1: | ||||
|         tty.die("spack ci rebuild-index requires an env containing a mirror") | ||||
|   | ||||
| @@ -81,7 +81,7 @@ | ||||
| # Same as above, but including keys for environments | ||||
| # this allows us to unify config reading between configs and environments | ||||
| all_schemas = copy.deepcopy(section_schemas) | ||||
| all_schemas.update(dict((key, spack.schema.env.schema) for key in spack.schema.env.keys)) | ||||
| all_schemas.update({spack.schema.env.TOP_LEVEL_KEY: spack.schema.env.schema}) | ||||
| 
 | ||||
| #: Path to the default configuration | ||||
| configuration_defaults_path = ("defaults", os.path.join(spack.paths.etc_path, "defaults")) | ||||
| @@ -111,14 +111,6 @@ | ||||
| overrides_base_name = "overrides-" | ||||
| 
 | ||||
| 
 | ||||
| def first_existing(dictionary, keys): | ||||
|     """Get the value of the first key in keys that is in the dictionary.""" | ||||
|     try: | ||||
|         return next(k for k in keys if k in dictionary) | ||||
|     except StopIteration: | ||||
|         raise KeyError("None of %s is in dict!" % str(keys)) | ||||
| 
 | ||||
| 
 | ||||
| class ConfigScope(object): | ||||
|     """This class represents a configuration scope. | ||||
| 
 | ||||
| @@ -838,12 +830,10 @@ def _config(): | ||||
| 
 | ||||
| def add_from_file(filename, scope=None): | ||||
|     """Add updates to a config from a filename""" | ||||
|     import spack.environment as ev | ||||
| 
 | ||||
|     # Get file as config dict | ||||
|     # Extract internal attributes, if we are dealing with an environment | ||||
|     data = read_config_file(filename) | ||||
|     if any(k in data for k in spack.schema.env.keys): | ||||
|         data = ev.config_dict(data) | ||||
|     if spack.schema.env.TOP_LEVEL_KEY in data: | ||||
|         data = data[spack.schema.env.TOP_LEVEL_KEY] | ||||
| 
 | ||||
|     # update all sections from config dict | ||||
|     # We have to iterate on keys to keep overrides from the file | ||||
|   | ||||
| @@ -37,7 +37,7 @@ def validate(configuration_file): | ||||
|         config = syaml.load(f) | ||||
| 
 | ||||
|     # Ensure we have a "container" attribute with sensible defaults set | ||||
|     env_dict = ev.config_dict(config) | ||||
|     env_dict = config[ev.TOP_LEVEL_KEY] | ||||
|     env_dict.setdefault( | ||||
|         "container", {"format": "docker", "images": {"os": "ubuntu:22.04", "spack": "develop"}} | ||||
|     ) | ||||
|   | ||||
| @@ -50,7 +50,7 @@ def create(configuration, last_phase=None): | ||||
|         configuration (dict): how to generate the current recipe | ||||
|         last_phase (str): last phase to be printed or None to print them all | ||||
|     """ | ||||
|     name = ev.config_dict(configuration)["container"]["format"] | ||||
|     name = configuration[ev.TOP_LEVEL_KEY]["container"]["format"] | ||||
|     return _writer_factory[name](configuration, last_phase) | ||||
| 
 | ||||
| 
 | ||||
| @@ -138,7 +138,7 @@ class PathContext(tengine.Context): | ||||
|     template_name: Optional[str] = None | ||||
| 
 | ||||
|     def __init__(self, config, last_phase): | ||||
|         self.config = ev.config_dict(config) | ||||
|         self.config = config[ev.TOP_LEVEL_KEY] | ||||
|         self.container_config = self.config["container"] | ||||
| 
 | ||||
|         # Operating system tag as written in the configuration file | ||||
|   | ||||
| @@ -337,6 +337,7 @@ | ||||
| """ | ||||
| 
 | ||||
| from .environment import ( | ||||
|     TOP_LEVEL_KEY, | ||||
|     Environment, | ||||
|     SpackEnvironmentError, | ||||
|     SpackEnvironmentViewError, | ||||
| @@ -345,7 +346,6 @@ | ||||
|     active_environment, | ||||
|     all_environment_names, | ||||
|     all_environments, | ||||
|     config_dict, | ||||
|     create, | ||||
|     create_in_dir, | ||||
|     deactivate, | ||||
| @@ -369,6 +369,7 @@ | ||||
| ) | ||||
| 
 | ||||
| __all__ = [ | ||||
|     "TOP_LEVEL_KEY", | ||||
|     "Environment", | ||||
|     "SpackEnvironmentError", | ||||
|     "SpackEnvironmentViewError", | ||||
| @@ -377,7 +378,6 @@ | ||||
|     "active_environment", | ||||
|     "all_environment_names", | ||||
|     "all_environments", | ||||
|     "config_dict", | ||||
|     "create", | ||||
|     "create_in_dir", | ||||
|     "deactivate", | ||||
|   | ||||
| @@ -53,6 +53,7 @@ | ||||
| import spack.version | ||||
| from spack.filesystem_view import SimpleFilesystemView, inverse_view_func_parser, view_func_parser | ||||
| from spack.installer import PackageInstaller | ||||
| from spack.schema.env import TOP_LEVEL_KEY | ||||
| from spack.spec import Spec | ||||
| from spack.spec_list import InvalidSpecConstraintError, SpecList | ||||
| from spack.util.path import substitute_path_variables | ||||
| @@ -361,19 +362,6 @@ def ensure_env_root_path_exists(): | ||||
|         fs.mkdirp(env_root_path()) | ||||
| 
 | ||||
| 
 | ||||
| def config_dict(yaml_data): | ||||
|     """Get the configuration scope section out of an spack.yaml""" | ||||
|     # TODO (env:): Remove env: as a possible top level keyword in v0.21 | ||||
|     key = spack.config.first_existing(yaml_data, spack.schema.env.keys) | ||||
|     if key == "env": | ||||
|         msg = ( | ||||
|             "using 'env:' as a top-level attribute of a Spack environment is deprecated and " | ||||
|             "will be removed in Spack v0.21. Please use 'spack:' instead." | ||||
|         ) | ||||
|         warnings.warn(msg) | ||||
|     return yaml_data[key] | ||||
| 
 | ||||
| 
 | ||||
| def all_environment_names(): | ||||
|     """List the names of environments that currently exist.""" | ||||
|     # just return empty if the env path does not exist.  A read-only | ||||
| @@ -821,8 +809,8 @@ def write_transaction(self): | ||||
|     def _construct_state_from_manifest(self): | ||||
|         """Read manifest file and set up user specs.""" | ||||
|         self.spec_lists = collections.OrderedDict() | ||||
| 
 | ||||
|         for item in config_dict(self.manifest).get("definitions", []): | ||||
|         env_configuration = self.manifest[TOP_LEVEL_KEY] | ||||
|         for item in env_configuration.get("definitions", []): | ||||
|             entry = copy.deepcopy(item) | ||||
|             when = _eval_conditional(entry.pop("when", "True")) | ||||
|             assert len(entry) == 1 | ||||
| @@ -834,13 +822,13 @@ def _construct_state_from_manifest(self): | ||||
|                 else: | ||||
|                     self.spec_lists[name] = user_specs | ||||
| 
 | ||||
|         spec_list = config_dict(self.manifest).get(user_speclist_name, []) | ||||
|         spec_list = env_configuration.get(user_speclist_name, []) | ||||
|         user_specs = SpecList( | ||||
|             user_speclist_name, [s for s in spec_list if s], self.spec_lists.copy() | ||||
|         ) | ||||
|         self.spec_lists[user_speclist_name] = user_specs | ||||
| 
 | ||||
|         enable_view = config_dict(self.manifest).get("view") | ||||
|         enable_view = env_configuration.get("view") | ||||
|         # enable_view can be boolean, string, or None | ||||
|         if enable_view is True or enable_view is None: | ||||
|             self.views = {default_view_name: ViewDescriptor(self.path, self.view_path_default)} | ||||
| @@ -855,14 +843,11 @@ def _construct_state_from_manifest(self): | ||||
|         else: | ||||
|             self.views = {} | ||||
| 
 | ||||
|         # Retrieve the current concretization strategy | ||||
|         configuration = config_dict(self.manifest) | ||||
| 
 | ||||
|         # Retrieve unification scheme for the concretizer | ||||
|         self.unify = spack.config.get("concretizer:unify", False) | ||||
| 
 | ||||
|         # Retrieve dev-build packages: | ||||
|         self.dev_specs = copy.deepcopy(configuration.get("develop", {})) | ||||
|         self.dev_specs = copy.deepcopy(env_configuration.get("develop", {})) | ||||
|         for name, entry in self.dev_specs.items(): | ||||
|             # spec must include a concrete version | ||||
|             assert Spec(entry["spec"]).versions.concrete_range_as_version | ||||
| @@ -982,7 +967,7 @@ def included_config_scopes(self): | ||||
| 
 | ||||
|         # load config scopes added via 'include:', in reverse so that | ||||
|         # highest-precedence scopes are last. | ||||
|         includes = config_dict(self.manifest).get("include", []) | ||||
|         includes = self.manifest[TOP_LEVEL_KEY].get("include", []) | ||||
|         missing = [] | ||||
|         for i, config_path in enumerate(reversed(includes)): | ||||
|             # allow paths to contain spack config/environment variables, etc. | ||||
| @@ -1075,10 +1060,7 @@ def env_file_config_scope(self): | ||||
|         """Get the configuration scope for the environment's manifest file.""" | ||||
|         config_name = self.env_file_config_scope_name() | ||||
|         return spack.config.SingleFileScope( | ||||
|             config_name, | ||||
|             self.manifest_path, | ||||
|             spack.schema.env.schema, | ||||
|             [spack.config.first_existing(self.manifest, spack.schema.env.keys)], | ||||
|             config_name, self.manifest_path, spack.schema.env.schema, [TOP_LEVEL_KEY] | ||||
|         ) | ||||
| 
 | ||||
|     def config_scopes(self): | ||||
| @@ -2684,8 +2666,8 @@ def add_user_spec(self, user_spec: str) -> None: | ||||
|         Args: | ||||
|             user_spec: user spec to be appended | ||||
|         """ | ||||
|         config_dict(self.pristine_yaml_content).setdefault("specs", []).append(user_spec) | ||||
|         config_dict(self.yaml_content).setdefault("specs", []).append(user_spec) | ||||
|         self.pristine_configuration.setdefault("specs", []).append(user_spec) | ||||
|         self.configuration.setdefault("specs", []).append(user_spec) | ||||
|         self.changed = True | ||||
| 
 | ||||
|     def remove_user_spec(self, user_spec: str) -> None: | ||||
| @@ -2698,8 +2680,8 @@ def remove_user_spec(self, user_spec: str) -> None: | ||||
|             SpackEnvironmentError: when the user spec is not in the list | ||||
|         """ | ||||
|         try: | ||||
|             config_dict(self.pristine_yaml_content)["specs"].remove(user_spec) | ||||
|             config_dict(self.yaml_content)["specs"].remove(user_spec) | ||||
|             self.pristine_configuration["specs"].remove(user_spec) | ||||
|             self.configuration["specs"].remove(user_spec) | ||||
|         except ValueError as e: | ||||
|             msg = f"cannot remove {user_spec} from {self}, no such spec exists" | ||||
|             raise SpackEnvironmentError(msg) from e | ||||
| @@ -2716,8 +2698,8 @@ def override_user_spec(self, user_spec: str, idx: int) -> None: | ||||
|             SpackEnvironmentError: when the user spec cannot be overridden | ||||
|         """ | ||||
|         try: | ||||
|             config_dict(self.pristine_yaml_content)["specs"][idx] = user_spec | ||||
|             config_dict(self.yaml_content)["specs"][idx] = user_spec | ||||
|             self.pristine_configuration["specs"][idx] = user_spec | ||||
|             self.configuration["specs"][idx] = user_spec | ||||
|         except ValueError as e: | ||||
|             msg = f"cannot override {user_spec} from {self}" | ||||
|             raise SpackEnvironmentError(msg) from e | ||||
| @@ -2733,14 +2715,14 @@ def add_definition(self, user_spec: str, list_name: str) -> None: | ||||
|         Raises: | ||||
|             SpackEnvironmentError: is no valid definition exists already | ||||
|         """ | ||||
|         defs = config_dict(self.pristine_yaml_content).get("definitions", []) | ||||
|         defs = self.pristine_configuration.get("definitions", []) | ||||
|         msg = f"cannot add {user_spec} to the '{list_name}' definition, no valid list exists" | ||||
| 
 | ||||
|         for idx, item in self._iterate_on_definitions(defs, list_name=list_name, err_msg=msg): | ||||
|             item[list_name].append(user_spec) | ||||
|             break | ||||
| 
 | ||||
|         config_dict(self.yaml_content)["definitions"][idx][list_name].append(user_spec) | ||||
|         self.configuration["definitions"][idx][list_name].append(user_spec) | ||||
|         self.changed = True | ||||
| 
 | ||||
|     def remove_definition(self, user_spec: str, list_name: str) -> None: | ||||
| @@ -2754,7 +2736,7 @@ def remove_definition(self, user_spec: str, list_name: str) -> None: | ||||
|             SpackEnvironmentError: if the user spec cannot be removed from the list, | ||||
|                 or the list does not exist | ||||
|         """ | ||||
|         defs = config_dict(self.pristine_yaml_content).get("definitions", []) | ||||
|         defs = self.pristine_configuration.get("definitions", []) | ||||
|         msg = ( | ||||
|             f"cannot remove {user_spec} from the '{list_name}' definition, " | ||||
|             f"no valid list exists" | ||||
| @@ -2767,7 +2749,7 @@ def remove_definition(self, user_spec: str, list_name: str) -> None: | ||||
|             except ValueError: | ||||
|                 pass | ||||
| 
 | ||||
|         config_dict(self.yaml_content)["definitions"][idx][list_name].remove(user_spec) | ||||
|         self.configuration["definitions"][idx][list_name].remove(user_spec) | ||||
|         self.changed = True | ||||
| 
 | ||||
|     def override_definition(self, user_spec: str, *, override: str, list_name: str) -> None: | ||||
| @@ -2782,7 +2764,7 @@ def override_definition(self, user_spec: str, *, override: str, list_name: str) | ||||
|         Raises: | ||||
|             SpackEnvironmentError: if the user spec cannot be overridden | ||||
|         """ | ||||
|         defs = config_dict(self.pristine_yaml_content).get("definitions", []) | ||||
|         defs = self.pristine_configuration.get("definitions", []) | ||||
|         msg = f"cannot override {user_spec} with {override} in the '{list_name}' definition" | ||||
| 
 | ||||
|         for idx, item in self._iterate_on_definitions(defs, list_name=list_name, err_msg=msg): | ||||
| @@ -2793,7 +2775,7 @@ def override_definition(self, user_spec: str, *, override: str, list_name: str) | ||||
|             except ValueError: | ||||
|                 pass | ||||
| 
 | ||||
|         config_dict(self.yaml_content)["definitions"][idx][list_name][sub_index] = override | ||||
|         self.configuration["definitions"][idx][list_name][sub_index] = override | ||||
|         self.changed = True | ||||
| 
 | ||||
|     def _iterate_on_definitions(self, definitions, *, list_name, err_msg): | ||||
| @@ -2825,24 +2807,24 @@ def set_default_view(self, view: Union[bool, str, pathlib.Path, Dict[str, str]]) | ||||
|                 True the default view is used for the environment, if False there's no view. | ||||
|         """ | ||||
|         if isinstance(view, dict): | ||||
|             config_dict(self.pristine_yaml_content)["view"][default_view_name].update(view) | ||||
|             config_dict(self.yaml_content)["view"][default_view_name].update(view) | ||||
|             self.pristine_configuration["view"][default_view_name].update(view) | ||||
|             self.configuration["view"][default_view_name].update(view) | ||||
|             self.changed = True | ||||
|             return | ||||
| 
 | ||||
|         if not isinstance(view, bool): | ||||
|             view = str(view) | ||||
| 
 | ||||
|         config_dict(self.pristine_yaml_content)["view"] = view | ||||
|         config_dict(self.yaml_content)["view"] = view | ||||
|         self.pristine_configuration["view"] = view | ||||
|         self.configuration["view"] = view | ||||
|         self.changed = True | ||||
| 
 | ||||
|     def remove_default_view(self) -> None: | ||||
|         """Removes the default view from the manifest file""" | ||||
|         view_data = config_dict(self.pristine_yaml_content).get("view") | ||||
|         view_data = self.pristine_configuration.get("view") | ||||
|         if isinstance(view_data, collections.abc.Mapping): | ||||
|             config_dict(self.pristine_yaml_content)["view"].pop(default_view_name) | ||||
|             config_dict(self.yaml_content)["view"].pop(default_view_name) | ||||
|             self.pristine_configuration["view"].pop(default_view_name) | ||||
|             self.configuration["view"].pop(default_view_name) | ||||
|             self.changed = True | ||||
|             return | ||||
| 
 | ||||
| @@ -2859,12 +2841,10 @@ def add_develop_spec(self, pkg_name: str, entry: Dict[str, str]) -> None: | ||||
|         if entry["path"] == pkg_name: | ||||
|             entry.pop("path") | ||||
| 
 | ||||
|         config_dict(self.pristine_yaml_content).setdefault("develop", {}).setdefault( | ||||
|             pkg_name, {} | ||||
|         ).update(entry) | ||||
|         config_dict(self.yaml_content).setdefault("develop", {}).setdefault(pkg_name, {}).update( | ||||
|         self.pristine_configuration.setdefault("develop", {}).setdefault(pkg_name, {}).update( | ||||
|             entry | ||||
|         ) | ||||
|         self.configuration.setdefault("develop", {}).setdefault(pkg_name, {}).update(entry) | ||||
|         self.changed = True | ||||
| 
 | ||||
|     def remove_develop_spec(self, pkg_name: str) -> None: | ||||
| @@ -2877,11 +2857,11 @@ def remove_develop_spec(self, pkg_name: str) -> None: | ||||
|             SpackEnvironmentError: if there is nothing to remove | ||||
|         """ | ||||
|         try: | ||||
|             del config_dict(self.pristine_yaml_content)["develop"][pkg_name] | ||||
|             del self.pristine_configuration["develop"][pkg_name] | ||||
|         except KeyError as e: | ||||
|             msg = f"cannot remove '{pkg_name}' from develop specs in {self}, entry does not exist" | ||||
|             raise SpackEnvironmentError(msg) from e | ||||
|         del config_dict(self.yaml_content)["develop"][pkg_name] | ||||
|         del self.configuration["develop"][pkg_name] | ||||
|         self.changed = True | ||||
| 
 | ||||
|     def absolutify_dev_paths(self, init_file_dir: Union[str, pathlib.Path]) -> None: | ||||
| @@ -2892,11 +2872,11 @@ def absolutify_dev_paths(self, init_file_dir: Union[str, pathlib.Path]) -> None: | ||||
|             init_file_dir: directory with the "spack.yaml" used to initialize the environment. | ||||
|         """ | ||||
|         init_file_dir = pathlib.Path(init_file_dir).absolute() | ||||
|         for _, entry in config_dict(self.pristine_yaml_content).get("develop", {}).items(): | ||||
|         for _, entry in self.pristine_configuration.get("develop", {}).items(): | ||||
|             expanded_path = os.path.normpath(str(init_file_dir / entry["path"])) | ||||
|             entry["path"] = str(expanded_path) | ||||
| 
 | ||||
|         for _, entry in config_dict(self.yaml_content).get("develop", {}).items(): | ||||
|         for _, entry in self.configuration.get("develop", {}).items(): | ||||
|             expanded_path = os.path.normpath(str(init_file_dir / entry["path"])) | ||||
|             entry["path"] = str(expanded_path) | ||||
|         self.changed = True | ||||
| @@ -2910,6 +2890,16 @@ def flush(self) -> None: | ||||
|             _write_yaml(self.pristine_yaml_content, f) | ||||
|         self.changed = False | ||||
| 
 | ||||
|     @property | ||||
|     def pristine_configuration(self): | ||||
|         """Return the dictionaries in the pristine YAML, without the top level attribute""" | ||||
|         return self.pristine_yaml_content[TOP_LEVEL_KEY] | ||||
| 
 | ||||
|     @property | ||||
|     def configuration(self): | ||||
|         """Return the dictionaries in the YAML, without the top level attribute""" | ||||
|         return self.yaml_content[TOP_LEVEL_KEY] | ||||
| 
 | ||||
|     def __len__(self): | ||||
|         return len(self.yaml_content) | ||||
| 
 | ||||
|   | ||||
| @@ -209,7 +209,7 @@ def update(data): | ||||
|     # Warn if deprecated section is still in the environment | ||||
|     ci_env = ev.active_environment() | ||||
|     if ci_env: | ||||
|         env_config = ev.config_dict(ci_env.manifest) | ||||
|         env_config = ci_env.manifest[ev.TOP_LEVEL_KEY] | ||||
|         if "gitlab-ci" in env_config: | ||||
|             tty.die("Error: `gitlab-ci` section detected with `ci`, these are not compatible") | ||||
| 
 | ||||
|   | ||||
| @@ -15,8 +15,8 @@ | ||||
| import spack.schema.packages | ||||
| import spack.schema.projections | ||||
| 
 | ||||
| #: legal first keys in the schema | ||||
| keys = ("spack", "env") | ||||
| #: Top level key in a manifest file | ||||
| TOP_LEVEL_KEY = "spack" | ||||
| 
 | ||||
| spec_list_schema = { | ||||
|     "type": "array", | ||||
| @@ -47,8 +47,8 @@ | ||||
|     "title": "Spack environment file schema", | ||||
|     "type": "object", | ||||
|     "additionalProperties": False, | ||||
|     "patternProperties": { | ||||
|         "^env|spack$": { | ||||
|     "properties": { | ||||
|         "spack": { | ||||
|             "type": "object", | ||||
|             "default": {}, | ||||
|             "additionalProperties": False, | ||||
|   | ||||
| @@ -32,7 +32,7 @@ def check_develop(self, env, spec, path=None): | ||||
|         assert dev_specs_entry["spec"] == str(spec) | ||||
| 
 | ||||
|         # check yaml representation | ||||
|         yaml = ev.config_dict(env.manifest) | ||||
|         yaml = env.manifest[ev.TOP_LEVEL_KEY] | ||||
|         assert spec.name in yaml["develop"] | ||||
|         yaml_entry = yaml["develop"][spec.name] | ||||
|         assert yaml_entry["spec"] == str(spec) | ||||
|   | ||||
		Reference in New Issue
	
	Block a user