Skip to content

fix(rocprofiler-compute): wire test-compression for TheRock install-mode CI - #7421

Merged
vedithal-amd merged 1 commit into
mainfrom
users/jamessiddeley-amd/rocprofiler-compute-compression-gtest
Aug 17, 2026
Merged

vedithal-amd merged 1 commit into
mainfrom
users/jamessiddeley-amd/rocprofiler-compute-compression-gtest

Conversation

@jamessiddeley-amd

@jamessiddeley-amd jamessiddeley-amd commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Added a test-compression gtest in #9711 of rocprofiler-compute,, but TheRock CI can't run it in install mode yet:

  • The post-hook sets install RPATH for the other gtests under libexec/rocprofiler-compute/tests/ but not test-compression
  • The subproject pulls bundled elfutils/libdrm/sqlite3 but not zlib, so pkg_check_modules(zlib) can link system libz instead of bundled sysdeps

The follow-up in rocm-systems repo is to add test-compression to test_categories.yaml after this merges.

Technical Details

  • Add test-compression to the RPATH post-hook foreach in profiler/post_hook_rocprofiler-compute.cmake.
  • Add ${THEROCK_BUNDLED_ZLIB} to rocprofiler-compute RUNTIME_DEPS in profiler/CMakeLists.txt.

Test Plan

  • Build with rocprofiler-compute + tests enabled.
  • Confirm test-compression installs under libexec/rocprofiler-compute/tests/ with correct RPATH.
  • Run ctest -R test-compression.

Test Result

  • Pending CI.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Aug 17, 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 PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

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

🚫 Please fix the failed policies

  • ❌ PR Description

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 Aug 17, 2026
@therock-pr-bot

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.

@therock-pr-bot therock-pr-bot Bot removed the Not ready to Review PR has unresolved policy failures — reviews blocked label Aug 17, 2026
@therock-pr-bot therock-pr-bot Bot added the Not ready to Review PR has unresolved policy failures — reviews blocked label Aug 17, 2026
@vedithal-amd
vedithal-amd marked this pull request as ready for review August 17, 2026 20:24
Copilot AI lite review requested due to automatic review settings August 17, 2026 20:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates TheRock’s rocprofiler-compute integration so install-mode CI can run the newly added test-compression gtest by ensuring it receives the correct install RPATH handling and that bundled zlib is treated as a runtime dependency when sysdeps bundling is enabled.

Changes:

  • Include test-compression in the rocprofiler-compute post-hook loop that sets THEROCK_INSTALL_RPATH_ORIGIN for installed gtests.
  • Add ${THEROCK_BUNDLED_ZLIB} to rocprofiler-compute’s RUNTIME_DEPS so install-mode runs prefer bundled zlib over a system libz when applicable.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
profiler/post_hook_rocprofiler-compute.cmake Extends the RPATH-origin post-hook to cover the new test-compression test target when present.
profiler/CMakeLists.txt Adds bundled zlib to rocprofiler-compute runtime deps to align install-mode behavior with sysdeps bundling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@vedithal-amd

Copy link
Copy Markdown
Contributor

gfx942 linux tests pass - merging

@vedithal-amd
vedithal-amd merged commit 0961399 into main Aug 17, 2026
233 of 270 checks passed
@vedithal-amd
vedithal-amd deleted the users/jamessiddeley-amd/rocprofiler-compute-compression-gtest branch August 17, 2026 22:05
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Aug 17, 2026
lumachad pushed a commit that referenced this pull request Aug 18, 2026
…ode CI (#7421)

## Motivation

Added a `test-compression` gtest in [#9711 of
rocprofiler-compute,](ROCm/rocm-systems#9711),
but TheRock CI can't run it in install mode yet:

- The post-hook sets install RPATH for the other gtests under
`libexec/rocprofiler-compute/tests/` but not `test-compression`
- The subproject pulls bundled elfutils/libdrm/sqlite3 but not zlib, so
`pkg_check_modules(zlib)` can link system `libz` instead of bundled
sysdeps

The follow-up in rocm-systems repo is to add `test-compression` to
`test_categories.yaml` after this merges.

## Technical Details

- Add `test-compression` to the RPATH post-hook foreach in
`profiler/post_hook_rocprofiler-compute.cmake`.
- Add `${THEROCK_BUNDLED_ZLIB}` to `rocprofiler-compute` `RUNTIME_DEPS`
in `profiler/CMakeLists.txt`.

## Test Plan

- Build with rocprofiler-compute + tests enabled.
- Confirm `test-compression` installs under
`libexec/rocprofiler-compute/tests/` with correct RPATH.
- Run `ctest -R test-compression`.

## Test Result

- Pending CI.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.
avansick-amd added a commit that referenced this pull request Aug 18, 2026
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Adam Van Sickle <adam.vansickle@amd.com>

## Motivation

`rocprofiler-compute` gained a `THEROCK_BUNDLED_ZLIB` runtime dep in
#7421, but the
committed consumer graph was never regenerated. The drift check compares
the whole
file, so any PR touching a graph-affecting path either fails it or has
to carry this
line in an unrelated diff. Same class as #7298 / #7297.

## Technical Details

Regenerated `test_tools/therock_consumer_graph.json` with
`check_consumer_graph_drift.py --write` from a full-tree
`THEROCK_ENABLE_ALL=ON`
`gfx94X-dcgpu` configure. No other files change.

The diff is one line: `rocprofiler-compute` added to `therock-zlib`'s
consumers.

`rocprofiler-compute` has a key in `fetch_test_configurations.py`, 
so changes to `therock-zlib` can now schedule its tests. 
That follows from the dependency #7421 added.

## Test Plan

- Regenerated locally, then confirmed the result matches what CI
independently
computed on #7376.

## Test Result

- Consumer graph drift check: pass.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.
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.

4 participants