Merge branch 'jennis/junctions_and_remotes' into 'master'

Revert default junction behaviour and add new junction config options

See merge request BuildStream/buildstream!1403

7 files changed, 148 insertions, 10 deletions

diff --git a/NEWS b/NEWS
index 5d5906500..6c93dde94 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,14 @@
 buildstream 1.3.1
 =================
 
+  o BREAKING CHANGE: Reverted the default behaviour of junctions. Subproject
+    elements will no longer interact with the parent project's remote (by
+    default). To enable this behaviour, a new "cache-junction-elements" boolean
+    can be optionally declared as part of your junction element's configuration.
+    Additionally, a new "ignore-junction-remotes" option has also been
+    introduced. This allows you to completely ignore subproject remotes when
+    using the parent project.
+
   o Added Basic support for the BuildBox sandbox. The sand box will only be used if the
     environment variable BST_FORCE_SANDBOX is set to `buildbox`. This is the first step in
     transitioning to only using BuildBox for local sandboxing.
diff --git a/doc/source/format_project.rst b/doc/source/format_project.rst
index 8f8a4cfff..d33a4974f 100644
--- a/doc/source/format_project.rst
+++ b/doc/source/format_project.rst
@@ -204,6 +204,7 @@ with an artifact share.
       server-cert: server.crt
     # A remote cache from which to upload/download built/prebuilt artifacts
     - url: https://foo.com:11002
+      push: true
       server-cert: server.crt
       client-cert: client.crt
       client-key: client.key
@@ -241,6 +242,7 @@ Exactly the same as artifact servers, source cache servers can be specified.
       server.cert: server.crt
     # A remote cache from which to upload/download prestaged sources
     - url: https://foo.com:11002
+      push: true
       server-cert: server.crt
       client-cert: client.crt
       client-key: client.key
diff --git a/src/buildstream/_basecache.py b/src/buildstream/_basecache.py
index 52b777fb2..5d93562fd 100644
--- a/src/buildstream/_basecache.py
+++ b/src/buildstream/_basecache.py
@@ -102,10 +102,14 @@ 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_specs + context_specs))
+        return list(utils._deduplicate(
+            project_extra_specs + project_specs + context_specs))
 
     # setup_remotes():
     #
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 9428ab4f6..dff101582 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -643,15 +643,17 @@ class Project():
         #
 
         # Load artifacts pull/push configuration for this project
-        project_specs = ArtifactCache.specs_from_config_node(config, self.directory)
-        override_specs = ArtifactCache.specs_from_config_node(
-            self._context.get_overrides(self.name), self.directory)
-
-        self.artifact_cache_specs = override_specs + project_specs
+        self.artifact_cache_specs = ArtifactCache.specs_from_config_node(config, self.directory)
 
+        # If there is a junction Element which specifies that we want to remotely cache
+        # its elements, append the junction's remotes to the artifact cache specs list
         if self.junction:
             parent = self.junction._get_project()
-            self.artifact_cache_specs = parent.artifact_cache_specs + self.artifact_cache_specs
+            if self.junction.cache_junction_elements:
+                self.artifact_cache_specs = parent.artifact_cache_specs + self.artifact_cache_specs
+
+            if self.junction.ignore_junction_remotes:
+                self.artifact_cache_specs = []
 
         # Load source caches with pull/push config
         self.source_cache_specs = SourceCache.specs_from_config_node(config, self.directory)
diff --git a/src/buildstream/_versions.py b/src/buildstream/_versions.py
index c439f59fb..cbaa52e4f 100644
--- a/src/buildstream/_versions.py
+++ b/src/buildstream/_versions.py
@@ -23,7 +23,7 @@
 # This version is bumped whenever enhancements are made
 # to the `project.conf` format or the core element format.
 #
-BST_FORMAT_VERSION = 24
+BST_FORMAT_VERSION = 25
 
 
 # The base BuildStream artifact version
diff --git a/src/buildstream/plugins/elements/junction.py b/src/buildstream/plugins/elements/junction.py
index b21ef0777..aec32516b 100644
--- a/src/buildstream/plugins/elements/junction.py
+++ b/src/buildstream/plugins/elements/junction.py
@@ -55,10 +55,22 @@ Overview
      # Note that this option cannot be used in conjunction with sources.
      target: sub-project.bst:sub-sub-project.bst
 
+     # Optionally declare whether elements within the junction project
+     # should interact with project remotes (default: False).
+     cache-junction-elements: False
+
+     # Optionally ignore junction remotes, this means that BuildStream
+     # will not attempt to pull artifacts from the junction project's
+     # remote(s) (default: False).
+     ignore-junction-remotes: False
+
 .. note::
 
    The configuration option to allow specifying junction targets is available
-   since :ref:`format version 24 <project_format_version>`.
+   since :ref:`format version 24 <project_format_version>` and the configuration
+   options allowing for junction project elements to interact with parent remotes
+   or to completely ignore junction project remotes are available since
+   :ref:`format version 25 <project_format_version>`.
 
 .. note::
 
@@ -180,6 +192,8 @@ class JunctionElement(Element):
         self.target = node.get_str('target', default=None)
         self.target_element = None
         self.target_junction = None
+        self.cache_junction_elements = node.get_bool('cache-junction-elements', default=False)
+        self.ignore_junction_remotes = node.get_bool('ignore-junction-remotes', default=False)
 
     def preflight(self):
         # "target" cannot be used in conjunction with:
diff --git a/tests/artifactcache/junctions.py b/tests/artifactcache/junctions.py
index 34d6916e8..1fafb11f1 100644
--- a/tests/artifactcache/junctions.py
+++ b/tests/artifactcache/junctions.py
@@ -57,7 +57,7 @@ def test_push_pull(cli, tmpdir, datafiles):
         # 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')
+        assert_not_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')
@@ -87,3 +87,111 @@ def test_push_pull(cli, tmpdir, datafiles):
         assert state == 'cached'
         state = cli.get_element_state(base_project, 'base-element.bst')
         assert state == 'cached'
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_caching_junction_elements(cli, tmpdir, datafiles):
+    project = os.path.join(str(datafiles), 'parent')
+    base_project = os.path.join(str(project), 'base')
+
+    # Load the junction element
+    junction_element = os.path.join(project, 'base.bst')
+    junction_data = _yaml.roundtrip_load(junction_element)
+
+    # Add the "cache-junction-elements" boolean to the junction Element
+    junction_data['config'] = {"cache-junction-elements": True}
+    _yaml.roundtrip_dump(junction_data, junction_element)
+
+    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
+        result = cli.run(project=project, args=['build', 'target.bst'])
+        assert result.exit_code == 0
+
+        # 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, 'base-element.bst')
+        assert state == 'cached'
+
+        project_set_artifacts(project, share.repo)
+        project_set_artifacts(base_project, base_share.repo)
+
+        # Now try bst artifact push
+        result = cli.run(project=project, args=['artifact', 'push', '--deps', 'all', 'target.bst'])
+        assert result.exit_code == 0
+
+        # And finally assert that the artifacts are in the right shares
+        #
+        # The parent project's cache should *also* contain elements from the junction
+        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')
+
+        # The junction project's cache should only contain elements in the junction project
+        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')
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_ignore_junction_remotes(cli, tmpdir, datafiles):
+    project = os.path.join(str(datafiles), 'parent')
+    base_project = os.path.join(str(project), 'base')
+
+    # Load the junction element
+    junction_element = os.path.join(project, 'base.bst')
+    junction_data = _yaml.roundtrip_load(junction_element)
+
+    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:
+
+        # Immediately declare the artifact caches in the appropriate project configs
+        project_set_artifacts(project, share.repo)
+        project_set_artifacts(base_project, base_share.repo)
+
+        # Build and populate the project remotes with their respective elements
+        result = cli.run(project=project, args=['build', 'target.bst'])
+        assert result.exit_code == 0
+
+        # And finally assert that the artifacts are in the right shares
+        #
+        # The parent project's cache should only contain project elements
+        assert_shared(cli, share, project, 'target.bst', project_name='parent')
+        assert_shared(cli, share, project, 'app.bst', project_name='parent')
+        assert_not_shared(cli, share, base_project, 'base-element.bst', project_name='base')
+
+        # The junction project's cache should only contain elements in the junction project
+        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')
+
+        # Ensure that, from now on, we ignore junction element remotes
+        junction_data['config'] = {"ignore-junction-remotes": True}
+        _yaml.roundtrip_dump(junction_data, junction_element)
+
+        # Now delete everything from the local cache and try to
+        # redownload from the shares.
+        #
+        cas = os.path.join(cli.directory, 'cas')
+        shutil.rmtree(cas)
+        artifact_dir = os.path.join(cli.directory, 'artifacts')
+        shutil.rmtree(artifact_dir)
+
+        # 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, 'base-element.bst')
+        assert state != 'cached'
+
+        # Now try bst artifact pull
+        result = cli.run(project=project, args=['artifact', 'pull', '--deps', 'all', 'target.bst'])
+        assert result.exit_code == 0
+
+        # And assert that they are again in the local cache, without having built
+        state = cli.get_element_state(project, 'target.bst')
+        assert state == 'cached'
+        # We shouldn't be able to download base-element!
+        state = cli.get_element_state(base_project, 'base-element.bst')
+        assert state != 'cached'