diff options
| -rw-r--r-- | etc/spack/defaults/config.yaml | 12 | ||||
| -rw-r--r-- | lib/spack/docs/basic_usage.rst | 41 | ||||
| -rw-r--r-- | lib/spack/docs/config_yaml.rst | 11 | ||||
| -rw-r--r-- | lib/spack/llnl/util/lock.py | 30 | ||||
| -rw-r--r-- | lib/spack/spack/database.py | 2 | ||||
| -rw-r--r-- | lib/spack/spack/main.py | 78 | ||||
| -rw-r--r-- | lib/spack/spack/schema/config.py | 1 | ||||
| -rw-r--r-- | lib/spack/spack/stage.py | 4 | ||||
| -rw-r--r-- | lib/spack/spack/test/llnl/util/lock.py | 105 | ||||
| -rw-r--r-- | lib/spack/spack/util/file_cache.py | 2 | ||||
| -rw-r--r-- | lib/spack/spack/util/lock.py | 91 |
11 files changed, 323 insertions, 54 deletions
diff --git a/etc/spack/defaults/config.yaml b/etc/spack/defaults/config.yaml index b5e517c98d..ef9d288173 100644 --- a/etc/spack/defaults/config.yaml +++ b/etc/spack/defaults/config.yaml @@ -18,13 +18,16 @@ config: # You can use $spack here to refer to the root of the spack instance. install_tree: $spack/opt/spack + # Locations where templates should be found template_dirs: - $spack/templates + # default directory layout directory_layout: "${ARCHITECTURE}/${COMPILERNAME}-${COMPILERVER}/${PACKAGE}-${VERSION}-${HASH}" + # Locations where different types of modules should be installed. module_roots: tcl: $spack/share/spack/modules @@ -74,6 +77,15 @@ config: dirty: false + # When set to true, concurrent instances of Spack will use locks to + # avoid modifying the install tree, database file, etc. If false, Spack + # will disable all locking, but you must NOT run concurrent instances + # of Spack. For filesystems that don't support locking, you should set + # this to false and run one Spack at a time, but otherwise we recommend + # enabling locks. + locks: true + + # The default number of jobs to use when running `make` in parallel. # If set to 4, for example, `spack install` will run `make -j4`. # If not set, all available cores are used by default. diff --git a/lib/spack/docs/basic_usage.rst b/lib/spack/docs/basic_usage.rst index bb426b4378..1a7576c74e 100644 --- a/lib/spack/docs/basic_usage.rst +++ b/lib/spack/docs/basic_usage.rst @@ -1093,22 +1093,43 @@ several variants: Filesystem requirements ----------------------- -Spack currently needs to be run from a filesystem that supports +By default, Spack needs to be run from a filesystem that supports ``flock`` locking semantics. Nearly all local filesystems and recent -versions of NFS support this, but parallel filesystems may be mounted -without ``flock`` support enabled. You can determine how your -filesystems are mounted with ``mount -p``. The output for a Lustre +versions of NFS support this, but parallel filesystems or NFS volumes may +be configured without ``flock`` support enabled. You can determine how +your filesystems are mounted with ``mount``. The output for a Lustre filesystem might look like this: .. code-block:: console - $ mount -l | grep lscratch - pilsner-mds1-lnet0@o2ib100:/lsd on /p/lscratchd type lustre (rw,nosuid,noauto,_netdev,lazystatfs,flock) - porter-mds1-lnet0@o2ib100:/lse on /p/lscratche type lustre (rw,nosuid,noauto,_netdev,lazystatfs,flock) + $ mount | grep lscratch + mds1-lnet0@o2ib100:/lsd on /p/lscratchd type lustre (rw,nosuid,lazystatfs,flock) + mds2-lnet0@o2ib100:/lse on /p/lscratche type lustre (rw,nosuid,lazystatfs,flock) -Note the ``flock`` option on both Lustre mounts. If you do not see -this or a similar option for your filesystem, you may need ot ask your -system administrator to enable ``flock``. +Note the ``flock`` option on both Lustre mounts. + +If you do not see this or a similar option for your filesystem, you have +a few options. First, you can move your Spack installation to a +filesystem that supports locking. Second, you could ask your system +administrator to enable ``flock`` for your filesystem. + +If none of those work, you can disable locking in one of two ways: + + 1. Run Spack with the ``-L`` or ``--disable-locks`` option to disable + locks on a call-by-call basis. + 2. Edit :ref:`config.yaml <config-yaml>` and set the ``locks`` option + to ``false`` to always disable locking. + +.. warning:: + + If you disable locking, concurrent instances of Spack will have no way + to avoid stepping on each other. You must ensure that there is only + **one** instance of Spack running at a time. Otherwise, Spack may end + up with a corrupted database file, or you may not be able to see all + installed packages in commands like ``spack find``. + + If you are unfortunate enough to run into this situation, you may be + able to fix it by running ``spack reindex``. This issue typically manifests with the error below: diff --git a/lib/spack/docs/config_yaml.rst b/lib/spack/docs/config_yaml.rst index da760f05e3..35bc22d72e 100644 --- a/lib/spack/docs/config_yaml.rst +++ b/lib/spack/docs/config_yaml.rst @@ -151,6 +151,17 @@ to ``false`` to disable these checks. Disabling this can expose you to attacks. Use at your own risk. -------------------- +``locks`` +-------------------- + +When set to ``true``, concurrent instances of Spack will use locks to +avoid modifying the install tree, database file, etc. If false, Spack +will disable all locking, but you must **not** run concurrent instances +of Spack. For filesystems that don't support locking, you should set +this to ``false`` and run one Spack at a time, but otherwise we recommend +enabling locks. + +-------------------- ``dirty`` -------------------- diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py index ed797d2ebf..0d072418cc 100644 --- a/lib/spack/llnl/util/lock.py +++ b/lib/spack/llnl/util/lock.py @@ -195,11 +195,13 @@ class Lock(object): """ if self._reads == 0 and self._writes == 0: - tty.debug('READ LOCK: {0.path}[{0._start}:{0._length}] [Acquiring]' - .format(self)) + self._debug( + 'READ LOCK: {0.path}[{0._start}:{0._length}] [Acquiring]' + .format(self)) self._lock(fcntl.LOCK_SH, timeout=timeout) # can raise LockError. - tty.debug('READ LOCK: {0.path}[{0._start}:{0._length}] [Acquired]' - .format(self)) + self._debug( + 'READ LOCK: {0.path}[{0._start}:{0._length}] [Acquired]' + .format(self)) self._reads += 1 return True else: @@ -218,12 +220,13 @@ class Lock(object): """ if self._writes == 0: - tty.debug( + self._debug( 'WRITE LOCK: {0.path}[{0._start}:{0._length}] [Acquiring]' .format(self)) self._lock(fcntl.LOCK_EX, timeout=timeout) # can raise LockError. - tty.debug('WRITE LOCK: {0.path}[{0._start}:{0._length}] [Acquired]' - .format(self)) + self._debug( + 'WRITE LOCK: {0.path}[{0._start}:{0._length}] [Acquired]' + .format(self)) self._writes += 1 return True else: @@ -243,8 +246,9 @@ class Lock(object): assert self._reads > 0 if self._reads == 1 and self._writes == 0: - tty.debug('READ LOCK: {0.path}[{0._start}:{0._length}] [Released]' - .format(self)) + self._debug( + 'READ LOCK: {0.path}[{0._start}:{0._length}] [Released]' + .format(self)) self._unlock() # can raise LockError. self._reads -= 1 return True @@ -265,8 +269,9 @@ class Lock(object): assert self._writes > 0 if self._writes == 1 and self._reads == 0: - tty.debug('WRITE LOCK: {0.path}[{0._start}:{0._length}] [Released]' - .format(self)) + self._debug( + 'WRITE LOCK: {0.path}[{0._start}:{0._length}] [Released]' + .format(self)) self._unlock() # can raise LockError. self._writes -= 1 return True @@ -274,6 +279,9 @@ class Lock(object): self._writes -= 1 return False + def _debug(self, *args): + tty.debug(*args) + class LockTransaction(object): """Simple nested transaction context manager that uses a file lock. diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index 3c62cc588f..4bc6891ec8 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -52,7 +52,6 @@ from yaml.error import MarkedYAMLError, YAMLError import llnl.util.tty as tty from llnl.util.filesystem import mkdirp -from llnl.util.lock import Lock, WriteTransaction, ReadTransaction import spack.store import spack.repo @@ -63,6 +62,7 @@ from spack.util.crypto import bit_length from spack.directory_layout import DirectoryLayoutError from spack.error import SpackError from spack.version import Version +from spack.util.lock import Lock, WriteTransaction, ReadTransaction # DB goes in this directory underneath the root diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py index 740367168f..7af096c821 100644 --- a/lib/spack/spack/main.py +++ b/lib/spack/spack/main.py @@ -250,7 +250,7 @@ class SpackArgumentParser(argparse.ArgumentParser): # epilog formatter.add_text("""\ {help}: - spack help --all list all available commands + spack help --all list all commands and options spack help <command> help on a specific command spack help --spec help on the spec syntax spack docs open http://spack.rtfd.io/ in a browser""" @@ -311,33 +311,50 @@ def make_argument_parser(**kwargs): # stat names in groups of 7, for nice wrapping. stat_lines = list(zip(*(iter(stat_names),) * 7)) - parser.add_argument('-h', '--help', action='store_true', - help="show this help message and exit") - parser.add_argument('--color', action='store', default='auto', - choices=('always', 'never', 'auto'), - help="when to colorize output (default: auto)") - parser.add_argument('-d', '--debug', action='store_true', - help="write out debug logs during compile") - parser.add_argument('-D', '--pdb', action='store_true', - help="run spack under the pdb debugger") - parser.add_argument('-k', '--insecure', action='store_true', - help="do not check ssl certificates when downloading") - parser.add_argument('-m', '--mock', action='store_true', - help="use mock packages instead of real ones") - parser.add_argument('-p', '--profile', action='store_true', - dest='spack_profile', - help="profile execution using cProfile") - parser.add_argument('-P', '--sorted-profile', default=None, metavar="STAT", - help="profile and sort by one or more of:\n[%s]" % - ',\n '.join([', '.join(line) for line in stat_lines])) - parser.add_argument('--lines', default=20, action='store', - help="lines of profile output or 'all' (default: 20)") - parser.add_argument('-v', '--verbose', action='store_true', - help="print additional output during builds") - parser.add_argument('-s', '--stacktrace', action='store_true', - help="add stacktraces to all printed statements") - parser.add_argument('-V', '--version', action='store_true', - help='show version number and exit') + parser.add_argument( + '-h', '--help', action='store_true', + help="show this help message and exit") + parser.add_argument( + '--color', action='store', default='auto', + choices=('always', 'never', 'auto'), + help="when to colorize output (default: auto)") + parser.add_argument( + '-d', '--debug', action='store_true', + help="write out debug logs during compile") + parser.add_argument( + '-D', '--pdb', action='store_true', + help="run spack under the pdb debugger") + parser.add_argument( + '-k', '--insecure', action='store_true', + help="do not check ssl certificates when downloading") + parser.add_argument( + '-l', '--enable-locks', action='store_true', dest='locks', + default=None, help="use filesystem locking (default)") + parser.add_argument( + '-L', '--disable-locks', action='store_false', dest='locks', + help="do not use filesystem locking (unsafe)") + parser.add_argument( + '-m', '--mock', action='store_true', + help="use mock packages instead of real ones") + parser.add_argument( + '-p', '--profile', action='store_true', dest='spack_profile', + help="profile execution using cProfile") + parser.add_argument( + '-P', '--sorted-profile', default=None, metavar="STAT", + help="profile and sort by one or more of:\n[%s]" % + ',\n '.join([', '.join(line) for line in stat_lines])) + parser.add_argument( + '--lines', default=20, action='store', + help="lines of profile output or 'all' (default: 20)") + parser.add_argument( + '-v', '--verbose', action='store_true', + help="print additional output during builds") + parser.add_argument( + '-s', '--stacktrace', action='store_true', + help="add stacktraces to all printed statements") + parser.add_argument( + '-V', '--version', action='store_true', + help='show version number and exit') return parser @@ -348,6 +365,11 @@ def setup_main_options(args): tty.set_debug(args.debug) tty.set_stacktrace(args.stacktrace) + # override lock configuration if passed on command line + if args.locks is not None: + spack.util.lock.check_lock_safety(spack.paths.prefix) + spack.config.set('config:locks', False, scope='command_line') + if args.debug: spack.util.debug.register_interrupt_handler() spack.config.set('config:debug', True, scope='command_line') diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py index d38e31cfd2..4a5b25ed63 100644 --- a/lib/spack/spack/schema/config.py +++ b/lib/spack/spack/schema/config.py @@ -66,6 +66,7 @@ schema = { 'verify_ssl': {'type': 'boolean'}, 'debug': {'type': 'boolean'}, 'checksum': {'type': 'boolean'}, + 'locks': {'type': 'boolean'}, 'dirty': {'type': 'boolean'}, 'build_jobs': {'type': 'integer', 'minimum': 1}, } diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index 0c460ddf5f..0c441b3269 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -34,7 +34,6 @@ from six import iteritems from six.moves.urllib.parse import urljoin import llnl.util.tty as tty -import llnl.util.lock from llnl.util.filesystem import mkdirp, can_access from llnl.util.filesystem import remove_if_dead_link, remove_linked_tree @@ -42,6 +41,7 @@ import spack.paths import spack.caches import spack.config import spack.error +import spack.util.lock import spack.fetch_strategy as fs import spack.util.pattern as pattern from spack.util.path import canonicalize_path @@ -231,7 +231,7 @@ class Stage(object): lock_id = prefix_bits(sha1, bit_length(sys.maxsize)) stage_lock_path = os.path.join(spack.paths.stage_path, '.lock') - Stage.stage_locks[self.name] = llnl.util.lock.Lock( + Stage.stage_locks[self.name] = spack.util.lock.Lock( stage_lock_path, lock_id, 1) self._lock = Stage.stage_locks[self.name] diff --git a/lib/spack/spack/test/llnl/util/lock.py b/lib/spack/spack/test/llnl/util/lock.py index c84573e75e..c279d7e918 100644 --- a/lib/spack/spack/test/llnl/util/lock.py +++ b/lib/spack/spack/test/llnl/util/lock.py @@ -73,8 +73,11 @@ from multiprocessing import Process import pytest from llnl.util.filesystem import touch + +import spack.util.lock +from spack.util.executable import which from spack.util.multiproc import Barrier -from llnl.util.lock import Lock, WriteTransaction, ReadTransaction, LockError +from spack.util.lock import Lock, WriteTransaction, ReadTransaction, LockError # @@ -183,15 +186,23 @@ def private_lock_path(lock_dir): lock_file = os.path.join(lock_dir, 'lockfile') if mpi: lock_file += '.%s' % comm.rank + yield lock_file + if os.path.exists(lock_file): + os.unlink(lock_file) + @pytest.fixture def lock_path(lock_dir): """This lock is shared among all processes in a multiproc test.""" lock_file = os.path.join(lock_dir, 'lockfile') + yield lock_file + if os.path.exists(lock_file): + os.unlink(lock_file) + def local_multiproc_test(*functions): """Order some processes using simple barrier synchronization.""" @@ -900,3 +911,95 @@ def test_transaction_with_context_manager_and_exception(lock_path): assert vals['exception'] assert not vals['exited_fn'] assert not vals['exception_fn'] + + +def test_disable_locking(private_lock_path): + """Ensure that locks do no real locking when disabled.""" + old_value = spack.config.get('config:locks') + + with spack.config.override('config:locks', False): + lock = Lock(private_lock_path) + + lock.acquire_read() + assert not os.path.exists(private_lock_path) + + lock.acquire_write() + assert not os.path.exists(private_lock_path) + + lock.release_write() + assert not os.path.exists(private_lock_path) + + lock.release_read() + assert not os.path.exists(private_lock_path) + + assert old_value == spack.config.get('config:locks') + + +def test_lock_checks_user(tmpdir): + """Ensure lock checks work.""" + path = str(tmpdir) + uid = os.getuid() + + # self-owned, own group + tmpdir.chown(uid, uid) + + # safe + tmpdir.chmod(0o744) + spack.util.lock.check_lock_safety(path) + + # safe + tmpdir.chmod(0o774) + spack.util.lock.check_lock_safety(path) + + # unsafe + tmpdir.chmod(0o777) + with pytest.raises(spack.error.SpackError): + spack.util.lock.check_lock_safety(path) + + # safe + tmpdir.chmod(0o474) + spack.util.lock.check_lock_safety(path) + + # safe + tmpdir.chmod(0o477) + spack.util.lock.check_lock_safety(path) + + +def test_lock_checks_group(tmpdir): + path = str(tmpdir) + uid = os.getuid() + + id_cmd = which('id') + if not id_cmd: + pytest.skip("Can't determine user's groups.") + + # find a legal gid to user that is NOT the user's uid + gids = [int(gid) for gid in id_cmd('-G', output=str).split(' ')] + gid = next((g for g in gids if g != uid), None) + if gid is None: + pytest.skip("Can't determine user's groups.") + + # self-owned, another group + tmpdir.chown(uid, gid) + + # safe + tmpdir.chmod(0o744) + spack.util.lock.check_lock_safety(path) + + # unsafe + tmpdir.chmod(0o774) + with pytest.raises(spack.error.SpackError): + spack.util.lock.check_lock_safety(path) + + # unsafe + tmpdir.chmod(0o777) + with pytest.raises(spack.error.SpackError): + spack.util.lock.check_lock_safety(path) + + # safe + tmpdir.chmod(0o474) + spack.util.lock.check_lock_safety(path) + + # safe + tmpdir.chmod(0o477) + spack.util.lock.check_lock_safety(path) diff --git a/lib/spack/spack/util/file_cache.py b/lib/spack/spack/util/file_cache.py index d0551faff9..56217e3a7c 100644 --- a/lib/spack/spack/util/file_cache.py +++ b/lib/spack/spack/util/file_cache.py @@ -26,9 +26,9 @@ import os import shutil from llnl.util.filesystem import mkdirp -from llnl.util.lock import Lock, ReadTransaction, WriteTransaction from spack.error import SpackError +from spack.util.lock import Lock, ReadTransaction, WriteTransaction class FileCache(object): diff --git a/lib/spack/spack/util/lock.py b/lib/spack/spack/util/lock.py new file mode 100644 index 0000000000..a36cf5876d --- /dev/null +++ b/lib/spack/spack/util/lock.py @@ -0,0 +1,91 @@ +############################################################################## +# Copyright (c) 2013-2018, 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/spack/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 +############################################################################## +"""Wrapper for ``llnl.util.lock`` allows locking to be enabled/disabled.""" +import os +import stat + +import llnl.util.lock +from llnl.util.lock import * # noqa + +import spack.config +import spack.error +import spack.paths + + +class Lock(llnl.util.lock.Lock): + """Lock that can be disabled. + + This overrides the ``_lock()`` and ``_unlock()`` methods from + ``llnl.util.lock`` so that all the lock API calls will succeed, but + the actual locking mechanism can be disabled via ``_enable_locks``. + """ + def __init__(self, *args, **kwargs): + super(Lock, self).__init__(*args, **kwargs) + self._enable = spack.config.get('config:locks', True) + + def _lock(self, op, timeout=0): + if self._enable: + super(Lock, self)._lock(op, timeout) + + def _unlock(self): + """Unlock call that always succeeds.""" + if self._enable: + super(Lock, self)._unlock() + + def _debug(self, *args): + if self._enable: + super(Lock, self)._debug(*args) + + +def check_lock_safety(path): + """Do some extra checks to ensure disabling locks is safe. + + This will raise an error if ``path`` can is group- or world-writable + AND the current user can write to the directory (i.e., if this user + AND others could write to the path). + + This is intended to run on the Spack prefix, but can be run on any + path for testing. + """ + if os.access(path, os.W_OK): + stat_result = os.stat(path) + uid, gid = stat_result.st_uid, stat_result.st_gid + mode = stat_result[stat.ST_MODE] + + writable = None + if (mode & stat.S_IWGRP) and (uid != gid): + # spack is group-writeable and the group is not the owner + writable = 'group' + elif (mode & stat.S_IWOTH): + # spack is world-writeable + writable = 'world' + + if writable: + msg = "Refusing to disable locks: spack is {0}-writable.".format( + writable) + long_msg = ( + "Running a shared spack without locks is unsafe. You must " + "restrict permissions on {0} or enable locks.").format(path) + raise spack.error.SpackError(msg, long_msg) |