From 6f3aeac74239ee2251eaf35010d4460115a38c22 Mon Sep 17 00:00:00 2001
From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Date: Tue, 13 Aug 2024 13:33:00 +0200
Subject: [PATCH] check: Fix handling of <remove-project /> element

When `repo-resource` creates a version string (which is a full XML
representation of a manifest), the `<remove-project>` tag is not
taken into account.
Because of this, we generate "false positives" for new Versions,
which could trigger unwanted builds.

For example, with manifest [1], we see that the Version string has
the device/amlogic/yukawa project twice, which is wrong.

Handle the <remove-project /> element to fix this.

[1] https://github.com/makohoek/demo-manifests/blob/main/aosp_remove_yukawa_project.xml
Fixes: https://github.com/makohoek/repo-resource/issues/36
Fixes: c23fc0c2fea1 ("Switch to getRevision code based on git ls-remote")
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 repo_resource/common.py     | 19 +++++++++++++++++++
 repo_resource/test_check.py | 27 +++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/repo_resource/common.py b/repo_resource/common.py
index 3649bce..6eca4cf 100644
--- a/repo_resource/common.py
+++ b/repo_resource/common.py
@@ -355,6 +355,7 @@ def __manifest_out(self, filename):
 
     def update_manifest(self, jobs):
         projects = []
+        removed_projects = []
 
         jobs = jobs or DEFAULT_CHECK_JOBS
         self.__change_to_workdir()
@@ -379,6 +380,24 @@ def update_manifest(self, jobs):
                     defaultBranch = defaults.get('revision') \
                         or self.__get_remote_revision(defaultRemote)
 
+                # Handle <remove-project /> element:
+                # - Should not be present in final XML (for version)
+                # - Should remove first matching project by name
+                for p in manifest.findall('remove-project'):
+                    project = p.get('name')
+                    removed_projects.append(project)
+                    manifest.remove(p)
+
+                for p in manifest.findall('project'):
+                    project = p.get('name')
+                    if project in removed_projects:
+                        manifest.remove(p)
+                        removed_projects.remove(project)
+                    # If there are no more projects to remove, we can
+                    # skip parsing the rest of the projects
+                    if removed_projects == []:
+                        break
+
                 for p in manifest.findall('project'):
                     project = p.get('name')
                     projectBranch = p.get('revision') or defaultBranch
diff --git a/repo_resource/test_check.py b/repo_resource/test_check.py
index 706c6fd..04e0284 100644
--- a/repo_resource/test_check.py
+++ b/repo_resource/test_check.py
@@ -59,6 +59,13 @@ def setUp(self):
                 'name': 'aosp_multiple_device_fixed.xml'
             }
         }
+        self.remove_project_source = {
+            'source': {
+                'url': 'https://github.com/makohoek/demo-manifests.git',
+                'revision': 'main',
+                'name': 'aosp_remove_yukawa_project.xml'
+            }
+        }
 
     def tearDown(self):
         p = common.CACHEDIR
@@ -359,6 +366,26 @@ def test_jobs_limit(self):
         print('fast: {} slow: {}'.format(fast_duration, slow_duration))
         self.assertTrue(fast_duration < slow_duration)
 
+    # test that the `<remove-project>` tag is correctly handled
+    # When rebuilding the Version string.
+    # See: https://github.com/makohoek/repo-resource/issues/36
+    def test_remove_project_version(self):
+        data = self.remove_project_source
+        instream = StringIO(json.dumps(data))
+        versions = check.check(instream)
+
+        expected_version = '<manifest><remote fetch="https://android.googlesource.com/" name="aosp"></remote><remote fetch="https://gitlab.baylibre.com/baylibre/amlogic/atv/aosp/" name="baylibre"></remote><default remote="aosp" revision="refs/tags/android-13.0.0_r62"></default><project name="device/amlogic/yukawa" path="device/amlogic/yukawa" remote="baylibre" revision="0f3423ec0c103d113c2c03953b3a79bb479fd5b7"></project></manifest>'  # noqa: E501
+
+        version = versions[0]['version']
+        self.assertEqual(version, expected_version)
+
+    def test_no_remove_project_element(self):
+        data = self.remove_project_source
+        instream = StringIO(json.dumps(data))
+        versions = check.check(instream)
+        version = versions[0]['version']
+        self.assertNotIn('remove-project', version)
+
 
 if __name__ == '__main__':
     unittest.main()