NEWS

@@ -2,6 +2,17 @@
 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 A `bst source-checkout` command has been added. This command allows an
+    element's sources to be checkout out into a user specified location. For
+    example, `bst source-checkout foo.bst some/path/` will collect the sources
+    of `foo.bst` and place them in the directory `some/path`.
+
   o All elements must now be suffixed with `.bst`
     Attempting to use an element that does not have the `.bst` extension,
     will result in a warning.

buildstream/_frontend/cli.py

@@ -697,11 +697,12 @@ def checkout(app, element, location, force, deps, integrate, hardlinks, tar):
 @click.option('--tar', 'tar', default=False, is_flag=True,
               help='Create a tarball from the element\'s sources instead of a '
                    'file tree.')
-@click.argument('element',
-                type=click.Path(readable=False))
+@click.option('--include-build-scripts', 'build_scripts', is_flag=True)
+@click.argument('element', type=click.Path(readable=False))
 @click.argument('location', type=click.Path())
 @click.pass_obj
-def source_checkout(app, element, location, force, deps, fetch_, except_, tar):
+def source_checkout(app, element, location, force, deps, fetch_, except_,
+                    tar, build_scripts):
     """Checkout sources of an element to the specified location
     """
     with app.initialized():
@@ -711,7 +712,8 @@ def source_checkout(app, element, location, force, deps, fetch_, except_, tar):
                                    deps=deps,
                                    fetch=fetch_,
                                    except_targets=except_,
-                                   tar=tar)
+                                   tar=tar,
+                                   include_build_scripts=build_scripts)
 
 
 ##################################################################
@@ -842,34 +844,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_)

buildstream/_stream.py

@@ -25,7 +25,7 @@ import stat
 import shlex
 import shutil
 import tarfile
-from contextlib import contextmanager
+from contextlib import contextmanager, suppress
 from tempfile import TemporaryDirectory
 
 from ._exceptions import StreamError, ImplError, BstError, set_last_task_error
@@ -453,7 +453,8 @@ class Stream():
                         deps='none',
                         fetch=False,
                         except_targets=(),
-                        tar=False):
+                        tar=False,
+                        include_build_scripts=False):
 
         self._check_location_writable(location, force=force, tar=tar)
 
@@ -469,10 +470,8 @@ class Stream():
 
         # Stage all sources determined by scope
         try:
-            if tar:
-                self._create_source_tarball(location, elements)
-            else:
-                self._write_element_sources(location, elements)
+            self._source_checkout(elements, location, force, deps, fetch,
+                                  except_targets, tar, include_build_scripts)
         except BstError as e:
             raise StreamError("Error while writing sources"
                               ": '{}'".format(e), detail=e.detail, reason=e.reason) from e
@@ -729,87 +728,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
@@ -1193,6 +1111,7 @@ class Stream():
     # Helper function for source_checkout()
     def _source_checkout(self, elements,
                          location=None,
+                         force=False,
                          deps='none',
                          fetch=False,
                          except_targets=(),
@@ -1203,35 +1122,20 @@ class Stream():
         # 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 = self._create_temp_source_dir(elements, include_build_scripts)
-        if tar:
-            self._create_tarball(temp_source_dir.name, location)
-        else:
-            try:
-                self._move_directory(temp_source_dir.name, location, force)
-            except OSError as e:
+        with TemporaryDirectory() as temp_source_dir:
+            self._write_element_sources(temp_source_dir, elements)
+            if include_build_scripts:
+                self._write_build_scripts(temp_source_dir, elements)
+
+            if tar:
+                self._create_tarball(temp_source_dir, location)
+            else:
                 try:
-                    shutil.rmtree(location)
-                except FileNotFoundError:
-                    pass
-                temp_source_dir.cleanup()
-                raise StreamError("Failed to checkout sources to {}: {}".format(location, e))
-
-        # Ensure the temporary directory is cleaned up. If it has been
-        # moved temp_source_dir will no longer exist with it's
-        # original name. This is expected.
-        try:
-            temp_source_dir.cleanup()
-        except FileNotFoundError:
-            pass
-
-    # Construct a TemporaryDirectory containing the sources of elements.
-    def _create_temp_source_dir(self, elements, include_build_scripts):
-        tempdir = TemporaryDirectory()
-        self._write_element_sources(tempdir.name, elements)
-        if include_build_scripts:
-            self._write_build_scripts(tempdir.name, elements)
-        return tempdir
+                    self._move_directory(temp_source_dir, location, force)
+                except OSError as e:
+                    with suppress(FileNotFoundError):
+                        shutil.rmtree(location)
+                    raise StreamError("Failed to checkout sources to {}: {}".format(location, e))
 
     # Utility to move the contents of one directory into the specified destination.
     # This is used because os.rename does not work when the target and destination
@@ -1263,17 +1167,28 @@ class Stream():
                 os.makedirs(element_source_dir)
                 element._stage_sources_at(element_source_dir)
 
-    # Create a tarball containing the sources of each element in elements
-    def _create_source_tarball(self, directory, elements):
-        with tarfile.open(name=directory, mode='w') as tf:
-            with TemporaryDirectory() as tmpdir:
-                self._write_element_sources(tmpdir, elements)
-                for item in os.listdir(tmpdir):
-                    file_to_add = os.path.join(tmpdir, item)
-                    tf.add(file_to_add, arcname=item)
+    # 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:

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

tests/completions/completions.py

@@ -16,7 +16,6 @@ MAIN_COMMANDS = [
     'shell ',
     'show ',
     'source-checkout ',
-    'source-bundle ',
     'track ',
     'workspace '
 ]

tests/frontend/help.py

@@ -25,7 +25,6 @@ def test_help_main(cli):
     ('push'),
     ('shell'),
     ('show'),
-    ('source-bundle'),
     ('track'),
     ('workspace')
 ])

tests/frontend/source_bundle.py

-#
-#  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()

tests/frontend/source_checkout.py

@@ -154,3 +154,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()