Merge branch 'becky/list_contents_long_option' into 'master'

Addition of --long option to list-contents

Closes #773

See merge request BuildStream/buildstream!1555

7 files changed, 147 insertions, 22 deletions

diff --git a/NEWS b/NEWS
index 5006dadce..cde99c7be 100644
--- a/NEWS
+++ b/NEWS
@@ -20,7 +20,8 @@ buildstream 1.3.1
 
   o Added `bst artifact list-contents` subcommand which can display the names
     of files in artifacts in your artifact cache, either by element name
-    or by direct artifact reference.
+    or by direct artifact reference. --long option can be used to display more
+    information; file type and size.
 
   o BREAKING CHANGE: Reverted the default behaviour of junctions. Subproject
     elements will no longer interact with the parent project's remote (by
diff --git a/src/buildstream/_frontend/cli.py b/src/buildstream/_frontend/cli.py
index c1b06b2d1..6a9f11c70 100644
--- a/src/buildstream/_frontend/cli.py
+++ b/src/buildstream/_frontend/cli.py
@@ -1265,9 +1265,11 @@ def artifact_log(app, artifacts, out):
 #                Artifact List-Contents Command                #
 ################################################################
 @artifact.command(name='list-contents', short_help="List the contents of an artifact")
+@click.option('--long', '-l', 'long_', default=False, is_flag=True,
+              help="Provides more information about the contents of the artifact.")
 @click.argument('artifacts', type=click.Path(), nargs=-1)
 @click.pass_obj
-def artifact_list_contents(app, artifacts):
+def artifact_list_contents(app, artifacts, long_):
     """List the contents of an artifact.
 
     Note that 'artifacts' can be element names, which must end in '.bst',
@@ -1276,8 +1278,12 @@ def artifact_list_contents(app, artifacts):
     """
     with app.initialized():
         elements_to_files = app.stream.artifact_list_contents(artifacts)
-        click.echo(app.logger._pretty_print_dictionary(elements_to_files))
-        sys.exit(0)
+        if not elements_to_files:
+            click.echo("None of the specified artifacts are cached.", err=True)
+            sys.exit(1)
+        else:
+            click.echo(app.logger._pretty_print_dictionary(elements_to_files, long_))
+            sys.exit(0)
 
 
 ###################################################################
diff --git a/src/buildstream/_frontend/widget.py b/src/buildstream/_frontend/widget.py
index e8299868c..181ee7d2e 100644
--- a/src/buildstream/_frontend/widget.py
+++ b/src/buildstream/_frontend/widget.py
@@ -31,7 +31,7 @@ from .. import Consistency, Scope
 from .. import __version__ as bst_version
 from .._exceptions import ImplError
 from .._message import MessageType
-
+from ..storage.directory import _FileType
 
 # These messages are printed a bit differently
 ERROR_MESSAGES = [MessageType.FAIL, MessageType.ERROR, MessageType.BUG]
@@ -802,14 +802,18 @@ class LogLine(Widget):
     # Args:
     #    values: A dictionary
     #    style_value: Whether to use the content profile for the values
+    #    list_long (Bool): whether to display verbose information about artifacts
     #
     # Returns:
     #    (str): The formatted values
     #
-    def _pretty_print_dictionary(self, values, style_value=True):
+    def _pretty_print_dictionary(self, values, long_=False, style_value=True):
         text = ''
         max_key_len = 0
-        max_key_len = max(len(key) for key in values.keys())
+        try:
+            max_key_len = max(len(key) for key in values.keys())
+        except ValueError:
+            text = ''
 
         for key, value in values.items():
             if isinstance(value, str) and '\n' in value:
@@ -819,8 +823,14 @@ class LogLine(Widget):
 
             text += self.format_profile.fmt("  {}:{}".format(key, ' ' * (max_key_len - len(key))))
 
-            value_list = "\n\t" + "\n\t".join(value)
-            if style_value:
+            value_list = "\n\t" + "\n\t".join((self._get_filestats(v, list_long=long_) for v in value))
+            if value == []:
+                message = "\n\tThis element has no associated artifacts"
+                if style_value:
+                    text += self.content_profile.fmt(message)
+                else:
+                    text += message
+            elif style_value:
                 text += self.content_profile.fmt(value_list)
             else:
                 text += value_list
@@ -864,3 +874,33 @@ class LogLine(Widget):
             report += line + '\n'
 
         return report
+
+    # _get_filestats()
+    #
+    # Gets the necessary information from a dictionary
+    #
+    # Args:
+    #    entry: A dictionary of info about the element
+    #    list_long (Bool): whether to display verbose information about artifacts
+    #
+    # Returns:
+    #    (str): The information about the element
+    #
+    def _get_filestats(self, entry, list_long=False):
+        if list_long:
+            size = str(entry["size"])
+            # Support files up to 99G, meaning maximum characters is 11
+            max_v_len = 11
+            if entry["type"] == _FileType.DIRECTORY:
+                return "drwxr-xr-x  dir    {}".format(entry["size"]) +\
+                       "{} ".format(' ' * (max_v_len - len(size))) + "{}".format(entry["name"])
+            elif entry["type"] == _FileType.SYMLINK:
+                return "lrwxrwxrwx  link   {}".format(entry["size"]) +\
+                       "{} ".format(' ' * (max_v_len - len(size))) + "{} -> {}".format(entry["name"], entry["target"])
+            elif entry["executable"]:
+                return "-rwxr-xr-x  exe    {}".format(entry["size"]) +\
+                       "{} ".format(' ' * (max_v_len - len(size))) + "{}".format(entry["name"])
+            else:
+                return "-rw-r--r--  reg    {}".format(entry["size"]) +\
+                       "{} ".format(' ' * (max_v_len - len(size))) + "{}".format(entry["name"])
+        return entry["name"]
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 95b306b47..9ff93fdc3 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -678,11 +678,14 @@ class Stream():
 
         elements_to_files = {}
         for obj in target_objects:
