Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common: resolve recursively include in root manifest #39

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

massonju
Copy link
Contributor

@massonju massonju commented Sep 6, 2024

The root manifest may have 'include' tag, we should resolve recursively these elements and add the contents of the subsequent manifests found in the root manifest.

setuptools must be present in the requirements, otherwise we have the
following errors:
```
$ make test
...
======================================================================
ERROR: repo_resource.test_check (unittest.loader._FailedTest.repo_resource.test_check)
----------------------------------------------------------------------
ImportError: Failed to import test module: repo_resource.test_check
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/unittest/loader.py", line 396, in _find_test_path
    module = self._get_module_from_name(name)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/unittest/loader.py", line 339, in _get_module_from_name
    __import__(name)
  File "/root/repo_resource/test_check.py", line 16, in <module>
    from . import check
  File "/root/repo_resource/check.py", line 19, in <module>
    from repo_resource import common
  File "/root/repo_resource/common.py", line 27, in <module>
    from repo import main as repo
  File "/usr/local/lib/python3.12/site-packages/repo/main.py", line 48, in <module>
    from repo.subcmds.version import Version
  File "/usr/local/lib/python3.12/site-packages/repo/subcmds/__init__.py", line 18, in <module>
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'
```

Signed-off-by: Julien Masson <[email protected]>
@massonju
Copy link
Contributor Author

massonju commented Sep 6, 2024

This PR depends on makohoek/demo-manifests#5

@makohoek makohoek self-requested a review September 10, 2024 08:26
@massonju massonju force-pushed the jmasson/resolve-include-manifest branch from 0783d2a to 12e56e4 Compare September 10, 2024 08:32
Copy link
Owner

@makohoek makohoek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great already. I have one nitpick and one question. Please have a look.

repo_resource/test_check.py Outdated Show resolved Hide resolved
repo_resource/common.py Show resolved Hide resolved
The root manifest may have 'include' tag, we should resolve
recursively these elements and add the contents of the subsequent
manifests found in the root manifest.

Signed-off-by: Julien Masson <[email protected]>
@massonju massonju force-pushed the jmasson/resolve-include-manifest branch from 12e56e4 to 877aba8 Compare September 10, 2024 08:53
@makohoek makohoek merged commit d4a269b into makohoek:main Sep 10, 2024
2 checks passed
@massonju
Copy link
Contributor Author

@makohoek thanks

@massonju massonju deleted the jmasson/resolve-include-manifest branch September 17, 2024 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants