Skip to content

Commit

Permalink
check: Fix handling of <remove-project /> element
Browse files Browse the repository at this point in the history
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: #36
Fixes: c23fc0c ("Switch to getRevision code based on git ls-remote")
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
  • Loading branch information
makohoek committed Aug 14, 2024
1 parent caadca4 commit 1b11ea4
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
19 changes: 19 additions & 0 deletions repo_resource/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 not removed_projects:
break

for p in manifest.findall('project'):
project = p.get('name')
projectBranch = p.get('revision') or defaultBranch
Expand Down
27 changes: 27 additions & 0 deletions repo_resource/test_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()

0 comments on commit 1b11ea4

Please sign in to comment.