+            ref = obj.get_artifact_name()
+            if not obj._cached():
+                self._message(MessageType.WARN, "{} is not cached".format(ref))
+                obj.name = {ref: "No artifact cached"}
+                continue
             if isinstance(obj, ArtifactElement):
-                obj.name = obj.get_artifact_name()
-            files = obj._get_artifact_relative_file_paths()
-            if files == []:
-                files = ["This element has no associated artifacts"]
+                obj.name = ref
+            files = [f for f in obj._walk_artifact_files()]
             elements_to_files[obj.name] = files
         return elements_to_files
 
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 633e29c88..950334695 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -2361,15 +2361,8 @@ class Element(Plugin):
         factory = self._get_project().config.element_factory
         return factory, self.__meta_kind, state
 
-    # _get_artifact_relative_path_files()
-    #
-    # Gets the file paths in the artifact and return them in a list
-    #
-    # Returns:
-    #   (list): A list of the file paths in the artifact
-    def _get_artifact_relative_file_paths(self):
-        casbd = self.__artifact.get_files()
-        return [f for f in casbd.list_relative_paths()]
+    def _walk_artifact_files(self):
+        yield from self.__artifact.get_files().walk()
 
     # _get_artifact()
     #
diff --git a/src/buildstream/storage/_casbaseddirectory.py b/src/buildstream/storage/_casbaseddirectory.py
index 424b7ef63..66b7a7259 100644
--- a/src/buildstream/storage/_casbaseddirectory.py
+++ b/src/buildstream/storage/_casbaseddirectory.py
@@ -574,6 +574,46 @@ class CasBasedDirectory(Directory):
             subdir = v.get_directory(self)
             yield from subdir._list_prefixed_relative_paths(prefix=os.path.join(prefix, k))
 
+    def walk(self):
+        """Provide a list of dictionaries containing information about the files.
+
+        Yields:
+          info (dict) - a dictionary containing name, type and size of the files.
+
+        """
+        yield from self._walk()
+
+    def _walk(self, prefix=""):
+        """ Walk through the files, collecting the required data
+
+        Arguments:
+          prefix (str): an optional prefix to the relative paths, this is
+                        also emitted by itself.
+
+        Yields:
+          info (dict) - a dictionary containing name, type and size of the files.
+
+          """
+        for leaf in sorted(self.index.keys()):
+            entry = self.index[leaf]
+            info = {
+                "name": os.path.join(prefix, leaf),
+                "type": entry.type
+            }
+            if entry.type == _FileType.REGULAR_FILE:
+                info["executable"] = entry.is_executable
+                info["size"] = self.get_size()
+            elif entry.type == _FileType.SYMLINK:
+                info["target"] = entry.target
+                info["size"] = len(entry.target)
+            if entry.type == _FileType.DIRECTORY:
+                directory = entry.get_directory(self)
+                info["size"] = len(directory.index)
+                yield info
+                yield from directory._walk(os.path.join(prefix, leaf))
+            else:
+                yield info
+
     def get_size(self):
         digest = self._get_digest()
         total = digest.size_bytes
diff --git a/tests/frontend/artifact_list_contents.py b/tests/frontend/artifact_list_contents.py
index 5bb08e3fa..626eb3fa7 100644
--- a/tests/frontend/artifact_list_contents.py
+++ b/tests/frontend/artifact_list_contents.py
@@ -96,3 +96,45 @@ def test_artifact_list_exact_contents_glob(cli, datafiles):
 
     for artifact in expected_artifacts:
         assert artifact in result.output
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_artifact_list_exact_contents_element_long(cli, datafiles):
+    project = str(datafiles)
+
+    # Ensure we have an artifact to read
+    result = cli.run(project=project, args=['build', 'import-bin.bst'])
+    assert result.exit_code == 0
+
+    # List the contents via the element name
+    result = cli.run(project=project, args=['artifact', 'list-contents', '--long', 'import-bin.bst'])
+    assert result.exit_code == 0
+    expected_output = ("import-bin.bst:\n"
+                       "\tdrwxr-xr-x  dir    1           usr\n"
+                       "\tdrwxr-xr-x  dir    1           usr/bin\n"
+                       "\t-rw-r--r--  reg    107         usr/bin/hello\n\n")
+
+    assert expected_output in result.output
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_artifact_list_exact_contents_ref_long(cli, datafiles):
+    project = str(datafiles)
+
+    # Get the cache key of our test element
+    key = cli.get_element_key(project, 'import-bin.bst')
+
+    # Ensure we have an artifact to read
+    result = cli.run(project=project, args=['build', 'import-bin.bst'])
+    assert result.exit_code == 0
+
+    # List the contents via the key
+    result = cli.run(project=project, args=['artifact', 'list-contents', '-l', 'test/import-bin/' + key])
+    assert result.exit_code == 0
+
+    expected_output = ("  test/import-bin/" + key + ":\n"
+                       "\tdrwxr-xr-x  dir    1           usr\n"
+                       "\tdrwxr-xr-x  dir    1           usr/bin\n"
+                       "\t-rw-r--r--  reg    107         usr/bin/hello\n\n")
+
+    assert expected_output in result.output