Merge branch 'tristan/options-order' into 'master'

Append lists when merging conditional statements

See merge request BuildStream/buildstream!2002

27 files changed, 192 insertions, 5 deletions

diff --git a/src/buildstream/node.pyx b/src/buildstream/node.pyx
index 394138311..9a0bd2eb6 100644
--- a/src/buildstream/node.pyx
+++ b/src/buildstream/node.pyx
@@ -214,7 +214,7 @@ cdef class Node:
 
     # _is_composite_list
     #
-    # Checks if the node is a Mapping with array composition
+    # Checks if the node is a Mapping with list composition
     # directives.
     #
     # Returns:
@@ -1070,14 +1070,14 @@ cdef class MappingNode(Node):
         cdef str key
 
         for key in self.value.keys():
-            if key in ['(>)', '(<)', '(=)']:
+            if key in ["(>)", "(<)", "(=)"]:
                 has_directives = True
             else:
                 has_keys = True
 
         if has_keys and has_directives:
             provenance = self.get_provenance()
-            raise LoadError("{}: Dictionary contains array composition directives and arbitrary keys"
+            raise LoadError("{}: Dictionary contains list composition directives and arbitrary keys"
                             .format(provenance), LoadErrorReason.INVALID_DATA)
 
         return has_directives
@@ -1412,8 +1412,16 @@ cdef class SequenceNode(Node):
                                   .format(self.get_provenance(),
                                           key,
                                           target_value.get_provenance()))
-        # Looks good, clobber it
-        target.value[key] = self
+
+        # If the target is a list of conditional statements, then we are
+        # also conditional statements, and we need to append ourselves
+        # to that list instead of overwriting it in order to preserve the
+        # conditional for later evaluation.
+        if type(target_value) is SequenceNode and key == "(?)":
+            (<SequenceNode> target.value[key]).value.extend(self.value)
+        else:
+            # Looks good, clobber it
+            target.value[key] = self
 
     cdef bint _is_composite_list(self) except *:
         return False
diff --git a/tests/format/include.py b/tests/format/include.py
index d57dd8c19..11659d459 100644
--- a/tests/format/include.py
+++ b/tests/format/include.py
@@ -219,6 +219,30 @@ def test_conditional_in_fragment(cli, datafiles):
     assert loaded.get_str("size") == "8"
 
 
+@pytest.mark.parametrize(
+    "project_dir",
+    [
+        "conditional-conflicts-project",
+        "conditional-conflicts-element",
+        "conditional-conflicts-options-included",
+        "conditional-conflicts-complex",
+        "conditional-conflicts-toplevel-precedence",
+    ],
+)
+@pytest.mark.datafiles(DATA_DIR)
+def test_preserve_conditionals(cli, datafiles, project_dir):
+    project = os.path.join(str(datafiles), project_dir)
+
+    result = cli.run(
+        project=project,
+        args=["-o", "build_arch", "i586", "show", "--deps", "none", "--format", "%{vars}", "element.bst"],
+    )
+    result.assert_success()
+    loaded = _yaml.load_data(result.output)
+    assert loaded.get_str("enable-work-around") == "true"
+    assert loaded.get_str("size") == "4"
+
+
 @pytest.mark.datafiles(DATA_DIR)
 def test_inner(cli, datafiles):
     project = os.path.join(str(datafiles), "inner")
