deploy: merge upstream 0.16.3

.flake8

@@ -29,4 +29,4 @@
 #
 [flake8]
 ignore = E129,E221,E241,E272,E731,W503,W504,F999,N801,N813,N814
-max-line-length = 79
+max-line-length = 88

.github/workflows/linux_unit_tests.yaml

@@ -68,7 +68,6 @@ jobs:
     - uses: codecov/codecov-action@v1
       with:
         flags: unittests,linux,${{ matrix.concretizer }}
-
   shell:
     runs-on: ubuntu-latest
     steps:

CHANGELOG.md

@@ -1,3 +1,15 @@
+# v0.16.3 (2021-09-21)
+
+* clang/llvm: fix version detection (#19978)
+* Fix use of quotes in Python build system (#22279)
+* Cray: fix extracting paths from module files (#23472)
+* Use AWS CloudFront for source mirror (#23978)
+* Ensure all roots of an installed environment are marked explicit in db (#24277)
+* Fix fetching for Python 3.8 and 3.9 (#24686)
+* locks: only open lockfiles once instead of for every lock held (#24794)
+* Remove the EOL centos:6 docker image
+
+
 # v0.16.2 (2021-05-22)
 
 * Major performance improvement for `spack load` and other commands. (#23661)

etc/spack/defaults/mirrors.yaml

@@ -1,2 +1,2 @@
 mirrors:
-  spack-public: https://spack-llnl-mirror.s3-us-west-2.amazonaws.com/
+  spack-public: https://mirror.spack.io

lib/spack/docs/containers.rst

@@ -126,9 +126,6 @@ are currently supported are summarized in the table below:
    * - Ubuntu 18.04
      - ``ubuntu:18.04``
      - ``spack/ubuntu-bionic``
-   * - CentOS 6
-     - ``centos:6``
-     - ``spack/centos6``
    * - CentOS 7
      - ``centos:7``
      - ``spack/centos7``

lib/spack/llnl/util/lock.py

@@ -26,6 +26,126 @@
 true_fn = lambda: True
 
 
+class OpenFile(object):
+    """Record for keeping track of open lockfiles (with reference counting).
+
+    There's really only one ``OpenFile`` per inode, per process, but we record the
+    filehandle here as it's the thing we end up using in python code.  You can get
+    the file descriptor from the file handle if needed -- or we could make this track
+    file descriptors as well in the future.
+    """
+    def __init__(self, fh):
+        self.fh = fh
+        self.refs = 0
+
+
+class OpenFileTracker(object):
+    """Track open lockfiles, to minimize number of open file descriptors.
+
+    The ``fcntl`` locks that Spack uses are associated with an inode and a process.
+    This is convenient, because if a process exits, it releases its locks.
+    Unfortunately, this also means that if you close a file, *all* locks associated
+    with that file's inode are released, regardless of whether the process has any
+    other open file descriptors on it.
+
+    Because of this, we need to track open lock files so that we only close them when
+    a process no longer needs them.  We do this by tracking each lockfile by its
+    inode and process id.  This has several nice properties:
+
+    1. Tracking by pid ensures that, if we fork, we don't inadvertently track the parent
+       process's lockfiles. ``fcntl`` locks are not inherited across forks, so we'll
+       just track new lockfiles in the child.
+    2. Tracking by inode ensures that referencs are counted per inode, and that we don't
+       inadvertently close a file whose inode still has open locks.
+    3. Tracking by both pid and inode ensures that we only open lockfiles the minimum
+       number of times necessary for the locks we have.
+
+    Note: as mentioned elsewhere, these locks aren't thread safe -- they're designed to
+    work in Python and assume the GIL.
+    """
+
+    def __init__(self):
+        """Create a new ``OpenFileTracker``."""
+        self._descriptors = {}
+
+    def get_fh(self, path):
+        """Get a filehandle for a lockfile.
+
+        This routine will open writable files for read/write even if you're asking
+        for a shared (read-only) lock. This is so that we can upgrade to an exclusive
+        (write) lock later if requested.
+
+        Arguments:
+          path (str): path to lock file we want a filehandle for
+        """
+        # Open writable files as 'r+' so we can upgrade to write later
+        os_mode, fh_mode = (os.O_RDWR | os.O_CREAT), 'r+'
+
+        pid = os.getpid()
+        open_file = None  # OpenFile object, if there is one
+        stat = None       # stat result for the lockfile, if it exists
+
+        try:
+            # see whether we've seen this inode/pid before
+            stat = os.stat(path)
+            key = (stat.st_ino, pid)
+            open_file = self._descriptors.get(key)
+
+        except OSError as e:
+            if e.errno != errno.ENOENT:  # only handle file not found
+                raise
+
+            # path does not exist -- fail if we won't be able to create it
+            parent = os.path.dirname(path) or '.'
+            if not os.access(parent, os.W_OK):
+                raise CantCreateLockError(path)
+
+        # if there was no already open file, we'll need to open one
+        if not open_file:
+            if stat and not os.access(path, os.W_OK):
+                # we know path exists but not if it's writable. If it's read-only,
+                # only open the file for reading (and fail if we're trying to get
+                # an exclusive (write) lock on it)
+                os_mode, fh_mode = os.O_RDONLY, 'r'
+
+            fd = os.open(path, os_mode)
+            fh = os.fdopen(fd, fh_mode)
+            open_file = OpenFile(fh)
+
+            # if we just created the file, we'll need to get its inode here
+            if not stat:
+                inode = os.fstat(fd).st_ino
+                key = (inode, pid)
+
+            self._descriptors[key] = open_file
+
+        open_file.refs += 1
+        return open_file.fh
+
+    def release_fh(self, path):
+        """Release a filehandle, only closing it if there are no more references."""
+        try:
+            inode = os.stat(path).st_ino
+        except OSError as e:
+            if e.errno != errno.ENOENT:  # only handle file not found
+                raise
+            inode = None  # this will not be in self._descriptors
+
+        key = (inode, os.getpid())
+        open_file = self._descriptors.get(key)
+        assert open_file, "Attempted to close non-existing lock path: %s" % path
+
+        open_file.refs -= 1
+        if not open_file.refs:
+            del self._descriptors[key]
+            open_file.fh.close()
+
+
+#: Open file descriptors for locks in this process. Used to prevent one process
+#: from opening the sam file many times for different byte range locks
+file_tracker = OpenFileTracker()
+
+
 def _attempts_str(wait_time, nattempts):
     # Don't print anything if we succeeded on the first try
     if nattempts <= 1:
@@ -46,7 +166,8 @@ class Lock(object):
     Note that this is for managing contention over resources *between*
     processes and not for managing contention between threads in a process: the
     functions of this object are not thread-safe. A process also must not
-    maintain multiple locks on the same file.
+    maintain multiple locks on the same file (or, more specifically, on
+    overlapping byte ranges in the same file).
     """
 
     def __init__(self, path, start=0, length=0, default_timeout=None,
@@ -151,25 +272,10 @@ def _lock(self, op, timeout=None):
 
         # Create file and parent directories if they don't exist.
         if self._file is None:
-            parent = self._ensure_parent_directory()
+            self._ensure_parent_directory()
+            self._file = file_tracker.get_fh(self.path)
 
-            # Open writable files as 'r+' so we can upgrade to write later
-            os_mode, fd_mode = (os.O_RDWR | os.O_CREAT), 'r+'
-            if os.path.exists(self.path):
-                if not os.access(self.path, os.W_OK):
-                    if op == fcntl.LOCK_SH:
-                        # can still lock read-only files if we open 'r'
-                        os_mode, fd_mode = os.O_RDONLY, 'r'
-                    else:
-                        raise LockROFileError(self.path)
-
-            elif not os.access(parent, os.W_OK):
-                raise CantCreateLockError(self.path)
-
-            fd = os.open(self.path, os_mode)
-            self._file = os.fdopen(fd, fd_mode)
-
-        elif op == fcntl.LOCK_EX and self._file.mode == 'r':
+        if op == fcntl.LOCK_EX and self._file.mode == 'r':
             # Attempt to upgrade to write lock w/a read-only file.
             # If the file were writable, we'd have opened it 'r+'
             raise LockROFileError(self.path)
@@ -282,7 +388,8 @@ def _unlock(self):
         """
         fcntl.lockf(self._file, fcntl.LOCK_UN,
                     self._length, self._start, os.SEEK_SET)
-        self._file.close()
+
+        file_tracker.release_fh(self.path)
         self._file = None
         self._reads = 0
         self._writes = 0

lib/spack/spack/__init__.py

@@ -5,7 +5,7 @@
 
 
 #: major, minor, patch version for Spack, in a tuple
-spack_version_info = (0, 16, 2)
+spack_version_info = (0, 16, 3)
 
 #: String containing Spack version joined with .'s
 spack_version = '.'.join(str(v) for v in spack_version_info)

lib/spack/spack/compilers/clang.py

@@ -159,11 +159,11 @@ def extract_version_from_output(cls, output):
 
         match = re.search(
             # Normal clang compiler versions are left as-is
-            r'clang version ([^ )]+)-svn[~.\w\d-]*|'
+            r'clang version ([^ )\n]+)-svn[~.\w\d-]*|'
             # Don't include hyphenated patch numbers in the version
             # (see https://github.com/spack/spack/pull/14365 for details)
-            r'clang version ([^ )]+?)-[~.\w\d-]*|'
-            r'clang version ([^ )]+)',
+            r'clang version ([^ )\n]+?)-[~.\w\d-]*|'
+            r'clang version ([^ )\n]+)',
             output
         )
         if match:

lib/spack/spack/container/images.json

@@ -21,13 +21,6 @@
       "build_tags": {
         "develop": "latest"
       }
-    },
-    "centos:6": {
-      "os_package_manager": "yum",
-      "build": "spack/centos6",
-      "build_tags": {
-        "develop": "latest"
-      }
     }
   },
   "os_package_managers": {

lib/spack/spack/environment.py

@@ -1421,6 +1421,13 @@ def install_all(self, args=None, **install_args):
         # DB read transaction to reduce time spent in this case.
         specs_to_install = self.uninstalled_specs()
 
+        # ensure specs already installed are marked explicit
+        all_specs = [cs for _, cs in self.concretized_specs()]
+        specs_installed = [s for s in all_specs if s.package.installed]
+        with spack.store.db.write_transaction():  # do all in one transaction
+            for spec in specs_installed:
+                spack.store.db.update_explicit(spec, True)
+
         if not specs_to_install:
             tty.msg('All of the packages are already installed')
             return

lib/spack/spack/schema/container.py

@@ -12,8 +12,7 @@
             'type': 'string',
             'enum': ['ubuntu:18.04',
                      'ubuntu:16.04',
-                     'centos:7',
-                     'centos:6']
+                     'centos:7']
         },
         'spack': {
             'type': 'string',

lib/spack/spack/test/cmd/env.py

@@ -176,6 +176,27 @@ def test_env_install_single_spec(install_mockery, mock_fetch):
     assert e.specs_by_hash[e.concretized_order[0]].name == 'cmake-client'
 
 
+def test_env_roots_marked_explicit(install_mockery, mock_fetch):
+    install = SpackCommand('install')
+    install('dependent-install')
+
+    # Check one explicit, one implicit install
+    dependent = spack.store.db.query(explicit=True)
+    dependency = spack.store.db.query(explicit=False)
+    assert len(dependent) == 1
+    assert len(dependency) == 1
+
+    env('create', 'test')
+    with ev.read('test') as e:
+        # make implicit install a root of the env
+        e.add(dependency[0].name)
+        e.concretize()
+        e.install_all()
+
+    explicit = spack.store.db.query(explicit=True)
+    assert len(explicit) == 2
+
+
 def test_env_modifications_error_on_activate(
         install_mockery, mock_fetch, monkeypatch, capfd):
     env('create', 'test')

lib/spack/spack/test/compilers/detection.py

@@ -96,7 +96,11 @@ def test_apple_clang_version_detection(
     ('clang version 8.0.0-3 (tags/RELEASE_800/final)\n'
      'Target: aarch64-unknown-linux-gnu\n'
      'Thread model: posix\n'
-     'InstalledDir: /usr/bin\n', '8.0.0')
+     'InstalledDir: /usr/bin\n', '8.0.0'),
+    ('clang version 11.0.0\n'
+     'Target: aarch64-unknown-linux-gnu\n'
+     'Thread model: posix\n'
+     'InstalledDir: /usr/bin\n', '11.0.0')
 ])
 def test_clang_version_detection(version_str, expected_version):
     version = spack.compilers.clang.Clang.extract_version_from_output(

lib/spack/spack/test/module_parsing.py

@@ -123,3 +123,9 @@ def test_get_argument_from_module_line():
     for bl in bad_lines:
         with pytest.raises(ValueError):
             get_path_args_from_module_line(bl)
+
+
+def test_lmod_quote_parsing():
+    lines = ['setenv("SOME_PARTICULAR_DIR","-L/opt/cray/pe/mpich/8.1.4/gtl/lib")']
+    result = get_path_from_module_contents(lines, 'some-module')
+    assert '/opt/cray/pe/mpich/8.1.4/gtl' == result

lib/spack/spack/util/module_cmd.py

@@ -213,9 +213,13 @@ def match_pattern_and_strip(line, pattern, strip=[]):
     def match_flag_and_strip(line, flag, strip=[]):
         flag_idx = line.find(flag)
         if flag_idx >= 0:
-            end = line.find(' ', flag_idx)
-            if end >= 0:
-                path = line[flag_idx + len(flag):end]
+            # Search for the first occurence of any separator marking the end of
+            # the path.
+            separators = (' ', '"', "'")
+            occurrences = [line.find(s, flag_idx) for s in separators]
+            indices = [idx for idx in occurrences if idx >= 0]
+            if indices:
+                path = line[flag_idx + len(flag):min(indices)]
             else:
                 path = line[flag_idx + len(flag):]
             path = strip_path(path, strip)