remote.py: Add support for marking downloaded files executable

Add an optional flag to make files executable after having downloaded them.

Instead of leaving the permissioning of downloaded file in remote.py up
to the user's umask; expressly set permissions to 0644 or 0755 if
executable.

Bump format version to 13.

4 files changed, 61 insertions, 5 deletions

diff --git a/buildstream/_versions.py b/buildstream/_versions.py
index 39ff30fc3..d774e5786 100644
--- a/buildstream/_versions.py
+++ b/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 = 12
+BST_FORMAT_VERSION = 13
 
 
 # The base BuildStream artifact version
diff --git a/buildstream/plugins/sources/remote.py b/buildstream/plugins/sources/remote.py
index ad4cdab8b..a0809cb10 100644
--- a/buildstream/plugins/sources/remote.py
+++ b/buildstream/plugins/sources/remote.py
@@ -35,6 +35,10 @@ remote - stage files from remote urls
    # If not specified, the basename of the url will be used.
    # filename: customfilename
 
+   # Optionally specify whether the downloaded file should be
+   # marked executable.
+   # executable: true
+
    # Specify the url. Using an alias defined in your project
    # configuration is encouraged. 'bst track' will update the
    # sha256sum in 'ref' to the downloaded file's sha256sum.
@@ -43,6 +47,8 @@ remote - stage files from remote urls
    # Specify the ref. It's a sha256sum of the file you download.
    ref: 6c9f6f68a131ec6381da82f2bff978083ed7f4f7991d931bfa767b7965ebc94b
 
+
+
 .. note::
 
    The ``remote`` plugin is available since :ref:`format version 10 <project_format_version>`
@@ -60,22 +66,31 @@ class RemoteSource(DownloadableFileSource):
         super().configure(node)
 
         self.filename = self.node_get_member(node, str, 'filename', os.path.basename(self.url))
+        self.executable = self.node_get_member(node, bool, 'executable', False)
 
         if os.sep in self.filename:
             raise SourceError('{}: filename parameter cannot contain directories'.format(self),
                               reason="filename-contains-directory")
-        self.node_validate(node, DownloadableFileSource.COMMON_CONFIG_KEYS + ['filename'])
+        self.node_validate(node, DownloadableFileSource.COMMON_CONFIG_KEYS + ['filename', 'executable'])
 
     def get_unique_key(self):
-        return super().get_unique_key() + [self.filename]
+        return super().get_unique_key() + [self.filename, self.executable]
 
     def stage(self, directory):
         # Same as in local plugin, don't use hardlinks to stage sources, they
         # are not write protected in the sandbox.
         dest = os.path.join(directory, self.filename)
         with self.timed_activity("Staging remote file to {}".format(dest)):
+
             utils.safe_copy(self._get_mirror_file(), dest)
 
+            # To prevent user's umask introducing variability here, explicitly set
+            # file modes.
+            if self.executable:
+                os.chmod(dest, 0o755)
+            else:
+                os.chmod(dest, 0o644)
+
 
 def setup():
     return RemoteSource
diff --git a/tests/sources/remote.py b/tests/sources/remote.py
index b7c8c08cf..d3968395f 100644
--- a/tests/sources/remote.py
+++ b/tests/sources/remote.py
@@ -1,4 +1,5 @@
 import os
+import stat
 import pytest
 
 from buildstream._exceptions import ErrorDomain
@@ -82,7 +83,14 @@ def test_simple_file_build(cli, tmpdir, datafiles):
     result.assert_success()
     # Note that the url of the file in target.bst is actually /dir/file
     # but this tests confirms we take the basename
-    assert(os.path.exists(os.path.join(checkoutdir, 'file')))
+    checkout_file = os.path.join(checkoutdir, 'file')
+    assert(os.path.exists(checkout_file))
+
+    mode = os.stat(checkout_file).st_mode
+    # Assert not executable by anyone
+    assert(not (mode & (stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)))
+    # Assert not writeable by anyone other than me
+    assert(not (mode & (stat.S_IWGRP | stat.S_IWOTH)))
 
 
 @pytest.mark.datafiles(os.path.join(DATA_DIR, 'single-file-custom-name'))
@@ -119,6 +127,7 @@ def test_unique_key(cli, tmpdir, datafiles):
     generate_project(project, tmpdir)
     assert cli.get_element_state(project, 'target.bst') == "fetch needed"
     assert cli.get_element_state(project, 'target-custom.bst') == "fetch needed"
+    assert cli.get_element_state(project, 'target-custom-executable.bst') == "fetch needed"
     # Try to fetch it
     result = cli.run(project=project, args=[
         'fetch', 'target.bst'
@@ -127,7 +136,31 @@ def test_unique_key(cli, tmpdir, datafiles):
     # We should download the file only once
     assert cli.get_element_state(project, 'target.bst') == 'buildable'
     assert cli.get_element_state(project, 'target-custom.bst') == 'buildable'
+    assert cli.get_element_state(project, 'target-custom-executable.bst') == 'buildable'
 
     # But the cache key is different because the 'filename' is different.
     assert cli.get_element_key(project, 'target.bst') != \
-        cli.get_element_key(project, 'target-custom.bst')
+        cli.get_element_key(project, 'target-custom.bst') != \
+        cli.get_element_key(project, 'target-custom-executable.bst')
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'unique-keys'))
+def test_executable(cli, tmpdir, datafiles):
+    '''This test confirms that the 'ecxecutable' parameter is honoured.
+    '''
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    generate_project(project, tmpdir)
+    checkoutdir = os.path.join(str(tmpdir), "checkout")
+    assert cli.get_element_state(project, 'target-custom-executable.bst') == "fetch needed"
+    # Try to fetch it
+    result = cli.run(project=project, args=[
+        'build', 'target-custom-executable.bst'
+    ])
+
+    result = cli.run(project=project, args=[
+        'checkout', 'target-custom-executable.bst', checkoutdir
+    ])
+    mode = os.stat(os.path.join(checkoutdir, 'some-custom-file')).st_mode
+    assert (mode & stat.S_IEXEC)
+    # Assert executable by anyone
+    assert(mode & (stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH))
diff --git a/tests/sources/remote/unique-keys/target-custom-executable.bst b/tests/sources/remote/unique-keys/target-custom-executable.bst
new file mode 100644
index 000000000..8a96678ce
--- /dev/null
+++ b/tests/sources/remote/unique-keys/target-custom-executable.bst
@@ -0,0 +1,8 @@
+kind: import
+description: test
+sources:
+- kind: remote
+  url: tmpdir:/dir/file
+  ref: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+  filename: some-custom-file
+  executable: true