Merge branch 'jennis/do_not_leak_project_specific_remotes' into 'master'

Do not leak subproject remotes

See merge request BuildStream/buildstream!1416

16 files changed, 42 insertions, 76 deletions

diff --git a/src/buildstream/_basecache.py b/src/buildstream/_basecache.py
index ef9414787..49930d4ca 100644
--- a/src/buildstream/_basecache.py
+++ b/src/buildstream/_basecache.py
@@ -103,14 +103,10 @@ class BaseCache():
     #
     @classmethod
     def _configured_remote_cache_specs(cls, context, project):
-        project_overrides = context.get_overrides(project.name)
-        project_extra_specs = cls.specs_from_config_node(project_overrides)
-
         project_specs = getattr(project, cls.spec_name)
         context_specs = getattr(context, cls.spec_name)
 
-        return list(utils._deduplicate(
-            project_extra_specs + project_specs + context_specs))
+        return list(utils._deduplicate(project_specs + context_specs))
 
     # setup_remotes():
     #
@@ -153,10 +149,10 @@ class BaseCache():
     #     on_failure (callable): Called if we fail to contact one of the caches.
     #
     def initialize_remotes(self, *, on_failure=None):
-        remote_specs = self.global_remote_specs
+        remote_specs = self.global_remote_specs.copy()
 
         for project in self.project_remote_specs:
-            remote_specs += self.project_remote_specs[project]
+            remote_specs.extend(self.project_remote_specs[project])
 
         remote_specs = list(utils._deduplicate(remote_specs))
 
diff --git a/tests/artifactcache/junctions.py b/tests/artifactcache/junctions.py
index ab041ce27..2798f032c 100644
--- a/tests/artifactcache/junctions.py
+++ b/tests/artifactcache/junctions.py
@@ -8,7 +8,7 @@ import pytest
 from buildstream import _yaml
 from buildstream.testing import cli  # pylint: disable=unused-import
 
