install : finer graned locking for install command

lib/spack/llnl/util/lock.py

@@ -28,9 +28,13 @@
 import time
 import socket
 
+import llnl.util.tty as tty
+
+
 __all__ = ['Lock', 'LockTransaction', 'WriteTransaction', 'ReadTransaction',
            'LockError']
 
+
 # Default timeout in seconds, after which locks will raise exceptions.
 _default_timeout = 60
 
@@ -120,6 +124,7 @@ def acquire_read(self, timeout=_default_timeout):
 
         """
         if self._reads == 0 and self._writes == 0:
+            tty.debug('READ LOCK : {0._file_path} [Acquiring]'.format(self))
             self._lock(fcntl.LOCK_SH, timeout)   # can raise LockError.
             self._reads += 1
             return True
@@ -139,6 +144,7 @@ def acquire_write(self, timeout=_default_timeout):
 
         """
         if self._writes == 0:
+            tty.debug('WRITE LOCK : {0._file_path} [Acquiring]'.format(self))
             self._lock(fcntl.LOCK_EX, timeout)   # can raise LockError.
             self._writes += 1
             return True
@@ -159,6 +165,7 @@ def release_read(self):
         assert self._reads > 0
 
         if self._reads == 1 and self._writes == 0:
+            tty.debug('READ LOCK : {0._file_path} [Released]'.format(self))
             self._unlock()      # can raise LockError.
             self._reads -= 1
             return True
@@ -179,6 +186,7 @@ def release_write(self):
         assert self._writes > 0
 
         if self._writes == 1 and self._reads == 0:
+            tty.debug('WRITE LOCK : {0._file_path} [Released]'.format(self))
             self._unlock()      # can raise LockError.
             self._writes -= 1
             return True

lib/spack/spack/cmd/install.py

@@ -84,7 +84,6 @@ def install(parser, args):
     specs = spack.cmd.parse_specs(args.packages, concretize=True)
     for spec in specs:
         package = spack.repo.get(spec)
