refactor: clean up Environment class

- remove redundant code in Environment.__init__
- use socket.gethostname() instead of which('hostname')
- refactor updating SpecList references
- refactor 'specs' literals to a single variable for default list name
- use six.string_types for python 2/3 compatibility
This commit is contained in:
Gregory Becker 2019-06-24 15:27:09 -07:00 committed by Todd Gamblin
parent 88295d927e
commit 6661cb1926
2 changed files with 65 additions and 74 deletions

View File

@ -315,7 +315,7 @@ def env_view(args):
if args.view_path: if args.view_path:
view_path = args.view_path view_path = args.view_path
else: else:
view_path = env.create_view_path view_path = env.view_path_default
env.update_default_view(view_path) env.update_default_view(view_path)
env.write() env.write()
elif args.action == ViewAction.disable: elif args.action == ViewAction.disable:

View File

@ -8,6 +8,7 @@
import sys import sys
import shutil import shutil
import copy import copy
import socket
import ruamel.yaml import ruamel.yaml
import six import six
@ -77,6 +78,8 @@
#: legal first keys in the spack.yaml manifest file #: legal first keys in the spack.yaml manifest file
env_schema_keys = ('spack', 'env') env_schema_keys = ('spack', 'env')
user_speclist_name = 'specs'
def valid_env_name(name): def valid_env_name(name):
return re.match(valid_environment_name_re, name) return re.match(valid_environment_name_re, name)
@ -403,14 +406,9 @@ def _eval_conditional(string):
'architecture': str(arch), 'architecture': str(arch),
're': re, 're': re,
'env': os.environ, 'env': os.environ,
'hostname': socket.gethostname()
} }
hostname_bin = which('hostname')
if hostname_bin:
hostname = str(hostname_bin(output=str, error=str)).strip()
valid_variables['hostname'] = hostname
else:
tty.warn("Spack was unable to find the executable `hostname`"
" hostname will be unavailable in conditionals")
return eval(string, valid_variables) return eval(string, valid_variables)
@ -443,6 +441,7 @@ def __init__(self, path, init_file=None, with_view=None):
else: else:
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
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:
@ -452,24 +451,13 @@ def __init__(self, path, init_file=None, with_view=None):
with open(self.lock_path) as f: with open(self.lock_path) as f:
self._read_lockfile(f) self._read_lockfile(f)
if default_manifest: if default_manifest:
# No manifest, set user specs from lockfile
self._set_user_specs_from_lockfile() self._set_user_specs_from_lockfile()
if os.path.exists(self.manifest_path):
# read the spack.yaml file, if exists
with open(self.manifest_path) as f:
self._read_manifest(f)
elif self.concretized_user_specs:
# if not, take user specs from the lockfile
self._set_user_specs_from_lockfile()
self.yaml = _read_yaml(default_manifest_yaml)
else:
# if there's no manifest or lockfile, use the default
self._read_manifest(default_manifest_yaml)
if with_view is False: if with_view is False:
self.views = None self.views = None
elif isinstance(with_view, six.string_types): elif isinstance(with_view, six.string_types):
self.views = {'default': self.create_view_descriptor(with_view)} self.views = {'default': self.view_descriptor_defaults(with_view)}
# If with_view is None, then defer to the view settings determined by # If with_view is None, then defer to the view settings determined by
# the manifest file # the manifest file
@ -477,31 +465,31 @@ def _read_manifest(self, f):
"""Read manifest file and set up user specs.""" """Read manifest file and set up user specs."""
self.yaml = _read_yaml(f) self.yaml = _read_yaml(f)
self.read_specs = OrderedDict() self.spec_lists = OrderedDict()
for item in list(self.yaml.values())[0].get('definitions', []): for item in config_dict(self.yaml).get('definitions', []):
entry = copy.deepcopy(item) entry = copy.deepcopy(item)
when = _eval_conditional(entry.pop('when', 'True')) when = _eval_conditional(entry.pop('when', 'True'))
assert len(entry) == 1 assert len(entry) == 1
if when: if when:
name, spec_list = list(entry.items())[0] name, spec_list = iter(entry.items()).next()
user_specs = SpecList(name, spec_list, self.read_specs.copy()) user_specs = SpecList(name, spec_list, self.spec_lists.copy())
if name in self.read_specs: if name in self.spec_lists:
self.read_specs[name].extend(user_specs) self.spec_lists[name].extend(user_specs)
else: else:
self.read_specs[name] = user_specs self.spec_lists[name] = user_specs
spec_list = config_dict(self.yaml).get('specs') spec_list = config_dict(self.yaml).get(user_speclist_name)
user_specs = SpecList('specs', [s for s in spec_list if s], user_specs = SpecList(user_speclist_name, [s for s in spec_list if s],
self.read_specs.copy()) self.spec_lists.copy())
self.read_specs['specs'] = user_specs self.spec_lists[user_speclist_name] = user_specs
enable_view = config_dict(self.yaml).get('view') enable_view = config_dict(self.yaml).get('view')
# enable_view can be boolean, string, or None # enable_view can be boolean, string, or None
if enable_view is True or enable_view is None: if enable_view is True or enable_view is None:
self.views = {'default': self.create_view_descriptor()} self.views = {'default': self.view_descriptor_defaults()}
elif isinstance(enable_view, six.string_types): elif isinstance(enable_view, six.string_types):
self.views = {'default': self.create_view_descriptor(enable_view)} self.views = {'default': self.view_descriptor_defaults(enable_view)}
elif enable_view: elif enable_view:
self.views = enable_view self.views = enable_view
else: else:
@ -509,18 +497,19 @@ def _read_manifest(self, f):
@property @property
def user_specs(self): def user_specs(self):
return self.read_specs['specs'] return self.spec_lists[user_speclist_name]
def _set_user_specs_from_lockfile(self): def _set_user_specs_from_lockfile(self):
"""Copy user_specs from a read-in lockfile.""" """Copy user_specs from a read-in lockfile."""
self.read_specs = { self.spec_lists = {
'specs': SpecList( user_speclist_name: SpecList(
'specs', [Spec(s) for s in self.concretized_user_specs] user_speclist_name, [Spec(s)
for s in self.concretized_user_specs]
) )
} }
def clear(self): def clear(self):
self.read_specs = {'specs': SpecList()} # specs read from yaml self.spec_lists = {user_speclist_name: SpecList()} # specs from yaml
self.concretized_user_specs = [] # user specs from last concretize self.concretized_user_specs = [] # user specs from last concretize
self.concretized_order = [] # roots of last concretize, in order self.concretized_order = [] # roots of last concretize, in order
self.specs_by_hash = {} # concretized specs by hash self.specs_by_hash = {} # concretized specs by hash
@ -573,13 +562,14 @@ def repos_path(self):
def log_path(self): def log_path(self):
return os.path.join(self.path, env_subdir_name, 'logs') return os.path.join(self.path, env_subdir_name, 'logs')
def create_view_descriptor(self, root=None): def view_descriptor_defaults(self, root=None):
# view descriptor with all values set to default
if root is None: if root is None:
root = self.create_view_path root = self.view_path_default
return {'root': root, 'projections': {}} return {'root': root, 'projections': {}}
@property @property
def create_view_path(self): def view_path_default(self):
return os.path.join(self.env_subdir_path, 'view') return os.path.join(self.env_subdir_path, 'view')
@property @property
@ -648,7 +638,21 @@ def destroy(self):
"""Remove this environment from Spack entirely.""" """Remove this environment from Spack entirely."""
shutil.rmtree(self.path) shutil.rmtree(self.path)
def add(self, user_spec, list_name='specs'): def update_stale_references(self, from_list=None):
"""Iterate over spec lists updating references."""
if not from_list:
from_list = iter(self.spec_lists.keys()).next()
index = list(self.spec_lists.keys()).index(from_list)
# spec_lists is an OrderedDict, all list entries after the modified
# list may refer to the modified list. Update stale references
for i, (name, speclist) in enumerate(
list(self.spec_lists.items())[index + 1:], index + 1):
new_reference = dict((n, self.spec_lists[n])
for n in list(self.spec_lists.keys())[:i])
speclist.update_reference(new_reference)
def add(self, user_spec, list_name=user_speclist_name):
"""Add a single user_spec (non-concretized) to the Environment """Add a single user_spec (non-concretized) to the Environment
Returns: Returns:
@ -658,39 +662,31 @@ def add(self, user_spec, list_name='specs'):
""" """
spec = Spec(user_spec) spec = Spec(user_spec)
if list_name not in self.read_specs: if list_name not in self.spec_lists:
raise SpackEnvironmentError( raise SpackEnvironmentError(
'No list %s exists in environment %s' % (list_name, self.name) 'No list %s exists in environment %s' % (list_name, self.name)
) )
if list_name == 'specs': if list_name == user_speclist_name:
if not spec.name: if not spec.name:
raise SpackEnvironmentError( raise SpackEnvironmentError(
'cannot add anonymous specs to an environment!') 'cannot add anonymous specs to an environment!')
elif not spack.repo.path.exists(spec.name): elif not spack.repo.path.exists(spec.name):
raise SpackEnvironmentError('no such package: %s' % spec.name) raise SpackEnvironmentError('no such package: %s' % spec.name)
list_to_change = self.read_specs[list_name] list_to_change = self.spec_lists[list_name]
existing = str(spec) in list_to_change.yaml_list existing = str(spec) in list_to_change.yaml_list
if not existing: if not existing:
list_to_change.add(str(spec)) list_to_change.add(str(spec))
index = list(self.read_specs.keys()).index(list_name) self.update_stale_references(list_name)
# read_specs is an OrderedDict, all list entries after the modified
# list may refer to the modified list. Update stale references
for i, (name, speclist) in enumerate(
list(self.read_specs.items())[index + 1:], index + 1):
new_reference = dict((n, self.read_specs[n])
for n in list(self.read_specs.keys())[:i])
speclist.update_reference(new_reference)
return bool(not existing) return bool(not existing)
def remove(self, query_spec, list_name='specs', force=False): def remove(self, query_spec, list_name=user_speclist_name, force=False):
"""Remove specs from an environment that match a query_spec""" """Remove specs from an environment that match a query_spec"""
query_spec = Spec(query_spec) query_spec = Spec(query_spec)
list_to_change = self.read_specs[list_name] list_to_change = self.spec_lists[list_name]
matches = [] matches = []
if not query_spec.concrete: if not query_spec.concrete:
@ -714,14 +710,7 @@ def remove(self, query_spec, list_name='specs', force=False):
if spec in list_to_change: if spec in list_to_change:
list_to_change.remove(spec) list_to_change.remove(spec)
# read_specs is an OrderedDict, all list entries after the modified self.update_stale_references(list_name)
# list may refer to the modified list. Update stale references
index = list(self.read_specs.keys()).index(list_name)
for i, (name, speclist) in enumerate(
list(self.read_specs.items())[index + 1:], index + 1):
new_reference = dict((n, self.read_specs[n])
for n in list(self.read_specs.keys())[:i])
speclist.update_reference(new_reference)
# If force, update stale concretized specs # If force, update stale concretized specs
# Only check specs removed by this operation # Only check specs removed by this operation
@ -858,9 +847,10 @@ def update_default_view(self, viewpath):
if self.default_view_path: if self.default_view_path:
self.views['default']['root'] = viewpath self.views['default']['root'] = viewpath
elif self.views: elif self.views:
self.views['default'] = self.create_view_descriptor(viewpath) self.views['default'] = self.view_descriptor_defaults(viewpath)
else: else:
self.views = {'default': self.create_view_descriptor(viewpath)} self.views = {'default': self.view_descriptor_defaults(
viewpath)}
else: else:
self.views.pop('default', None) self.views.pop('default', None)
@ -1163,8 +1153,8 @@ def write(self):
# invalidate _repo cache # invalidate _repo cache
self._repo = None self._repo = None
# put any chagnes in the definitions in the YAML # put any changes in the definitions in the YAML
for name, speclist in list(self.read_specs.items())[:-1]: for name, speclist in list(self.spec_lists.items())[:-1]:
conf = config_dict(self.yaml) conf = config_dict(self.yaml)
active_yaml_lists = [l for l in conf.get('definitions', []) active_yaml_lists = [l for l in conf.get('definitions', [])
if name in l and if name in l and
@ -1174,12 +1164,12 @@ def write(self):
for ayl in active_yaml_lists: for ayl in active_yaml_lists:
# If it's not a string, it's a matrix. Those can't have changed # If it's not a string, it's a matrix. Those can't have changed
ayl[name][:] = [s for s in ayl.setdefault(name, []) ayl[name][:] = [s for s in ayl.setdefault(name, [])
if not isinstance(s, str) or if not isinstance(s, six.string_types) or
Spec(s) in speclist.specs] Spec(s) in speclist.specs]
# Put the new specs into the first active list from the yaml # Put the new specs into the first active list from the yaml
new_specs = [entry for entry in speclist.yaml_list new_specs = [entry for entry in speclist.yaml_list
if isinstance(entry, str) and if isinstance(entry, six.string_types) and
not any(entry in ayl[name] not any(entry in ayl[name]
for ayl in active_yaml_lists)] for ayl in active_yaml_lists)]
list_for_new_specs = active_yaml_lists[0].setdefault(name, []) list_for_new_specs = active_yaml_lists[0].setdefault(name, [])
@ -1188,14 +1178,15 @@ def write(self):
# put the new user specs in the YAML. # put the new user specs in the YAML.
# This can be done directly because there can't be multiple definitions # This can be done directly because there can't be multiple definitions
# nor when clauses for `specs` list. # nor when clauses for `specs` list.
yaml_spec_list = config_dict(self.yaml).setdefault('specs', []) yaml_spec_list = config_dict(self.yaml).setdefault(user_speclist_name,
[])
yaml_spec_list[:] = self.user_specs.yaml_list yaml_spec_list[:] = self.user_specs.yaml_list
if self.views and len(self.views) == 1 and self.default_view_path: if self.views and len(self.views) == 1 and self.default_view_path:
path = self.default_view_path path = self.default_view_path
if self.views['default'] == self.create_view_descriptor(): if self.views['default'] == self.view_descriptor_defaults():
view = True view = True
elif self.views['default'] == self.create_view_descriptor(path): elif self.views['default'] == self.view_descriptor_defaults(path):
view = path view = path
else: else:
view = self.views view = self.views