llvm_tools: Preserve comments to manifest_utils

Comments should be preserved in the manifest files. While the
code seems to work, the test itself was broken because it
did not use the custom parser.

Expand upon the test to better show that everything works.

BUG=None
TEST=./run_tests_for.py llvm_tools/manifest_utils_unittest.py

Change-Id: I9f6292b8418374cbed777c4f786d498fcb64842b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/4713968
Reviewed-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Ryan Beltran <ryanbeltran@chromium.org>
Commit-Queue: Jordan Abrahams-Whitehead <ajordanr@google.com>
Tested-by: Jordan Abrahams-Whitehead <ajordanr@google.com>

2 files changed, 18 insertions, 4 deletions

diff --git a/llvm_tools/manifest_utils.py b/llvm_tools/manifest_utils.py
index 83d0ca9d..67eae4f3 100644
--- a/llvm_tools/manifest_utils.py
+++ b/llvm_tools/manifest_utils.py
@@ -31,6 +31,13 @@ class UpdateManifestError(Exception):
     pass
 
 
+def make_xmlparser():
+    """Return a new xmlparser with custom TreeBuilder."""
+    return ElementTree.XMLParser(
+        target=ElementTree.TreeBuilder(insert_comments=True)
+    )
+
+
 def update_chromeos_manifest(revision: str, src_tree: Path) -> Path:
     """Replaces the manifest project revision with 'revision'.
 
@@ -53,9 +60,7 @@ def update_chromeos_manifest(revision: str, src_tree: Path) -> Path:
         FormattingError: The manifest could not be reformatted.
     """
     manifest_path = get_chromeos_manifest_path(src_tree)
-    parser = ElementTree.XMLParser(
-        target=ElementTree.TreeBuilder(insert_comments=True)
-    )
+    parser = make_xmlparser()
     xmltree = ElementTree.parse(manifest_path, parser)
     update_chromeos_manifest_tree(revision, xmltree.getroot())
     with atomic_write_file.atomic_write(manifest_path, mode="wb") as f:
diff --git a/llvm_tools/manifest_utils_unittest.py b/llvm_tools/manifest_utils_unittest.py
index c99b01ab..9a21d990 100644
--- a/llvm_tools/manifest_utils_unittest.py
+++ b/llvm_tools/manifest_utils_unittest.py
@@ -19,6 +19,8 @@ on toolchain projects (llvm-project, etc.) which are public.
 
 MANIFEST_FIXTURE = """<?xml version="1.0" encoding="UTF-8"?>
 <manifest>
+  <!-- Comment that should not be removed.
+       Multiple lines. -->
   <include name="_remotes.xml" />
   <default revision="refs/heads/main"
            remote="cros"
@@ -54,13 +56,20 @@ class TestManifestUtils(unittest.TestCase):
     """Test manifest_utils."""
 
     def test_update_chromeos_manifest(self):
-        root = ElementTree.fromstring(MANIFEST_FIXTURE)
+        root = ElementTree.fromstring(
+            MANIFEST_FIXTURE,
+            parser=manifest_utils.make_xmlparser(),
+        )
         manifest_utils.update_chromeos_manifest_tree("wxyz", root)
         string_root1 = ElementTree.tostring(root)
         self.assertRegex(
             str(string_root1, encoding="utf-8"),
             r'revision="wxyz"',
         )
+        self.assertRegex(
+            str(string_root1, encoding="utf-8"),
+            r"<!-- Comment that should not be removed.",
+        )
         self.assertNotRegex(
             str(string_root1, encoding="utf-8"),
             r'revision="abcd"',