'spack install' can overwrite an existing installation (#5384)

'spack install' can now reinstall a spec even if it has dependents, via
the --overwrite option. This option moves the current installation in a
temporary directory. If the reinstallation is successful the temporary
is removed, otherwise a rollback is performed.

lib/spack/llnl/util/filesystem.py

@@ -24,6 +24,7 @@
 ##############################################################################
 import collections
 import errno
+import hashlib
 import fileinput
 import fnmatch
 import glob
@@ -31,12 +32,13 @@
 import os
 import re
 import shutil
-import six
 import stat
 import subprocess
 import sys
+import tempfile
 from contextlib import contextmanager
 
+import six
 from llnl.util import tty
 from llnl.util.lang import dedupe
 
@@ -282,6 +284,60 @@ def working_dir(dirname, **kwargs):
     os.chdir(orig_dir)
 
 
+@contextmanager
+def replace_directory_transaction(directory_name, tmp_root=None):
+    """Moves a directory to a temporary space. If the operations executed
+    within the context manager don't raise an exception, the directory is
+    deleted. If there is an exception, the move is undone.
+
+    Args:
+        directory_name (path): absolute path of the directory name
+        tmp_root (path): absolute path of the parent directory where to create
+            the temporary
+
+    Returns:
+        temporary directory where ``directory_name`` has been moved
+    """
+    # Check the input is indeed a directory with absolute path.
+    # Raise before anything is done to avoid moving the wrong directory
+    assert os.path.isdir(directory_name), \
+        '"directory_name" must be a valid directory'
+    assert os.path.isabs(directory_name), \
+        '"directory_name" must contain an absolute path'
+
+    directory_basename = os.path.basename(directory_name)
+
+    if tmp_root is not None:
+        assert os.path.isabs(tmp_root)
+
+    tmp_dir = tempfile.mkdtemp(dir=tmp_root)
+    tty.debug('TEMPORARY DIRECTORY CREATED [{0}]'.format(tmp_dir))
+
+    shutil.move(src=directory_name, dst=tmp_dir)
+    tty.debug('DIRECTORY MOVED [src={0}, dest={1}]'.format(
+        directory_name, tmp_dir
+    ))
+
+    try:
+        yield tmp_dir
+    except (Exception, KeyboardInterrupt, SystemExit):
+        # Delete what was there, before copying back the original content
+        if os.path.exists(directory_name):
+            shutil.rmtree(directory_name)
+        shutil.move(
+            src=os.path.join(tmp_dir, directory_basename),
+            dst=os.path.dirname(directory_name)
+        )
+        tty.debug('DIRECTORY RECOVERED [{0}]'.format(directory_name))
+
+        msg = 'the transactional move of "{0}" failed.'
+        raise RuntimeError(msg.format(directory_name))
+    else:
+        # Otherwise delete the temporary directory
+        shutil.rmtree(tmp_dir)
+        tty.debug('TEMPORARY DIRECTORY DELETED [{0}]'.format(tmp_dir))
+
+
 @contextmanager
 def hide_files(*file_list):
     try:
@@ -294,6 +350,32 @@ def hide_files(*file_list):
             shutil.move(bak, f)
 
 
+def hash_directory(directory):
+    """Hashes recursively the content of a directory.
+
+    Args:
+        directory (path): path to a directory to be hashed
+
+    Returns:
+        hash of the directory content
+    """
+    assert os.path.isdir(directory), '"directory" must be a directory!'
+
+    md5_hash = hashlib.md5()
+
+    # Adapted from https://stackoverflow.com/a/3431835/771663
+    for root, dirs, files in os.walk(directory):
+        for name in sorted(files):
+            filename = os.path.join(root, name)
+            # TODO: if caching big files becomes an issue, convert this to
+            # TODO: read in chunks. Currently it's used only for testing
+            # TODO: purposes.
+            with open(filename, 'rb') as f:
+                md5_hash.update(f.read())
+
+    return md5_hash.hexdigest()
+
+
 def touch(path):
     """Creates an empty file at the specified path."""
     perms = (os.O_WRONLY | os.O_CREAT | os.O_NONBLOCK | os.O_NOCTTY)

lib/spack/spack/cmd/install.py

@@ -61,6 +61,9 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-j', '--jobs', action='store', type=int,
         help="explicitly set number of make jobs. default is #cpus")
+    subparser.add_argument(
+        '--overwrite', action='store_true',
+        help="reinstall an existing spec, even if it has dependents")
     subparser.add_argument(
         '--keep-prefix', action='store_true',
         help="don't remove the install prefix if installation fails")
