Merge branch 'phil/source-checkout-options' into 'master'

Retire bst source bundle command

Closes #672

See merge request BuildStream/buildstream!959

8 files changed, 167 insertions, 179 deletions

diff --git a/NEWS b/NEWS
index 1212b2f4b..91897f677 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,12 @@
 buildstream 1.3.1
 =================
 
+  o BREAKING CHANGE: The bst source-bundle command has been removed. The
+    functionality it provided has been replaced by the `--include-build-scripts`
+    option of the `bst source-checkout` command. To produce a tarball containing
+    an element's sources and generated build scripts you can do the command
+    `bst source-checkout --include-build-scripts --tar foo.bst some-file.tar`
+
   o BREAKING CHANGE: Default strip-commands have been removed as they are too
     specific. Recommendation if you are building in Linux is to use the
     ones being used in freedesktop-sdk project, for example
diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py
index 4900b28a7..f2f87e721 100644
--- a/buildstream/_frontend/cli.py
+++ b/buildstream/_frontend/cli.py
@@ -725,6 +725,8 @@ def checkout(app, element, location, force, deps, integrate, hardlinks, tar):
 #                  Source Checkout Command                      #
 ##################################################################
 @cli.command(name='source-checkout', short_help='Checkout sources for an element')
+@click.option('--force', '-f', default=False, is_flag=True,
+              help="Allow files to be overwritten")
 @click.option('--except', 'except_', multiple=True,
               type=click.Path(readable=False),
               help="Except certain dependencies")
@@ -733,11 +735,15 @@ def checkout(app, element, location, force, deps, integrate, hardlinks, tar):
               help='The dependencies whose sources to checkout (default: none)')
 @click.option('--fetch', 'fetch_', default=False, is_flag=True,
               help='Fetch elements if they are not fetched')
-@click.argument('element', required=False,
-                type=click.Path(readable=False))
+@click.option('--tar', 'tar', default=False, is_flag=True,
+              help='Create a tarball from the element\'s sources instead of a '
+                   'file tree.')
+@click.option('--include-build-scripts', 'build_scripts', is_flag=True)
+@click.argument('element', required=False, type=click.Path(readable=False))
 @click.argument('location', type=click.Path(), required=False)
 @click.pass_obj
-def source_checkout(app, element, location, deps, fetch_, except_):
+def source_checkout(app, element, location, force, deps, fetch_, except_,
+                    tar, build_scripts):
     """Checkout sources of an element to the specified location
     """
     if not element and not location:
@@ -757,9 +763,12 @@ def source_checkout(app, element, location, deps, fetch_, except_):
 
         app.stream.source_checkout(element,
                                    location=location,
+                                   force=force,
                                    deps=deps,
                                    fetch=fetch_,
-                                   except_targets=except_)
+                                   except_targets=except_,
+                                   tar=tar,
+                                   include_build_scripts=build_scripts)
 
 
 ##################################################################
@@ -906,34 +915,3 @@ def workspace_list(app):
 
     with app.initialized():
         app.stream.workspace_list()
-
-
-##################################################################
-#                     Source Bundle Command                      #
-##################################################################
-@cli.command(name="source-bundle", short_help="Produce a build bundle to be manually executed")
-@click.option('--except', 'except_', multiple=True,
-              type=click.Path(readable=False),
-              help="Elements to except from the tarball")
-@click.option('--compression', default='gz',
-              type=click.Choice(['none', 'gz', 'bz2', 'xz']),
-              help="Compress the tar file using the given algorithm.")
-@click.option('--track', 'track_', default=False, is_flag=True,
-              help="Track new source references before bundling")
-@click.option('--force', '-f', default=False, is_flag=True,
-              help="Overwrite an existing tarball")
-@click.option('--directory', default=os.getcwd(),
-              help="The directory to write the tarball to")
-@click.argument('element',
-                type=click.Path(readable=False))
-@click.pass_obj
-def source_bundle(app, element, force, directory,
-                  track_, compression, except_):
-    """Produce a source bundle to be manually executed
-    """
-    with app.initialized():
-        app.stream.source_bundle(element, directory,
-                                 track_first=track_,
-                                 force=force,
-                                 compression=compression,
-                                 except_targets=except_)
diff --git a/buildstream/_stream.py b/buildstream/_stream.py
index 85f77e281..04eb409a5 100644
--- a/buildstream/_stream.py
+++ b/buildstream/_stream.py
@@ -25,8 +25,8 @@ import stat
 import shlex
 import shutil
 import tarfile
-from contextlib import contextmanager
-from tempfile import TemporaryDirectory
+import tempfile
+from contextlib import contextmanager, suppress
 
 from ._exceptions import StreamError, ImplError, BstError, set_last_task_error
 from ._message import Message, MessageType
