diff options
| -rw-r--r-- | lib/spack/spack/binary_distribution.py | 31 | ||||
| -rw-r--r-- | lib/spack/spack/test/bindist.py | 40 |
2 files changed, 63 insertions, 8 deletions
diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 7e156ba998..499236ff20 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -1407,23 +1407,38 @@ def needs_rebuild(spec, mirror_url, rebuild_on_errors=False): spec_yaml = syaml.load(yaml_contents) + yaml_spec = spec_yaml['spec'] + name = spec.name + + # The "spec" key in the yaml is a list of objects, each with a single + # key that is the package name. While the list usually just contains + # a single object, we iterate over the list looking for the object + # with the name of this concrete spec as a key, out of an abundance + # of caution. + cached_pkg_specs = [item[name] for item in yaml_spec if name in item] + cached_target = cached_pkg_specs[0] if cached_pkg_specs else None + # If either the full_hash didn't exist in the .spec.yaml file, or it # did, but didn't match the one we computed locally, then we should # just rebuild. This can be simplified once the dag_hash and the # full_hash become the same thing. - if ('full_hash' not in spec_yaml or - spec_yaml['full_hash'] != pkg_full_hash): - if 'full_hash' in spec_yaml: + rebuild = False + if not cached_target or 'full_hash' not in cached_target: + reason = 'full_hash was missing from remote spec.yaml' + rebuild = True + else: + full_hash = cached_target['full_hash'] + if full_hash != pkg_full_hash: reason = 'hash mismatch, remote = {0}, local = {1}'.format( - spec_yaml['full_hash'], pkg_full_hash) - else: - reason = 'full_hash was missing from remote spec.yaml' + full_hash, pkg_full_hash) + rebuild = True + + if rebuild: tty.msg('Rebuilding {0}, reason: {1}'.format( spec.short_spec, reason)) tty.msg(spec.tree()) - return True - return False + return rebuild def check_specs_against_mirrors(mirrors, specs, output_file=None, diff --git a/lib/spack/spack/test/bindist.py b/lib/spack/spack/test/bindist.py index 142b4a72a5..69c95ff97f 100644 --- a/lib/spack/spack/test/bindist.py +++ b/lib/spack/spack/test/bindist.py @@ -19,6 +19,7 @@ import spack.cmd.buildcache as buildcache import spack.cmd.install as install import spack.cmd.uninstall as uninstall import spack.cmd.mirror as mirror +from spack.main import SpackCommand import spack.mirror import spack.util.gpg from spack.directory_layout import YamlDirectoryLayout @@ -31,6 +32,11 @@ ndef_install_path_scheme = '${PACKAGE}/${VERSION}/${ARCHITECTURE}-${COMPILERNAME mirror_path_def = None mirror_path_rel = None +mirror_cmd = SpackCommand('mirror') +install_cmd = SpackCommand('install') +uninstall_cmd = SpackCommand('uninstall') +buildcache_cmd = SpackCommand('buildcache') + @pytest.fixture(scope='function') def cache_directory(tmpdir): @@ -569,3 +575,37 @@ def test_built_spec_cache(tmpdir, margs = mparser.parse_args( ['rm', '--scope', 'site', 'test-mirror-rel']) mirror.mirror(mparser, margs) + + +def test_spec_needs_rebuild(install_mockery_mutable_config, mock_packages, + mock_fetch, monkeypatch, tmpdir): + """Make sure needs_rebuild properly comares remote full_hash + against locally computed one, avoiding unnecessary rebuilds""" + + # Create a temp mirror directory for buildcache usage + mirror_dir = tmpdir.join('mirror_dir') + mirror_url = 'file://{0}'.format(mirror_dir.strpath) + + mirror_cmd('add', 'test-mirror', mirror_url) + + s = Spec('libdwarf').concretized() + + # Install a package + install_cmd(s.name) + + # Put installed package in the buildcache + buildcache_cmd('create', '-u', '-a', '-d', mirror_dir.strpath, s.name) + + rebuild = bindist.needs_rebuild(s, mirror_url, rebuild_on_errors=True) + + assert not rebuild + + # Now monkey patch Spec to change the full hash on the package + def fake_full_hash(spec): + print('fake_full_hash') + return 'tal4c7h4z0gqmixb1eqa92mjoybxn5l6' + monkeypatch.setattr(spack.spec.Spec, 'full_hash', fake_full_hash) + + rebuild = bindist.needs_rebuild(s, mirror_url, rebuild_on_errors=True) + + assert rebuild |