feat: add unified tags mapping across environment builders #991
Conversation
Signed-off-by: rn23thakur <[email protected]>
|
@rn23thakur , thank you for all your efforts and your time. just a note: you dont need to close a pull-request, and restart your changes with another pull request. you could have left it open and improved it. Anyway, I am happy to review your changes. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for extracting and populating tags from package metadata keywords across different Python package managers (Poetry, Pipenv, and standard environment).
Key changes:
- Introduces a new
metadata2tagsfunction inpackaging.pyto parse keywords from package metadata - Updates Poetry, Pipenv, and environment builders to populate component tags from package keywords
- Adds logic to conditionally update component tags when the
tagsattribute exists
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cyclonedx_py/_internal/utils/packaging.py | Adds metadata2tags function to parse keywords from package metadata into tags |
| cyclonedx_py/_internal/poetry.py | Imports and uses tag parsing for Poetry lock file packages |
| cyclonedx_py/_internal/pipenv.py | Imports and uses tag parsing for Pipenv lock file packages |
| cyclonedx_py/_internal/environment.py | Imports and uses metadata2tags for environment-based components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from .cli_common import add_argument_mc_type | ||
| from .utils.cdx import make_bom | ||
| from .utils.packaging import normalize_packagename | ||
| from .utils.packaging import normalize_packagename, to_tags |
There was a problem hiding this comment.
The imported function to_tags does not exist in utils.packaging. The actual function defined in that module is metadata2tags. This will cause an ImportError at runtime.
| from .utils.packaging import normalize_packagename, to_tags | |
| from .utils.packaging import normalize_packagename, metadata2tags |
| ) | ||
|
|
||
| if hasattr(component, 'tags'): | ||
| component.tags.update(to_tags(package.get('keywords'))) |
There was a problem hiding this comment.
The function to_tags does not exist. Based on the implementation in utils/packaging.py, this should call metadata2tags. However, metadata2tags expects a PackageMetadata object, not a raw keywords value. A separate function needs to be created that accepts keywords as a string or list, or this code needs to be adjusted to match the actual function signature.
| from .utils.args import arparse_split | ||
| from .utils.cdx import make_bom | ||
| from .utils.packaging import normalize_packagename | ||
| from .utils.packaging import normalize_packagename, to_tags |
There was a problem hiding this comment.
The imported function to_tags does not exist in utils.packaging. The actual function defined in that module is metadata2tags. This will cause an ImportError at runtime.
| from .utils.packaging import normalize_packagename, to_tags | |
| from .utils.packaging import normalize_packagename, metadata2tags |
| external_references=self.__make_extrefs(package_name, package_data, source_urls), | ||
| ) | ||
| if hasattr(component, 'tags'): | ||
| component.tags.update(to_tags(package_data.get('keywords'))) |
There was a problem hiding this comment.
The function to_tags does not exist. Based on the implementation in utils/packaging.py, this should call metadata2tags. However, metadata2tags expects a PackageMetadata object, not a raw keywords value. A separate function needs to be created that accepts keywords as a string or list, or this code needs to be adjusted to match the actual function signature.
| _KEYWORDS_SPLIT_MATCHER = re_compile(r'[;, ]+') | ||
|
|
||
|
|
||
| def metadata2tags(metadata: 'PackageMetadata') -> Generator[str, None, None]: |
There was a problem hiding this comment.
| from ..py_interop.packagemetadata import PackageMetadata | ||
|
|
||
|
|
||
| _KEYWORDS_SPLIT_MATCHER = re_compile(r'[;, ]+') |
There was a problem hiding this comment.
let's discuss: why did you use a regular expression, instead of just splitting on comma(,)?
(I am not saying you are wrong,I just dont understand the solution, as I might not know all the details.)
| """ | ||
| keywords_string = metadata.get('Keywords', '') | ||
| if keywords_string: | ||
| yield from ( |
There was a problem hiding this comment.
can this me simplified?
pseudo code:
filter(None, map(str.strip,
_KEYWORDS_SPLIT_MATCHER.split(keywords_string)
))
jkowalleck
changed the title
this tool works on specific setups, we all them test beds.
feel free to ask further question. |
2 participants
Hey, I gave it another shot, tried to read and follow the existing patterns. I'm still not sure how to write tests for these, or how to run the pre-existing ones and interpret their results.