Warn on duplicate LLSD keys in autobuild package definitions

autobuild/configfile.py

@@ -6,6 +6,7 @@
 import re
 import string
 import sys
+import xml.etree.ElementTree as ET
 from io import StringIO
 
 import llsd
@@ -37,6 +38,58 @@ class NoVersionFileKeyError(common.AutobuildError):
     pass
 
 
+def _iter_duplicate_llsd_keys(element, path=()):
+    """
+    Yield dotted LLSD map paths that are defined more than once in raw XML.
+
+    We inspect the ElementTree instead of the parsed LLSD structure because
+    llsd.parse() has already applied the existing "last definition wins"
+    behavior by the time it returns a dict-like object.
+    """
+    if element.tag == 'map':
+        seen = set()
+        children = list(element)
+        index = 0
+        while index < len(children):
+            key_element = children[index]
+            if key_element.tag != 'key':
+                yield from _iter_duplicate_llsd_keys(key_element, path)
+                index += 1
+                continue
+
+            key = key_element.text or ''
+            value_element = children[index + 1] if index + 1 < len(children) else None
+            key_path = path + (key,)
+            if key in seen:
+                yield '.'.join(key_path)
+            else:
+                seen.add(key)
+            if value_element is not None:
+                yield from _iter_duplicate_llsd_keys(value_element, key_path)
+            index += 2
+    elif element.tag in ('array', 'llsd'):
+        for child in list(element):
+            yield from _iter_duplicate_llsd_keys(child, path)
+
+
+def _warn_duplicate_llsd_keys(xml_bytes, source):
+    """
+    Log duplicate LLSD map keys without changing Autobuild's merge semantics.
+
+    This runs before parsing so package metadata can warn about repeated keys
+    introduced by merges while still allowing the later value to override the
+    earlier one, as Autobuild has historically done.
+    """
+    try:
+        root = ET.fromstring(xml_bytes)
+    except ET.ParseError:
+        return
+
+    for key_path in _iter_duplicate_llsd_keys(root):
+        logger.warning("File '%s' contains duplicate LLSD key '%s'; later definitions override earlier ones",
+                       source, key_path)
+
+
 class ConfigurationDescription(common.Serialized):
     """
     An autobuild configuration.
@@ -217,6 +270,9 @@ def __load(self, path):
             if not autobuild_xml:
                 logger.warning("Configuration file '%s' is empty" % self.path)
                 return
+            # Warn on repeated raw LLSD keys before llsd.parse() collapses them
+            # into a single dict entry.
+            _warn_duplicate_llsd_keys(autobuild_xml, self.path)
             try:
                 saved_data = llsd.parse(autobuild_xml)
             except llsd.LLSDParseError:
@@ -431,6 +487,7 @@ def __init__(self, path=None, stream=None, parsed_llsd=None, convert_platform=No
         elif stream:
             metadata_xml = stream.read()
         if metadata_xml:
+            _warn_duplicate_llsd_keys(metadata_xml, self.path or '<stream>')
             try:
                 parsed_llsd = llsd.parse(metadata_xml)
             except llsd.LLSDParseError:

tests/test_configfile.py

@@ -61,6 +61,61 @@ def test_configuration_inherit(self):
         # check that we fall back to the 32 bit version if no 64 bit is found
         assert reloaded.get_platform('darwin64').build_directory == 'darwin_build'
 
+    def test_configuration_warns_on_duplicate_llsd_keys(self):
+        tmp_file = self.get_tmp_file()
+        with open(tmp_file, 'wb') as f:
+            f.write(b"""<?xml version="1.0" ?>
+<llsd>
+  <map>
+    <key>installables</key>
+    <map>
+      <key>icu4c</key>
+      <map>
+        <key>name</key>
+        <string>icu4c</string>
+        <key>platforms</key>
+        <map>
+          <key>linux64</key>
+          <map>
+            <key>archive</key>
+            <map>
+              <key>hash</key>
+              <string>1111111111111111111111111111111111111111</string>
+              <key>hash_algorithm</key>
+              <string>sha1</string>
+              <key>url</key>
+              <string>https://example.com/first.tar.zst</string>
+            </map>
+          </map>
+          <key>linux64</key>
+          <map>
+            <key>archive</key>
+            <map>
+              <key>hash</key>
+              <string>2222222222222222222222222222222222222222</string>
+              <key>hash_algorithm</key>
+              <string>sha1</string>
+              <key>url</key>
+              <string>https://example.com/second.tar.zst</string>
+            </map>
+          </map>
+        </map>
+      </map>
+    </map>
+    <key>type</key>
+    <string>autobuild</string>
+    <key>version</key>
+    <string>1.3</string>
+  </map>
+</llsd>
+""")
+
+        with self.assertLogs(configfile.logger, level='WARNING') as captured:
+            config = configfile.ConfigurationDescription(tmp_file)
+
+        assert config.installables['icu4c'].platforms['linux64'].archive.url == 'https://example.com/second.tar.zst'
+        assert any("installables.icu4c.platforms.linux64" in message for message in captured.output)
+
     def test_configuration_save_expanded(self):
         config = self.fake_config()
         # pretend to expand variables -- doesn't matter that there are no