From cfbac14cba4ddc18a1addc2f02f8ff09947716b0 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Fri, 25 Oct 2019 16:01:45 -0700
Subject: bugfix: restore upstream lock safety; update tests

Restore upstream lock safety; avoid calling methods directly on upstream DB in test.
---
 lib/spack/spack/database.py      | 13 +++++-----
 lib/spack/spack/test/database.py | 54 +++++++++++++++++++++++++++++++---------
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py
index 022a2d3871..a1748fc585 100644
--- a/lib/spack/spack/database.py
+++ b/lib/spack/spack/database.py
@@ -257,11 +257,6 @@ class Database(object):
 
         self.is_upstream = is_upstream
 
-        if self.is_upstream:
-            self.lock = ForbiddenLock()
-        else:
-            self.lock = Lock(self._lock_path)
-
         # initialize rest of state.
         self.db_lock_timeout = (
             spack.config.get('config:db_lock_timeout') or _db_lock_timeout)
@@ -273,8 +268,12 @@ class Database(object):
                               if self.package_lock_timeout else 'No timeout')
         tty.debug('PACKAGE LOCK TIMEOUT: {0}'.format(
                   str(timeout_format_str)))
-        self.lock = Lock(self._lock_path,
-                         default_timeout=self.db_lock_timeout)
+
+        if self.is_upstream:
+            self.lock = ForbiddenLock()
+        else:
+            self.lock = Lock(self._lock_path,
+                             default_timeout=self.db_lock_timeout)
         self._data = {}
 
         self.upstream_dbs = list(upstream_dbs) if upstream_dbs else []
diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py
index 099894e0d2..06ddc83a47 100644
--- a/lib/spack/spack/test/database.py
+++ b/lib/spack/spack/test/database.py
@@ -159,6 +159,31 @@ def test_add_to_upstream_after_downstream(upstream_and_downstream_db):
             spack.store.db = orig_db
 
 
+@pytest.mark.usefixtures('config')
+def test_cannot_write_upstream(tmpdir_factory, test_store, gen_mock_layout):
+    roots = [str(tmpdir_factory.mktemp(x)) for x in ['a', 'b']]
+    layouts = [gen_mock_layout(x) for x in ['/ra/', '/rb/']]
+
+    x = MockPackage('x', [], [])
+    mock_repo = MockPackageMultiRepo([x])
+
+    # Instantiate the database that will be used as the upstream DB and make
+    # sure it has an index file
+    upstream_db_independent = spack.database.Database(roots[1])
+    with upstream_db_independent.write_transaction():
+        pass
+
+    upstream_dbs = spack.store._construct_upstream_dbs_from_install_roots(
+        [roots[1]], _test=True)
+
+    with spack.repo.swap(mock_repo):
+        spec = spack.spec.Spec('x')
+        spec.concretize()
+
+        with pytest.raises(spack.database.ForbiddenLockError):
+            upstream_dbs[0].add(spec, layouts[1])
+
+
 @pytest.mark.usefixtures('config')
 def test_recursive_upstream_dbs(tmpdir_factory, test_store, gen_mock_layout):
     roots = [str(tmpdir_factory.mktemp(x)) for x in ['a', 'b', 'c']]
@@ -183,22 +208,27 @@ def test_recursive_upstream_dbs(tmpdir_factory, test_store, gen_mock_layout):
         db_a = spack.database.Database(roots[0], upstream_dbs=[db_b, db_c])
         db_a.add(spec['x'], layouts[0])
 
-        dbs = spack.store._construct_upstream_dbs_from_install_roots(
-            roots, _test=True)
+        upstream_dbs_from_scratch = (
+            spack.store._construct_upstream_dbs_from_install_roots(
+                [roots[1], roots[2]], _test=True))
+        db_a_from_scratch = spack.database.Database(
+            roots[0], upstream_dbs=upstream_dbs_from_scratch)
 
-        assert dbs[0].db_for_spec_hash(spec.dag_hash()) == dbs[0]
-        assert dbs[0].db_for_spec_hash(spec['y'].dag_hash()) == dbs[1]
-        assert dbs[0].db_for_spec_hash(spec['z'].dag_hash()) == dbs[2]
+        assert db_a_from_scratch.db_for_spec_hash(spec.dag_hash()) == (
+            db_a_from_scratch)
+        assert db_a_from_scratch.db_for_spec_hash(spec['y'].dag_hash()) == (
+            upstream_dbs_from_scratch[0])
+        assert db_a_from_scratch.db_for_spec_hash(spec['z'].dag_hash()) == (
+            upstream_dbs_from_scratch[1])
 
-        dbs[0]._check_ref_counts()
-        dbs[1]._check_ref_counts()
-        dbs[2]._check_ref_counts()
+        db_a_from_scratch._check_ref_counts()
+        upstream_dbs_from_scratch[0]._check_ref_counts()
+        upstream_dbs_from_scratch[1]._check_ref_counts()
 
-        assert (dbs[0].installed_relatives(spec) ==
+        assert (db_a_from_scratch.installed_relatives(spec) ==
                 set(spec.traverse(root=False)))
-        assert (dbs[0].installed_relatives(spec['z'], direction='parents') ==
-                set([spec, spec['y']]))
-        assert not dbs[2].installed_relatives(spec['z'], direction='parents')
+        assert (db_a_from_scratch.installed_relatives(
+                spec['z'], direction='parents') == set([spec, spec['y']]))
 
 
 @pytest.fixture()
-- 
cgit v1.2.3-70-g09d2