-        with spack.installed_db.write_transaction():
         package.do_install(
             keep_prefix=args.keep_prefix,
             keep_stage=args.keep_stage,

lib/spack/spack/database.py

@@ -33,7 +33,7 @@
   2. It will allow us to track external installations as well as lost
      packages and their dependencies.
 
-Prior ot the implementation of this store, a direcotry layout served
+Prior to the implementation of this store, a directory layout served
 as the authoritative database of packages in Spack.  This module
 provides a cache and a sanity checking mechanism for what is in the
 filesystem.

lib/spack/spack/package.py

@@ -39,8 +39,17 @@
 import textwrap
 import time
 import string
+import contextlib
+from urlparse import urlparse
+from StringIO import StringIO
 
+import llnl.util.lock
 import llnl.util.tty as tty
+from llnl.util.filesystem import *
+from llnl.util.lang import *
+from llnl.util.link_tree import LinkTree
+from llnl.util.tty.log import log_output
+
 import spack
 import spack.build_environment
 import spack.compilers
@@ -53,11 +62,6 @@
 import spack.url
 import spack.util.web
 
-from StringIO import StringIO
-from llnl.util.filesystem import *
-from llnl.util.lang import *
-from llnl.util.link_tree import LinkTree
-from llnl.util.tty.log import log_output
 from spack.stage import Stage, ResourceStage, StageComposite
 from spack.util.environment import dump_environment
 from spack.util.executable import ProcessError, which
@@ -346,6 +350,9 @@ def __init__(self, spec):
         # this determines how the package should be built.
         self.spec = spec
 
+        # Lock on the prefix shared resource. Will be set in prefix property
+        self._prefix_lock = None
+
         # Name of package is the name of its module, without the
         # containing module names.
         self.name = self.module.__name__
@@ -691,6 +698,22 @@ def installed_dependents(self):
                     dependents.append(spec)
         return dependents
 
+    @property
+    def prefix_lock(self):
+        if self._prefix_lock is None:
+            dirname = join_path(os.path.dirname(self.spec.prefix), '.locks')
+            basename = os.path.basename(self.spec.prefix)
+            lock_file = join_path(dirname, basename)
+
+            if not os.path.exists(lock_file):
+                tty.debug('TOUCH FILE : {0}'.format(lock_file))
+                os.makedirs(dirname)
+                touch(lock_file)
+
+            self._prefix_lock = llnl.util.lock.Lock(lock_file)
+
+        return self._prefix_lock
+
     @property
     def prefix(self):
         """Get the prefix into which this package should be installed."""
@@ -875,6 +898,22 @@ def _resource_stage(self, resource):
         resource_stage_folder = '-'.join(pieces)
         return resource_stage_folder
 
+    @contextlib.contextmanager
+    def _prefix_read_lock(self):
+        try:
+            self.prefix_lock.acquire_read(60)
+            yield self
+        finally:
+            self.prefix_lock.release_read()
+
+    @contextlib.contextmanager
+    def _prefix_write_lock(self):
+        try:
+            self.prefix_lock.acquire_write(60)
+            yield self
+        finally:
+            self.prefix_lock.release_write()
+
     install_phases = set(['configure', 'build', 'install', 'provenance'])
 
     def do_install(self,
@@ -926,8 +965,12 @@ def do_install(self,
 
         # Ensure package is not already installed
         layout = spack.install_layout
-        if 'install' in install_phases and layout.check_installed(self.spec):
+        with self._prefix_read_lock():
-            tty.msg("%s is already installed in %s" % (self.name, self.prefix))
+            if ('install' in install_phases and
+                layout.check_installed(self.spec)):
+
+                tty.msg(
+                    "%s is already installed in %s" % (self.name, self.prefix))
                 rec = spack.installed_db.get_record(self.spec)
                 if (not rec.explicit) and explicit:
                     with spack.installed_db.write_transaction():
@@ -983,7 +1026,7 @@ def build_process():
             self.build_directory = join_path(self.stage.path, 'spack-build')
             self.source_directory = self.stage.source_path
 
-            with self.stage:
+            with contextlib.nested(self.stage, self._prefix_write_lock()):
                 # Run the pre-install hook in the child process after
                 # the directory is created.
                 spack.hooks.pre_install(self)
@@ -1077,8 +1120,9 @@ def build_process():
                          wrap=False)
             raise
 
-        # note: PARENT of the build process adds the new package to
+        # Parent of the build process adds the new package to
         # the database, so that we don't need to re-read from file.
+        # NOTE: add() implicitly acquires a write-lock
         spack.installed_db.add(
             self.spec, spack.install_layout, explicit=explicit)
 

lib/spack/spack/stage.py

@@ -29,6 +29,7 @@
 from urlparse import urljoin
 
 import llnl.util.tty as tty
+import llnl.util.lock
 from llnl.util.filesystem import *
 
 import spack.util.pattern as pattern
@@ -88,8 +89,9 @@ class Stage(object):
     similar, and are intended to persist for only one run of spack.
     """
 
-    def __init__(self, url_or_fetch_strategy,
+    def __init__(
-                 name=None, mirror_path=None, keep=False, path=None):
+            self, url_or_fetch_strategy,
+            name=None, mirror_path=None, keep=False, path=None, lock=True):
         """Create a stage object.
            Parameters:
              url_or_fetch_strategy
@@ -147,6 +149,15 @@ def __init__(self, url_or_fetch_strategy,
         # Flag to decide whether to delete the stage folder on exit or not
         self.keep = keep
 
+        # File lock for the stage directory
+        self._lock_file = None
+        self._lock = None
+        if lock:
+            self._lock_file = join_path(spack.stage_path, self.name + '.lock')
+            if not os.path.exists(self._lock_file):
+                touch(self._lock_file)
+            self._lock = llnl.util.lock.Lock(self._lock_file)
+
     def __enter__(self):
         """
         Entering a stage context will create the stage directory
@@ -154,6 +165,8 @@ def __enter__(self):
         Returns:
             self
         """
+        if self._lock is not None:
+            self._lock.acquire_write(timeout=60)
         self.create()
         return self
 
@@ -175,6 +188,9 @@ def __exit__(self, exc_type, exc_val, exc_tb):
         if exc_type is None and not self.keep:
             self.destroy()
 
+        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.