diff options
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/docs/config_yaml.rst | 31 | ||||
-rw-r--r-- | lib/spack/llnl/util/filesystem.py | 20 | ||||
-rw-r--r-- | lib/spack/spack/cmd/location.py | 3 | ||||
-rw-r--r-- | lib/spack/spack/compilers/clang.py | 3 | ||||
-rw-r--r-- | lib/spack/spack/config.py | 1 | ||||
-rw-r--r-- | lib/spack/spack/package.py | 4 | ||||
-rw-r--r-- | lib/spack/spack/patch.py | 5 | ||||
-rw-r--r-- | lib/spack/spack/paths.py | 1 | ||||
-rw-r--r-- | lib/spack/spack/stage.py | 182 | ||||
-rw-r--r-- | lib/spack/spack/test/cmd/location.py | 3 | ||||
-rw-r--r-- | lib/spack/spack/test/conftest.py | 54 | ||||
-rw-r--r-- | lib/spack/spack/test/data/config.yaml | 2 | ||||
-rw-r--r-- | lib/spack/spack/test/llnl/util/filesystem.py | 15 | ||||
-rw-r--r-- | lib/spack/spack/test/patch.py | 18 | ||||
-rw-r--r-- | lib/spack/spack/test/stage.py | 360 | ||||
-rw-r--r-- | lib/spack/spack/test/util/spack_lock_wrapper.py | 4 |
16 files changed, 399 insertions, 307 deletions
diff --git a/lib/spack/docs/config_yaml.rst b/lib/spack/docs/config_yaml.rst index 7e2759349b..ef3ad1d03a 100644 --- a/lib/spack/docs/config_yaml.rst +++ b/lib/spack/docs/config_yaml.rst @@ -85,38 +85,39 @@ See :ref:`modules` for details. Spack is designed to run out of a user home directory, and on many systems the home directory is a (slow) network file system. On most systems, -building in a temporary file system results in faster builds than building -in the home directory. Usually, there is also more space available in -the temporary location than in the home directory. So, Spack tries to -create build stages in temporary space. +building in a temporary file system is faster. Usually, there is also more +space available in the temporary location than in the home directory. If the +username is not already in the path, Spack will also append the value of +`$user` to the path. + +.. warning:: We highly recommend appending `spack-stage` to `$tempdir` in order + to ensure `spack clean` does not delete everything in `$tempdir`. By default, Spack's ``build_stage`` is configured like this: .. code-block:: yaml build_stage: - - $tempdir - - $spack/var/spack/stage + - $tempdir/spack-stage -This is an ordered list of paths that Spack should search when trying to +This can be an ordered list of paths that Spack should search when trying to find a temporary directory for the build stage. The list is searched in order, and Spack will use the first directory to which it has write access. -See :ref:`config-file-variables` for more on ``$tempdir`` and ``$spack``. +Specifying `~/.spack/stage` first will ensure each user builds in their home +directory. The historic Spack stage path `$spack/var/spack/stage` will build +directly inside the Spack instance. See :ref:`config-file-variables` for more +on ``$tempdir`` and ``$spack``. When Spack builds a package, it creates a temporary directory within the -``build_stage``, and it creates a symbolic link to that directory in -``$spack/var/spack/stage``. This is used to track the temporary -directory. After the package is successfully installed, Spack deletes +``build_stage``. After the package is successfully installed, Spack deletes the temporary directory it used to build. Unsuccessful builds are not deleted, but you can manually purge them with :ref:`spack clean --stage <cmd-spack-clean>`. .. note:: - The last item in the list is ``$spack/var/spack/stage``. If this is the - only writable directory in the ``build_stage`` list, Spack will build - *directly* in ``$spack/var/spack/stage`` and will not link to temporary - space. + The build will fail if there is no writable directory in the ``build_stage`` + list. -------------------- ``source_cache`` diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index e58f657aea..19feb1686f 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -347,12 +347,22 @@ def copy_tree(src, dest, symlinks=True, ignore=None, _permissions=False): else: tty.debug('Copying {0} to {1}'.format(src, dest)) - mkdirp(dest) + abs_src = os.path.abspath(src) + if not abs_src.endswith(os.path.sep): + abs_src += os.path.sep + abs_dest = os.path.abspath(dest) + if not abs_dest.endswith(os.path.sep): + abs_dest += os.path.sep + + # Stop early to avoid unnecessary recursion if being asked to copy from a + # parent directory. + if abs_dest.startswith(abs_src): + raise ValueError('Cannot copy ancestor directory {0} into {1}'. + format(abs_src, abs_dest)) - src = os.path.abspath(src) - dest = os.path.abspath(dest) + mkdirp(dest) - for s, d in traverse_tree(src, dest, order='pre', + for s, d in traverse_tree(abs_src, abs_dest, order='pre', follow_symlinks=not symlinks, ignore=ignore, follow_nonexisting=True): @@ -361,7 +371,7 @@ def copy_tree(src, dest, symlinks=True, ignore=None, _permissions=False): if symlinks: target = os.readlink(s) if os.path.isabs(target): - new_target = re.sub(src, dest, target) + new_target = re.sub(abs_src, abs_dest, target) if new_target != target: tty.debug("Redirecting link {0} to {1}" .format(target, new_target)) diff --git a/lib/spack/spack/cmd/location.py b/lib/spack/spack/cmd/location.py index 38d1eab174..d6716233f9 100644 --- a/lib/spack/spack/cmd/location.py +++ b/lib/spack/spack/cmd/location.py @@ -14,6 +14,7 @@ import spack.cmd import spack.environment import spack.paths import spack.repo +import spack.stage description = "print out locations of packages and spack directories" section = "basic" @@ -76,7 +77,7 @@ def location(parser, args): print(spack.repo.path.first_repo().root) elif args.stages: - print(spack.paths.stage_path) + print(spack.stage.get_stage_root()) else: specs = spack.cmd.parse_specs(args.spec) diff --git a/lib/spack/spack/compilers/clang.py b/lib/spack/spack/compilers/clang.py index e4b109588b..e53332363b 100644 --- a/lib/spack/spack/compilers/clang.py +++ b/lib/spack/spack/compilers/clang.py @@ -12,6 +12,7 @@ import llnl.util.lang import llnl.util.tty as tty import spack.paths +import spack.stage from spack.compiler import Compiler, UnsupportedCompilerFlag from spack.util.executable import Executable from spack.version import ver @@ -279,7 +280,7 @@ class Clang(Compiler): raise OSError(msg) real_root = os.path.dirname(os.path.dirname(real_root)) - developer_root = os.path.join(spack.paths.stage_path, + developer_root = os.path.join(spack.stage.get_stage_root(), 'xcode-select', self.name, str(self.version)) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 8114ec21b4..c73b5047f5 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -101,6 +101,7 @@ config_defaults = { 'checksum': True, 'dirty': False, 'build_jobs': min(16, multiprocessing.cpu_count()), + 'build_stage': '$tempdir/spack-stage', } } diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 07d2c28679..5f3cfef6f0 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1769,9 +1769,7 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): else: partial = True - stage_is_managed_in_spack = self.stage.path.startswith( - spack.paths.stage_path) - if restage and stage_is_managed_in_spack: + if restage and self.stage.managed_by_spack: self.stage.destroy() self.stage.create() diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py index 09aa48d9da..cb012af7ff 100644 --- a/lib/spack/spack/patch.py +++ b/lib/spack/spack/patch.py @@ -192,9 +192,8 @@ class UrlPatch(Patch): fetch_digest = self.archive_sha256 fetcher = fs.URLFetchStrategy(self.url, fetch_digest) - mirror = os.path.join( - os.path.dirname(stage.mirror_path), - os.path.basename(self.url)) + mirror = os.path.join(os.path.dirname(stage.mirror_path), + os.path.basename(self.url)) self.stage = spack.stage.Stage(fetcher, mirror_path=mirror) self.stage.create() diff --git a/lib/spack/spack/paths.py b/lib/spack/spack/paths.py index 2f7ccf3b5d..d91b01d87c 100644 --- a/lib/spack/spack/paths.py +++ b/lib/spack/spack/paths.py @@ -38,7 +38,6 @@ operating_system_path = os.path.join(module_path, 'operating_systems') test_path = os.path.join(module_path, "test") hooks_path = os.path.join(module_path, "hooks") var_path = os.path.join(prefix, "var", "spack") -stage_path = os.path.join(var_path, "stage") repos_path = os.path.join(var_path, "repos") share_path = os.path.join(prefix, "share", "spack") diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index 6c52afb529..aa9920ca58 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import grp import os import stat import sys @@ -16,7 +17,7 @@ from six.moves.urllib.parse import urljoin import llnl.util.tty as tty from llnl.util.filesystem import mkdirp, can_access, install, install_tree -from llnl.util.filesystem import remove_if_dead_link, remove_linked_tree +from llnl.util.filesystem import remove_linked_tree import spack.paths import spack.caches @@ -28,45 +29,54 @@ import spack.util.pattern as pattern import spack.util.path as sup from spack.util.crypto import prefix_bits, bit_length + +# The well-known stage source subdirectory name. _source_path_subdir = 'spack-src' + +# The temporary stage name prefix. _stage_prefix = 'spack-stage-' def _first_accessible_path(paths): - """Find a tmp dir that exists that we can access.""" + """Find the first path that is accessible, creating it if necessary.""" for path in paths: try: - # try to create the path if it doesn't exist. + # Ensure the user has access, creating the directory if necessary. path = sup.canonicalize_path(path) - mkdirp(path) - - # ensure accessible - if not can_access(path): - continue - - # return it if successful. - return path + if os.path.exists(path): + if can_access(path): + return path + else: + # The path doesn't exist so create it and adjust permissions + # and group as needed (e.g., shared ``$tempdir``). + prefix = os.path.sep + parts = path.strip(os.path.sep).split(os.path.sep) + for part in parts: + prefix = os.path.join(prefix, part) + if not os.path.exists(prefix): + break + parent = os.path.dirname(prefix) + group = grp.getgrgid(os.stat(parent).st_gid)[0] + mkdirp(path, group=group, default_perms='parents') + + if can_access(path): + return path except OSError as e: - tty.debug('OSError while checking temporary path %s: %s' % ( + tty.debug('OSError while checking stage path %s: %s' % ( path, str(e))) - continue return None -# cached temporary root -_tmp_root = None -_use_tmp_stage = True +# Cached stage path root +_stage_root = None -def get_tmp_root(): - global _tmp_root, _use_tmp_stage +def get_stage_root(): + global _stage_root - if not _use_tmp_stage: - return None - - if _tmp_root is None: + if _stage_root is None: candidates = spack.config.get('config:build_stage') if isinstance(candidates, string_types): candidates = [candidates] @@ -75,23 +85,16 @@ def get_tmp_root(): if not path: raise StageError("No accessible stage paths in:", candidates) - # Return None to indicate we're using a local staging area. - if path == sup.canonicalize_path(spack.paths.stage_path): - _use_tmp_stage = False - return None - # Ensure that any temp path is unique per user, so users don't # fight over shared temporary space. user = getpass.getuser() if user not in path: - path = os.path.join(path, user, 'spack-stage') - else: - path = os.path.join(path, 'spack-stage') + path = os.path.join(path, user) + mkdirp(path) - mkdirp(path) - _tmp_root = path + _stage_root = path - return _tmp_root + return _stage_root class Stage(object): @@ -139,6 +142,9 @@ class Stage(object): """Shared dict of all stage locks.""" stage_locks = {} + """Most staging is managed by Spack. DIYStage is one exception.""" + managed_by_spack = True + def __init__( self, url_or_fetch_strategy, name=None, mirror_path=None, keep=False, path=None, lock=True, @@ -165,6 +171,17 @@ class Stage(object): is deleted on exit when no exceptions are raised. Pass True to keep the stage intact even if no exceptions are raised. + + path + If provided, the stage path to use for associated builds. + + lock + True if the stage directory file lock is to be used, False + otherwise. + + search_fn + The search function that provides the fetch strategy + instance. """ # TODO: fetch/stage coupling needs to be reworked -- the logic # TODO: here is convoluted and not modular enough. @@ -184,20 +201,19 @@ class Stage(object): self.srcdir = None - # TODO : this uses a protected member of tempfile, but seemed the only - # TODO : way to get a temporary name besides, the temporary link name - # TODO : won't be the same as the temporary stage area in tmp_root + # TODO: This uses a protected member of tempfile, but seemed the only + # TODO: way to get a temporary name. It won't be the same as the + # TODO: temporary stage area in _stage_root. self.name = name if name is None: self.name = _stage_prefix + next(tempfile._get_candidate_names()) self.mirror_path = mirror_path - # Try to construct here a temporary name for the stage directory - # If this is a named stage, then construct a named path. + # Use the provided path or construct an optionally named stage path. if path is not None: self.path = path else: - self.path = os.path.join(spack.paths.stage_path, self.name) + self.path = os.path.join(get_stage_root(), self.name) # Flag to decide whether to delete the stage folder on exit or not self.keep = keep @@ -210,7 +226,7 @@ class Stage(object): if self.name not in Stage.stage_locks: sha1 = hashlib.sha1(self.name.encode('utf-8')).digest() lock_id = prefix_bits(sha1, bit_length(sys.maxsize)) - stage_lock_path = os.path.join(spack.paths.stage_path, '.lock') + stage_lock_path = os.path.join(get_stage_root(), '.lock') Stage.stage_locks[self.name] = spack.util.lock.Lock( stage_lock_path, lock_id, 1) @@ -254,44 +270,6 @@ class Stage(object): if self._lock is not None: self._lock.release_write() - def _need_to_create_path(self): - """Makes sure nothing weird has happened since the last time we - looked at path. Returns True if path already exists and is ok. - Returns False if path needs to be created.""" - # Path doesn't exist yet. Will need to create it. - if not os.path.exists(self.path): - return True - - # Path exists but points at something else. Blow it away. - if not os.path.isdir(self.path): - os.unlink(self.path) - return True - - # Path looks ok, but need to check the target of the link. - if os.path.islink(self.path): - tmp_root = get_tmp_root() - if tmp_root is not None: - real_path = os.path.realpath(self.path) - real_tmp = os.path.realpath(tmp_root) - - # If we're using a tmp dir, it's a link, and it points at the - # right spot, then keep it. - if (real_path.startswith(real_tmp) and - os.path.exists(real_path)): - return False - else: - # otherwise, just unlink it and start over. - os.unlink(self.path) - return True - - else: - # If we're not tmp mode, then it's a link and we want a - # directory. - os.unlink(self.path) - return True - - return False - @property def expected_archive_files(self): """Possible archive file paths.""" @@ -455,30 +433,18 @@ class Stage(object): self.fetcher.reset() def create(self): - """Creates the stage directory. - - If get_tmp_root() is None, the stage directory is created - directly under spack.paths.stage_path, otherwise this will attempt to - create a stage in a temporary directory and link it into - spack.paths.stage_path. - """ - # Create the top-level stage directory - mkdirp(spack.paths.stage_path) - remove_if_dead_link(self.path) - - # If a tmp_root exists then create a directory there and then link it - # in the stage area, otherwise create the stage directory in self.path - if self._need_to_create_path(): - tmp_root = get_tmp_root() - if tmp_root is not None: - # tempfile.mkdtemp already sets mode 0700 - tmp_dir = tempfile.mkdtemp('', _stage_prefix, tmp_root) - tty.debug('link %s -> %s' % (self.path, tmp_dir)) - os.symlink(tmp_dir, self.path) - else: - # emulate file permissions for tempfile.mkdtemp - mkdirp(self.path, mode=stat.S_IRWXU) + Ensures the top-level (config:build_stage) directory exists. + """ + # Emulate file permissions for tempfile.mkdtemp. + if not os.path.exists(self.path): + print("TLD: %s does not exist, creating it" % self.path) + mkdirp(self.path, mode=stat.S_IRWXU) + elif not os.path.isdir(self.path): + print("TLD: %s is not a directory, replacing it" % self.path) + os.remove(self.path) + mkdirp(self.path, mode=stat.S_IRWXU) + # Make sure we can actually do something with the stage we made. ensure_access(self.path) self.created = True @@ -562,7 +528,7 @@ class ResourceStage(Stage): @pattern.composite(method_list=[ 'fetch', 'create', 'created', 'check', 'expand_archive', 'restage', - 'destroy', 'cache_local']) + 'destroy', 'cache_local', 'managed_by_spack']) class StageComposite: """Composite for Stage type objects. The first item in this composite is considered to be the root package, and operations that return a value are @@ -612,6 +578,9 @@ class DIYStage(object): directory naming convention. """ + """DIY staging is, by definition, not managed by Spack.""" + managed_by_spack = False + def __init__(self, path): if path is None: raise ValueError("Cannot construct DIYStage without a path.") @@ -659,7 +628,7 @@ class DIYStage(object): tty.msg("Sources for DIY stages are not cached") -def ensure_access(file=spack.paths.stage_path): +def ensure_access(file): """Ensure we can access a directory and die with an error if we can't.""" if not can_access(file): tty.die("Insufficient permissions for %s" % file) @@ -667,9 +636,10 @@ def ensure_access(file=spack.paths.stage_path): def purge(): """Remove all build directories in the top-level stage path.""" - if os.path.isdir(spack.paths.stage_path): - for stage_dir in os.listdir(spack.paths.stage_path): - stage_path = os.path.join(spack.paths.stage_path, stage_dir) + root = get_stage_root() + if os.path.isdir(root): + for stage_dir in os.listdir(root): + stage_path = os.path.join(root, stage_dir) remove_linked_tree(stage_path) diff --git a/lib/spack/spack/test/cmd/location.py b/lib/spack/spack/test/cmd/location.py index 6dbc4d94d4..f48cdb75e7 100644 --- a/lib/spack/spack/test/cmd/location.py +++ b/lib/spack/spack/test/cmd/location.py @@ -11,6 +11,7 @@ from llnl.util.filesystem import mkdirp import spack.environment as ev from spack.main import SpackCommand, SpackCommandError import spack.paths +import spack.stage # Everything here uses (or can use) the mock config and database. @@ -128,4 +129,4 @@ def test_location_stage_dir(mock_spec): @pytest.mark.db def test_location_stages(mock_spec): """Tests spack location --stages.""" - assert location('--stages').strip() == spack.paths.stage_path + assert location('--stages').strip() == spack.stage.get_stage_root() diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 53493eda91..f8c8e2a8bc 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -121,14 +121,40 @@ def reset_compiler_cache(): spack.compilers._compiler_cache = {} -@pytest.fixture(scope='session', autouse=True) -def mock_stage(tmpdir_factory): - """Mocks up a fake stage directory for use by tests.""" - stage_path = spack.paths.stage_path - new_stage = str(tmpdir_factory.mktemp('mock_stage')) - spack.paths.stage_path = new_stage - yield new_stage - spack.paths.stage_path = stage_path +@pytest.fixture +def clear_stage_root(monkeypatch): + """Ensure spack.stage._stage_root is not set at test start.""" + monkeypatch.setattr(spack.stage, '_stage_root', None) + yield + + +@pytest.fixture(scope='function', autouse=True) +def mock_stage(clear_stage_root, tmpdir_factory, request): + """Establish the temporary build_stage for the mock archive.""" + # Workaround to skip mock_stage for 'nomockstage' test cases + if 'nomockstage' not in request.keywords: + new_stage = tmpdir_factory.mktemp('mock-stage') + new_stage_path = str(new_stage) + + # Set test_stage_path as the default directory to use for test stages. + current = spack.config.get('config:build_stage') + spack.config.set('config', + {'build_stage': new_stage_path}, scope='user') + + # Ensure the source directory exists + source_path = new_stage.join(spack.stage._source_path_subdir) + source_path.ensure(dir=True) + + yield new_stage_path + + spack.config.set('config', {'build_stage': current}, scope='user') + + # Clean up the test stage directory + if os.path.isdir(new_stage_path): + shutil.rmtree(new_stage_path) + else: + # Must yield a path to avoid a TypeError on test teardown + yield str(tmpdir_factory) @pytest.fixture(scope='session') @@ -138,7 +164,7 @@ def ignore_stage_files(): Used to track which leftover files in the stage have been seen. """ # to start with, ignore the .lock file at the stage root. - return set(['.lock']) + return set(['.lock', spack.stage._source_path_subdir]) def remove_whatever_it_is(path): @@ -173,17 +199,19 @@ def check_for_leftover_stage_files(request, mock_stage, ignore_stage_files): to tests that need it. """ + stage_path = mock_stage + yield files_in_stage = set() - if os.path.exists(spack.paths.stage_path): - files_in_stage = set( - os.listdir(spack.paths.stage_path)) - ignore_stage_files + if os.path.exists(stage_path): + stage_files = os.listdir(stage_path) + files_in_stage = set(stage_files) - ignore_stage_files if 'disable_clean_stage_check' in request.keywords: # clean up after tests that are expected to be dirty for f in files_in_stage: - path = os.path.join(spack.paths.stage_path, f) + path = os.path.join(stage_path, f) remove_whatever_it_is(path) else: ignore_stage_files |= files_in_stage diff --git a/lib/spack/spack/test/data/config.yaml b/lib/spack/spack/test/data/config.yaml index 2fcd10ad97..501001ff00 100644 --- a/lib/spack/spack/test/data/config.yaml +++ b/lib/spack/spack/test/data/config.yaml @@ -5,7 +5,7 @@ config: - $spack/lib/spack/spack/test/data/templates - $spack/lib/spack/spack/test/data/templates_again build_stage: - - $tempdir + - $tempdir/spack-stage-test - /nfs/tmp2/$user - $spack/var/spack/stage source_cache: $spack/var/spack/cache diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index 4fc360ec4e..f70844978e 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -107,6 +107,21 @@ class TestCopyTree: assert os.path.exists('dest/sub/directory/a/b/2') + def test_parent_dir(self, stage): + """Test copying to from a parent directory.""" + + # Make sure we get the right error if we try to copy a parent into + # a descendent directory. + with pytest.raises(ValueError, matches="Cannot copy"): + with fs.working_dir(str(stage)): + fs.copy_tree('source', 'source/sub/directory') + + # Only point with this check is to make sure we don't try to perform + # the copy. + with pytest.raises(IOError, matches="No such file or directory"): + with fs.working_dir(str(stage)): + fs.copy_tree('foo/ba', 'foo/bar') + def test_symlinks_true(self, stage): """Test copying with symlink preservation.""" diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py index 5ea3ba65bc..c3df33dc8b 100644 --- a/lib/spack/spack/test/patch.py +++ b/lib/spack/spack/test/patch.py @@ -32,10 +32,10 @@ url2_archive_sha256 = 'abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcda @pytest.fixture() -def mock_stage(tmpdir, monkeypatch): - # don't disrupt the spack install directory with tests. - mock_path = str(tmpdir) - monkeypatch.setattr(spack.paths, 'stage_path', mock_path) +def mock_patch_stage(tmpdir_factory, monkeypatch): + # Don't disrupt the spack install directory with tests. + mock_path = str(tmpdir_factory.mktemp('mock-patch-stage')) + monkeypatch.setattr(spack.stage, '_stage_root', mock_path) return mock_path @@ -52,7 +52,7 @@ data_path = os.path.join(spack.paths.test_path, 'data', 'patch') '252c0af58be3d90e5dc5e0d16658434c9efa5d20a5df6c10bf72c2d77f780866', None) ]) -def test_url_patch(mock_stage, filename, sha256, archive_sha256): +def test_url_patch(mock_patch_stage, filename, sha256, archive_sha256): # Make a patch object url = 'file://' + filename pkg = spack.repo.get('patch') @@ -61,13 +61,9 @@ def test_url_patch(mock_stage, filename, sha256, archive_sha256): # make a stage with Stage(url) as stage: # TODO: url isn't used; maybe refactor Stage - # TODO: there is probably a better way to mock this. - stage.mirror_path = mock_stage # don't disrupt the spack install - - # Fake a source path and ensure the directory exists - with working_dir(stage.path): - mkdirp(spack.stage._source_path_subdir) + stage.mirror_path = mock_patch_stage + mkdirp(stage.source_path) with working_dir(stage.source_path): # write a file to be patched with open('foo.txt', 'w') as f: diff --git a/lib/spack/spack/test/stage.py b/lib/spack/spack/test/stage.py index 9f5e7363f5..b8b217032a 100644 --- a/lib/spack/spack/test/stage.py +++ b/lib/spack/spack/test/stage.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) """Test that the Stage class works correctly.""" +import grp import os import collections import shutil @@ -12,7 +13,7 @@ import getpass import pytest -from llnl.util.filesystem import working_dir +from llnl.util.filesystem import mkdirp, working_dir import spack.paths import spack.stage @@ -20,6 +21,7 @@ import spack.util.executable from spack.resource import Resource from spack.stage import Stage, StageComposite, ResourceStage, DIYStage +from spack.util.path import canonicalize_path # The following values are used for common fetch and stage mocking fixtures: _archive_base = 'test-files' @@ -38,6 +40,10 @@ _include_readme = 1 _include_hidden = 2 _include_extra = 3 +# Some standard unix directory that does NOT include the username +_non_user_root = os.path.join(os.path.sep, 'opt') + + # Mock fetch directories are expected to appear as follows: # # TMPDIR/ @@ -133,7 +139,7 @@ def check_destroy(stage, stage_name): assert not os.path.exists(stage_path) # tmp stage needs to remove tmp dir too. - if spack.stage._use_tmp_stage: + if not stage.managed_by_spack: target = os.path.realpath(stage_path) assert not os.path.exists(target) @@ -145,153 +151,102 @@ def check_setup(stage, stage_name, archive): # Ensure stage was created in the spack stage directory assert os.path.isdir(stage_path) - if spack.stage.get_tmp_root(): - # Check that the stage dir is really a symlink. - assert os.path.islink(stage_path) - - # Make sure it points to a valid directory - target = os.path.realpath(stage_path) - assert os.path.isdir(target) - assert not os.path.islink(target) - - # Make sure the directory is in the place we asked it to - # be (see setUp, tearDown, and use_tmp) - assert target.startswith(str(archive.test_tmp_dir)) + # Make sure it points to a valid directory + target = os.path.realpath(stage_path) + assert os.path.isdir(target) + assert not os.path.islink(target) - else: - # Make sure the stage path is NOT a link for a non-tmp stage - assert not os.path.islink(stage_path) + # Make sure the directory is in the place we asked it to + # be (see setUp, tearDown, and use_tmp) + assert target.startswith(str(archive.stage_path)) def get_stage_path(stage, stage_name): """Figure out where a stage should be living. This depends on whether it's named. """ + stage_path = spack.stage.get_stage_root() if stage_name is not None: # If it is a named stage, we know where the stage should be - return os.path.join(spack.paths.stage_path, stage_name) + return os.path.join(stage_path, stage_name) else: # If it's unnamed, ensure that we ran mkdtemp in the right spot. assert stage.path is not None - assert stage.path.startswith(spack.paths.stage_path) + assert stage.path.startswith(stage_path) return stage.path @pytest.fixture -def no_path_for_stage(monkeypatch): - """Ensure there is no accessible path for staging.""" - def _no_stage_path(paths): - return None - - monkeypatch.setattr(spack.stage, '_first_accessible_path', _no_stage_path) - yield - - -@pytest.fixture -def no_tmp_root_stage(monkeypatch): - """ - Disable use of a temporary root for staging. - - Note that it can be important for other tests that the previous settings be - restored when the test case is over. - """ - monkeypatch.setattr(spack.stage, '_tmp_root', None) - monkeypatch.setattr(spack.stage, '_use_tmp_stage', False) - yield - - -@pytest.fixture -def non_user_path_for_stage(config): - """ - Use a non-user path for staging. - - Note that it can be important for other tests that the previous settings be - restored when the test case is over. - """ +def bad_stage_path(): + """Temporarily ensure there is no accessible path for staging.""" current = spack.config.get('config:build_stage') - spack.config.set('config', {'build_stage': ['/var/spack/non-path']}, - scope='user') + spack.config.set('config', {'build_stage': '/no/such/path'}, scope='user') yield spack.config.set('config', {'build_stage': current}, scope='user') @pytest.fixture -def stage_path_for_stage(config): - """ - Use the basic stage_path for staging. +def non_user_path_for_stage(monkeypatch): + """Temporarily use a Linux-standard non-user path for staging. """ + def _can_access(path, perms): + return True - Note that it can be important for other tests that the previous settings be - restored when the test case is over. - """ current = spack.config.get('config:build_stage') - spack.config.set('config', - {'build_stage': spack.paths.stage_path}, scope='user') + spack.config.set('config', {'build_stage': [_non_user_root]}, scope='user') + monkeypatch.setattr(os, 'access', _can_access) yield spack.config.set('config', {'build_stage': current}, scope='user') @pytest.fixture -def tmp_path_for_stage(tmpdir, config): +def instance_path_for_stage(): """ - Use a built-in, temporary, test directory for staging. + Temporarily use the "traditional" spack instance stage path for staging. Note that it can be important for other tests that the previous settings be restored when the test case is over. """ current = spack.config.get('config:build_stage') - spack.config.set('config', {'build_stage': [str(tmpdir)]}, scope='user') + base = canonicalize_path(os.path.join('$spack', 'test-stage')) + mkdirp(base) + path = tempfile.mkdtemp(dir=base) + spack.config.set('config', {'build_stage': path}, scope='user') yield spack.config.set('config', {'build_stage': current}, scope='user') + shutil.rmtree(base) @pytest.fixture -def tmp_root_stage(monkeypatch): - """ - Enable use of a temporary root for staging. - - Note that it can be important for other tests that the previous settings be - restored when the test case is over. - """ - monkeypatch.setattr(spack.stage, '_tmp_root', None) - monkeypatch.setattr(spack.stage, '_use_tmp_stage', True) - yield - - -@pytest.fixture -def tmpdir_for_stage(config, mock_stage_archive): +def tmp_path_for_stage(tmpdir): """ - Use the mock_stage_archive's temporary directory for staging. + Use a temporary test directory for staging. Note that it can be important for other tests that the previous settings be restored when the test case is over. """ - archive = mock_stage_archive() current = spack.config.get('config:build_stage') - spack.config.set( - 'config', - {'build_stage': [str(archive.test_tmp_dir)]}, - scope='user') - yield + spack.config.set('config', {'build_stage': [str(tmpdir)]}, scope='user') + yield tmpdir spack.config.set('config', {'build_stage': current}, scope='user') @pytest.fixture -def tmp_build_stage_dir(tmpdir, config): +def tmp_build_stage_dir(tmpdir): """Establish the temporary build_stage for the mock archive.""" - test_tmp_path = tmpdir.join('tmp') + test_stage_path = tmpdir.join('stage') - # Set test_tmp_path as the default test directory to use for stages. + # Set test_stage_path as the default directory to use for test stages. current = spack.config.get('config:build_stage') spack.config.set('config', - {'build_stage': [str(test_tmp_path)]}, scope='user') + {'build_stage': [str(test_stage_path)]}, scope='user') - yield (tmpdir, test_tmp_path) + yield (tmpdir, test_stage_path) spack.config.set('config', {'build_stage': current}, scope='user') @pytest.fixture -def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request): +def mock_stage_archive(clear_stage_root, tmp_build_stage_dir, request): """ Create the directories and files for the staged mock archive. @@ -301,7 +256,7 @@ def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request): # Mock up a stage area that looks like this: # # tmpdir/ test_files_dir - # tmp/ test_tmp_path (where stage should be) + # stage/ test_stage_path (where stage should be) # <_archive_base>/ archive_dir_path # <_readme_fn> Optional test_readme (contains _readme_contents) # <_extra_fn> Optional extra file (contains _extra_contents) @@ -309,8 +264,8 @@ def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request): # <_archive_fn> archive_url = file:///path/to/<_archive_fn> # def create_stage_archive(expected_file_list=[_include_readme]): - tmpdir, test_tmp_path = tmp_build_stage_dir - test_tmp_path.ensure(dir=True) + tmpdir, test_stage_path = tmp_build_stage_dir + test_stage_path.ensure(dir=True) # Create the archive directory and associated file archive_dir = tmpdir.join(_archive_base) @@ -349,10 +304,10 @@ def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request): tar(*tar_args) Archive = collections.namedtuple( - 'Archive', ['url', 'tmpdir', 'test_tmp_dir', 'archive_dir'] + 'Archive', ['url', 'tmpdir', 'stage_path', 'archive_dir'] ) return Archive(url=archive_url, tmpdir=tmpdir, - test_tmp_dir=test_tmp_path, archive_dir=archive_dir) + stage_path=test_stage_path, archive_dir=archive_dir) return create_stage_archive @@ -368,22 +323,33 @@ def mock_noexpand_resource(tmpdir): @pytest.fixture def mock_expand_resource(tmpdir): """Sets up an expandable resource in tmpdir prior to staging.""" - resource_dir = tmpdir.join('resource-expand') + # Mock up an expandable resource: + # + # tmpdir/ test_files_dir + # resource-expand/ resource source dir + # resource-file.txt resource contents (contains 'test content') + # resource.tar.gz archive of resource content + # + subdir = 'resource-expand' + resource_dir = tmpdir.join(subdir) + resource_dir.ensure(dir=True) + 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) + + filename = 'resource-file.txt' + test_file = resource_dir.join(filename) test_file.write('test content\n') with tmpdir.as_cwd(): tar = spack.util.executable.which('tar', required=True) - tar('czf', str(archive_name), 'resource-expand') + tar('czf', str(archive_name), subdir) MockResource = collections.namedtuple( 'MockResource', ['url', 'files']) - return MockResource(archive_url, ['resource-file.txt']) + return MockResource(archive_url, [filename]) @pytest.fixture @@ -404,7 +370,7 @@ def composite_stage_with_expanding_resource( resource_stage = ResourceStage( test_resource_fetcher, root_stage, test_resource) composite_stage.append(resource_stage) - return composite_stage, root_stage, resource_stage + return composite_stage, root_stage, resource_stage, mock_expand_resource @pytest.fixture @@ -445,7 +411,6 @@ class TestStage(object): stage_name = 'spack-test-stage' - @pytest.mark.usefixtures('tmpdir_for_stage') def test_setup_and_destroy_name_with_tmp(self, mock_stage_archive): archive = mock_stage_archive() with Stage(archive.url, name=self.stage_name) as stage: @@ -458,14 +423,12 @@ class TestStage(object): check_setup(stage, self.stage_name, archive) check_destroy(stage, self.stage_name) - @pytest.mark.usefixtures('tmpdir_for_stage') def test_setup_and_destroy_no_name_with_tmp(self, mock_stage_archive): archive = mock_stage_archive() with Stage(archive.url) as stage: check_setup(stage, None, archive) check_destroy(stage, None) - @pytest.mark.usefixtures('tmpdir_for_stage') def test_noexpand_stage_file( self, mock_stage_archive, mock_noexpand_resource): """When creating a stage with a nonexpanding URL, the 'archive_file' @@ -479,7 +442,6 @@ class TestStage(object): assert os.path.exists(stage.archive_file) @pytest.mark.disable_clean_stage_check - @pytest.mark.usefixtures('tmpdir_for_stage') def test_composite_stage_with_noexpand_resource( self, mock_stage_archive, mock_noexpand_resource): archive = mock_stage_archive() @@ -505,12 +467,10 @@ class TestStage(object): 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_stage_archive, mock_expand_resource, - composite_stage_with_expanding_resource): + self, composite_stage_with_expanding_resource): - composite_stage, root_stage, resource_stage = ( + composite_stage, root_stage, resource_stage, mock_resource = ( composite_stage_with_expanding_resource) composite_stage.create() @@ -519,23 +479,24 @@ class TestStage(object): assert composite_stage.expanded # Archive is expanded - for fname in mock_expand_resource.files: + for fname in mock_resource.files: file_path = os.path.join( root_stage.source_path, 'resource-dir', fname) assert os.path.exists(file_path) + # Perform a little cleanup + shutil.rmtree(root_stage.path) + @pytest.mark.disable_clean_stage_check - @pytest.mark.usefixtures('tmpdir_for_stage') def test_composite_stage_with_expand_resource_default_placement( - self, mock_stage_archive, mock_expand_resource, - composite_stage_with_expanding_resource): + self, composite_stage_with_expanding_resource): """For a resource which refers to a compressed archive which expands to a directory, check that by default the resource is placed in the source_path of the root stage with the name of the decompressed directory. """ - composite_stage, root_stage, resource_stage = ( + composite_stage, root_stage, resource_stage, mock_resource = ( composite_stage_with_expanding_resource) resource_stage.resource.placement = None @@ -544,11 +505,14 @@ class TestStage(object): composite_stage.fetch() composite_stage.expand_archive() - for fname in mock_expand_resource.files: + for fname in mock_resource.files: file_path = os.path.join( root_stage.source_path, 'resource-expand', fname) assert os.path.exists(file_path) + # Perform a little cleanup + shutil.rmtree(root_stage.path) + def test_setup_and_destroy_no_name_without_tmp(self, mock_stage_archive): archive = mock_stage_archive() with Stage(archive.url) as stage: @@ -710,42 +674,108 @@ class TestStage(object): assert source_path.endswith(spack.stage._source_path_subdir) assert not os.path.exists(source_path) - def test_first_accessible_path_error(self): - """Test _first_accessible_path handling of an OSError.""" - with tempfile.NamedTemporaryFile() as _file: - assert spack.stage._first_accessible_path([_file.name]) is None + def test_first_accessible_path(self, tmpdir): + """Test _first_accessible_path names.""" + spack_dir = tmpdir.join('spack-test-fap') + name = str(spack_dir) + files = [os.path.join(os.path.sep, 'no', 'such', 'path'), name] + + # Ensure the tmpdir path is returned since the user should have access + path = spack.stage._first_accessible_path(files) + assert path == name + assert os.path.isdir(path) + + # Ensure an existing path is returned + spack_subdir = spack_dir.join('existing').ensure(dir=True) + subdir = str(spack_subdir) + path = spack.stage._first_accessible_path([subdir]) + assert path == subdir + + # Cleanup + shutil.rmtree(str(name)) - def test_get_tmp_root_no_use(self, no_tmp_root_stage): - """Ensure not using tmp root results in no path.""" - assert spack.stage.get_tmp_root() is None + def test_first_accessible_perms(self, tmpdir): + """Test _first_accessible_path permissions.""" + name = str(tmpdir.join('first', 'path')) + path = spack.stage._first_accessible_path([name]) - def test_get_tmp_root_no_stage_path(self, tmp_root_stage, - no_path_for_stage): - """Ensure using tmp root with no stage path raises StageError.""" + # Ensure the non-existent path was created + assert path == name + assert os.path.isdir(path) + + # Ensure the non-existent subdirectories have their parent's perms + prefix = str(tmpdir) + status = os.stat(prefix) + group = grp.getgrgid(status.st_gid)[0] + parts = path[len(prefix):].split(os.path.sep) + for part in parts: + prefix = os.path.join(prefix, part) + prefix_status = os.stat(prefix) + assert group == grp.getgrgid(os.stat(prefix).st_gid)[0] + assert status.st_mode == prefix_status.st_mode + + # Cleanup + shutil.rmtree(os.path.dirname(name)) + + def test_get_stage_root_bad_path(self, clear_stage_root, bad_stage_path): + """Ensure an invalid stage path root raises a StageError.""" with pytest.raises(spack.stage.StageError, match="No accessible stage paths in"): - spack.stage.get_tmp_root() - - def test_get_tmp_root_non_user_path(self, tmp_root_stage, - non_user_path_for_stage): - """Ensure build_stage of tmp root with non-user path includes user.""" - path = spack.stage.get_tmp_root() - assert path.endswith(os.path.join(getpass.getuser(), 'spack-stage')) - - def test_get_tmp_root_use(self, tmp_root_stage, tmp_path_for_stage): - """Ensure build_stage of tmp root provides has right ancestors.""" - path = spack.stage.get_tmp_root() - shutil.rmtree(path) - assert path.rfind('test_get_tmp_root_use') > 0 - assert path.endswith('spack-stage') - - def test_get_tmp_root_stage_path(self, tmp_root_stage, - stage_path_for_stage): - """ - Ensure build_stage of tmp root with stage_path means use local path. - """ - assert spack.stage.get_tmp_root() is None - assert not spack.stage._use_tmp_stage + assert spack.stage.get_stage_root() is None + + # Make sure the cached stage path values are unchanged. + assert spack.stage._stage_root is None + + def test_get_stage_root_non_user_path(self, clear_stage_root, + non_user_path_for_stage): + """Ensure a non-user stage root includes the username.""" + # The challenge here is whether the user has access to the standard + # non-user path. If not, the path should still appear in the error. + try: + path = spack.stage.get_stage_root() + assert getpass.getuser() in path.split(os.path.sep) + + # Make sure the cached stage path values are changed appropriately. + assert spack.stage._stage_root == path + except OSError as e: + expected = os.path.join(_non_user_root, getpass.getuser()) + assert expected in str(e) + + # Make sure the cached stage path values are unchanged. + assert spack.stage._stage_root is None + + def test_get_stage_root_tmp(self, clear_stage_root, tmp_path_for_stage): + """Ensure a temp path stage root is a suitable temp path.""" + assert spack.stage._stage_root is None + + tmpdir = tmp_path_for_stage + path = spack.stage.get_stage_root() + assert path == str(tmpdir) + assert 'test_get_stage_root_tmp' in path + + # Make sure the cached stage path values are changed appropriately. + assert spack.stage._stage_root == path + + # Add then purge a couple of directories + dir1 = tmpdir.join('dir1') + dir1.ensure(dir=True) + dir2 = tmpdir.join('dir2') + dir2.ensure(dir=True) + + spack.stage.purge() + assert not os.path.exists(str(dir1)) + assert not os.path.exists(str(dir2)) + + def test_get_stage_root_in_spack(self, clear_stage_root, + instance_path_for_stage): + """Ensure an instance path stage root is a suitable path.""" + assert spack.stage._stage_root is None + + path = spack.stage.get_stage_root() + assert 'spack' in path.split(os.path.sep) + + # Make sure the cached stage path values are changed appropriately. + assert spack.stage._stage_root == path def test_stage_constructor_no_fetcher(self): """Ensure Stage constructor with no URL or fetch strategy fails.""" @@ -814,3 +844,41 @@ class TestStage(object): assert os.path.isfile(readmefn) with open(readmefn) as _file: _file.read() == _readme_contents + + +@pytest.fixture +def tmp_build_stage_nondir(tmpdir): + """Establish the temporary build_stage pointing to non-directory.""" + test_stage_path = tmpdir.join('stage', 'afile') + test_stage_path.ensure(dir=False) + + # Set test_stage_path as the default directory to use for test stages. + current = spack.config.get('config:build_stage') + stage_dir = os.path.dirname(str(test_stage_path)) + spack.config.set('config', {'build_stage': [stage_dir]}, scope='user') + + yield test_stage_path + + spack.config.set('config', {'build_stage': current}, scope='user') + + +def test_stage_create_replace_path(tmp_build_stage_dir): + """Ensure stage creation replaces a non-directory path.""" + _, test_stage_path = tmp_build_stage_dir + test_stage_path.ensure(dir=True) + + nondir = test_stage_path.join('afile') + nondir.ensure(dir=False) + path = str(nondir) + + stage = Stage(path, name='') + stage.create() # Should ensure the path is converted to a dir + + assert os.path.isdir(stage.path) + + +def test_cannot_access(): + """Ensure can_access dies with the expected error.""" + with pytest.raises(SystemExit, matches='Insufficient permissions'): + # It's far more portable to use a non-existent filename. + spack.stage.ensure_access('/no/such/file') diff --git a/lib/spack/spack/test/util/spack_lock_wrapper.py b/lib/spack/spack/test/util/spack_lock_wrapper.py index 1a0e41c7f6..c64af82e37 100644 --- a/lib/spack/spack/test/util/spack_lock_wrapper.py +++ b/lib/spack/spack/test/util/spack_lock_wrapper.py @@ -38,6 +38,8 @@ def test_disable_locking(tmpdir): assert old_value == spack.config.get('config:locks') +# "Disable" mock_stage fixture to avoid subdir permissions issues on cleanup. +@pytest.mark.nomockstage def test_lock_checks_user(tmpdir): """Ensure lock checks work with a self-owned, self-group repo.""" uid = os.getuid() @@ -70,6 +72,8 @@ def test_lock_checks_user(tmpdir): lk.check_lock_safety(path) +# "Disable" mock_stage fixture to avoid subdir permissions issues on cleanup. +@pytest.mark.nomockstage def test_lock_checks_group(tmpdir): """Ensure lock checks work with a self-owned, non-self-group repo.""" uid = os.getuid() |