@@ -449,11 +449,14 @@ class Stream():
     #
     def source_checkout(self, target, *,
                         location=None,
+                        force=False,
                         deps='none',
                         fetch=False,
-                        except_targets=()):
+                        except_targets=(),
+                        tar=False,
+                        include_build_scripts=False):
 
-        self._check_location_writable(location)
+        self._check_location_writable(location, force=force, tar=tar)
 
         elements, _ = self._load((target,), (),
                                  selection=deps,
@@ -467,7 +470,8 @@ class Stream():
 
         # Stage all sources determined by scope
         try:
-            self._write_element_sources(location, elements)
+            self._source_checkout(elements, location, force, deps,
+                                  fetch, tar, include_build_scripts)
         except BstError as e:
             raise StreamError("Error while writing sources"
                               ": '{}'".format(e), detail=e.detail, reason=e.reason) from e
@@ -728,87 +732,6 @@ class Stream():
             'workspaces': workspaces
         })
 
-    # source_bundle()
-    #
-    # Create a host buildable tarball bundle for the given target.
-    #
-    # Args:
-    #    target (str): The target element to bundle
-    #    directory (str): The directory to output the tarball
-    #    track_first (bool): Track new source references before bundling
-    #    compression (str): The compression type to use
-    #    force (bool): Overwrite an existing tarball
-    #
-    def source_bundle(self, target, directory, *,
-                      track_first=False,
-                      force=False,
-                      compression="gz",
-                      except_targets=()):
-
-        if track_first:
-            track_targets = (target,)
-        else:
-            track_targets = ()
-
-        elements, track_elements = self._load((target,), track_targets,
-                                              selection=PipelineSelection.ALL,
-                                              except_targets=except_targets,
-                                              track_selection=PipelineSelection.ALL,
-                                              fetch_subprojects=True)
-
-        # source-bundle only supports one target
-        target = self.targets[0]
-
-        self._message(MessageType.INFO, "Bundling sources for target {}".format(target.name))
-
-        # Find the correct filename for the compression algorithm
-        tar_location = os.path.join(directory, target.normal_name + ".tar")
-        if compression != "none":
-            tar_location += "." + compression
-
-        # Attempt writing a file to generate a good error message
-        # early
-        #
-        # FIXME: A bit hackish
-        try:
-            open(tar_location, mode="x")
-            os.remove(tar_location)
-        except IOError as e:
-            raise StreamError("Cannot write to {0}: {1}"
-                              .format(tar_location, e)) from e
-
-        # Fetch and possibly track first
-        #
-        self._fetch(elements, track_elements=track_elements)
-
-        # We don't use the scheduler for this as it is almost entirely IO
-        # bound.
-
-        # Create a temporary directory to build the source tree in
-        builddir = self._context.builddir
-        os.makedirs(builddir, exist_ok=True)
-        prefix = "{}-".format(target.normal_name)
-
-        with TemporaryDirectory(prefix=prefix, dir=builddir) as tempdir:
-            source_directory = os.path.join(tempdir, 'source')
-            try:
-                os.makedirs(source_directory)
-            except OSError as e:
-                raise StreamError("Failed to create directory: {}"
-                                  .format(e)) from e
-
-            # Any elements that don't implement _write_script
-            # should not be included in the later stages.
-            elements = [
-                element for element in elements
-                if self._write_element_script(source_directory, element)
-            ]
-
-            self._write_element_sources(os.path.join(tempdir, "source"), elements)
-            self._write_build_script(tempdir, elements)
-            self._collect_sources(tempdir, tar_location,
-                                  target.normal_name, compression)
-
     # redirect_element_names()
     #
     # Takes a list of element names and returns a list where elements have been
@@ -1189,6 +1112,54 @@ class Stream():
 
         sandbox_vroot.export_files(directory, can_link=True, can_destroy=True)
 
+    # Helper function for source_checkout()
+    def _source_checkout(self, elements,
+                         location=None,
+                         force=False,
+                         deps='none',
+                         fetch=False,
+                         tar=False,
+                         include_build_scripts=False):
+        location = os.path.abspath(location)
+        location_parent = os.path.abspath(os.path.join(location, ".."))
+
+        # Stage all our sources in a temporary directory. The this
+        # directory can be used to either construct a tarball or moved
+        # to the final desired location.
+        temp_source_dir = tempfile.TemporaryDirectory(dir=location_parent)
+        try:
+            self._write_element_sources(temp_source_dir.name, elements)
+            if include_build_scripts:
+                self._write_build_scripts(temp_source_dir.name, elements)
+            if tar:
+                self._create_tarball(temp_source_dir.name, location)
+            else:
+                self._move_directory(temp_source_dir.name, location, force)
+        except OSError as e:
+            raise StreamError("Failed to checkout sources to {}: {}"
+                              .format(location, e)) from e
+        finally:
+            with suppress(FileNotFoundError):
+                temp_source_dir.cleanup()
+
+    # Move a directory src to dest. This will work across devices and
+    # may optionaly overwrite existing files.
+    def _move_directory(self, src, dest, force=False):
+        def is_empty_dir(path):
+            return os.path.isdir(dest) and not os.listdir(dest)
+
+        try:
+            os.rename(src, dest)
+            return
+        except OSError:
+            pass
+
+        if force or is_empty_dir(dest):
+            try:
+                utils.link_files(src, dest)
+            except utils.UtilError as e:
+                raise StreamError("Failed to move directory: {}".format(e)) from e
+
     # Write the element build script to the given directory
     def _write_element_script(self, directory, element):
         try:
@@ -1205,8 +1176,28 @@ class Stream():
                 os.makedirs(element_source_dir)
                 element._stage_sources_at(element_source_dir, mount_workspaces=False)
 
+    # Create a tarball from the content of directory
+    def _create_tarball(self, directory, tar_name):
+        try:
+            with utils.save_file_atomic(tar_name, mode='wb') as f:
+                # This TarFile does not need to be explicitly closed
+                # as the underlying file object will be closed be the
+                # save_file_atomic contect manager
+                tarball = tarfile.open(fileobj=f, mode='w')
+                for item in os.listdir(str(directory)):
+                    file_to_add = os.path.join(directory, item)
+                    tarball.add(file_to_add, arcname=item)
+        except OSError as e:
+            raise StreamError("Failed to create tar archive: {}".format(e)) from e
+
+    # Write all the build_scripts for elements in the directory location
+    def _write_build_scripts(self, location, elements):
+        for element in elements:
+            self._write_element_script(location, element)
+        self._write_master_build_script(location, elements)
+
     # Write a master build script to the sandbox
-    def _write_build_script(self, directory, elements):
+    def _write_master_build_script(self, directory, elements):
 
         module_string = ""
         for element in elements:
diff --git a/doc/source/using_commands.rst b/doc/source/using_commands.rst
index 18affc6e5..90d86dcbb 100644
--- a/doc/source/using_commands.rst
+++ b/doc/source/using_commands.rst
@@ -86,13 +86,6 @@ project's main directory.
 
 ----
 
-.. _invoking_source_bundle:
-
-.. click:: buildstream._frontend.cli:source_bundle
-   :prog: bst source bundle
-
-----
-
 .. _invoking_workspace:
 
 .. click:: buildstream._frontend.cli:workspace
diff --git a/tests/completions/completions.py b/tests/completions/completions.py
index af35fb23a..9b32baadd 100644
--- a/tests/completions/completions.py
+++ b/tests/completions/completions.py
@@ -16,7 +16,6 @@ MAIN_COMMANDS = [
     'shell ',
     'show ',
     'source-checkout ',
-    'source-bundle ',
     'track ',
     'workspace '
 ]
diff --git a/tests/frontend/help.py b/tests/frontend/help.py
index bdafe851b..2a0c502e3 100644
--- a/tests/frontend/help.py
+++ b/tests/frontend/help.py
@@ -25,7 +25,6 @@ def test_help_main(cli):
     ('push'),
     ('shell'),
     ('show'),
-    ('source-bundle'),
     ('track'),
     ('workspace')
 ])
diff --git a/tests/frontend/source_bundle.py b/tests/frontend/source_bundle.py
deleted file mode 100644
index f72e80a3b..000000000
--- a/tests/frontend/source_bundle.py
+++ /dev/null
@@ -1,48 +0,0 @@
-#
-#  Copyright (C) 2018 Bloomberg Finance LP
-#
-#  This program is free software; you can redistribute it and/or
-#  modify it under the terms of the GNU Lesser General Public
-#  License as published by the Free Software Foundation; either
-#  version 2 of the License, or (at your option) any later version.
-#
-#  This library is distributed in the hope that it will be useful,
-#  but WITHOUT ANY WARRANTY; without even the implied warranty of
-#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-#  Lesser General Public License for more details.
-#
-#  You should have received a copy of the GNU Lesser General Public
-#  License along with this library. If not, see <http://www.gnu.org/licenses/>.
-#
-#  Authors: Chandan Singh <csingh43@bloomberg.net>
-#
-
-import os
-import tarfile
-
-import pytest
-
-from tests.testutils import cli
-
-# Project directory
-DATA_DIR = os.path.join(
-    os.path.dirname(os.path.realpath(__file__)),
-    "project",
-)
-
-
-@pytest.mark.datafiles(DATA_DIR)
-def test_source_bundle(cli, tmpdir, datafiles):
-    project_path = os.path.join(datafiles.dirname, datafiles.basename)
-    element_name = 'source-bundle/source-bundle-hello.bst'
-    normal_name = 'source-bundle-source-bundle-hello'
-
-    # Verify that we can correctly produce a source-bundle
-    args = ['source-bundle', element_name, '--directory', str(tmpdir)]
-    result = cli.run(project=project_path, args=args)
-    result.assert_success()
-
-    # Verify that the source-bundle contains our sources and a build script
-    with tarfile.open(os.path.join(str(tmpdir), '{}.tar.gz'.format(normal_name))) as bundle:
-        assert os.path.join(normal_name, 'source', normal_name, 'llamas.txt') in bundle.getnames()
-        assert os.path.join(normal_name, 'build.sh') in bundle.getnames()
diff --git a/tests/frontend/source_checkout.py b/tests/frontend/source_checkout.py
index 08230ce5d..f6067498d 100644
--- a/tests/frontend/source_checkout.py
+++ b/tests/frontend/source_checkout.py
@@ -1,5 +1,7 @@
 import os
 import pytest
+import tarfile
+from pathlib import Path
 
 from tests.testutils import cli
 
@@ -56,6 +58,39 @@ def test_source_checkout(datafiles, cli, tmpdir_factory, with_workspace, guess_e
 
 
 @pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize('force_flag', ['--force', '-f'])
+def test_source_checkout_force(datafiles, cli, force_flag):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    checkout = os.path.join(cli.directory, 'source-checkout')
+    target = 'checkout-deps.bst'
+
+    os.makedirs(os.path.join(checkout, 'some-thing'))
+    # Path(os.path.join(checkout, 'some-file')).touch()
+
+    result = cli.run(project=project, args=['source-checkout', force_flag, target, '--deps', 'none', checkout])
+    result.assert_success()
+
+    assert os.path.exists(os.path.join(checkout, 'checkout-deps', 'etc', 'buildstream', 'config'))
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_source_checkout_tar(datafiles, cli):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    checkout = os.path.join(cli.directory, 'source-checkout.tar')
+    target = 'checkout-deps.bst'
+
+    result = cli.run(project=project, args=['source-checkout', '--tar', target, '--deps', 'none', checkout])
+    result.assert_success()
+
+    assert os.path.exists(checkout)
+    with tarfile.open(checkout) as tf:
+        expected_content = os.path.join(checkout, 'checkout-deps', 'etc', 'buildstream', 'config')
+        tar_members = [f.name for f in tf]
+        for member in tar_members:
+            assert member in expected_content
+
+
+@pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.parametrize('deps', [('build'), ('none'), ('run'), ('all')])
 def test_source_checkout_deps(datafiles, cli, deps):
     project = os.path.join(datafiles.dirname, datafiles.basename)
@@ -135,3 +170,38 @@ def test_source_checkout_fetch(datafiles, cli, fetch):
         assert os.path.exists(os.path.join(checkout, 'remote-import-dev', 'pony.h'))
     else:
         result.assert_main_error(ErrorDomain.PIPELINE, 'uncached-sources')
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_source_checkout_build_scripts(cli, tmpdir, datafiles):
+    project_path = os.path.join(datafiles.dirname, datafiles.basename)
+    element_name = 'source-bundle/source-bundle-hello.bst'
+    normal_name = 'source-bundle-source-bundle-hello'
+    checkout = os.path.join(str(tmpdir), 'source-checkout')
+
+    args = ['source-checkout', '--include-build-scripts', element_name, checkout]
+    result = cli.run(project=project_path, args=args)
+    result.assert_success()
+
+    # There sould be a script for each element (just one in this case) and a top level build script
+    expected_scripts = ['build.sh', 'build-' + normal_name]
+    for script in expected_scripts:
+        assert script in os.listdir(checkout)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_source_checkout_tar_buildscripts(cli, tmpdir, datafiles):
+    project_path = os.path.join(datafiles.dirname, datafiles.basename)
+    element_name = 'source-bundle/source-bundle-hello.bst'
+    normal_name = 'source-bundle-source-bundle-hello'
+    tar_file = os.path.join(str(tmpdir), 'source-checkout.tar')
+
+    args = ['source-checkout', '--include-build-scripts', '--tar', element_name, tar_file]
+    result = cli.run(project=project_path, args=args)
+    result.assert_success()
+
+    expected_scripts = ['build.sh', 'build-' + normal_name]
+
+    with tarfile.open(tar_file, 'r') as tf:
+        for script in expected_scripts:
+            assert script in tf.getnames()