From 91de23ce650ef4dd007b94f67c26e1e6901be354 Mon Sep 17 00:00:00 2001 From: Scott Wittenburg Date: Fri, 30 Apr 2021 16:51:43 -0600 Subject: [PATCH] install cmd: --no-add in an env installs w/out concretize and add In an active concretize environment, support installing one or more cli specs only if they are already present in the environment. The `--no-add` option is the default for root specs, but optional for dependency specs. I.e. if you `spack install ` in an environment, the dependency-only spec `depspec` will be added as a root of the environment before being installed. In addition, `spack install --no-add ` fails if it does not find an unambiguous match for `spec`. --- lib/spack/docs/environments.rst | 12 ++++ lib/spack/spack/cmd/install.py | 67 +++++++++++++++-- lib/spack/spack/environment.py | 18 +++-- lib/spack/spack/test/cmd/install.py | 107 ++++++++++++++++++++++++++++ share/spack/spack-completion.bash | 2 +- 5 files changed, 197 insertions(+), 9 deletions(-) diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index 76b974ff62..12f6f4121c 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -356,6 +356,18 @@ command also stores a Spack repo containing the ``package.py`` file used at install time for each package in the ``repos/`` directory in the Environment. +The ``--no-add`` option can be used in a concrete environment to tell +spack to install specs already present in the environment but not to +add any new root specs to the environment. For root specs provided +to ``spack install`` on the command line, ``--no-add`` is the default, +while for dependency specs on the other hand, it is optional. In other +words, if there is an unambiguous match in the active concrete environment +for a root spec provided to ``spack install`` on the command line, spack +does not require you to specify the ``--no-add` option to prevent the spec +from being added again. At the same time, a spec that already exists in the +environment, but only as a dependency, will be added to the environment as a +root spec without the ``--no-add`` option. + ^^^^^^^ Loading ^^^^^^^ diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 26d824c128..53e924cb9e 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -139,6 +139,10 @@ def setup_parser(subparser): subparser.add_argument( '--only-concrete', action='store_true', default=False, help='(with environment) only install already concretized specs') + subparser.add_argument( + '--no-add', action='store_true', default=False, + help="""(with environment) only install specs provided as argument +if they are already in the concretized environment""") subparser.add_argument( '-f', '--file', action='append', default=[], dest='specfiles', metavar='SPEC_YAML_FILE', @@ -205,11 +209,66 @@ def install_specs(cli_args, kwargs, specs): try: if env: + specs_to_install = [] + specs_to_add = [] for abstract, concrete in specs: - with env.write_transaction(): - concrete = env.concretize_and_add(abstract, concrete) - env.write(regenerate_views=False) - env.install_all(cli_args, **kwargs) + # This won't find specs added to the env since last + # concretize, therefore should we consider enforcing + # concretization of the env before allowing to install + # specs? + m_spec = env.matching_spec(abstract) + + # If there is any ambiguity in the above call to matching_spec + # (i.e. if more than one spec in the environment matches), then + # SpackEnvironmentError is rasied, with a message listing the + # the matches. Getting to this point means there were either + # no matches or exactly one match. + + if not m_spec: + tty.debug('{0} matched nothing in the env'.format( + abstract.name)) + # no matches in the env + if cli_args.no_add: + msg = ('You asked to install {0} without adding it ' + + '(--no-add), but no such spec exists in ' + + 'environment').format(abstract.name) + tty.die(msg) + else: + tty.debug('adding {0} as a root'.format(abstract.name)) + specs_to_add.append((abstract, concrete)) + + continue + + tty.debug('exactly one match for {0} in env -> {1}'.format( + m_spec.name, m_spec.dag_hash())) + + if m_spec in env.roots() or cli_args.no_add: + # either the single match is a root spec (and --no-add is + # the default for roots) or --no-add was stated explictly + tty.debug('just install {0}'.format(m_spec.name)) + specs_to_install.append(m_spec) + else: + # the single match is not a root (i.e. it's a dependency), + # and --no-add was not specified, so we'll add it as a + # root before installing + tty.debug('add {0} then install it'.format(m_spec.name)) + specs_to_add.append((abstract, concrete)) + + if specs_to_add: + tty.debug('Adding the following specs as roots:') + for abstract, concrete in specs_to_add: + tty.debug(' {0}'.format(abstract.name)) + with env.write_transaction(): + specs_to_install.append( + env.concretize_and_add(abstract, concrete)) + env.write(regenerate_views=False) + + # Install the validated list of cli specs + if specs_to_install: + tty.debug('Installing the following cli specs:') + for s in specs_to_install: + tty.debug(' {0}'.format(s.name)) + env.install_specs(specs_to_install, args=cli_args, **kwargs) else: installs = [(concrete.package, kwargs) for _, concrete in specs] builder = PackageInstaller(installs) diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index ab02f46222..46541fb32a 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -1440,21 +1440,31 @@ def install_all(self, args=None, **install_args): args (Namespace): argparse namespace with command arguments install_args (dict): keyword install arguments """ + self.install_specs(None, args=args, **install_args) + + def install_specs(self, specs=None, args=None, **install_args): from spack.installer import PackageInstaller + tty.debug('Assessing installation status of environment packages') # If "spack install" is invoked repeatedly for a large environment # where all specs are already installed, the operation can take # a large amount of time due to repeatedly acquiring and releasing # locks, this does an initial check across all specs within a single - # DB read transaction to reduce time spent in this case. - specs_to_install = self.uninstalled_specs() + # DB read transaction to reduce time spent in this case. In the next + # three lines we remove any already-installed root specs from the list + # to install. However, uninstalled_specs() only considers root specs, + # so this will allow dep specs to be unnecessarily re-installed. + uninstalled_roots = self.uninstalled_specs() + specs_to_install = specs or uninstalled_roots + specs_to_install = [s for s in specs_to_install + if s not in self.roots() or s in uninstalled_roots] if not specs_to_install: tty.msg('All of the packages are already installed') return - tty.debug('Processing {0} uninstalled specs' - .format(len(specs_to_install))) + tty.debug('Processing {0} uninstalled specs'.format( + len(specs_to_install))) install_args['overwrite'] = install_args.get( 'overwrite', []) + self._get_overwrite_specs() diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index 73d701c06f..cbdea73931 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -32,6 +32,7 @@ mirror = SpackCommand('mirror') uninstall = SpackCommand('uninstall') buildcache = SpackCommand('buildcache') +find = SpackCommand('find') @pytest.fixture() @@ -712,6 +713,112 @@ def test_install_only_dependencies_of_all_in_env( assert os.path.exists(dep.prefix) +def test_install_no_add_in_env(tmpdir, mock_fetch, install_mockery, + mutable_mock_env_path): + # To test behavior of --no-add option, we create the following environment: + # + # mpileaks + # ^callpath + # ^dyninst + # ^libelf@0.8.13 # or latest, really + # ^libdwarf + # ^mpich + # libelf@0.8.10 + # a~bvv + # ^b + # a + # ^b + e = ev.create('test') + e.add('mpileaks') + e.add('libelf@0.8.10') # so env has both root and dep libelf specs + e.add('a') + e.add('a ~bvv') + e.concretize() + env_specs = e.all_specs() + + a_spec = None + b_spec = None + mpi_spec = None + + # First find and remember some target concrete specs in the environment + for e_spec in env_specs: + if e_spec.satisfies(Spec('a ~bvv')): + a_spec = e_spec + elif e_spec.name == 'b': + b_spec = e_spec + elif e_spec.satisfies(Spec('mpi')): + mpi_spec = e_spec + + assert(a_spec) + assert(a_spec.concrete) + + assert(b_spec) + assert(b_spec.concrete) + assert(b_spec not in e.roots()) + + assert(mpi_spec) + assert(mpi_spec.concrete) + + # Activate the environment + with e: + # Assert using --no-add with a spec not in the env fails + inst_out = install( + '--no-add', 'boost', fail_on_error=False, output=str) + + assert('no such spec exists in environment' in inst_out) + + # Ensure using --no-add with an ambiguous spec fails + with pytest.raises(ev.SpackEnvironmentError) as err: + inst_out = install( + '--no-add', 'a', output=str) + + assert('a matches multiple specs in the env' in str(err)) + + # With "--no-add", install an unambiguous dependency spec (that already + # exists as a dep in the environment) using --no-add and make sure it + # gets installed (w/ deps), but is not added to the environment. + install('--no-add', 'dyninst') + + find_output = find('-l', output=str) + assert('dyninst' in find_output) + assert('libdwarf' in find_output) + assert('libelf' in find_output) + assert('callpath' not in find_output) + + post_install_specs = e.all_specs() + assert all([s in env_specs for s in post_install_specs]) + + # Make sure we can install a concrete dependency spec from a spec.yaml + # file on disk, using the ``--no-add` option, and the spec is installed + # but not added as a root + mpi_spec_yaml_path = tmpdir.join('{0}.yaml'.format(mpi_spec.name)) + with open(mpi_spec_yaml_path.strpath, 'w') as fd: + fd.write(mpi_spec.to_yaml(hash=ht.full_hash)) + + install('--no-add', '-f', mpi_spec_yaml_path.strpath) + assert(mpi_spec not in e.roots()) + + find_output = find('-l', output=str) + assert(mpi_spec.name in find_output) + + # Without "--no-add", install an unambiguous depependency spec (that + # already exists as a dep in the environment) without --no-add and make + # sure it is added as a root of the environment as well as installed. + assert(b_spec not in e.roots()) + + install('b') + + assert(b_spec in e.roots()) + assert(b_spec not in e.uninstalled_specs()) + + # Without "--no-add", install a novel spec and make sure it is added + # as a root and installed. + install('bowtie') + + assert(any([s.name == 'bowtie' for s in e.roots()])) + assert(not any([s.name == 'bowtie' for s in e.uninstalled_specs()])) + + def test_install_help_does_not_show_cdash_options(capsys): """ Make sure `spack install --help` does not describe CDash arguments diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 0c4bb8eba1..0c17f29312 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -1063,7 +1063,7 @@ _spack_info() { _spack_install() { if $list_options then - SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --overwrite --fail-fast --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --monitor --monitor-no-auth --monitor-keep-going --monitor-host --monitor-prefix --include-build-deps --no-check-signature --require-full-hash-match --show-log-on-error --source -n --no-checksum --deprecated -v --verbose --fake --only-concrete -f --file --clean --dirty --test --run-tests --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all" + SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --overwrite --fail-fast --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --monitor --monitor-no-auth --monitor-keep-going --monitor-host --monitor-prefix --include-build-deps --no-check-signature --require-full-hash-match --show-log-on-error --source -n --no-checksum --deprecated -v --verbose --fake --only-concrete --no-add -f --file --clean --dirty --test --run-tests --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all" else _all_packages fi