diff --git a/tests/format/include/conditional-conflicts-complex/element.bst b/tests/format/include/conditional-conflicts-complex/element.bst
new file mode 100644
index 000000000..4d7f70266
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/element.bst
@@ -0,0 +1 @@
+kind: manual
diff --git a/tests/format/include/conditional-conflicts-complex/enable_work_around.yml b/tests/format/include/conditional-conflicts-complex/enable_work_around.yml
new file mode 100644
index 000000000..7e56ae727
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/enable_work_around.yml
@@ -0,0 +1,5 @@
+variables:
+  enable-work-around: "false"
+  (?):
+  - build_arch == "i586":
+      enable-work-around: "true"
diff --git a/tests/format/include/conditional-conflicts-complex/extra_conf.yml b/tests/format/include/conditional-conflicts-complex/extra_conf.yml
new file mode 100644
index 000000000..a25eabe23
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/extra_conf.yml
@@ -0,0 +1,5 @@
+(?):
+- build_arch == "i586":
+    (@): extra_conf_i586.yml
+- build_arch == "x86_64":
+    (@): extra_conf_x86_64.yml
diff --git a/tests/format/include/conditional-conflicts-complex/extra_conf_i586.yml b/tests/format/include/conditional-conflicts-complex/extra_conf_i586.yml
new file mode 100644
index 000000000..caf872b98
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/extra_conf_i586.yml
@@ -0,0 +1,4 @@
+variables:
+  (?):
+  - build_arch == "i586":
+      size: 4
diff --git a/tests/format/include/conditional-conflicts-complex/extra_conf_x86_64.yml b/tests/format/include/conditional-conflicts-complex/extra_conf_x86_64.yml
new file mode 100644
index 000000000..9c5f64630
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/extra_conf_x86_64.yml
@@ -0,0 +1,4 @@
+variables:
+  (?):
+  - build_arch == "x86_64":
+      size: 8
diff --git a/tests/format/include/conditional-conflicts-complex/options.yml b/tests/format/include/conditional-conflicts-complex/options.yml
new file mode 100644
index 000000000..8bb07305a
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/options.yml
@@ -0,0 +1,9 @@
+
+options:
+  build_arch:
+    type: arch
+    description: Architecture
+    variable: build_arch
+    values:
+      - i586
+      - x86_64
diff --git a/tests/format/include/conditional-conflicts-complex/project.conf b/tests/format/include/conditional-conflicts-complex/project.conf
new file mode 100644
index 000000000..dd58ac234
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/project.conf
@@ -0,0 +1,7 @@
+name: test
+min-version: 2.0
+
+(@):
+- extra_conf.yml
+- work_around.yml
+- options.yml
diff --git a/tests/format/include/conditional-conflicts-complex/work_around.yml b/tests/format/include/conditional-conflicts-complex/work_around.yml
new file mode 100644
index 000000000..d7e3ec1dc
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-complex/work_around.yml
@@ -0,0 +1,3 @@
+(?):
+- build_arch == "i586":
+    (@): enable_work_around.yml
diff --git a/tests/format/include/conditional-conflicts-element/element.bst b/tests/format/include/conditional-conflicts-element/element.bst
new file mode 100644
index 000000000..bb1ba25d4
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-element/element.bst
@@ -0,0 +1,5 @@
+kind: manual
+
+(@):
+  - extra_conf.yml
+  - work_around.yml
diff --git a/tests/format/include/conditional-conflicts-element/extra_conf.yml b/tests/format/include/conditional-conflicts-element/extra_conf.yml
new file mode 100644
index 000000000..dd58c9855
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-element/extra_conf.yml
@@ -0,0 +1,6 @@
+variables:
+  (?):
+    - build_arch == "i586":
+        size: "4"
+    - build_arch == "x86_64":
+        size: "8"
diff --git a/tests/format/include/conditional-conflicts-element/project.conf b/tests/format/include/conditional-conflicts-element/project.conf
new file mode 100644
index 000000000..c6259d5f2
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-element/project.conf
@@ -0,0 +1,11 @@
+name: test
+min-version: 2.0
+
+options:
+  build_arch:
+    type: arch
+    description: Architecture
+    variable: build_arch
+    values:
+      - i586
+      - x86_64
diff --git a/tests/format/include/conditional-conflicts-element/work_around.yml b/tests/format/include/conditional-conflicts-element/work_around.yml
new file mode 100644
index 000000000..a527fe124
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-element/work_around.yml
@@ -0,0 +1,5 @@
+variables:
+  enable-work-around: "false"
+  (?):
+    - build_arch == "i586":
+        enable-work-around: "true"
diff --git a/tests/format/include/conditional-conflicts-options-included/element.bst b/tests/format/include/conditional-conflicts-options-included/element.bst
new file mode 100644
index 000000000..4d7f70266
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-options-included/element.bst
@@ -0,0 +1 @@
+kind: manual
diff --git a/tests/format/include/conditional-conflicts-options-included/extra_conf.yml b/tests/format/include/conditional-conflicts-options-included/extra_conf.yml
new file mode 100644
index 000000000..dd58c9855
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-options-included/extra_conf.yml
@@ -0,0 +1,6 @@
+variables:
+  (?):
+    - build_arch == "i586":
+        size: "4"
+    - build_arch == "x86_64":
+        size: "8"
diff --git a/tests/format/include/conditional-conflicts-options-included/options.yml b/tests/format/include/conditional-conflicts-options-included/options.yml
new file mode 100644
index 000000000..8bb07305a
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-options-included/options.yml
@@ -0,0 +1,9 @@
+
+options:
+  build_arch:
+    type: arch
+    description: Architecture
+    variable: build_arch
+    values:
+      - i586
+      - x86_64
diff --git a/tests/format/include/conditional-conflicts-options-included/project.conf b/tests/format/include/conditional-conflicts-options-included/project.conf
new file mode 100644
index 000000000..364723e0c
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-options-included/project.conf
@@ -0,0 +1,7 @@
+name: test
+min-version: 2.0
+
+(@):
+- options.yml
+- extra_conf.yml
+- work_around.yml
diff --git a/tests/format/include/conditional-conflicts-options-included/work_around.yml b/tests/format/include/conditional-conflicts-options-included/work_around.yml
new file mode 100644
index 000000000..a527fe124
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-options-included/work_around.yml
@@ -0,0 +1,5 @@
+variables:
+  enable-work-around: "false"
+  (?):
+    - build_arch == "i586":
+        enable-work-around: "true"
diff --git a/tests/format/include/conditional-conflicts-project/element.bst b/tests/format/include/conditional-conflicts-project/element.bst
new file mode 100644
index 000000000..4d7f70266
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-project/element.bst
@@ -0,0 +1 @@
+kind: manual
diff --git a/tests/format/include/conditional-conflicts-project/extra_conf.yml b/tests/format/include/conditional-conflicts-project/extra_conf.yml
new file mode 100644
index 000000000..dd58c9855
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-project/extra_conf.yml
@@ -0,0 +1,6 @@
+variables:
+  (?):
+    - build_arch == "i586":
+        size: "4"
+    - build_arch == "x86_64":
+        size: "8"
diff --git a/tests/format/include/conditional-conflicts-project/project.conf b/tests/format/include/conditional-conflicts-project/project.conf
new file mode 100644
index 000000000..6978fa813
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-project/project.conf
@@ -0,0 +1,15 @@
+name: test
+min-version: 2.0
+
+options:
+  build_arch:
+    type: arch
+    description: Architecture
+    variable: build_arch
+    values:
+      - i586
+      - x86_64
+
+(@):
+  - extra_conf.yml
+  - work_around.yml
diff --git a/tests/format/include/conditional-conflicts-project/work_around.yml b/tests/format/include/conditional-conflicts-project/work_around.yml
new file mode 100644
index 000000000..a527fe124
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-project/work_around.yml
@@ -0,0 +1,5 @@
+variables:
+  enable-work-around: "false"
+  (?):
+    - build_arch == "i586":
+        enable-work-around: "true"
diff --git a/tests/format/include/conditional-conflicts-toplevel-precedence/element.bst b/tests/format/include/conditional-conflicts-toplevel-precedence/element.bst
new file mode 100644
index 000000000..4d7f70266
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-toplevel-precedence/element.bst
@@ -0,0 +1 @@
+kind: manual
diff --git a/tests/format/include/conditional-conflicts-toplevel-precedence/extra_conf.yml b/tests/format/include/conditional-conflicts-toplevel-precedence/extra_conf.yml
new file mode 100644
index 000000000..dd58c9855
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-toplevel-precedence/extra_conf.yml
@@ -0,0 +1,6 @@
+variables:
+  (?):
+    - build_arch == "i586":
+        size: "4"
+    - build_arch == "x86_64":
+        size: "8"
diff --git a/tests/format/include/conditional-conflicts-toplevel-precedence/project.conf b/tests/format/include/conditional-conflicts-toplevel-precedence/project.conf
new file mode 100644
index 000000000..cdbaea502
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-toplevel-precedence/project.conf
@@ -0,0 +1,23 @@
+name: test
+min-version: 2.0
+
+options:
+  build_arch:
+    type: arch
+    description: Architecture
+    variable: build_arch
+    values:
+      - i586
+      - x86_64
+
+# The work_around.yml sets this to false in it's conditional
+# and we set it to true, testing here that the including
+# fragment still takes precedence over any included fragments.
+variables:
+  (?):
+  - build_arch == "i586":
+      enable-work-around: "true"
+
+(@):
+  - extra_conf.yml
+  - work_around.yml
diff --git a/tests/format/include/conditional-conflicts-toplevel-precedence/work_around.yml b/tests/format/include/conditional-conflicts-toplevel-precedence/work_around.yml
new file mode 100644
index 000000000..e9991aaba
--- /dev/null
+++ b/tests/format/include/conditional-conflicts-toplevel-precedence/work_around.yml
@@ -0,0 +1,5 @@
+variables:
+  enable-work-around: "true"
+  (?):
+    - build_arch == "i586":
+        enable-work-around: "false"