Deprecate `--track-save` option from `bst build`

It makes no sense to support this now that we are adding project.refs,
instead of removing the option completely, we document it as
deprecated and print a warning.

  o _frontend/cli.py: Mark the option as deprecated, warn if it's used

  o _scheduler/trackqueue.py: Remove save optionality

  o _pipeline.py: Remove save optionality

  o tests/frontend/buildtrack.py: Stop testing for no-save functionality,

    This test was actually broken, and only save functionality was being
    tested. This was due to using a list [True] or [False] in the
    @pytest.mark.parameterize() statement, both of which are truthy values.

4 files changed, 27 insertions, 37 deletions

diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py
index 1f8ebbbb9..c0bf74117 100644
--- a/buildstream/_frontend/cli.py
+++ b/buildstream/_frontend/cli.py
@@ -180,7 +180,7 @@ def cli(context, **kwargs):
               type=click.Path(dir_okay=False, readable=True),
               help="Except certain dependencies from tracking")
 @click.option('--track-save', default=False, is_flag=True,
-              help="Write out the tracked references to their element files")
+              help="Deprecated: This is ignored")
 @click.argument('elements', nargs=-1,
                 type=click.Path(dir_okay=False, readable=True))
 @click.pass_obj
@@ -188,22 +188,25 @@ def build(app, elements, all_, track_, track_save, track_all, track_except):
     """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")
+        click.echo("ERROR: --track-except cannot be used without --track or --track-all", err=True)
         sys.exit(-1)
 
-    if track_save and not (track_ or track_all):
-        click.echo("ERROR: --track-save cannot be used without --track or --track-all")
-        sys.exit(-1)
+    if track_save:
+        click.echo("WARNING: --track-save is deprecated, saving is now unconditional", err=True)
 
     if track_all:
         track_ = elements
 
-    app.initialize(elements, except_=track_except, rewritable=track_save,
+    rewritable = False
+    if track_:
+        rewritable = True
+
+    app.initialize(elements, except_=track_except, rewritable=rewritable,
                    use_configured_remote_caches=True, track_elements=track_,
                    fetch_subprojects=True)
     app.print_heading()
     try:
-        app.pipeline.build(app.scheduler, all_, track_, track_save)
+        app.pipeline.build(app.scheduler, all_, track_)
         app.print_summary()
     except PipelineError as e:
         app.print_error(e)
diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py
index 42dab3b01..15fcf42fd 100644
--- a/buildstream/_pipeline.py
+++ b/buildstream/_pipeline.py
@@ -318,7 +318,7 @@ class Pipeline():
     def track(self, scheduler, dependencies):
 
         dependencies = list(dependencies)
-        track = TrackQueue(save=True)
+        track = TrackQueue()
         track.enqueue(dependencies)
         self.session_elements = len(dependencies)
 
@@ -409,10 +409,8 @@ class Pipeline():
     #                      which are required to build the target.
     #    track_first (list): Elements whose sources to track prior to
     #                        building
-    #    save (bool): Whether to save the tracking results in the
-    #                 elements
     #
-    def build(self, scheduler, build_all, track_first, save):
+    def build(self, scheduler, build_all, track_first):
         if self.unused_workspaces:
             self.message(MessageType.WARN, "Unused workspaces",
                          detail="\n".join([el for el, _
@@ -450,7 +448,7 @@ class Pipeline():
         push = None
         queues = []
         if track_plan:
-            track = TrackQueue(save=save)
+            track = TrackQueue()
             queues.append(track)
         if self.artifacts.has_fetch_remotes():
             pull = PullQueue()
diff --git a/buildstream/_scheduler/trackqueue.py b/buildstream/_scheduler/trackqueue.py
index 50ae804ac..c92239053 100644
--- a/buildstream/_scheduler/trackqueue.py
+++ b/buildstream/_scheduler/trackqueue.py
@@ -38,10 +38,6 @@ class TrackQueue(Queue):
     complete_name = "Tracked"
     queue_type = QueueType.FETCH
 
-    def __init__(self, save=True):
-        super(TrackQueue, self).__init__()
-        self.save = save
-
     def process(self, element):
         return element._track()
 
@@ -72,19 +68,18 @@ class TrackQueue(Queue):
 
                 # Here we are in master process, what to do if writing
                 # to the disk fails for some reason ?
-                if self.save:
-                    try:
-                        _yaml.dump(toplevel, fullname)
-                    except OSError as e:
-                        # FIXME: We currently dont have a clear path to
-                        #        fail the scheduler from the main process, so
-                        #        this will just warn and BuildStream will exit
-                        #        with a success code.
-                        #
-                        source.warn("Failed to update project file",
-                                    detail="{}: Failed to rewrite "
-                                    "tracked source to file {}: {}"
-                                    .format(source, fullname, e))
+                try:
+                    _yaml.dump(toplevel, fullname)
+                except OSError as e:
+                    # FIXME: We currently dont have a clear path to
+                    #        fail the scheduler from the main process, so
+                    #        this will just warn and BuildStream will exit
+                    #        with a success code.
+                    #
+                    source.warn("Failed to update project file",
+                                detail="{}: Failed to rewrite "
+                                "tracked source to file {}: {}"
+                                .format(source, fullname, e))
 
         context = element._get_context()
         context._push_message_depth(True)
diff --git a/tests/frontend/buildtrack.py b/tests/frontend/buildtrack.py
index c351080b9..9ce1396f9 100644
--- a/tests/frontend/buildtrack.py
+++ b/tests/frontend/buildtrack.py
@@ -28,7 +28,6 @@ def create_element(repo, name, path, dependencies, ref=None):
     _yaml.dump(element, os.path.join(path, name))
 
 
-@pytest.mark.parametrize("save", [([True]), ([False])])
 @pytest.mark.datafiles(os.path.join(DATA_DIR))
 @pytest.mark.parametrize("exceptions,excepted", [
     # Test with no exceptions
@@ -63,7 +62,7 @@ def create_element(repo, name, path, dependencies, ref=None):
     ])
 ])
 def test_build_track(cli, datafiles, tmpdir, track_targets,
-                     exceptions, tracked, excepted, save):
+                     exceptions, tracked, excepted):
     project = os.path.join(datafiles.dirname, datafiles.basename)
     dev_files_path = os.path.join(project, 'files', 'dev-files')
     element_path = os.path.join(project, 'elements')
@@ -101,8 +100,6 @@ def test_build_track(cli, datafiles, tmpdir, track_targets,
             create_element(repo, element, element_path, dependencies, ref=ref)
 
     args = ['build']
-    if save:
-        args += ['--track-save']
     args += itertools.chain.from_iterable(zip(itertools.repeat('--track'), track_targets))
     args += itertools.chain.from_iterable(zip(itertools.repeat('--track-except'), exceptions))
     args += ['0.bst']
@@ -119,10 +116,7 @@ def test_build_track(cli, datafiles, tmpdir, track_targets,
         source_dir = os.path.join(project, 'cache', 'sources')
         shutil.rmtree(source_dir)
 
-        if not save:
-            assert cli.get_element_state(project, target) == 'no reference'
-        else:
-            assert cli.get_element_state(project, target) == 'fetch needed'
+        assert cli.get_element_state(project, target) == 'fetch needed'
 
 
 @pytest.mark.datafiles(os.path.join(DATA_DIR))