-from tests.testutils import create_artifact_share
+from tests.testutils import create_artifact_share, assert_shared, assert_not_shared
 
 
 DATA_DIR = os.path.join(
@@ -17,17 +17,6 @@ DATA_DIR = os.path.join(
 )
 
 
-# Assert that a given artifact is in the share
-#
-def assert_shared(cli, share, project_name, project, element_name):
-    # NOTE: 'test' here is the name of the project
-    # specified in the project.conf we are testing with.
-    #
-    if not share.has_artifact(cli.get_artifact_name(project, project_name, element_name)):
-        raise AssertionError("Artifact share at {} does not contain the expected element {}"
-                             .format(share.repo, element_name))
-
-
 def project_set_artifacts(project, url):
     project_conf_file = os.path.join(project, 'project.conf')
     project_config = _yaml.load(project_conf_file)
@@ -40,10 +29,10 @@ def project_set_artifacts(project, url):
 
 @pytest.mark.datafiles(DATA_DIR)
 def test_push_pull(cli, tmpdir, datafiles):
-    project = os.path.join(str(datafiles), 'foo')
+    project = os.path.join(str(datafiles), 'parent')
     base_project = os.path.join(str(project), 'base')
 
-    with create_artifact_share(os.path.join(str(tmpdir), 'artifactshare-foo')) as share,\
+    with create_artifact_share(os.path.join(str(tmpdir), 'artifactshare-parent')) as share,\
         create_artifact_share(os.path.join(str(tmpdir), 'artifactshare-base')) as base_share:
 
         # First build it without the artifact cache configured
@@ -53,7 +42,7 @@ def test_push_pull(cli, tmpdir, datafiles):
         # Assert that we are now cached locally
         state = cli.get_element_state(project, 'target.bst')
         assert state == 'cached'
-        state = cli.get_element_state(base_project, 'target.bst')
+        state = cli.get_element_state(base_project, 'base-element.bst')
         assert state == 'cached'
 
         project_set_artifacts(project, share.repo)
@@ -64,8 +53,16 @@ def test_push_pull(cli, tmpdir, datafiles):
         assert result.exit_code == 0
 
         # And finally assert that the artifacts are in the right shares
-        assert_shared(cli, share, 'foo', project, 'target.bst')
-        assert_shared(cli, base_share, 'base', base_project, 'target.bst')
+        #
+        # In the parent project's cache
+        assert_shared(cli, share, project, 'target.bst', project_name='parent')
+        assert_shared(cli, share, project, 'app.bst', project_name='parent')
+        assert_shared(cli, share, base_project, 'base-element.bst', project_name='base')
+
+        # In the junction project's cache
+        assert_not_shared(cli, base_share, project, 'target.bst', project_name='parent')
+        assert_not_shared(cli, base_share, project, 'app.bst', project_name='parent')
+        assert_shared(cli, base_share, base_project, 'base-element.bst', project_name='base')
 
         # Now we've pushed, delete the user's local artifact cache
         # directory and try to redownload it from the share
@@ -78,7 +75,7 @@ def test_push_pull(cli, tmpdir, datafiles):
         # Assert that nothing is cached locally anymore
         state = cli.get_element_state(project, 'target.bst')
         assert state != 'cached'
-        state = cli.get_element_state(base_project, 'target.bst')
+        state = cli.get_element_state(base_project, 'base-element.bst')
         assert state != 'cached'
 
         # Now try bst artifact pull
@@ -88,5 +85,5 @@ def test_push_pull(cli, tmpdir, datafiles):
         # And assert that they are again in the local cache, without having built
         state = cli.get_element_state(project, 'target.bst')
         assert state == 'cached'
-        state = cli.get_element_state(base_project, 'target.bst')
+        state = cli.get_element_state(base_project, 'base-element.bst')
         assert state == 'cached'
diff --git a/tests/artifactcache/junctions/foo/project.conf b/tests/artifactcache/junctions/foo/project.conf
deleted file mode 100644
index 5a240e3ed..000000000
--- a/tests/artifactcache/junctions/foo/project.conf
+++ /dev/null
@@ -1 +0,0 @@
-name: foo
diff --git a/tests/artifactcache/junctions/foo/.bst/workspaces.yml b/tests/artifactcache/junctions/parent/.bst/workspaces.yml
index e69de29bb..e69de29bb 100644
--- a/tests/artifactcache/junctions/foo/.bst/workspaces.yml
+++ b/tests/artifactcache/junctions/parent/.bst/workspaces.yml
diff --git a/tests/artifactcache/junctions/foo/app.bst b/tests/artifactcache/junctions/parent/app.bst
index e658628b0..ad43ddb9b 100644
--- a/tests/artifactcache/junctions/foo/app.bst
+++ b/tests/artifactcache/junctions/parent/app.bst
@@ -4,4 +4,4 @@ sources:
   path: foo.txt
 depends:
 - junction: base.bst
-  filename: target.bst
+  filename: base-element.bst
diff --git a/tests/artifactcache/junctions/foo/base.bst b/tests/artifactcache/junctions/parent/base.bst
index 10ce559a9..10ce559a9 100644
--- a/tests/artifactcache/junctions/foo/base.bst
+++ b/tests/artifactcache/junctions/parent/base.bst
diff --git a/tests/artifactcache/junctions/foo/base/target.bst b/tests/artifactcache/junctions/parent/base/base-element.bst
index 2b61c518b..2b61c518b 100644
--- a/tests/artifactcache/junctions/foo/base/target.bst
+++ b/tests/artifactcache/junctions/parent/base/base-element.bst
diff --git a/tests/artifactcache/junctions/foo/base/base.txt b/tests/artifactcache/junctions/parent/base/base.txt
index a496efee8..a496efee8 100644
--- a/tests/artifactcache/junctions/foo/base/base.txt
+++ b/tests/artifactcache/junctions/parent/base/base.txt
diff --git a/tests/artifactcache/junctions/foo/base/project.conf b/tests/artifactcache/junctions/parent/base/project.conf
index 951ea1a34..951ea1a34 100644
--- a/tests/artifactcache/junctions/foo/base/project.conf
+++ b/tests/artifactcache/junctions/parent/base/project.conf
diff --git a/tests/artifactcache/junctions/foo/foo.txt b/tests/artifactcache/junctions/parent/foo.txt
index 257cc5642..257cc5642 100644
--- a/tests/artifactcache/junctions/foo/foo.txt
+++ b/tests/artifactcache/junctions/parent/foo.txt
diff --git a/tests/artifactcache/junctions/parent/project.conf b/tests/artifactcache/junctions/parent/project.conf
new file mode 100644
index 000000000..620fb42dc
--- /dev/null
+++ b/tests/artifactcache/junctions/parent/project.conf
@@ -0,0 +1 @@
+name: parent
diff --git a/tests/artifactcache/junctions/foo/target.bst b/tests/artifactcache/junctions/parent/target.bst
index 70b78a3fc..092fd9668 100644
--- a/tests/artifactcache/junctions/foo/target.bst
+++ b/tests/artifactcache/junctions/parent/target.bst
@@ -1,5 +1,5 @@
 kind: stack
 depends:
 - junction: base.bst
-  filename: target.bst
+  filename: base-element.bst
 - app.bst
diff --git a/tests/frontend/pull.py b/tests/frontend/pull.py
index 25ff265b1..a87a311d2 100644
--- a/tests/frontend/pull.py
+++ b/tests/frontend/pull.py
@@ -7,7 +7,7 @@ import stat
 import pytest
 from buildstream import utils
 from buildstream.testing import cli  # pylint: disable=unused-import
-from tests.testutils import create_artifact_share, generate_junction
+from tests.testutils import create_artifact_share, generate_junction, assert_shared, assert_not_shared
 
 
 # Project directory
@@ -17,28 +17,6 @@ DATA_DIR = os.path.join(
 )
 
 
-# Assert that a given artifact is in the share
-#
-def assert_shared(cli, share, project, element_name):
-    # NOTE: 'test' here is the name of the project
-    # specified in the project.conf we are testing with.
-    #
-    if not share.has_artifact(cli.get_artifact_name(project, 'test', element_name)):
-        raise AssertionError("Artifact share at {} does not contain the expected element {}"
-                             .format(share.repo, element_name))
-
-
-# Assert that a given artifact is NOT in the share
-#
-def assert_not_shared(cli, share, project, element_name):
-    # NOTE: 'test' here is the name of the project
-    # specified in the project.conf we are testing with.
-    #
-    if share.has_artifact(cli.get_artifact_name(project, 'test', element_name)):
-        raise AssertionError("Artifact share at {} unexpectedly contains the element {}"
-                             .format(share.repo, element_name))
-
-
 # Tests that:
 #
 #  * `bst build` pushes all build elements to configured 'push' cache
diff --git a/tests/frontend/push.py b/tests/frontend/push.py
index a9b072cf7..93b7425b2 100644
--- a/tests/frontend/push.py
+++ b/tests/frontend/push.py
@@ -28,7 +28,8 @@ import pytest
 
 from buildstream._exceptions import ErrorDomain
 from buildstream.testing import cli  # pylint: disable=unused-import
-from tests.testutils import create_artifact_share, create_element_size, generate_junction, wait_for_cache_granularity
+from tests.testutils import create_artifact_share, create_element_size, generate_junction, \
+    wait_for_cache_granularity, assert_shared, assert_not_shared
 
 
 # Project directory
@@ -38,28 +39,6 @@ DATA_DIR = os.path.join(
 )
 
 
-# Assert that a given artifact is in the share
-#
-def assert_shared(cli, share, project, element_name):
-    # NOTE: 'test' here is the name of the project
-    # specified in the project.conf we are testing with.
-    #
-    if not share.has_artifact(cli.get_artifact_name(project, 'test', element_name)):
-        raise AssertionError("Artifact share at {} does not contain the expected element {}"
-                             .format(share.repo, element_name))
-
-
-# Assert that a given artifact is NOT in the share
-#
-def assert_not_shared(cli, share, project, element_name):
-    # NOTE: 'test' here is the name of the project
-    # specified in the project.conf we are testing with.
-    #
-    if share.has_artifact(cli.get_artifact_name(project, 'test', element_name)):
-        raise AssertionError("Artifact share at {} unexpectedly contains the element {}"
-                             .format(share.repo, element_name))
-
-
 # Tests that:
 #
 #  * `bst artifact push` fails if there are no remotes configured for pushing
diff --git a/tests/testutils/__init__.py b/tests/testutils/__init__.py
index 2e1f72138..9a904f007 100644
--- a/tests/testutils/__init__.py
+++ b/tests/testutils/__init__.py
@@ -23,7 +23,7 @@
 #           William Salmon <will.salmon@codethink.co.uk>
 #
 
-from .artifactshare import create_artifact_share
+from .artifactshare import create_artifact_share, assert_shared, assert_not_shared
 from .element_generators import create_element_size, update_element_size
 from .junction import generate_junction
 from .runner_integration import wait_for_cache_granularity
diff --git a/tests/testutils/artifactshare.py b/tests/testutils/artifactshare.py
index bc69a87d8..a5522c8eb 100644
--- a/tests/testutils/artifactshare.py
+++ b/tests/testutils/artifactshare.py
@@ -207,3 +207,19 @@ def create_artifact_share(directory, *, total_space=None, free_space=None,
 
 
 statvfs_result = namedtuple('statvfs_result', 'f_blocks f_bfree f_bsize f_bavail')
+
+
+# Assert that a given artifact is in the share
+#
+def assert_shared(cli, share, project, element_name, *, project_name='test'):
+    if not share.has_artifact(cli.get_artifact_name(project, project_name, element_name)):
+        raise AssertionError("Artifact share at {} does not contain the expected element {}"
+                             .format(share.repo, element_name))
+
+
+# Assert that a given artifact is not in the share
+#
+def assert_not_shared(cli, share, project, element_name, *, project_name='test'):
+    if share.has_artifact(cli.get_artifact_name(project, project_name, element_name)):
+        raise AssertionError("Artifact share at {} unexpectedly contains the element {}"
+                             .format(share.repo, element_name))