Skip to content

stage: fix resources being deleted from local cache #10152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
scheibelp merged 2 commits into from
Jan 5, 2019
+101 −7
Merged

stage: fix resources being deleted from local cache #10152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c79a16
stage: fix resources being deleted from local cache (#10152)
michaelkuhn Dec 19, 2018
08d0734
Add tests for expanding/non-expanding resources
scheibelp Jan 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions lib/spack/spack/stage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@
import sys
import errno
import hashlib
import shutil
import tempfile
import getpass
from six import string_types
from six import iteritems
from six.moves.urllib.parse import urljoin

import llnl.util.tty as tty
from llnl.util.filesystem import mkdirp, can_access
from llnl.util.filesystem import mkdirp, can_access, copy, copy_tree
from llnl.util.filesystem import remove_if_dead_link, remove_linked_tree

import spack.paths
Expand Down Expand Up @@ -406,7 +405,7 @@ def generate_fetchers():
self.fetcher = fetcher
self.fetcher.fetch()
break
except spack.fetch_strategy.NoCacheError as e:
except spack.fetch_strategy.NoCacheError:
# Don't bother reporting when something is not cached.
continue
except spack.error.SpackError as e:
Expand Down Expand Up @@ -545,7 +544,13 @@ def _add_to_root_stage(self):
'{stage}\n\tdestination : {destination}'.format(
stage=source_path, destination=destination_path
))
shutil.move(os.path.realpath(source_path), destination_path)

src = os.path.realpath(source_path)

if os.path.isdir(src):
copy_tree(src, destination_path)
Copy link
Copy Markdown
Contributor

@healther healther Dec 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity why doesn't copy handle this distinction?

Copy link
Copy Markdown
Member

@adamjstewart adamjstewart Dec 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I can't think of a reason why we couldn't redesign copy and install to support both files and directories. It's just a design choice. For comparison, Python's shutil library has separate copy and copytree methods. Python's distutils library also has separate dir_util.copy_tree and file_util.copy_file methods.

else:
copy(src, destination_path)


@pattern.composite(method_list=[
Expand Down
95 changes: 92 additions & 3 deletions lib/spack/spack/test/stage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
import spack.paths
import spack.stage
import spack.util.executable
from spack.stage import Stage

from spack.resource import Resource
from spack.stage import Stage, StageComposite, ResourceStage


def check_expand_archive(stage, stage_name, mock_archive):
Expand Down Expand Up @@ -92,7 +94,7 @@ def get_stage_path(stage, stage_name):


@pytest.fixture()
def tmpdir_for_stage(mock_archive, mutable_config):
def tmpdir_for_stage(mock_archive):
"""Uses a temporary directory for staging"""
current = spack.paths.stage_path
spack.config.set(
Expand All @@ -104,7 +106,7 @@ def tmpdir_for_stage(mock_archive, mutable_config):


@pytest.fixture()
def mock_archive(tmpdir, monkeypatch, mutable_config):
def mock_archive(tmpdir, monkeypatch):
"""Creates a mock archive with the structure expected by the tests"""
# Mock up a stage area that looks like this:
#
Expand Down Expand Up @@ -147,6 +149,52 @@ def mock_archive(tmpdir, monkeypatch, mutable_config):
)


@pytest.fixture()
def mock_noexpand_resource(tmpdir):
test_resource = tmpdir.join('resource-no-expand.sh')
test_resource.write("an example resource")
return str(test_resource)


@pytest.fixture()
def mock_expand_resource(tmpdir):
resource_dir = tmpdir.join('resource-expand')
archive_name = 'resource.tar.gz'
archive = tmpdir.join(archive_name)
archive_url = 'file://' + str(archive)
test_file = resource_dir.join('resource-file.txt')
resource_dir.ensure(dir=True)
test_file.write('test content\n')
current = tmpdir.chdir()
tar = spack.util.executable.which('tar', required=True)
tar('czf', str(archive_name), 'resource-expand')
current.chdir()

MockResource = collections.namedtuple(
'MockResource', ['url', 'files'])

return MockResource(archive_url, ['resource-file.txt'])


@pytest.fixture()
def composite_stage_with_expanding_resource(
mock_archive, mock_expand_resource):
composite_stage = StageComposite()
root_stage = Stage(mock_archive.url)
composite_stage.append(root_stage)

test_resource_fetcher = spack.fetch_strategy.from_kwargs(
url=mock_expand_resource.url)
# Specify that the resource files are to be placed in the 'resource-dir'
# directory
test_resource = Resource(
'test_resource', test_resource_fetcher, '', 'resource-dir')
resource_stage = ResourceStage(
test_resource_fetcher, root_stage, test_resource)
composite_stage.append(resource_stage)
return composite_stage, root_stage, resource_stage


@pytest.fixture()
def failing_search_fn():
"""Returns a search function that fails! Always!"""
Expand Down Expand Up @@ -202,6 +250,47 @@ def test_setup_and_destroy_no_name_with_tmp(self, mock_archive):
check_setup(stage, None, mock_archive)
check_destroy(stage, None)

@pytest.mark.disable_clean_stage_check
@pytest.mark.usefixtures('tmpdir_for_stage')
def test_composite_stage_with_noexpand_resource(
self, mock_archive, mock_noexpand_resource):
composite_stage = StageComposite()
root_stage = Stage(mock_archive.url)
composite_stage.append(root_stage)

resource_dst_name = 'resource-dst-name.sh'
test_resource_fetcher = spack.fetch_strategy.from_kwargs(
url='file://' + mock_noexpand_resource, expand=False)
test_resource = Resource(
'test_resource', test_resource_fetcher, resource_dst_name, None)
resource_stage = ResourceStage(
test_resource_fetcher, root_stage, test_resource)
composite_stage.append(resource_stage)

composite_stage.create()
composite_stage.fetch()
composite_stage.expand_archive()
assert os.path.exists(
os.path.join(composite_stage.source_path, resource_dst_name))

@pytest.mark.disable_clean_stage_check
@pytest.mark.usefixtures('tmpdir_for_stage')
def test_composite_stage_with_expand_resource(
self, mock_archive, mock_expand_resource,
composite_stage_with_expanding_resource):

composite_stage, root_stage, resource_stage = (
composite_stage_with_expanding_resource)

composite_stage.create()
composite_stage.fetch()
composite_stage.expand_archive()

for fname in mock_expand_resource.files:
file_path = os.path.join(
root_stage.source_path, 'resource-dir', fname)
assert os.path.exists(file_path)

def test_setup_and_destroy_no_name_without_tmp(self, mock_archive):
with Stage(mock_archive.url) as stage:
check_setup(stage, None, mock_archive)
Expand Down