-
Notifications
You must be signed in to change notification settings - Fork 52
[WIP] konflux: hermetic build #748
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds hermetic build support: Tekton PipelineRun params, RPM input and lockfiles, a UBI9 repo, hermetic Python requirements and pyproject tooling/groups, and Containerfile changes to conditionally source /cachi2/cachi2.env and run a hermetic uv sync path. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Tekton as Tekton Pipeline
participant Builder as Container build
participant Cachi as /cachi2/cachi2.env
participant UV as uv (package sync)
rect rgb(245,250,255)
note right of Tekton: PipelineRun invoked with params: hermetic, prefetch-input, build-source-image
Tekton->>Builder: start build
Builder->>Cachi: test for /cachi2/cachi2.env
alt cachi2 present (hermetic)
Builder->>Cachi: source env
Cachi-->>Builder: provide PIP_FIND_LINKS / PIP_NO_INDEX
Builder->>UV: uv sync --locked --group llslibdev-hermetic --no-index --find-links ...
else cachi2 absent (non-hermetic)
Builder->>UV: uv sync --locked --group llslibdev ...
end
UV->>Builder: sync result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.tekton/lightspeed-stack-pull-request.yaml (1)
30-30: Replace TODO comments with tracked issues.Informal TODO comments should be converted to tracked issues or linked via ADR/documentation. Line 30 references platform expansion; line 34 mentions coordinating changes with the on-push pipeline.
These items would be easier to track and communicate if converted to:
- GitHub issues with acceptance criteria
- An ADR (Architecture Decision Record) documenting the multi-platform strategy
- Comments in docs linking to the implementation plan
For now, consider adding issue numbers or links to clarify ownership and timeline.
Also applies to: 34-34
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (3)
.tekton/lightspeed-stack-pull-request.yaml (3)
35-40: Verify hermetic build enablement and coordinate on-push pipeline updates.This change activates hermetic (network-isolated) builds alongside source-image generation and dependency prefetching. These are significant behavioral changes that require:
- Verify the hermetic build works end-to-end: Confirm all build steps (especially custom ones in your Containerfile) remain compatible with network isolation.
- Confirm on-push pipeline sync: The TODO on line 34 indicates the on-push pipeline also needs these parameter additions. Ensure both pipelines are kept in sync to maintain consistent behavior across PR and merge workflows.
- Test the prefetch configuration: The
prefetch-inputJSON (line 38) pre-downloads pip dependencies. Verify this works with your project's dependency structure and any custom pip configurations.Run the following checks if available:
- Trigger a test build on this PR branch and verify the build completes successfully with hermetic mode enabled.
- Inspect the on-push pipeline configuration to confirm it also needs updating (see
.tekton/lightspeed-stack-on-push.yamlor similar).- Validate that all build-time dependencies are captured by the prefetch configuration.
1-40: PR requires completion before merge: checklist items outstanding.Per the PR objectives, this PR is marked WIP and the following checklist items remain unchecked:
- Self-review performed
- PR passed pre-merge test jobs
- Core feature tests added (if applicable)
The Testing section contains no concrete test steps or results. Before this can be merged:
- Complete self-review, documenting any concerns or design rationale.
- Run the full pre-merge test suite and provide results.
- Add or update tests to cover the hermetic build behavior if not already present.
- Update the on-push pipeline to maintain consistency (as noted in the TODO).
Please confirm:
- Have pre-merge tests been executed? If so, share the results.
- Are there existing tests for the build pipeline configuration? Do they need updates?
- Is the on-push pipeline update being handled in this PR or a follow-up?
38-38: JSON syntax is valid; manual verification of task compatibility required.The prefetch-input JSON at line 38 is syntactically valid and properly passed to the
prefetch-dependencies-oci-tatask via theinputparameter (line 186). Path"."correctly resolves to the cloned project root since the task runs afterclone-repository. However, verify that the remote task bundle (version 0.2) accepts the string value"allow_binary": "true"as configured—consult the task documentation or test execution to confirm compatibility.
279ac6d to
57b1036
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- rpms.in.yaml
- ubi.repo
- rpms.lock.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Use uv as the package manager; prefix commands with uv run
Applied to files:
requirements.hermetic.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (azure)
- GitHub Check: e2e_tests (ci)
🔇 Additional comments (3)
requirements.hermetic.txt (1)
1-1: Upgrade uv to 0.9.7 after verifying hermetic build reproducibility.uv 0.8.15 is outdated; uv 0.9.7 was released October 30, 2025. However, the default
--python-platform linuxtarget changed from manylinux_2_17 to manylinux_2_28, which affects wheel resolution. Since the project pins a custom PyTorch CPU wheel index and uses a lockedrequirements.txtgenerated viauv pip compile, upgrading to 0.9.7 may causerequirements.txtto diverge, breaking hermetic reproducibility. Regeneraterequirements.txtwith uv 0.9.7 and verify the lock file is identical or intentionally updated before committing the upgrade..tekton/lightspeed-stack-pull-request.yaml (2)
37-38: Theallow_binary: truesetting is standard Cachi2/Hermeto practice and does not conflict with hermetic builds.The
allow_binaryparameter in Cachi2 controls whether the prefetcher will accept binary distributions (wheels) for Python packages instead of forcing source distributions. This setting operates during the prefetch phase—precisely where dependencies are cached before the hermetic build phase begins. Hermetic builds emphasize no external network access during the build itself, not during prefetching. Pre-fetching binary wheels (when available) is a standard optimization and does not weaken hermetic guarantees; the wheels are vendored and available for reproducible builds.The pipeline includes security scanning tasks (clair-scan, sast-snyk-check) that can audit the final artifacts, and the setting is intentional and documented. No action required.
35-40: All specified parameters are properly integrated and configuration files exist.The verification confirms:
Parameters are defined in the pipeline spec: build-source-image (lines 91-94), prefetch-input (lines 82-85), hermetic (lines 78-81) are all declared as string parameters with appropriate defaults.
Parameters are passed to downstream tasks:
prefetch-inputis correctly passed to the prefetch-dependencies task (line 186 as "input") and buildah-remote-oci-ta task (line 225 as "PREFETCH_INPUT")hermeticis passed to buildah-remote-oci-ta (line 223 as "HERMETIC")build-source-imageis used in the source-build-oci-ta task's when condition (line 320)Task references are correct: prefetch-dependencies-oci-ta:0.2, buildah-remote-oci-ta:0.6, and source-build-oci-ta:0.3 are all referenced at the specified lines.
Configuration files exist: rpms.in.yaml, rpms.lock.yaml, and ubi.repo are present in the repository root.
prefetch-input JSON is well-formed with valid fields (type, path, allow_binary).
Note: The prefetch-input JSON schema validation against the external Konflux task bundle schema cannot be performed locally; verify this against the official Konflux CI task catalog documentation if needed.
| value: quay.io/redhat-user-workloads/lightspeed-core-tenant/lightspeed-stack:on-pr-{{revision}} | ||
| - name: image-expires-after | ||
| value: 5d | ||
| # todo: add arm64. refer to https://konflux.pages.redhat.com/docs/users/getting-started/multi-platform-builds.html#arm64-2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete TODOs block mergeability.
Lines 30 and 34 contain TODO comments that indicate incomplete work:
- Line 30: ARM64 multi-platform support not yet added
- Line 34: on-push pipeline not updated to match PR pipeline changes
These must be resolved or tracked as separate issues before merge.
Create follow-up issues or PRs to address:
- Add ARM64 platform support (refer to the linked Konflux documentation)
- Apply the same hermetic/prefetch changes to
.tekton/lightspeed-stack-on-push.yaml
Also applies to: 34-34
🤖 Prompt for AI Agents
In .tekton/lightspeed-stack-pull-request.yaml around lines 30 and 34, there are
TODO comments left for adding ARM64 multi-platform support and updating the
on-push pipeline to match PR-pipeline hermetic/prefetch changes; resolve by
either implementing ARM64 build support per the Konflux multi-platform docs (add
arm64 entries, multi-platform build config and any necessary image/platform
matrix) and by applying the same hermetic/prefetch changes to
.tekton/lightspeed-stack-on-push.yaml, or open two follow-up issues/PRs that
clearly track: (1) adding ARM64 support with a link to the Konflux guidance, and
(2) updating the on-push pipeline to mirror the PR pipeline changes — reference
the new issues/PR numbers in these TODOs and remove or replace the inline TODO
comments so the file is mergeable.
be8d77a to
96fbb5e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
1-195: PR is marked "[WIP] Do Not Merge!" — address before review.The PR objectives indicate this is marked as work-in-progress with incomplete author checklists (self-review, pre-merge tests, feature tests all unchecked). This should be finalized before requesting substantive code review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)pyproject.toml(1 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- rpms.lock.yaml
- .tekton/lightspeed-stack-pull-request.yaml
- rpms.in.yaml
- requirements.hermetic.txt
- ubi.repo
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
🧠 Learnings (1)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to pyproject.toml : ALWAYS check pyproject.toml for existing dependencies before adding new ones
Applied to files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
deee0f1 to
3acd44b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.tekton/lightspeed-stack-pull-request.yaml (1)
30-30: Resolve TODOs before merge.The TODO comments at lines 30 and 34 were previously flagged as merge blockers. These must be addressed by either:
- Implementing the ARM64 support and updating the on-push pipeline, or
- Creating tracked issues and updating the TODOs with issue references.
Also applies to: 34-34
pyproject.toml (1)
208-209: Reconsider pdm in hermetic group—already flagged in previous review.The hermetic group includes both uv==0.8.15 and pdm>=2.21.0. A previous review comment identified that pdm is already the project's build backend and conflicts with the hermetic tooling approach that uses uv. Including both package managers in the same group creates confusion about which tool is intended for hermetic builds.
Based on the Containerfile (line 30) which uses
uv sync, the hermetic group should only include uv, not pdm. Remove pdm from this group.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- requirements.hermetic.txt
- ubi.repo
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
🧠 Learnings (2)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to pyproject.toml : ALWAYS check pyproject.toml for existing dependencies before adding new ones
Applied to files:
pyproject.toml
📚 Learning: 2025-08-18T10:57:39.266Z
Learnt from: matysek
Repo: lightspeed-core/lightspeed-stack PR: 292
File: pyproject.toml:59-59
Timestamp: 2025-08-18T10:57:39.266Z
Learning: In the lightspeed-stack project, transitive dependencies like faiss-cpu are intentionally pinned as top-level dependencies to maintain better control over the dependency graph and avoid version conflicts when bundling ML/LLM tooling packages.
Applied to files:
pyproject.toml
🪛 GitHub Actions: E2E Tests
Containerfile
[error] 29-33: RUN ... uv sync --locked --no-dev --group llslibdev; failed: The lockfile at uv.lock needs to be updated. To update, run 'uv lock'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: build-pr
🔇 Additional comments (2)
.tekton/lightspeed-stack-pull-request.yaml (1)
35-40: LGTM: Hermetic build parameters correctly configured.The three new parameters (build-source-image, prefetch-input, hermetic) are properly structured for Konflux hermetic builds. The prefetch-input correctly specifies both rpm and pip dependency types, with allow_binary enabled for pip packages.
rpms.lock.yaml (1)
1-40: Lockfile structure is correct but may be incomplete.The lockfile is well-formed and includes oniguruma (jq's dependency) and jq for both architectures. However, gcc and patch declared in rpms.in.yaml are not present here. This was flagged in the rpms.in.yaml review.
d46f456 to
5ebdc69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Containerfile (1)
28-33: Consider extracting the common command to reduce duplication.Both branches execute the same
uv synccommand. You can simplify the conditional to only source the cachi2 environment when present, then run the command once:-# Source cachi2 environment for hermetic builds if available, otherwise use normal installation -RUN if [ -f /cachi2/cachi2.env ]; then \ - . /cachi2/cachi2.env && uv sync --locked --no-dev --group llslibdev; \ - else \ - uv sync --locked --no-dev --group llslibdev; \ - fi +# Source cachi2 environment for hermetic builds if available +RUN if [ -f /cachi2/cachi2.env ]; then . /cachi2/cachi2.env; fi && \ + uv sync --locked --no-dev --group llslibdevThis reduces duplication and makes future command changes easier to maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- rpms.in.yaml
- .tekton/lightspeed-stack-pull-request.yaml
- rpms.lock.yaml
- requirements.hermetic.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-pr
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (1)
ubi.repo (1)
17-17: The ubi.repo SRPM URLs are correctly configured.The verification confirms that UBI9 source RPM repositories require the
$basearchvariable in the URL path. Testing shows:
- URLs with
$basearchreturn HTTP 302 (valid redirects)- URLs without
$basearchreturn HTTP 404 (not found)Red Hat's CDN organizes source packages with architecture-specific URL paths, making the current configuration correct. No changes are needed.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.tekton/lightspeed-stack-pull-request.yaml (1)
30-30: Unresolved TODOs block mergeability — duplicate of past review.Lines 30 and 34 contain TODO comments that remain unresolved:
- Line 30: ARM64 multi-platform support not yet added
- Line 34: on-push pipeline not updated to match PR pipeline changes
These were flagged in the previous review as blocking issues. Either resolve them or create and reference follow-up tracking issues before merging.
Also applies to: 34-34
🧹 Nitpick comments (1)
pyproject.toml (1)
178-221: Eliminate duplication: llslibdev-hermetic mirrors entire llslibdev group.Lines 178–221 duplicate the entire
llslibdevgroup (lines 130–177) verbatim. This creates a maintenance burden: any change tollslibdevmust be manually synchronized tollslibdev-hermetic. The comment at line 179 acknowledges this ("the same as llslibdev, just using default index"), but the actual difference in index routing is handled separately at lines 95–98.Explore one of these alternatives:
Option 1: Use group composition (if uv supports it)
llslibdev-hermetic = [ { include-group = "llslibdev" }, ]This inherits all dependencies from
llslibdevwhile allowing the torch source scoping to apply per-group routing.Option 2: Keep duplication with a maintenance warning
Add a prominent comment at line 178 explicitly warning that changes tollslibdev(lines 130–177) must be mirrored tollslibdev-hermetic, and clarify that torch variant routing is controlled by the source scoping at lines 95–98, not by group differences.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- rpms.in.yaml
- requirements.hermetic.txt
- ubi.repo
- rpms.lock.yaml
- Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (2)
pyproject.toml (2)
95-98: Verify torch variant routing for llslibdev-hermetic.Line 96 routes
llslibdevgroup topytorch-cpu(CPU variant), but line 97 routesllslibdev-hermetictopypi-default, which defaults to the GPU variant on PyPI. The past review suggested both groups should use the CPU variant for consistency. Clarify whether the GPU variant is intentional for hermetic builds, or whether line 97 should also referencepytorch-cpu.
222-225: The review comment is based on an incorrect premise and should be dismissed.The review claims that
requirements.hermetic.txt specifies uv==0.8.15 as the hermetic tool, not pdm, but the actual file contents show thatrequirements.hermetic.txtcontains bothuv==0.8.15andpdm>=2.21.0. Therefore, includingpdm>=2.21.0in thetool-hermeticgroup is correct and maintains consistency with the hermetic requirements file. Having pdm as both the build-backend and in the hermetic tooling group is intentional for ensuring reproducible builds.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pyproject.toml (2)
178-221: Consider group composition to reduce duplication.The
llslibdev-hermeticgroup duplicates all 44 dependencies fromllslibdev. While this works, any future changes tollslibdevmust be manually synchronized. If your tooling supports it, consider using group composition/inclusion to reference the base group and avoid the maintenance burden.
95-98: Torch variant mismatch between groups—hermetic will use GPU instead of CPU.The torch source configuration directs
llslibdevto thepytorch-cpuindex but routesllslibdev-hermetictopypi-default, which defaults to the GPU variant. This creates an unintended hardware dependency difference between the two groups that should otherwise be equivalent.Apply this diff to align both groups to the CPU variant:
torch = [ { index = "pytorch-cpu", group = "llslibdev" }, - { index = "pypi-default", group = "llslibdev-hermetic" } + { index = "pytorch-cpu", group = "llslibdev-hermetic" } ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- rpms.in.yaml
- requirements.hermetic.txt
- ubi.repo
- rpms.lock.yaml
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: e2e_tests (azure)
- GitHub Check: e2e_tests (ci)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (5)
Containerfile (1)
28-33: LGTM! Clean hermetic build conditional.The conditional correctly switches between hermetic and standard builds by detecting the cachi2 environment file and selecting the appropriate dependency group. Both branches maintain lockfile integrity with the
--lockedflag..tekton/lightspeed-stack-pull-request.yaml (2)
30-30: TODOs are tracked but need resolution before merge.The TODO comments for ARM64 support (line 30) and on-push pipeline updates (line 34) were flagged in previous reviews as blocking mergeability. Since this PR is still marked WIP, these are acceptable for now, but ensure they're addressed or converted to tracked issues before removing the WIP status.
Also applies to: 34-34
35-40: LGTM! Hermetic build parameters are correctly configured.The new parameters enable hermetic builds with appropriate prefetch configuration for both RPM and Python dependencies. The
allow_binary: truesetting for pip is appropriate for this Python project.pyproject.toml (2)
89-93: LGTM! PyPI default index correctly configured.The explicit pypi-default index is properly configured for hermetic builds that need to use the standard PyPI repository.
99-105: LGTM! Conflict declaration prevents mixing incompatible groups.The mutual exclusion between
llslibdevandllslibdev-hermeticis correctly configured to prevent simultaneous installation.
| tool-hermetic = [ | ||
| "uv==0.8.15", | ||
| "pdm>=2.21.0", | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify pdm is needed for hermetic tooling.
The tool-hermetic group includes both uv==0.8.15 and pdm>=2.21.0. A previous critical review comment suggested removing pdm from this group since uv is the hermetic build tool and pdm is already the build backend. Confirm that pdm is intentionally included here for hermetic builds, or remove it if uv alone is sufficient.
🏁 Script executed:
#!/bin/bash
# Check if pdm is used in hermetic build context beyond being the build backend
echo "=== Check Containerfile for pdm usage ==="
rg -n "pdm" Containerfile || echo "No pdm references in Containerfile"
echo "=== Check hermetic requirements file ==="
if [ -f "requirements.hermetic.txt" ]; then
cat requirements.hermetic.txt
else
echo "requirements.hermetic.txt not found"
fi
echo "=== Check Tekton pipeline for pdm usage ==="
rg -n "pdm" .tekton/lightspeed-stack-pull-request.yaml || echo "No pdm references in pipeline"Length of output: 214
🏁 Script executed:
#!/bin/bash
# Check pyproject.toml for build-backend and hermetic-related info
echo "=== Check build-backend in pyproject.toml ==="
rg -n "build-backend|tool-hermetic" pyproject.toml -A2 -B2
# Look for build/hermetic documentation or scripts
echo "=== Check for build-related documentation ==="
fd -e md -e txt -e rst | xargs rg -l "hermetic\|build.*tool" 2>/dev/null | head -10
# Check if any script invokes pdm during build/hermetic context
echo "=== Check for pdm invocation in scripts/docs ==="
rg -n "pdm\s+(install|build|sync|lock)" --type=py --type=sh --type=bash 2>/dev/null || echo "No pdm invocation patterns found"
# Check Makefile if exists
echo "=== Check Makefile for hermetic/build targets ==="
if [ -f "Makefile" ]; then
rg -n "hermetic|pdm|uv" Makefile
else
echo "Makefile not found"
fiLength of output: 297
🏁 Script executed:
#!/bin/bash
# Check pyproject.toml for build-backend configuration
echo "=== Extract build-backend and tool-hermetic from pyproject.toml ==="
rg -n "build-backend" pyproject.toml -A1 -B1
echo "=== Extract tool-hermetic section ==="
rg -n "\[tool\.hermetic\]" pyproject.toml -A10
echo "=== Extract build-system section ==="
rg -n "\[build-system\]" pyproject.toml -A10
echo "=== Search for any hermetic-related comments in pyproject.toml ==="
rg -n "hermetic" pyproject.toml -B2 -A2
echo "=== Check if uv is documented as the build tool ==="
rg -n "uv" pyproject.toml | head -20Length of output: 1462
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about hermetic builds or why pdm is needed
echo "=== Search for hermetic documentation in README/docs ==="
rg -n "hermetic" README.md docs/ 2>/dev/null || echo "No hermetic references in README/docs"
echo "=== Check git history comments for tool-hermetic context ==="
git log -p --all -S "tool-hermetic" -- pyproject.toml 2>/dev/null | head -100 || echo "Git history not available"
echo "=== Check if pdm is imported or called in any Python files ==="
rg -n "import pdm|from pdm" --type=py 2>/dev/null || echo "No pdm imports found"
echo "=== Check requirements.hermetic.txt history if it exists ==="
if [ -f "requirements.hermetic.txt" ]; then
git log -p --all -- requirements.hermetic.txt 2>/dev/null | head -100 || echo "Git history not available"
fiLength of output: 431
🏁 Script executed:
#!/bin/bash
# Check README context around hermetic builds mention
echo "=== README context for hermetic builds ==="
sed -n '730,750p' README.md
# Double-check: is pdm ever invoked as a command in any files?
echo "=== Search for pdm command invocations (not just imports) ==="
rg -n "pdm" --type=sh --type=bash --type=yaml --type=makefile 2>/dev/null || echo "No pdm found in build files"
# Check the full tool-hermetic group definition
echo "=== Full tool-hermetic group from pyproject.toml ==="
rg -n "tool-hermetic" pyproject.toml -A5Length of output: 783
Remove pdm from tool-hermetic group; it is unused in hermetic builds.
Verification confirms that pdm is not invoked during hermetic builds. No pdm references appear in the Containerfile, Tekton pipeline, or any build scripts. The hermetic build context uses only uv. Since pdm's sole role is as the project's build backend (specified in build-system), it does not belong in the tool-hermetic dependency group. Remove "pdm>=2.21.0" from lines 222-225.
🤖 Prompt for AI Agents
In pyproject.toml around lines 222 to 225, the tool-hermetic dependency group
incorrectly includes "pdm>=2.21.0" which is unused in hermetic builds; remove
the pdm entry so the group contains only "uv==0.8.15". Update the file by
deleting the "pdm>=2.21.0" line and ensure trailing commas/formatting remain
valid (leave the tool-hermetic list with only the uv entry).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pyproject.toml (1)
222-225: Remove pdm from tool-hermetic; it is unused in hermetic builds.Previous review verified that pdm is not invoked during hermetic builds (no references in Containerfile, Tekton pipeline, or build scripts). The hermetic context uses only uv. Since pdm's sole role is the project's build backend (
pdm.backendinbuild-systemat line 247), it does not belong intool-hermetic. Remove line 224 ("pdm>=2.21.0") to keep only the uv entry.Apply this diff:
tool-hermetic = [ "uv==0.8.15", - "pdm>=2.21.0", ]
🧹 Nitpick comments (1)
pyproject.toml (1)
178-225: Eliminate duplication between llslibdev and llslibdev-hermetic groups; document synchronization requirement.Lines 178–221 duplicate all 44 lines of the llslibdev group verbatim (lines 130–177), then append build tools in lines 222–225. This creates a maintenance burden: any dependency update to llslibdev must be manually mirrored to llslibdev-hermetic.
If uv's dependency group syntax supports group composition (e.g.,
include-group), refactor to reference the base group and only override/append differing entries. Otherwise, add a prominent comment warning maintainers to mirror llslibdev changes to llslibdev-hermetic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- rpms.in.yaml
- .tekton/lightspeed-stack-pull-request.yaml
- rpms.lock.yaml
- ubi.repo
- requirements.hermetic.txt
- Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (1)
pyproject.toml (1)
90-98: Clarify intentionality of torch variant split across indices.Past review (marked resolved in fadad34) recommended extending CPU variant to both llslibdev and llslibdev-hermetic. However, the current code specifies
pytorch-cpufor llslibdev butpypi-defaultfor llslibdev-hermetic, causing hermetic builds to resolve torch from the default GPU-first PyPI index—contradicting the stated intent.Is this split intentional (perhaps hermetic builds require GPU torch despite non-hermetic using CPU)? If so, add an explanatory comment. If not, apply the suggested fix to use
pytorch-cpufor both groups.
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pyproject.toml (2)
178-221: Acknowledge: duplication between llslibdev groups flagged in earlier review.The complete duplication of the
llslibdevgroup intollslibdev-hermetic(43 lines) was already flagged as a major refactor suggestion in previous reviews. This creates a maintenance burden where changes tollslibdevmust be manually synchronized tollslibdev-hermetic.The earlier review suggested either using group composition (if supported by uv) or adding prominent documentation about the need to keep both groups synchronized.
222-225: Remove pdm from tool-hermetic—it's unused and already the build backend.The
tool-hermeticgroup includespdm>=2.21.0, but pdm is not used in hermetic builds. As noted in earlier reviews, pdm is already specified as the build backend (pdm-backendin[build-system]at line 246) and serves no purpose in the hermetic tooling group. The hermetic build workflow only requiresuv.Apply this diff to remove pdm:
tool-hermetic = [ "uv==0.8.15", - "pdm>=2.21.0", ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- rpms.lock.yaml
- requirements.hermetic.txt
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (azure)
- GitHub Check: e2e_tests (ci)
🔇 Additional comments (6)
.tekton/lightspeed-stack-pull-request.yaml (1)
35-40: LGTM—hermetic build parameters correctly configured.The new parameters (
build-source-image: 'true',prefetch-inputwith rpm and pip types, andhermetic: 'true') are correctly configured for Konflux hermetic builds. The prefetch-input JSON structure properly specifies both RPM and Python dependency prefetching withallow_binaryenabled for pip packages.Note: The TODO comments at lines 30 and 34 regarding ARM64 support and on-push pipeline updates were already flagged in previous reviews.
pyproject.toml (2)
90-93: LGTM—pypi-default index correctly configured.The explicit pypi-default index is properly configured to provide an alternative source for packages when not using the specialized pytorch-cpu index. The
explicit = trueflag ensures it's only used when explicitly referenced.
99-105: LGTM—conflicts declaration prevents mixed group usage.The conflicts block correctly prevents simultaneous use of the
llslibdevandllslibdev-hermeticgroups, ensuring users explicitly choose between the standard and hermetic build paths.rpms.in.yaml (1)
1-4: LGTM—RPM input configuration correctly structured.The configuration properly declares the required packages (
gcc,jq,patch), references the UBI repository file, and specifies both x86_64 and aarch64 architectures. This aligns with the hermetic build requirements and multi-platform support indicated in the Tekton pipeline configuration.Note: The earlier review comment about lockfile completeness (gcc and patch missing from rpms.lock.yaml) was marked as addressed in commits d9da223 to c621aaa.
ubi.repo (1)
1-62: LGTM—UBI9 repository configuration correctly structured.The repository configuration properly defines all three UBI9 repository categories (BaseOS, AppStream, CodeReady Builder) with their respective RPM, Debug, and Source variants. The configuration follows standard yum/dnf repository file format:
- Correct baseurl patterns using
$basearchvariable for multi-architecture support- GPG verification enabled (
gpgcheck = 1) with proper key path for all repositories- Main RPM repositories appropriately enabled (
enabled = 1)- Debug and Source repositories appropriately disabled (
enabled = 0)- URLs point to the public Red Hat UBI CDN
This configuration provides the reproducible package sources required for hermetic builds.
Containerfile (1)
28-33: No action required—requirements.txtexists and is correctly referenced.The verification confirms that
requirements.txtexists in the repository, and the Containerfile correctly references it on line 30 for the hermetic build path. The code is valid as written.
79004b6 to
a0b7bd9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
pyproject.toml (3)
222-225: Remove pdm from tool-hermetic—unresolved from past review.Despite past review comments claiming this was addressed (commit showing as resolved), the
tool-hermeticgroup still includespdm>=2.21.0. Past verification confirmed that pdm is not invoked during hermetic builds and serves only as the project's build backend. This contradicts the hermetic tooling purpose and should be removed.Apply this diff to remove pdm:
tool-hermetic = [ "uv==0.8.15", - "pdm>=2.21.0", ]
178-221: Eliminate 44-line duplication—hermetic group copies entire llslibdev.Despite past review comments claiming this was addressed (commit be34f46), the
llslibdev-hermeticgroup still duplicates all 44 lines ofllslibdevverbatim. This creates significant maintenance burden: any change tollslibdevmust be manually synchronized tollslibdev-hermetic. The comment at line 179 claims "the same as llslibdev, just using default index", but the actual package pins are identical—only the torch source routing differs (line 95-98).Consider documenting the duplication risk prominently if tool-level group composition is not supported:
llslibdev-hermetic = [ + # WARNING: This group duplicates llslibdev verbatim. Any changes to llslibdev + # must be manually synchronized here. The torch GPU/CPU variant is controlled + # by [tool.uv.sources] torch configuration (lines 95-98), not by these pins. # the same as llslibdev, just using default index. "matplotlib>=3.10.0",Alternatively, if UV supports group references or composition, refactor to reference
llslibdevinstead of duplicating it.
95-98: Torch variant mismatch persists—hermetic group resolves GPU variant from PyPI.Despite past review comments claiming this was addressed (commit fadad34), the torch source configuration still specifies
index = "pytorch-cpu"forllslibdevbutindex = "pypi-default"forllslibdev-hermetic. This means:
llslibdev: CPU-only torch variant (~200MB smaller)llslibdev-hermetic: GPU torch variant from PyPI (larger, includes CUDA)This inconsistency contradicts the comment at line 179 stating "the same as llslibdev" and will cause hermetic builds to pull a different, heavier torch package.
Apply this diff to use the CPU variant for both groups:
torch = [ { index = "pytorch-cpu", group = "llslibdev" }, - { index = "pypi-default", group = "llslibdev-hermetic" } + { index = "pytorch-cpu", group = "llslibdev-hermetic" } ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .tekton/lightspeed-stack-pull-request.yaml
- rpms.in.yaml
- Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
🧠 Learnings (1)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Use uv as the package manager; prefix commands with uv run
Applied to files:
requirements.hermetic.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (4)
pyproject.toml (2)
99-105: LGTM—conflict prevents accidental mixing of hermetic and non-hermetic groups.The UV conflicts declaration correctly ensures that
llslibdevandllslibdev-hermeticcannot be installed together, preventing unintended dependency resolution issues.
90-93: Based on my verification, I found that the pyproject.toml configuration uses two explicit index entries to support different build modes:
pytorch-cpuindex for standard builds (llslibdev group)pypi-defaultindex for hermetic builds (llslibdev-hermetic group)The torch package is explicitly pinned to each index per group. The configuration uses dependency groups with explicit conflicts to ensure builds use only one mode at a time. The hermetic build variant (llslibdev-hermetic) uses PyPI directly rather than pytorch.org/whl, likely because Konflux hermetic builds have limited network access to external registries.
Marking
pypi-defaultasexplicit = trueactually reduces variability by restricting torch to a pinned source, which is appropriate for reproducible hermetic builds. This is the opposite of the concern stated in the review comment.
Configuration is correct for hermetic build support.
The
pypi-defaultindex is necessary and correctly configured. It explicitly pins torch to PyPI for thellslibdev-hermeticgroup, ensuring reproducibility in hermetic builds. Marking it asexplicit = truerestricts its use to only the pinned package (torch), reducing—not increasing—variability. The dual-index setup appropriately supports both standard (pytorch-cpu) and hermetic (pypi-default) build workflows with dependency group conflicts to prevent mixing them.rpms.lock.yaml (1)
1-355: I need to search for scripts or tooling that generates the lock file.Now let me search the codebase for generation scripts and build documentation:Based on my verification, the review comment's concerns are valid but the request is incomplete. Here are my findings:
Verified Facts:
- The rpms.lock.yaml file is auto-generated from rpms.in.yaml using the rpm-lockfile-prototype tool via:
rpm-lockfile-prototype --outfile=rpms.lock.yaml rpms.in.yaml- The rpm-lockfile-prototype tool uses live dnf metadata to resolve a given rpms.in.yaml file into an rpms.lock.yaml file in which every RPM is pinned to a repository and version
- The packages in the lock file (27+) correctly include gcc, jq, patch plus all their transitive dependencies
- Missing: The codebase has no documented process for regenerating the lockfile
Verify lockfile is auto-generated and matches rpms.in.yaml.
The rpms.lock.yaml file is correctly auto-generated from rpms.in.yaml. To generate the lockfile, the rpm-lockfile-prototype tool is run with:
rpm-lockfile-prototype --outfile=shared-ingress/rpms.lock.yaml shared-ingress/rpms.in.yamlHowever, the codebase lacks documentation on how developers should regenerate this lockfile when updating rpms.in.yaml. Add regeneration instructions to README or CONTRIBUTING.md, specifying:
- The tool to use (rpm-lockfile-prototype)
- Required dependencies (dnf, python3)
- Command to run for regeneration
- When regeneration is needed (whenever rpms.in.yaml changes)
ubi.repo (1)
1-62: Security configuration is correct; verification request is based on incorrect assumptions about build process.The
ubi.repofile is used by Cachi2 (Red Hat's hermetic build tool) for build-time RPM dependency resolution viarpms.in.yaml, not as runtime container configuration. The file is never copied into the final container image, as evidenced by the absence of anyCOPY/ADDcommands for it in the Containerfile. The base UBI image already includes the standard Red Hat GPG key (RPM-GPG-KEY-redhat-release), and GPG key validation during builds is handled by the Konflux/Tekton CI/CD pipeline, not the container itself.The original review comment's security assessment remains valid:
- HTTPS URLs are used
- GPG checking is enabled
- Only main RPM repositories are enabled at runtime
- Standard Red Hat GPG key location is referenced
No changes needed.
| @@ -0,0 +1,2 @@ | |||
| uv==0.8.15 | |||
| pdm>=2.21.0 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove pdm from hermetic requirements—it's the build backend, not a hermetic tool.
Based on past verification, pdm is not invoked during hermetic builds and serves only as the project's build backend (pdm-backend in [build-system]). The hermetic build workflow uses uv. Including pdm>=2.21.0 here is inconsistent with the hermetic tooling purpose.
Apply this diff to remove pdm:
uv==0.8.15
-pdm>=2.21.0Based on learnings
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pdm>=2.21.0 | |
| uv==0.8.15 |
🤖 Prompt for AI Agents
In requirements.hermetic.txt around line 2, the file incorrectly lists
"pdm>=2.21.0" which should be removed because pdm is the build backend, not a
hermetic runtime tool; edit the file to delete that line so the hermetic
requirements only contain tools actually used at runtime by the hermetic build
workflow (ensure no leftover whitespace or blank-line artifacts remain).
|
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
rpms.lock.yaml (1)
1-355: RPM lockfile structure looks correct.The lockfile format is valid and includes:
- Proper metadata (lockfileVersion, lockfileVendor)
- Complete package information for both aarch64 and x86_64
- SHA256 checksums for integrity verification
- Official Red Hat UBI repository URLs
This supports reproducible hermetic builds by pinning exact RPM versions. Ensure this file is regenerated when RPM dependencies change (via
rpms.in.yamlupdates).Maintenance note: This lockfile should be version-controlled and regenerated using the appropriate tooling (e.g., cachi2) whenever
rpms.in.yamlis updated to ensure consistency between the input specification and the locked versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .tekton/lightspeed-stack-pull-request.yaml
- rpms.in.yaml
- ubi.repo
- pyproject.toml
- requirements.hermetic.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (azure)
- GitHub Check: e2e_tests (ci)
| # (avoid accidental inclusion of local directories or env files or credentials) | ||
| COPY ${LSC_SOURCE_DIR}/src ./src | ||
| COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ./ | ||
| COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ${LSC_SOURCE_DIR}/requirements.txt ./ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify requirements.txt usage in hermetic build path.
The requirements.txt file is now copied but doesn't appear to be used in either the hermetic or standard build paths (lines 29-33). Ensure this file serves a purpose in the hermetic build workflow, or consider if it should be removed to avoid confusion.
Run the following script to check if requirements.txt is referenced elsewhere:
🏁 Script executed:
#!/bin/bash
# Description: Verify requirements.txt usage across the codebase
echo "=== Searching for requirements.txt references ==="
rg -n "requirements\.txt" --type dockerfile --type yaml --type toml -C2
echo -e "\n=== Check if requirements.hermetic.txt exists ==="
fd -t f "requirements.*\.txt$"Length of output: 178
I need to examine the Containerfile more closely and search for actual usage of requirements.txt in the build process.The web search gave general context about hermetic builds, but I need to examine the actual Containerfile. Let me search for the specific build logic in the codebase.
🏁 Script executed:
#!/bin/bash
# Search for requirements.txt usage in build commands and view Containerfile around lines 29-33
echo "=== View Containerfile lines 20-40 ==="
head -40 Containerfile | tail -20
echo -e "\n=== Search for pip or requirements usage in Containerfile ==="
rg -n "pip|requirements" Containerfile -C1
echo -e "\n=== Search for hermetic or RUN commands in Containerfile ==="
rg -n "hermetic|RUN" Containerfile -C1Length of output: 344
I need to clone the repository to access the Containerfile. Let me retry:
🏁 Script executed:
#!/bin/bash
# View Containerfile and check requirements.txt usage
echo "=== View entire Containerfile ==="
cat Containerfile
echo -e "\n=== Search for requirements.txt usage ==="
rg -n "requirements\.txt" -C2
echo -e "\n=== Search for hermetic build logic ==="
rg -n "hermetic" -C3Length of output: 43388
requirements.txt is copied but unused in the Containerfile build—remove it from line 25.
The hermetic build uses uv pip sync --group llslibdev-hermetic pyproject.toml, and the standard build uses uv sync --locked --no-dev --group llslibdev. Neither references requirements.txt. The file is generated as an artifact (per Makefile and README.md), but does not serve a purpose in the container build process. Copying it adds unnecessary bloat and creates confusion.
Remove ${LSC_SOURCE_DIR}/requirements.txt from the COPY statement on line 25.
🤖 Prompt for AI Agents
In Containerfile around line 25, the COPY statement currently includes
${LSC_SOURCE_DIR}/requirements.txt which is unused in the container build;
remove ${LSC_SOURCE_DIR}/requirements.txt from that COPY invocation so only
pyproject.toml, LICENSE, README.md, uv.lock (and any other needed files) are
copied, then rebuild to verify image size and that the hermetic and standard
build flows still succeed.
5f4c673 to
90686ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
pyproject.toml (3)
95-98: Torch variant mismatch—llslibdev-hermetic should use pytorch-cpu, not pypi-default.The torch configuration (lines 95–98) specifies
pytorch-cpuindex forllslibdev(CPU-only, smaller) butpypi-defaultforllslibdev-hermetic(GPU variant, much larger). This contradicts the intent that hermetic builds should match the dev environment. A past review flagged this same issue and marked it addressed, but the code still shows the inconsistency.Apply this diff to align both groups on the CPU variant:
torch = [ { index = "pytorch-cpu", group = "llslibdev" }, - { index = "pypi-default", group = "llslibdev-hermetic" } + { index = "pytorch-cpu", group = "llslibdev-hermetic" } ]Verify: Was commit fadad34 actually merged to this branch, and if so, why does the torch variant still differ?
178-221: Eliminate duplication—llslibdev-hermetic copies entire llslibdev group.Lines 178–221 duplicate all 44 lines of the
llslibdevgroup verbatim, creating maintenance risk: future changes tollslibdevmust be manually mirrored to avoid divergence. A past review flagged this same maintenance burden and marked it addressed, but the full duplication remains.If your uv version supports group composition, refactor to reference the existing group:
llslibdev-hermetic = [ { include-group = "llslibdev" }, "torch==2.7.1", # re-pin here if needed to override index scoping ]Otherwise, add a prominent comment warning that changes to
llslibdevmust be manually synced tollslibdev-hermetic, and clarify that the torch GPU/CPU variant is determined by the index scoping (line 97), not by duplicate pins here.
222-226: Remove pdm and pdm-backend from tool-hermetic—not used in hermetic builds.The
tool-hermeticdependency group includespdm>=2.21.0andpdm-backend==2.4.5, but hermetic builds use onlyuv(line 223). Since pdm is already the project's build backend (specified in[build-system]line 248), it does not belong in this group. A past review flagged this and marked it addressed, but both entries remain.Apply this diff to remove the unused build-system entries:
tool-hermetic = [ "uv==0.8.15", - "pdm>=2.21.0", - "pdm-backend==2.4.5", ]
🧹 Nitpick comments (1)
.tekton/lightspeed-stack-pull-request.yaml (1)
30-30: TODO comments block merge—resolve or track as separate issues.Lines 30 and 34 contain incomplete work markers that prevent merging:
- Line 30: ARM64 multi-platform support not yet implemented
- Line 34: On-push pipeline not synchronized with PR pipeline changes
Given that the PR is marked [WIP] and the author checklist is unchecked, these are documented known gaps. Before merge, either implement both features or open two follow-up issues with explicit references and remove/replace the TODO comments so the file is merge-ready.
I can help generate the follow-up issue templates or implement ARM64 support per the Konflux multi-platform documentation if you'd like.
Also applies to: 34-34
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- rpms.lock.yaml
- ubi.repo
- requirements.hermetic.txt
- Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml
Files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (azure)
- GitHub Check: e2e_tests (ci)
🔇 Additional comments (2)
.tekton/lightspeed-stack-pull-request.yaml (1)
35-40: Parameter additions are properly integrated.The three new parameters (
build-source-image,prefetch-input,hermetic) align with pipeline spec definitions and are correctly forwarded to dependent tasks (buildah-remote, source-build, etc.). The prefetch-input JSON structure matches the prefetch-dependencies task expectations.rpms.in.yaml (1)
1-4: Structure and architecture alignment look correct.The RPM configuration correctly declares all required packages (gcc, jq, patch), references the ubi.repo repository, and specifies both target architectures. This aligns with the Tekton prefetch-input configuration and the build-platforms specified in the PipelineRun.
d3c6138 to
9a447a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
Containerfile (1)
28-28: Remove unused requirements.txt from COPY statement.This issue was previously flagged:
requirements.txtis copied but not used in either the hermetic or standard build paths. Both invokeuv syncwithpyproject.toml. Remove${LSC_SOURCE_DIR}/requirements.txtfrom the COPY statement.pyproject.toml (1)
222-226: Remove pdm and pdm-backend from tool-hermetic group.A previous review comment identified that pdm is not invoked during hermetic builds and requested its removal from this group. Verification confirmed no pdm usage in the Containerfile, pipeline, or build scripts. The hermetic build uses only uv. Since pdm's role is limited to being the build backend (
[build-system]), it should not be in this dependency group.Apply this diff:
tool-hermetic = [ "uv==0.8.15", - "pdm>=2.21.0", - "pdm-backend==2.4.5", ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .tekton/lightspeed-stack-pull-request.yaml
- ubi.repo
- rpms.in.yaml
- requirements.hermetic.txt
- rpms.lock.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (1)
pyproject.toml (1)
89-105: Incomplete hermetic torch configuration—intentional staging for WIP?The torch source for
llslibdev-hermetic(line 97), thepypi-defaultindex (lines 90-93), and the conflicts block (lines 99-105) are all commented out. This is consistent with thellslibdev-hermeticdependency group also being commented out (lines 178-221), but it leaves hermetic build support incomplete. Given the PR is marked[WIP]and "Do Not Merge," this may be intentional staging.If hermetic builds are intended to be testable in this PR, uncomment these blocks. Otherwise, document that hermetic configuration is staged but not yet active.
| # Install pdm and pdm-backend for hermetic builds | ||
| RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused pdm installation—hermetic builds use only uv.
This conditional installation of pdm and pdm-backend was previously flagged as unnecessary. Verification confirms that the hermetic build path (line 36) invokes only uv sync, and no pdm commands appear elsewhere in the Containerfile or pipeline configuration. Since pdm's sole role is as the build backend (specified in [build-system] of pyproject.toml), installing it here adds unnecessary dependencies and image bloat.
Remove these lines:
-# Install pdm and pdm-backend for hermetic builds
-RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Install pdm and pdm-backend for hermetic builds | |
| RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi |
🤖 Prompt for AI Agents
In Containerfile around lines 22 to 23, remove the conditional RUN that installs
pdm and pdm-backend (the line starting with "RUN if [ -f /cachi2/cachi2.env ];
then pip3.12 install \"pdm>=2.21.0\" \"pdm-backend\"; fi") because hermetic
builds only use "uv sync" and no pdm commands are used elsewhere; delete that
RUN step to avoid unnecessary dependencies and image bloat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pyproject.toml (1)
222-226: Remove pdm from tool-hermetic; it is unused in hermetic builds.The
tool-hermeticgroup includes"pdm>=2.21.0"(line 224), but verification from past review confirms pdm is not invoked during hermetic builds. No pdm references appear in the Containerfile, Tekton pipeline, or build scripts. Since pdm's sole role is as the project's build backend (specified in[build-system]line 247), it does not belong in the hermetic tooling group.Additionally, the version constraint
2.21.0is outdated; the latest stable release is 2.26.1 (October 2025).Apply this diff to remove pdm and keep only uv for hermetic builds:
tool-hermetic = [ "uv==0.8.15", - "pdm>=2.21.0", "pdm-backend==2.4.5", ]Verify whether
pdm-backend==2.4.5is also needed in hermetic builds or should be removed as well.
🧹 Nitpick comments (1)
pyproject.toml (1)
178-221: Duplication of llslibdev group remains; consider refactoring for maintainability.The
llslibdev-hermeticgroup duplicates all 43 lines ofllslibdevverbatim before adding hermetic tools. This creates a maintenance burden: any future change tollslibdev(dependencies, versions, comments) must be manually mirrored tollslibdev-hermeticto keep them in sync.Past review suggested using group composition (e.g.,
include-group) if the tool supports it, or at minimum documenting the duplication risk with a prominent comment. The current implementation has neither.If uv supports dependency group composition, refactor to:
llslibdev-hermetic = [ { include-group = "llslibdev" }, "uv==0.8.15", ]If composition is not supported, add a comment warning that changes to
llslibdevmust be mirrored here to prevent divergence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(2 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Containerfile
- .tekton/lightspeed-stack-pull-request.yaml
- ubi.repo
- requirements.hermetic.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (3)
rpms.in.yaml (1)
1-4: ✅ RPM configuration is complete and consistent with lockfile.All three declared packages (gcc, jq, patch) are now present in rpms.lock.yaml for both x86_64 and aarch64 architectures with resolved NEVRA entries. The reference to
./ubi.repois valid, and the architecture list matches the lockfile structure.rpms.lock.yaml (1)
1-355: ✅ Lockfile structure and content are valid.The lockfile includes all declared packages with proper NEVRA entries, sha256 checksums, and architecture-specific URLs for both aarch64 and x86_64. Repository IDs (ubi-9-baseos-rpms, ubi-9-appstream-rpms) align with the ubi.repo configuration.
pyproject.toml (1)
95-98: Torch source still uses PyPI default for hermetic group (GPU variant mismatch).Line 97 specifies
index = "pypi-default"forllslibdev-hermetic, which resolves torch to the PyPI GPU variant. This contradicts the CPU-only variant used byllslibdev(line 96:pytorch-cpu), causing the hermetic build to pull a significantly larger, architecture-incompatible torch package.Past review flagged this issue as "Addressed in commit f35507a" but the fix was not applied to the current code.
Apply this diff to align hermetic with non-hermetic torch resolution:
torch = [ { index = "pytorch-cpu", group = "llslibdev" }, - { index = "pypi-default", group = "llslibdev-hermetic" } + { index = "pytorch-cpu", group = "llslibdev-hermetic" } ]
Signed-off-by: Haoyu Sun <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (6)
pyproject.toml (3)
178-221: Duplication remains between llslibdev groups—consider composition.The llslibdev-hermetic group duplicates 43 of 44 lines from llslibdev, differing only in torch's platform marker (line 212). Past review flagged this maintenance burden: changes to llslibdev must be manually mirrored. While marked as addressed in a previous commit, the duplication persists.
If UV supports group composition, consider:
llslibdev-hermetic = [ { include-group = "llslibdev" }, # Override torch with platform marker "torch==2.7.1; sys_platform == 'linux'", ]Otherwise, add a prominent comment warning that llslibdev changes must be mirrored here.
Based on learnings
95-98: Torch variant mismatch persists—hermetic group uses GPU variant.This issue remains from past review: llslibdev uses
pytorch-cpuindex (CPU-only), while llslibdev-hermetic usespypi-default(GPU variant, significantly larger). This contradicts the stated intent that hermetic should mirror llslibdev except for build tools.Apply this diff to use CPU variant for both:
torch = [ { index = "pytorch-cpu", group = "llslibdev" }, - { index = "pypi-default", group = "llslibdev-hermetic" } + { index = "pytorch-cpu", group = "llslibdev-hermetic" } ]Based on learnings
222-226: Remove pdm from tool-hermetic—not used in hermetic builds.Past verification confirmed pdm is not invoked during hermetic builds. The hermetic workflow uses only uv. Since pdm's role is solely as the build backend (in
[build-system]), it does not belong in the tool-hermetic dependency group.Apply this diff:
tool-hermetic = [ "uv==0.8.15", - "pdm>=2.21.0", - "pdm-backend==2.4.5", ]Based on learnings
Containerfile (2)
22-23: Remove unused pdm installation—hermetic builds use only uv.This conditional pdm installation remains from a previous review. Verification shows hermetic builds invoke only
uv sync(line 36), and no pdm commands are used elsewhere. Installing pdm here adds unnecessary dependencies and image bloat.Apply this diff:
-# Install pdm and pdm-backend for hermetic builds -RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi -Based on learnings
28-28: Remove unused requirements.txt from COPY statement.This file is copied but never used in the build. Past review confirmed neither the hermetic build (
uv sync --group llslibdev-hermetic) nor the standard build (uv sync --group llslibdev) references requirements.txt. It adds unnecessary bloat.Apply this diff:
-COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ${LSC_SOURCE_DIR}/requirements.txt ./ +COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ./Based on learnings
requirements.hermetic.txt (1)
1-3: Remove pdm and pdm-backend—only uv is used in hermetic builds.Past review confirmed pdm is not invoked during hermetic builds and serves only as the build backend. The hermetic workflow uses uv exclusively. Including these packages is inconsistent with hermetic tooling requirements.
Apply this diff:
uv==0.8.15 -pdm>=2.21.0 -pdm-backendBased on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.tekton/lightspeed-stack-pull-request.yaml(1 hunks)Containerfile(1 hunks)pyproject.toml(3 hunks)requirements.hermetic.txt(1 hunks)rpms.in.yaml(1 hunks)rpms.lock.yaml(1 hunks)ubi.repo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- rpms.in.yaml
- .tekton/lightspeed-stack-pull-request.yaml
- ubi.repo
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (4)
pyproject.toml (2)
90-93: LGTM: pypi-default index added for hermetic group.The explicit pypi-default index enables hermetic builds to use PyPI packages while the standard build uses pytorch-cpu index. The
explicit = trueflag ensures packages must explicitly reference this index.
99-105: LGTM: UV conflicts prevent mixing hermetic and non-hermetic groups.The conflict declaration correctly prevents simultaneous installation of llslibdev and llslibdev-hermetic groups, enforcing clean separation between standard and hermetic builds.
Containerfile (1)
31-39: Hermetic build conditional verified as correct.The uv.lock lockfile exists and contains the llslibdev-hermetic group, confirming that past critical issues (missing hermetic group and outdated lockfile) have been resolved. The conditional logic correctly routes to the hermetic path with proper environment sourcing and Python version specification.
rpms.lock.yaml (1)
1-355: Lockfile is current and valid.All three packages specified in rpms.in.yaml are present in rpms.lock.yaml for both architectures:
- gcc (lines 18, 197)
- jq (lines 137, 309)
- patch (lines 74, 246)
Additional packages are transitive dependencies, which is correct for a hermetic build lockfile. All packages include required metadata (URL, checksum, size, EVR, source RPM). The lockfile is properly formatted and up-to-date with the input specification.
| "RAG" | ||
| ] | ||
| requires-python = ">=3.12,<3.14" | ||
| requires-python = ">=3.12,<3.13" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Python version restricted to 3.12.x—verify intentional for hermetic builds.
The Python requirement is narrowed from a broader range to >=3.12,<3.13, restricting to Python 3.12.x only. This is typical for hermetic builds requiring specific versions, but verify this doesn't break compatibility for users on different Python versions.
Verify this restriction is documented and intentional:
🏁 Script executed:
#!/bin/bash
# Description: Check if Python version restriction is documented
echo "=== Check README for Python version requirements ==="
rg -n "python.*3\.12|requires.*python" README.md -i -A2 -B2 || echo "Not documented in README"
echo -e "\n=== Check if CI uses Python 3.12 ==="
rg -n "python.*3\.12|python-version.*3\.12" .github/workflows/ .tekton/ -A1 -B1 || echo "Not found in CI config"Length of output: 2664
Python version restriction breaks documented 3.13 support.
The change restricts to >=3.12,<3.13 (3.12.x only), but the README documents support for both Python 3.12 and 3.13, and CI tests run on both versions. Update to >=3.12,<3.14 to maintain advertised compatibility, or clarify documentation if 3.13 support is being dropped.
🤖 Prompt for AI Agents
In pyproject.toml around line 21, the Python version constraint was narrowed to
">=3.12,<3.13" which contradicts README and CI that claim support for Python
3.13; update the constraint to ">=3.12,<3.14" to allow both 3.12 and 3.13, or if
3.13 is intentionally dropped, update README and CI to reflect the stricter
requirement — make the change consistent across pyproject.toml, README, and CI
matrix.
|
@raptorsun: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Do Not Merge!
Description
activate hermetic build
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Chores