Skip to content

pnpm development dependencies#1587

Merged
slimreaper35 merged 3 commits into
hermetoproject:mainfrom
slimreaper35:pnpm-enhacements
Jun 10, 2026
Merged

pnpm development dependencies#1587
slimreaper35 merged 3 commits into
hermetoproject:mainfrom
slimreaper35:pnpm-enhacements

Conversation

@slimreaper35

Copy link
Copy Markdown
Member

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request implements tracking of non-development dependencies for pnpm projects by traversing the dependency graph using a BFS algorithm on lockfile snapshots, enabling accurate setting of the npm_development property on SBOM components. The review feedback suggests optimizing the BFS traversal by marking nodes as visited when queued to prevent redundant processing, and adding defensive checks to avoid potential KeyError and TypeError exceptions when parsing malformed lockfiles.

Comment thread hermeto/core/package_managers/javascript/pnpm/resolver.py Outdated
Comment thread hermeto/core/package_managers/javascript/pnpm/project.py
Comment thread hermeto/core/package_managers/javascript/pnpm/resolver.py Outdated
Comment thread hermeto/core/package_managers/javascript/pnpm/project.py Outdated
Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py Outdated
Comment thread hermeto/core/package_managers/javascript/pnpm/project.py Outdated
Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py
Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py Outdated
Comment thread hermeto/core/package_managers/javascript/pnpm/resolver.py

@eskultety eskultety left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Almost there, one more iteration and I'm good.

Comment thread hermeto/core/package_managers/javascript/pnpm/resolver.py
Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py Outdated
@slimreaper35

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the PNPM package manager implementation to trace transitive non-development dependencies and support JSR registry prefixes, adding development properties to the generated SBOM components. Feedback on these changes highlights several robustness issues, including potential AttributeError or TypeError when parsing empty YAML nodes in snapshots, and a potential KeyError if the lockfile is malformed or missing the root importer. Additionally, it is recommended to update and restore the deleted unit test for permissive mode rather than removing it, and to revert the temporary repository and branch configurations in the integration tests before merging.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread hermeto/core/package_managers/javascript/pnpm/resolver.py
Comment thread hermeto/core/package_managers/javascript/pnpm/project.py Outdated
Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py
Comment thread tests/integration/test_pnpm.py Outdated

@taylormadore taylormadore left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just test nits remaining. Otherwise LGTM

Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py Outdated
Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py
Comment thread tests/unit/package_managers/javascript/pnpm/test_resolver.py
The package.json part is already covered below in the
`test_create_root_component` function.  Otherwise, it's just mocking and
length comparison. Coverage is not affected.

Signed-off-by: Michal Šoltis <msoltis@redhat.com>
1. Find all runtime dependencies and optional dependencies first
2. Those that are left are development dependencies

Set the official CycloneDX property.

The overall logic requires parsing snapshots section from the lockfile
and matching them with constructed package IDs from the importers
section in the lockfile.

--
https://github.com/CycloneDX/cyclonedx-property-taxonomy/blob/main/cdx/npm.md

Assisted-by: Cursor
Signed-off-by: Michal Šoltis <msoltis@redhat.com>
Signed-off-by: Michal Šoltis <msoltis@redhat.com>
@slimreaper35 slimreaper35 added this pull request to the merge queue Jun 10, 2026
Merged via the queue into hermetoproject:main with commit c385a7a Jun 10, 2026
13 checks passed
@slimreaper35 slimreaper35 deleted the pnpm-enhacements branch June 10, 2026 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants