From 0dac630fe6a852016d4b79e8bee6af4b70e7e0f7 Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Fri, 27 Oct 2017 17:15:20 +0100
Subject: Catch attempts to compose a list

Attempting to compose a list property would result in an unhandled
exception:

    AttributeError: 'CommentedSeq' object has no attribute 'get'

We now at least detect the situation and produce an exception that
the frontend will report neatly:

    Error loading pipeline: element.bst [line 11 column 4]: Only values of type 'dict' can be composed.

I was getting this error from an attempt to conditionally extend the
sources list:

    sources:
      (?):
        arch == "x86_64":
          - url: http://example.com/x86_64

The correct way to do this is to move the conditional into the parent
dict, e.g.:

    (?):
      arch == "x86_64":
        sources:
          - url: https://example.com/x86_64

It would be nice if the error message could hint at how the user can do
what they want, but it doesn't seem possible in this case.
---
 buildstream/_yaml.py                                | 10 +++++++---
 tests/format/list-directive-type-error/element.bst  |  6 ++++++
 tests/format/list-directive-type-error/project.conf |  7 +++++++
 tests/format/listdirectiveerrors.py                 | 15 +++++++++++++++
 4 files changed, 35 insertions(+), 3 deletions(-)
 create mode 100644 tests/format/list-directive-type-error/element.bst
 create mode 100644 tests/format/list-directive-type-error/project.conf

diff --git a/buildstream/_yaml.py b/buildstream/_yaml.py
index bba59c73c..8aa2ec714 100644
--- a/buildstream/_yaml.py
+++ b/buildstream/_yaml.py
@@ -743,13 +743,17 @@ def composite_dict(target, source, path=None):
 # Like composite_dict(), but raises an all purpose LoadError for convenience
 #
 def composite(target, source):
-    provenance = node_get_provenance(source)
+    if not hasattr(source, 'get'):
+        raise LoadError(LoadErrorReason.ILLEGAL_COMPOSITE,
+                        "Only values of type 'dict' can be composed.")
+
+    source_provenance = node_get_provenance(source)
     try:
         composite_dict(target, source)
     except CompositeTypeError as e:
         error_prefix = ""
-        if provenance:
-            error_prefix = "[%s]: " % str(provenance)
+        if source_provenance:
+            error_prefix = "[%s]: " % str(source_provenance)
         raise LoadError(LoadErrorReason.ILLEGAL_COMPOSITE,
                         "%sExpected '%s' type for configuration '%s', instead received '%s'" %
                         (error_prefix,
diff --git a/tests/format/list-directive-type-error/element.bst b/tests/format/list-directive-type-error/element.bst
new file mode 100644
index 000000000..4ac7d95bb
--- /dev/null
+++ b/tests/format/list-directive-type-error/element.bst
@@ -0,0 +1,6 @@
+kind: autotools
+
+sources:
+  (?):
+    - arch == "x86_64":
+      - url: https://example.com/x86_64
diff --git a/tests/format/list-directive-type-error/project.conf b/tests/format/list-directive-type-error/project.conf
new file mode 100644
index 000000000..dde56d7b8
--- /dev/null
+++ b/tests/format/list-directive-type-error/project.conf
@@ -0,0 +1,7 @@
+name: test
+
+options:
+  arch:
+    type: arch
+    description: Example architecture option
+    values: [ x86_32, x86_64 ]
diff --git a/tests/format/listdirectiveerrors.py b/tests/format/listdirectiveerrors.py
index 001b8a49d..9f2cfaaca 100644
--- a/tests/format/listdirectiveerrors.py
+++ b/tests/format/listdirectiveerrors.py
@@ -39,3 +39,18 @@ def test_element_error(cli, datafiles, target):
     assert result.exception
     assert isinstance(result.exception, LoadError)
     assert result.exception.reason == LoadErrorReason.TRAILING_LIST_DIRECTIVE
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_project_error(cli, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename, 'list-directive-type-error')
+    result = cli.run(project=project, silent=True, args=[
+        'show',
+        '--deps', 'none',
+        '--format', '%{vars}',
+        'element.bst'])
+
+    assert result.exit_code != 0
+    assert result.exception
+    assert isinstance(result.exception, LoadError)
+    assert result.exception.reason == LoadErrorReason.ILLEGAL_COMPOSITE
-- 
cgit v1.2.1