Skip to content

fix(packaging): tolerate empty roots in split artifact manifests - #8702

Closed
ammarwa wants to merge 1 commit into
mainfrom
users/aelwazir/fix-empty-kpack-roots
Closed

ammarwa wants to merge 1 commit into
mainfrom
users/aelwazir/fix-empty-kpack-roots

Conversation

@ammarwa

@ammarwa ammarwa commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

fix(packaging): tolerate empty roots in split artifact manifests

Summary

When multiple GPU targets share a native package, split artifact manifests can retain a source root whose device images have moved into a shared kpack under another root. Native package root validation rejects that absent directory, preventing the device package from being produced and leaving unsatisfied DEB dependencies.

Skip absent manifest roots in kpack mode, matching ArtifactFlattener's existing behavior. Existing roots still undergo collision validation. Preserve both selected target kpacks.

Evidence

Build https://github.com/ROCm/TheRock/actions/runs/36947069646, attempt 1, source 392c7de. Both sparse device archives retain the rocSPARSE root in their manifests but put their shared kpack under hipSPARSELt. Each kpack indexes 340 rocSPARSE and 3 hipSPARSELt images for its respective target; the generic archive contains librocsparse.so. All three downloaded archives match their published SHA-256 checksums.

Related: #8327
Jira Super Epic: ROCM-31030. This PR addresses the split-artifact packaging defect within that campaign.

Validation

  • Regression fails on unchanged mainline with Missing package source directory.
  • python3 build_tools/packaging/linux/tests/build_package_test.py: 45 tests pass.
  • Regression checks both target kpack filenames and bytes survive DEB staging.
  • Existing conflicting-file and file/directory collision tests remain passing.
  • git diff --check passes.
  • Full DEB build/install and GPU execution not run. External dpkg invocation is mocked in unit coverage.

@therock-pr-bot

therock-pr-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ❌ Fail Error: Check concluded with failure.
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 2 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description
  • ❌ pre-commit

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot therock-pr-bot Bot added the Not ready to Review PR has unresolved policy failures — reviews blocked label Oct 2, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 2, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

marbre added a commit that referenced this pull request Oct 14, 2026
## Motivation

When multiple selected GPU targets share one native package owner, split
artifact manifests can retain source roots whose device images have
moved into
a shared kpack under another root. The grouped-root validation
introduced by
#8327 rejects those absent directories before the DEB and RPM backends
can
apply their established filtering.

The device-package builder then converts that exception into an empty
result.
Other variants can still be produced, causing package generation to
appear
successful and emit meta packages that depend on device packages that
were
never created. The failure is only discovered later during simulated or
native
package installation.

This was first reported in
#8327 (comment) and
reproduces
when #8350 enables `gfx1250-strict` alongside `gfx1250`.

This PR carries forward the fix and artifact evidence from #8702, then
adds RPM
coverage and makes unexpected partial package failures fatal.

Progress on #8278.

## Evidence

The original #8702 investigation used
https://github.com/ROCm/TheRock/actions/runs/36947069646. Both sparse
device
artifact manifests retained the rocSPARSE root after their device images
moved
into shared kpacks under hipSPARSELt. Each target kpack still contained
340
rocSPARSE and 3 hipSPARSELt images, while the generic artifact contained
`librocsparse.so`.

The current failure reproduces in #8350 run
https://github.com/ROCm/TheRock/actions/runs/38419442156:

- DEB package generation fails its simulated installation after the
affected
  `gfx1250` device packages are omitted:

https://github.com/ROCm/TheRock/actions/runs/38419442156/job/115420436430?pr=8350
- RPM generation completes despite omitting the affected packages:

https://github.com/ROCm/TheRock/actions/runs/38419442156/job/115420436578?pr=8350
- The downstream RHEL 8 and SLES 16 installation jobs then fail on
unresolved
  dependencies:
-
https://github.com/ROCm/TheRock/actions/runs/38419442156/job/115430597884?pr=8350
-
https://github.com/ROCm/TheRock/actions/runs/38419442156/job/115430596637?pr=8350

The omitted packages include `amdrocm-sparse`, `amdrocm-blas-test`,
`amdrocm-rand-test`, `amdrocm-ccl-test`, and `amdrocm-rccl-test` for the
shared
`gfx1250` package owner.

## Technical Details

- Ignore absent manifest roots during kpack artifact discovery, matching
the
  existing artifact-catalog behavior.
- Continue collision validation for every existing package root.
- Log skipped roots at debug level so stale manifest entries remain
  diagnosable.
- Preserve the selected `gfx1250` and `gfx1250-strict` kpack payloads in
their
  shared `gfx1250` native package.
- Propagate unexpected device-package exceptions instead of converting
them to
  empty results.
- Keep intentional empty device variants skippable.
- Return a nonzero status after the build summary whenever any base
package is
  recorded as failed.
- Add matching DEB and RPM regressions for combined-owner packages with
absent
  manifest roots.

The implementation is architecture-independent packaging logic.
Regression
fixtures use the motivating combined `gfx1250` and `gfx1250-strict`
ownership
case and verify that both selected payloads survive. No GPU execution is
required for this change.

## Test Plan

- `python build_tools/packaging/linux/tests/build_package_test.py`
- `gfx125X,gfx1250-strict` dev build:
https://github.com/ROCm/TheRock/actions/runs/38459803112

## Test Result

- 52 packaging tests passed.
- The DEB regression verifies both selected kpack filenames and bytes
survive
  staging when manifests contain absent roots.
- The RPM regression verifies both valid target roots are retained in
the
  generated spec and absent roots are excluded.
- Device-package exceptions propagate, while intentional empty variants
remain
  skippable.
- Generic recorded package failures exit with status 1 after printing
the build
  summary.

## Related

- Tracking issue: #8278
- Original fix and evidence: #8702
- Native package ownership change: #8327
- Reproducing family enablement: #8350

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.

Generated with Codex

---------

Co-authored-by: Ammar ELWazir <3832908+ammarwa@users.noreply.github.com>
@marbre

marbre commented Oct 14, 2026

Copy link
Copy Markdown
Member

Superseded by #8987.

@marbre marbre closed this Oct 14, 2026
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Not ready to Review PR has unresolved policy failures — reviews blocked

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants