_frontend/cli.py, _pipeline.py: Add options for cross junction tracking.

This patch makes cross junction tracking disabled by default, which
was the initial intention when landing project.refs but never got around
to doing this (intended to get addressing of junctioned elements via
command line sorted first, but didnt happen).

This adds the following options to enable cross-junction tracking:

  o bst build -J / --track-cross-junctions

  o bst fetch -J / --track-cross-junctions

  o bst track -J / --cross-junctions

This also fixes `bst fetch --track` which had a bug, it was avoiding
to track and fetch elements which are in a `cached` consistency state,
which is wrong when `--track` is specified.

This also updates some test cases which were broken by
this change.

This fixes issue #354

5 files changed, 112 insertions, 50 deletions

diff --git a/buildstream/_frontend/app.py b/buildstream/_frontend/app.py
index 2d5842c32..fa6a1b800 100644
--- a/buildstream/_frontend/app.py
+++ b/buildstream/_frontend/app.py
@@ -414,7 +414,7 @@ class App():
         # If we're going to checkout, we need at least a fetch,
         # if we were asked to track first, we're going to fetch anyway.
         if not no_checkout or track_first:
-            self.pipeline.fetch(self.scheduler, [target], track_first)
+            self.pipeline.fetch(self.scheduler, [target], track_first=track_first)
 
         if not no_checkout and target._get_consistency() != Consistency.CACHED:
             raise PipelineError("Could not stage uncached source. " +
diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py
index bd4fd0e5b..0234ebb4f 100644
--- a/buildstream/_frontend/cli.py
+++ b/buildstream/_frontend/cli.py
@@ -205,16 +205,19 @@ def init(app, project_name, format_version, element_path, force):
 @click.option('--track-except', multiple=True,
               type=click.Path(dir_okay=False, readable=True),
               help="Except certain dependencies from tracking")
+@click.option('--track-cross-junctions', '-J', default=False, is_flag=True,
+              help="Allow tracking to cross junction boundaries")
 @click.option('--track-save', default=False, is_flag=True,
               help="Deprecated: This is ignored")
 @click.argument('elements', nargs=-1,
                 type=click.Path(dir_okay=False, readable=True))
 @click.pass_obj
-def build(app, elements, all_, track_, track_save, track_all, track_except):
+def build(app, elements, all_, track_, track_save, track_all, track_except, track_cross_junctions):
     """Build elements in a pipeline"""
 
-    if track_except and not (track_ or track_all):
-        click.echo("ERROR: --track-except cannot be used without --track or --track-all", err=True)
+    if (track_except or track_cross_junctions) and not (track_ or track_all):
+        click.echo("ERROR: The --track-except and --track-cross-junctions options "
+                   "can only be used with --track or --track-all", err=True)
         sys.exit(-1)
 
     if track_save:
@@ -230,7 +233,8 @@ def build(app, elements, all_, track_, track_save, track_all, track_except):
     with app.initialized(elements, session_name="Build", except_=track_except, rewritable=rewritable,
                          use_configured_remote_caches=True, track_elements=track_,
                          fetch_subprojects=True):
-        app.pipeline.build(app.scheduler, all_, track_)
+        app.pipeline.build(app.scheduler, build_all=all_, track_first=track_,
+                           track_cross_junctions=track_cross_junctions)
 
 
 ##################################################################
@@ -245,10 +249,12 @@ def build(app, elements, all_, track_, track_save, track_all, track_except):
               help='The dependencies to fetch (default: plan)')
 @click.option('--track', 'track_', default=False, is_flag=True,
               help="Track new source references before fetching")
+@click.option('--track-cross-junctions', '-J', default=False, is_flag=True,
+              help="Allow tracking to cross junction boundaries")
 @click.argument('elements', nargs=-1,
                 type=click.Path(dir_okay=False, readable=True))
 @click.pass_obj
-def fetch(app, elements, deps, track_, except_):
+def fetch(app, elements, deps, track_, except_, track_cross_junctions):
     """Fetch sources required to build the pipeline
 
     By default this will only try to fetch sources which are
@@ -263,11 +269,16 @@ def fetch(app, elements, deps, track_, except_):
         plan:  Only dependencies required for the build plan
         all:   All dependencies
     """
+    if track_cross_junctions and not track_:
+        click.echo("ERROR: The --track-cross-junctions option can only be used with --track", err=True)
+        sys.exit(-1)
+
     with app.initialized(elements, session_name="Fetch", except_=except_, rewritable=track_,
                          track_elements=elements if track_ else None,
                          fetch_subprojects=True):
         dependencies = app.pipeline.deps_elements(deps)
-        app.pipeline.fetch(app.scheduler, dependencies, track_)
+        app.pipeline.fetch(app.scheduler, dependencies, track_first=track_,
+                           track_cross_junctions=track_cross_junctions)
 
 
 ##################################################################
@@ -280,10 +291,12 @@ def fetch(app, elements, deps, track_, except_):
 @click.option('--deps', '-d', default='none',
               type=click.Choice(['none', 'all']),
               help='The dependencies to track (default: none)')
+@click.option('--cross-junctions', '-J', default=False, is_flag=True,
+              help="Allow crossing junction boundaries")
 @click.argument('elements', nargs=-1,
                 type=click.Path(dir_okay=False, readable=True))
 @click.pass_obj
-def track(app, elements, deps, except_):
+def track(app, elements, deps, except_, cross_junctions):
     """Consults the specified tracking branches for new versions available
     to build and updates the project with any newly available references.
 
@@ -293,13 +306,13 @@ def track(app, elements, deps, except_):
     Specify `--deps` to control which sources to track:
 
     \b
-        none:  No dependencies, just the element itself
-        all:   All dependencies
+        none:  No dependencies, just the specified elements
+        all:   All dependencies of all specified elements
     """
     with app.initialized(elements, session_name="Track", except_=except_, rewritable=True,
                          track_elements=elements, fetch_subprojects=True):
         dependencies = app.pipeline.deps_elements(deps)
-        app.pipeline.track(app.scheduler, dependencies)
+        app.pipeline.track(app.scheduler, dependencies, cross_junctions=cross_junctions)
 
 
 ##################################################################
diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py
index 969c3f6e5..5cef4ceb2 100644
--- a/buildstream/_pipeline.py
+++ b/buildstream/_pipeline.py
@@ -222,19 +222,23 @@ class Pipeline():
     # Args:
     #    scheduler (Scheduler): The scheduler to run this pipeline on
     #    dependencies (list): List of elements to track
+    #    cross_junctions (bool): Whether to allow cross junction tracking
     #
     # If no error is encountered while tracking, then the project files
     # are rewritten inline.
     #
-    def track(self, scheduler, dependencies):
+    def track(self, scheduler, dependencies, *, cross_junctions=False):
 
         dependencies = list(dependencies)
+        if cross_junctions:
+            self._assert_junction_tracking(dependencies)
+        else:
+            dependencies = self._filter_cross_junctions(dependencies)
+
         track = TrackQueue()
         track.enqueue(dependencies)
         self.session_elements = len(dependencies)
 
-        self._assert_junction_tracking(dependencies, build=False)
-
         _, status = scheduler.run([track])
         if status == SchedStatus.ERROR:
             raise PipelineError()
@@ -249,30 +253,44 @@ class Pipeline():
     #    scheduler (Scheduler): The scheduler to run this pipeline on
     #    dependencies (list): List of elements to fetch
     #    track_first (bool): Track new source references before fetching
+    #    track_cross_junctions (bool): Whether to allow cross junction tracking
     #
-    def fetch(self, scheduler, dependencies, track_first):
-
-        plan = dependencies
+    def fetch(self, scheduler, dependencies, *, track_first=False, track_cross_junctions=False):
+        fetch_plan = dependencies
+        track_plan = []
 
         # Assert that we have a consistent pipeline, or that
         # the track option will make it consistent
-        if not track_first:
-            self._assert_consistent(plan)
+        if track_first:
+            track_plan = fetch_plan
 
-        # Filter out elements with cached sources, we already have them.
-        cached = [elt for elt in plan if elt._get_consistency() == Consistency.CACHED]
-        plan = [elt for elt in plan if elt not in cached]
+            if track_cross_junctions:
+                self._assert_junction_tracking(track_plan)
+            else:
+                track_plan = self._filter_cross_junctions(track_plan)
 
-        self.session_elements = len(plan)
+            # Subtract the track elements from the fetch elements, they will be added separately
+            track_elements = set(track_plan)
+            fetch_plan = [e for e in fetch_plan if e not in track_elements]
+        else:
+            # If we're not going to track first, we need to make sure
+            # the elements are not in an inconsistent state
+            self._assert_consistent(fetch_plan)
+
+        # Filter out elements with cached sources, only from the fetch plan
+        # let the track plan resolve new refs.
+        cached = [elt for elt in fetch_plan if elt._get_consistency() == Consistency.CACHED]
+        fetch_plan = [elt for elt in fetch_plan if elt not in cached]
+
+        self.session_elements = len(track_plan) + len(fetch_plan)
 
         fetch = FetchQueue()
+        fetch.enqueue(fetch_plan)
         if track_first:
             track = TrackQueue()
-            track.enqueue(plan)
+            track.enqueue(track_plan)
             queues = [track, fetch]
         else:
-            track = None
-            fetch.enqueue(plan)
             queues = [fetch]
 
         _, status = scheduler.run(queues)
@@ -300,8 +318,9 @@ class Pipeline():
     #                      which are required to build the target.
     #    track_first (list): Elements whose sources to track prior to
     #                        building
+    #    track_cross_junctions (bool): Whether to allow cross junction tracking
     #
-    def build(self, scheduler, build_all, track_first):
+    def build(self, scheduler, *, build_all=False, track_first=False, track_cross_junctions=False):
         unused_workspaces = self._collect_unused_workspaces()
         if unused_workspaces:
             self._message(MessageType.WARN, "Unused workspaces",
@@ -318,7 +337,10 @@ class Pipeline():
         if track_first:
             track_plan = self.get_elements_to_track(track_first)
 
-        self._assert_junction_tracking(track_plan, build=True)
+        if track_cross_junctions:
+            self._assert_junction_tracking(track_plan)
+        else:
+            track_plan = self._filter_cross_junctions(track_plan)
 
         if build_all:
             plan = self.dependencies(Scope.ALL)
@@ -573,7 +595,7 @@ class Pipeline():
                                 .format(tar_location, e)) from e
 
         plan = list(dependencies)
-        self.fetch(scheduler, plan, track_first)
+        self.fetch(scheduler, plan, track_first=track_first)
 
         # We don't use the scheduler for this as it is almost entirely IO
         # bound.
@@ -723,6 +745,23 @@ class Pipeline():
                 detail += "  " + element.name + "\n"
             raise PipelineError("Inconsistent pipeline", detail=detail, reason="inconsistent-pipeline")
 
+    # _filter_cross_junction()
+    #
+    # Filters out cross junction elements from the elements
+    #
+    # Args:
+    #    elements (list of Element): The list of elements to be tracked
+    #
+    # Returns:
+    #    (list): A filtered list of `elements` which does
+    #            not contain any cross junction elements.
+    #
+    def _filter_cross_junctions(self, elements):
+        return [
+            element for element in elements
+            if element._get_project() is self.project
+        ]
+
     # _assert_junction_tracking()
     #
     # Raises an error if tracking is attempted on junctioned elements and
@@ -730,12 +769,8 @@ class Pipeline():
     #
     # Args:
     #    elements (list of Element): The list of elements to be tracked
-    #    build (bool): Whether this is being called for `bst build`, otherwise `bst track`
     #
-    # The `build` argument is only useful for suggesting an appropriate
-    # alternative to the user
-    #
-    def _assert_junction_tracking(self, elements, *, build):
+    def _assert_junction_tracking(self, elements):
 
         # We can track anything if the toplevel project uses project.refs
         #
@@ -750,13 +785,8 @@ class Pipeline():
         for element in elements:
             element_project = element._get_project()
             if element_project is not self.project:
-                suggestion = '--except'
-                if build:
-                    suggestion = '--track-except'
-
                 detail = "Requested to track sources across junction boundaries\n" + \
-                         "in a project which does not use separate source references.\n\n" + \
-                         "Try using `{}` arguments to limit the scope of tracking.".format(suggestion)
+                         "in a project which does not use project.refs ref-storage."
 
                 raise PipelineError("Untrackable sources", detail=detail, reason="untrackable-sources")
 
diff --git a/tests/completions/completions.py b/tests/completions/completions.py
index e253e9d26..cc98cb940 100644
--- a/tests/completions/completions.py
+++ b/tests/completions/completions.py
@@ -99,7 +99,9 @@ def test_commands(cli, cmd, word_idx, expected):
 
     # Test that options of subcommands also complete
     ('bst --no-colors build -', 3, ['--all ', '--track ', '--track-all ',
-                                    '--track-except ', '--track-save ']),
+                                    '--track-except ',
+                                    '--track-cross-junctions ', '-J ',
+                                    '--track-save ']),
 
     # Test the behavior of completing after an option that has a
     # parameter that cannot be completed, vs an option that has
diff --git a/tests/frontend/track.py b/tests/frontend/track.py
index 6fa61f343..5142dee45 100644
--- a/tests/frontend/track.py
+++ b/tests/frontend/track.py
@@ -215,8 +215,9 @@ def test_track_optional(cli, tmpdir, datafiles, ref_storage):
 
 
 @pytest.mark.datafiles(os.path.join(TOP_DIR, 'track-cross-junction'))
+@pytest.mark.parametrize("cross_junction", [('cross'), ('nocross')])
 @pytest.mark.parametrize("ref_storage", [('inline'), ('project.refs')])
-def test_track_cross_junction(cli, tmpdir, datafiles, ref_storage):
+def test_track_cross_junction(cli, tmpdir, datafiles, cross_junction, ref_storage):
     project = os.path.join(datafiles.dirname, datafiles.basename)
     dev_files_path = os.path.join(project, 'files')
     target_path = os.path.join(project, 'target.bst')
@@ -267,14 +268,27 @@ def test_track_cross_junction(cli, tmpdir, datafiles, ref_storage):
     assert get_subproject_element_state() == 'no reference'
 
     # Track recursively across the junction
-    result = cli.run(project=project, args=['track', '--deps', 'all', 'target.bst'])
+    args = ['track', '--deps', 'all']
+    if cross_junction == 'cross':
+        args += ['--cross-junctions']
+    args += ['target.bst']
+
+    result = cli.run(project=project, args=args)
 
     if ref_storage == 'inline':
-        #
-        # Cross junction tracking is not allowed when the toplevel project
-        # is using inline ref storage.
-        #
-        result.assert_main_error(ErrorDomain.PIPELINE, 'untrackable-sources')
+
+        if cross_junction == 'cross':
+            #
+            # Cross junction tracking is not allowed when the toplevel project
+            # is using inline ref storage.
+            #
+            result.assert_main_error(ErrorDomain.PIPELINE, 'untrackable-sources')
+        else:
+            #
+            # No cross juction tracking was requested
+            #
+            result.assert_success()
+            assert get_subproject_element_state() == 'no reference'
     else:
         #
         # Tracking is allowed with project.refs ref storage
@@ -282,9 +296,12 @@ def test_track_cross_junction(cli, tmpdir, datafiles, ref_storage):
         result.assert_success()
 
         #
-        # Assert that we now have a ref for the subproject element
+        # If cross junction tracking was enabled, we should now be buildable
         #
-        assert get_subproject_element_state() == 'buildable'
+        if cross_junction == 'cross':
+            assert get_subproject_element_state() == 'buildable'
+        else:
+            assert get_subproject_element_state() == 'no reference'
 
 
 @pytest.mark.datafiles(os.path.join(TOP_DIR, 'consistencyerror'))