@@ -84,7 +87,7 @@ def setup_parser(subparser):
         help="display verbose build output while installing")
     subparser.add_argument(
         '--fake', action='store_true',
-        help="fake install. just remove prefix and create a fake file")
+        help="fake install for debug purposes.")
     subparser.add_argument(
         '-f', '--file', action='store_true',
         help="install from file. Read specs to install from .yaml files")
@@ -121,6 +124,7 @@ def setup_parser(subparser):
         default=None,
         help="filename for the log file. if not passed a default will be used"
     )
+    arguments.add_common_arguments(subparser, ['yes_to_all'])
 
 
 # Needed for test cases
@@ -314,6 +318,61 @@ def default_log_file(spec):
     return fs.join_path(dirname, basename)
 
 
+def install_spec(cli_args, kwargs, spec):
+
+    saved_do_install = PackageBase.do_install
+    decorator = lambda fn: fn
+
+    # Check if we were asked to produce some log for dashboards
+    if cli_args.log_format is not None:
+        # Compute the filename for logging
+        log_filename = cli_args.log_file
+        if not log_filename:
+            log_filename = default_log_file(spec)
+
+        # Create the test suite in which to log results
+        test_suite = TestSuite(spec)
+
+        # Temporarily decorate PackageBase.do_install to monitor
+        # recursive calls.
+        decorator = junit_output(spec, test_suite)
+
+    # Do the actual installation
+    try:
+        # decorate the install if necessary
+        PackageBase.do_install = decorator(PackageBase.do_install)
+
+        if cli_args.things_to_install == 'dependencies':
+            # Install dependencies as-if they were installed
+            # for root (explicit=False in the DB)
+            kwargs['explicit'] = False
+            for s in spec.dependencies():
+                p = spack.repo.get(s)
+                p.do_install(**kwargs)
+        else:
+            package = spack.repo.get(spec)
+            kwargs['explicit'] = True
+            package.do_install(**kwargs)
+
+    except InstallError as e:
+        if cli_args.show_log_on_error:
+            e.print_context()
+            if not os.path.exists(e.pkg.build_log_path):
+                tty.error("'spack install' created no log.")
+            else:
+                sys.stderr.write('Full build log:\n')
+                with open(e.pkg.build_log_path) as log:
+                    shutil.copyfileobj(log, sys.stderr)
+        raise
+
+    finally:
+        PackageBase.do_install = saved_do_install
+
+    # Dump test output if asked to
+    if cli_args.log_format is not None:
+        test_suite.dump(log_filename)
+
+
 def install(parser, args, **kwargs):
     if not args.package:
         tty.die("install requires at least one package argument")
@@ -360,56 +419,39 @@ def install(parser, args, **kwargs):
     if len(specs) == 0:
         tty.error('The `spack install` command requires a spec to install.')
 
-    for spec in specs:
+    if args.overwrite:
-        saved_do_install = PackageBase.do_install
+        # If we asked to overwrite an existing spec we must ensure that:
-        decorator = lambda fn: fn
+        # 1. We have only one spec
+        # 2. The spec is already installed
+        assert len(specs) == 1, \
+            "only one spec is allowed when overwriting an installation"
 
-        # Check if we were asked to produce some log for dashboards
+        spec = specs[0]
-        if args.log_format is not None:
+        t = spack.store.db.query(spec)
-            # Compute the filename for logging
+        assert len(t) == 1, "to overwrite a spec you must install it first"
-            log_filename = args.log_file
-            if not log_filename:
-                log_filename = default_log_file(spec)
 
-            # Create the test suite in which to log results
+        # Give the user a last chance to think about overwriting an already
-            test_suite = TestSuite(spec)
+        # existing installation
+        if not args.yes_to_all:
+            tty.msg('The following package will be reinstalled:\n')
 
-            # Temporarily decorate PackageBase.do_install to monitor
+            display_args = {
-            # recursive calls.
+                'long': True,
-            decorator = junit_output(spec, test_suite)
+                'show_flags': True,
+                'variants': True
+            }
 
-        # Do the actual installation
+            spack.cmd.display_specs(t, **display_args)
-        try:
+            answer = tty.get_yes_or_no(
-            # decorate the install if necessary
+                'Do you want to proceed?', default=False
-            PackageBase.do_install = decorator(PackageBase.do_install)
+            )
+            if not answer:
+                tty.die('Reinstallation aborted.')
 
