Revert "Override partial installs by default" (#3918)
* Revert "Override partial installs by default (#3530)"
This reverts commit a65c37f15d
.
This commit is contained in:
@@ -72,9 +72,6 @@ def setup_parser(subparser):
|
||||
subparser.add_argument(
|
||||
'--fake', action='store_true', dest='fake',
|
||||
help="fake install. just remove prefix and create a fake file")
|
||||
subparser.add_argument(
|
||||
'--force', action='store_true', dest='force',
|
||||
help='Install again even if package is already installed.')
|
||||
|
||||
cd_group = subparser.add_mutually_exclusive_group()
|
||||
arguments.add_common_arguments(cd_group, ['clean', 'dirty'])
|
||||
@@ -322,12 +319,6 @@ def install(parser, args, **kwargs):
|
||||
tty.error('The `spack install` command requires a spec to install.')
|
||||
|
||||
for spec in specs:
|
||||
if args.force:
|
||||
for s in spec.traverse():
|
||||
if s.package.installed:
|
||||
tty.msg("Clearing %s for new installation" % s.name)
|
||||
s.package.do_uninstall(force=True)
|
||||
|
||||
# Check if we were asked to produce some log for dashboards
|
||||
if args.log_format is not None:
|
||||
# Compute the filename for logging
|
||||
|
@@ -239,17 +239,6 @@ def build_packages_path(self, spec):
|
||||
return join_path(self.path_for_spec(spec), self.metadata_dir,
|
||||
self.packages_dir)
|
||||
|
||||
def _completion_marker_file(self, spec):
|
||||
return join_path(self.path_for_spec(spec), self.metadata_dir,
|
||||
'complete')
|
||||
|
||||
def mark_complete(self, spec):
|
||||
with open(self._completion_marker_file(spec), 'w'):
|
||||
pass
|
||||
|
||||
def completed_install(self, spec):
|
||||
return os.path.exists(self._completion_marker_file(spec))
|
||||
|
||||
def create_install_directory(self, spec):
|
||||
_check_concrete(spec)
|
||||
|
||||
@@ -263,34 +252,26 @@ def create_install_directory(self, spec):
|
||||
def check_installed(self, spec):
|
||||
_check_concrete(spec)
|
||||
path = self.path_for_spec(spec)
|
||||
spec_file_path = self.spec_file_path(spec)
|
||||
|
||||
if not os.path.isdir(path):
|
||||
return None
|
||||
elif not self.completed_install(spec):
|
||||
raise InconsistentInstallDirectoryError(
|
||||
'The prefix %s contains a partial install' % path)
|
||||
|
||||
self.check_metadata_consistency(spec)
|
||||
return path
|
||||
|
||||
def check_metadata_consistency(self, spec):
|
||||
spec_file_path = self.spec_file_path(spec)
|
||||
|
||||
if not os.path.isfile(spec_file_path):
|
||||
raise InconsistentInstallDirectoryError(
|
||||
'Install prefix exists but contains no spec.yaml:',
|
||||
" " + spec.prefix)
|
||||
" " + path)
|
||||
|
||||
installed_spec = self.read_spec(spec_file_path)
|
||||
if installed_spec == spec:
|
||||
return
|
||||
return path
|
||||
|
||||
# DAG hashes currently do not include build dependencies.
|
||||
#
|
||||
# TODO: remove this when we do better concretization and don't
|
||||
# ignore build-only deps in hashes.
|
||||
elif installed_spec == spec.copy(deps=('link', 'run')):
|
||||
return
|
||||
return path
|
||||
|
||||
if spec.dag_hash() == installed_spec.dag_hash():
|
||||
raise SpecHashCollisionError(spec, installed_spec)
|
||||
|
@@ -856,7 +856,7 @@ def provides(self, vpkg_name):
|
||||
|
||||
@property
|
||||
def installed(self):
|
||||
return spack.store.layout.completed_install(self.spec)
|
||||
return os.path.isdir(self.prefix)
|
||||
|
||||
@property
|
||||
def prefix_lock(self):
|
||||
@@ -1174,16 +1174,10 @@ def do_install(self,
|
||||
(self.name, self.spec.external))
|
||||
return
|
||||
|
||||
self.repair_partial(keep_prefix)
|
||||
|
||||
# Ensure package is not already installed
|
||||
layout = spack.store.layout
|
||||
with self._prefix_read_lock():
|
||||
if (keep_prefix and os.path.isdir(self.prefix) and
|
||||
(not self.installed)):
|
||||
tty.msg(
|
||||
"Continuing from partial install of %s" % self.name)
|
||||
elif layout.check_installed(self.spec):
|
||||
if layout.check_installed(self.spec):
|
||||
tty.msg(
|
||||
"%s is already installed in %s" % (self.name, self.prefix))
|
||||
rec = spack.store.db.get_record(self.spec)
|
||||
@@ -1311,11 +1305,9 @@ def build_process(input_stream):
|
||||
|
||||
try:
|
||||
# Create the install prefix and fork the build process.
|
||||
if (not keep_prefix) or (not os.path.isdir(self.prefix)):
|
||||
spack.store.layout.create_install_directory(self.spec)
|
||||
spack.store.layout.create_install_directory(self.spec)
|
||||
# Fork a child to do the actual installation
|
||||
spack.build_environment.fork(self, build_process, dirty=dirty)
|
||||
spack.store.layout.mark_complete(self.spec)
|
||||
# If we installed then we should keep the prefix
|
||||
keep_prefix = True if self.last_phase is None else keep_prefix
|
||||
# note: PARENT of the build process adds the new package to
|
||||
@@ -1340,43 +1332,6 @@ def build_process(input_stream):
|
||||
if not keep_prefix:
|
||||
self.remove_prefix()
|
||||
|
||||
def repair_partial(self, continue_with_partial=False):
|
||||
"""If continue_with_partial is not set, this ensures that the package
|
||||
is either fully-installed or that the prefix is removed. If the
|
||||
package is installed but there is no DB entry then this adds a
|
||||
record. If continue_with_partial is not set this also clears the
|
||||
stage directory to start an installation from scratch.
|
||||
"""
|
||||
layout = spack.store.layout
|
||||
with self._prefix_read_lock():
|
||||
if (os.path.isdir(self.prefix) and not self.installed and
|
||||
not continue_with_partial):
|
||||
spack.hooks.pre_uninstall(self)
|
||||
self.remove_prefix()
|
||||
try:
|
||||
spack.store.db.remove(self.spec)
|
||||
except KeyError:
|
||||
pass
|
||||
spack.hooks.post_uninstall(self)
|
||||
tty.msg("Removed partial install for %s" %
|
||||
self.spec.short_spec)
|
||||
elif self.installed and layout.check_installed(self.spec):
|
||||
try:
|
||||
spack.store.db.get_record(self.spec)
|
||||
except KeyError:
|
||||
tty.msg("Repairing db for %s" % self.name)
|
||||
spack.store.db.add(self.spec)
|
||||
|
||||
if continue_with_partial and not self.installed:
|
||||
try:
|
||||
layout.check_metadata_consistency(self.spec)
|
||||
except directory_layout.DirectoryLayoutError:
|
||||
self.remove_prefix()
|
||||
|
||||
if not continue_with_partial:
|
||||
self.stage.destroy()
|
||||
self.stage.create()
|
||||
|
||||
def _do_install_pop_kwargs(self, kwargs):
|
||||
"""Pops kwargs from do_install before starting the installation
|
||||
|
||||
|
@@ -30,8 +30,6 @@
|
||||
from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite
|
||||
from spack.spec import Spec
|
||||
|
||||
import os
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def install_mockery(tmpdir, config, builtin_mock):
|
||||
@@ -79,123 +77,6 @@ def test_install_and_uninstall(mock_archive):
|
||||
raise
|
||||
|
||||
|
||||
def mock_remove_prefix(*args):
|
||||
raise MockInstallError(
|
||||
"Intentional error",
|
||||
"Mock remove_prefix method intentionally fails")
|
||||
|
||||
|
||||
@pytest.mark.usefixtures('install_mockery')
|
||||
def test_partial_install(mock_archive):
|
||||
spec = Spec('canfail')
|
||||
spec.concretize()
|
||||
pkg = spack.repo.get(spec)
|
||||
fake_fetchify(mock_archive.url, pkg)
|
||||
remove_prefix = spack.package.Package.remove_prefix
|
||||
try:
|
||||
spack.package.Package.remove_prefix = mock_remove_prefix
|
||||
try:
|
||||
pkg.do_install()
|
||||
except MockInstallError:
|
||||
pass
|
||||
spack.package.Package.remove_prefix = remove_prefix
|
||||
setattr(pkg, 'succeed', True)
|
||||
pkg.do_install()
|
||||
assert pkg.installed
|
||||
finally:
|
||||
spack.package.Package.remove_prefix = remove_prefix
|
||||
try:
|
||||
delattr(pkg, 'succeed')
|
||||
except AttributeError:
|
||||
pass
|
||||
|
||||
|
||||
@pytest.mark.usefixtures('install_mockery')
|
||||
def test_partial_install_keep_prefix(mock_archive):
|
||||
spec = Spec('canfail')
|
||||
spec.concretize()
|
||||
pkg = spack.repo.get(spec)
|
||||
fake_fetchify(mock_archive.url, pkg)
|
||||
remove_prefix = spack.package.Package.remove_prefix
|
||||
try:
|
||||
spack.package.Package.remove_prefix = mock_remove_prefix
|
||||
try:
|
||||
pkg.do_install()
|
||||
except MockInstallError:
|
||||
pass
|
||||
# Don't repair remove_prefix at this point, set keep_prefix so that
|
||||
# Spack continues with a partial install
|
||||
setattr(pkg, 'succeed', True)
|
||||
pkg.do_install(keep_prefix=True)
|
||||
assert pkg.installed
|
||||
finally:
|
||||
spack.package.Package.remove_prefix = remove_prefix
|
||||
try:
|
||||
delattr(pkg, 'succeed')
|
||||
except AttributeError:
|
||||
pass
|
||||
|
||||
|
||||
@pytest.mark.usefixtures('install_mockery')
|
||||
def test_partial_install_keep_prefix_check_metadata(mock_archive):
|
||||
spec = Spec('canfail')
|
||||
spec.concretize()
|
||||
pkg = spack.repo.get(spec)
|
||||
fake_fetchify(mock_archive.url, pkg)
|
||||
remove_prefix = spack.package.Package.remove_prefix
|
||||
try:
|
||||
spack.package.Package.remove_prefix = mock_remove_prefix
|
||||
try:
|
||||
pkg.do_install()
|
||||
except MockInstallError:
|
||||
pass
|
||||
os.remove(spack.store.layout.spec_file_path(spec))
|
||||
spack.package.Package.remove_prefix = remove_prefix
|
||||
setattr(pkg, 'succeed', True)
|
||||
pkg.do_install(keep_prefix=True)
|
||||
assert pkg.installed
|
||||
spack.store.layout.check_metadata_consistency(spec)
|
||||
finally:
|
||||
spack.package.Package.remove_prefix = remove_prefix
|
||||
try:
|
||||
delattr(pkg, 'succeed')
|
||||
except AttributeError:
|
||||
pass
|
||||
|
||||
|
||||
@pytest.mark.usefixtures('install_mockery')
|
||||
def test_install_succeeds_but_db_add_fails(mock_archive):
|
||||
"""If an installation succeeds but the database is not updated, make sure
|
||||
that the database is updated for a future install."""
|
||||
spec = Spec('cmake')
|
||||
spec.concretize()
|
||||
pkg = spack.repo.get(spec)
|
||||
fake_fetchify(mock_archive.url, pkg)
|
||||
remove_prefix = spack.package.Package.remove_prefix
|
||||
db_add = spack.store.db.add
|
||||
|
||||
def mock_db_add(*args, **kwargs):
|
||||
raise MockInstallError(
|
||||
"Intentional error", "Mock db add method intentionally fails")
|
||||
|
||||
try:
|
||||
spack.package.Package.remove_prefix = mock_remove_prefix
|
||||
spack.store.db.add = mock_db_add
|
||||
try:
|
||||
pkg.do_install()
|
||||
except MockInstallError:
|
||||
pass
|
||||
assert pkg.installed
|
||||
|
||||
spack.package.Package.remove_prefix = remove_prefix
|
||||
spack.store.db.add = db_add
|
||||
pkg.do_install()
|
||||
assert spack.store.db.get_record(spec)
|
||||
except:
|
||||
spack.package.Package.remove_prefix = remove_prefix
|
||||
raise
|
||||
|
||||
|
||||
@pytest.mark.usefixtures('install_mockery')
|
||||
def test_store(mock_archive):
|
||||
spec = Spec('cmake-client').concretized()
|
||||
@@ -221,7 +102,3 @@ def test_failing_build(mock_archive):
|
||||
pkg = spec.package
|
||||
with pytest.raises(spack.build_environment.ChildError):
|
||||
pkg.do_install()
|
||||
|
||||
|
||||
class MockInstallError(spack.error.SpackError):
|
||||
pass
|
||||
|
Reference in New Issue
Block a user