Skip to content

Commit

Permalink
fix: same manifest folder rules shouldn't be declared multi times
Browse files Browse the repository at this point in the history
warn users with a warning, raise exception when user passed
--check-manifest-rules flag
  • Loading branch information
hfudev committed Jan 16, 2024
1 parent 70a1372 commit d813ef9
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 8 deletions.
7 changes: 1 addition & 6 deletions idf_build_apps/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,7 @@ def find_apps(
Manifest.CHECK_MANIFEST_RULES = check_manifest_rules

if manifest_files:
rules = set()
for _manifest_file in to_list(manifest_files):
LOGGER.debug('Loading manifest file: %s', _manifest_file)
rules.update(Manifest.from_file(_manifest_file).rules)
manifest = Manifest(rules)
App.MANIFEST = manifest
App.MANIFEST = Manifest.from_files(to_list(manifest_files))

modified_components = to_list(modified_components)
modified_files = to_list(modified_files)
Expand Down
25 changes: 24 additions & 1 deletion idf_build_apps/manifest/manifest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
# SPDX-License-Identifier: Apache-2.0

import logging
Expand Down Expand Up @@ -162,6 +162,29 @@ def __init__(
) -> None:
self.rules = sorted(rules, key=lambda x: x.folder)

@classmethod
def from_files(cls, paths: t.List[str]) -> 'Manifest':
# folder, defined_at dict
_known_folders: t.Dict[str, str] = dict()

rules: t.List[FolderRule] = []
for path in paths:
_manifest = cls.from_file(path)

for rule in _manifest.rules:
if rule.folder in _known_folders:
msg = f'Folder "{rule.folder}" is already defined in {_known_folders[rule.folder]}'
if cls.CHECK_MANIFEST_RULES:
raise InvalidManifest(msg)
else:
warnings.warn(msg)

_known_folders[rule.folder] = path

rules.extend(_manifest.rules)

return Manifest(rules)

@classmethod
def from_file(cls, path: str) -> 'Manifest':
with open(path) as f:
Expand Down
35 changes: 34 additions & 1 deletion tests/test_manifest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
# SPDX-License-Identifier: Apache-2.0

import os
Expand Down Expand Up @@ -86,6 +86,39 @@ def test_manifest_with_anchor(tmpdir, monkeypatch):
assert manifest.enable_build_targets('bar') == []


def test_from_files_duplicates(tmp_path, monkeypatch):
yaml_file_1 = tmp_path / 'test1.yml'
yaml_file_1.write_text(
"""
foo:
enable:
- if: IDF_TARGET == "esp32"
""",
encoding='utf8',
)

yaml_file_2 = tmp_path / 'test2.yml'
yaml_file_2.write_text(
"""
foo:
enable:
- if: IDF_TARGET == "esp32"
""",
encoding='utf8',
)

monkeypatch.setattr(idf_build_apps.manifest.manifest.Manifest, 'CHECK_MANIFEST_RULES', True)
folder_path = os.path.join(os.getcwd(), 'foo')
os.makedirs(folder_path)

with pytest.raises(InvalidManifest, match=f'Folder "{folder_path}" is already defined in {str(yaml_file_1)}'):
Manifest.from_files([str(yaml_file_1), str(yaml_file_2)])

monkeypatch.setattr(idf_build_apps.manifest.manifest.Manifest, 'CHECK_MANIFEST_RULES', False)
with pytest.warns(UserWarning, match=f'Folder "{folder_path}" is already defined in {str(yaml_file_1)}'):
Manifest.from_files([str(yaml_file_1), str(yaml_file_2)])


class TestIfParser:
def test_idf_version(self, monkeypatch):
monkeypatch.setattr(idf_build_apps.manifest.if_parser, 'IDF_VERSION', Version('5.9.0'))
Expand Down

0 comments on commit d813ef9

Please sign in to comment.