-            if args.things_to_install == 'dependencies':
+        with fs.replace_directory_transaction(specs[0].prefix):
-                # Install dependencies as-if they were installed
+            install_spec(args, kwargs, specs[0])
-                # for root (explicit=False in the DB)
-                kwargs['explicit'] = False
-                for s in spec.dependencies():
-                    p = spack.repo.get(s)
-                    p.do_install(**kwargs)
 
-            else:
+    else:
-                package = spack.repo.get(spec)
-                kwargs['explicit'] = True
-                package.do_install(**kwargs)
 
-        except InstallError as e:
+        for spec in specs:
-            if args.show_log_on_error:
+            install_spec(args, kwargs, spec)
-                e.print_context()
-                if not os.path.exists(e.pkg.build_log_path):
-                    tty.error("'spack install' created no log.")
-                else:
-                    sys.stderr.write('Full build log:\n')
-                    with open(e.pkg.build_log_path) as log:
-                        shutil.copyfileobj(log, sys.stderr)
-            raise
-
-        finally:
-            PackageBase.do_install = saved_do_install
-
-        # Dump test output if asked to
-        if args.log_format is not None:
-            test_suite.dump(log_filename)

lib/spack/spack/test/cmd/install.py

@@ -28,6 +28,8 @@
 
 import pytest
 
+import llnl.util.filesystem as fs
+
 import spack
 import spack.cmd.install
 from spack.spec import Spec
@@ -197,3 +199,38 @@ def test_show_log_on_error(builtin_mock, mock_archive, mock_fetch,
     errors = [line for line in out.split('\n')
               if 'configure: error: cannot run C compiled programs' in line]
     assert len(errors) == 2
+
+
+def test_install_overwrite(
+        builtin_mock, mock_archive, mock_fetch, config, install_mockery
+):
+    # It's not possible to overwrite something that is not yet installed
+    with pytest.raises(AssertionError):
+        install('--overwrite', 'libdwarf')
+
+    # --overwrite requires a single spec
+    with pytest.raises(AssertionError):
+        install('--overwrite', 'libdwarf', 'libelf')
+
+    # Try to install a spec and then to reinstall it.
+    spec = Spec('libdwarf')
+    spec.concretize()
+
+    install('libdwarf')
+
+    assert os.path.exists(spec.prefix)
+    expected_md5 = fs.hash_directory(spec.prefix)
+
+    # Modify the first installation to be sure the content is not the same
+    # as the one after we reinstalled
+    with open(os.path.join(spec.prefix, 'only_in_old'), 'w') as f:
+        f.write('This content is here to differentiate installations.')
+
+    bad_md5 = fs.hash_directory(spec.prefix)
+
+    assert bad_md5 != expected_md5
+
+    install('--overwrite', '-y', 'libdwarf')
+    assert os.path.exists(spec.prefix)
+    assert fs.hash_directory(spec.prefix) == expected_md5
+    assert fs.hash_directory(spec.prefix) != bad_md5

lib/spack/spack/test/util/filesystem.py

@@ -0,0 +1,61 @@
+##############################################################################
+# Copyright (c) 2013-2017, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/llnl/spack
+# Please also see the NOTICE and LICENSE files for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+
+import llnl.util.filesystem as fs
+
+
+def test_move_transaction_commit(tmpdir):
+
+    fake_library = tmpdir.mkdir('lib').join('libfoo.so')
+    fake_library.write('Just some fake content.')
+
+    old_md5 = fs.hash_directory(str(tmpdir))
+
+    with fs.replace_directory_transaction(str(tmpdir.join('lib'))):
+        fake_library = tmpdir.mkdir('lib').join('libfoo.so')
+        fake_library.write('Other content.')
+        new_md5 = fs.hash_directory(str(tmpdir))
+
+    assert old_md5 != fs.hash_directory(str(tmpdir))
+    assert new_md5 == fs.hash_directory(str(tmpdir))
+
+
+def test_move_transaction_rollback(tmpdir):
+
+    fake_library = tmpdir.mkdir('lib').join('libfoo.so')
+    fake_library.write('Just some fake content.')
+
+    h = fs.hash_directory(str(tmpdir))
+
+    try:
+        with fs.replace_directory_transaction(str(tmpdir.join('lib'))):
+            assert h != fs.hash_directory(str(tmpdir))
+            fake_library = tmpdir.mkdir('lib').join('libfoo.so')
+            fake_library.write('Other content.')
+            raise RuntimeError('')
+    except RuntimeError:
+        pass
+
+    assert h == fs.hash_directory(str(tmpdir))