Repository navigation
Fix PACKIT-5421: Check if package already built with fixed dependency - #833
majamassarini wants to merge 7 commits into
Conversation
PR Summary by QodoAvoid rebuilds when packages already use fixed dependencies
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Partially checked builds appear fixed
|
477a694 to
6656a92
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 6656a92 |
6656a92 to
0e07871
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 0e07871 |
0e07871 to
1750c9c
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 1750c9c |
1750c9c to
41d6358
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 41d6358 |
41d6358 to
0c2c323
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 0c2c323 |
0c2c323 to
b111e6c
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit b111e6c |
01988a5 to
0d52213
Compare
|
/agentic_review |
35c6a51 to
2e45fdd
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 2e45fdd |
142bee6 to
9495861
Compare
9495861 to
3c27d9f
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 3c27d9f |
3c27d9f to
2f9fe55
Compare
Ymir was incorrectly recommending rebuild when a package was already built with the fixed dependency version. Now checks if the package's current build already used the fix before recommending rebuild. Implementation uses two verification methods: 1. root.log inspection (primary): Downloads build logs from Brew to see which exact dependency NVR was installed. Most reliable since it shows what actually happened during the build. Handles epochs, subpackages, and concurrent architecture probes. 2. Timestamp comparison (fallback): When root.log unavailable (logs expire after some time), compare package completion time vs dependency fix time. Use timestamp fallback only for manual verification, not auto-resolution. When root.log is unavailable, timestamp comparison can suggest whether a package likely has the fix, but cannot prove it due to: - Overlapping builds (buildroot updated mid-build) - Buildroot tagging delays - Build system caching of dependencies root.log is preferred because it's precise and handles edge cases (cached packages, epoch overrides). Timestamp fallback is needed because build logs are not preserved indefinitely. Performance: concurrent Koji lookups, HEAD checks before downloads, 30s overall timeout across all architectures. Security: validates package names, escapes JQL, validates NVR format. Addresses: PACKIT-5421 Related: PACKIT-5289 Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Distinguish between 'CVE doesn't apply' and 'fix already shipped' scenarios: 1. CVE doesn't apply (NOT_AFFECTED): - Vulnerable code not present, version not affected, etc. - Routes to reproducer for verification - Gets ymir_triaged_not_affected label 2. Fix already shipped (ALREADY_FIXED): - Rebase/backport/rebuild already completed - Package has fix, needs errata processing - Routes to comment_in_jira, not reproducer - Gets ymir_triaged_already_fixed label Changes: - Added Resolution.ALREADY_FIXED enum and AlreadyFixedData model - Updated triage prompt: use 'already-fixed' for completed rebases - Added output format example with package_nvr and package_issue_key - Excluded ALREADY_FIXED from reproducer handoff - Added to _should_update_jira and comment_in_jira routing - Included in _RESOLUTION_TO_LABEL mapping For rebuilds: verify_rebuild_buildroot detects already-fixed via root.log For rebases: triage detects version already at target via spec file check Addresses code review feedback on PACKIT-5421 Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…ifications Three improvements to rebuild detection accuracy: 1. **Multi-architecture verification**: Fetch and verify all architecture root.logs agree on dependency version. Different archs can have different buildroots during long builds. Request clarification on conflicts. 2. **Active build handling**: Search both closed and active (not yet closed) builds. Trust closed builds immediately. Request clarification when only active builds exist, since they might be rejected before closure. 3. **Metadata failure handling**: Return clarification instead of False when Koji metadata unavailable. Missing metadata is indeterminate, not proof rebuild is needed. New clarification reasons: architecture_dependency_conflict, active_builds_not_closed, timestamp_comparison_failed. Prevents false positives and false negatives by requesting human verification for ambiguous cases. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Documents how Ymir determines whether a package needs to be rebuilt for a CVE fix, including multi-architecture verification, active build handling, and clarification scenarios. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Retry automatically on Jira/Koji outages instead of asking the user. Fetch root.log only for architectures actually built (from Koji RPMs). Skip noarch-only builds (no per-arch logs). Re-raise for recent builds, fall back to timestamps for old ones past Brew retention. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
2f9fe55 to
560a4d3
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 560a4d3 |
…logic, handle partial logs
- Fix noarch installed_pkgs.log location: fetch from noarch/ directory instead of trying builder architectures
- Remove date-based retry logic for missing logs: when logs are missing (404/410), don't retry - fall back to timestamp comparison immediately
- Fix partial log handling: return partial_architecture_coverage clarification when some (but not all) arch logs available, instead of discarding partial data and falling back to timestamp comparison
- Update documentation to reflect noarch/ directory and removal of date-based logic
- Use Koji API to resolve independently versioned subpackages
The issue: _parse_dependency_from_installed_pkgs_log constructs a source NVR as
{dep_component}-{version}-{release} using a subpackage's version. For packages
like perl where subpackages have independent versions (e.g., perl-Errno-1.30),
this creates non-existent Koji builds like perl-1.30, causing TransientInfrastructureError.
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
560a4d3 to
510ddcc
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 510ddcc |
Summary
Fixes PACKIT-5421 by checking if a package was already built with the fixed dependency before recommending rebuild.
What Changed
Core Feature: Already-Built Detection
check_package_built_with_fixed_dependency()to verify if the latest build used the fixed dependencyMulti-Architecture Verification
0:vs omitted epoch are equivalent)Active Build Handling
Infrastructure Failure Handling
TransientInfrastructureErrorfor Jira/Koji temporary unavailabilityResolution Types
Edge Cases - Clarification Requested For:
🤖 Generated with Claude Code