Repository navigation
[hipSOLVER][rocSOLVER] Pin build-time dependency fetches to immutable commits - #8794
Merged
Merged
Conversation
… commits Replace mutable branch/tag refs in FetchContent and ExternalProject declarations with pinned 40-char commit SHAs so configure-time fetches cannot be altered upstream: - rocm-cmake (both libs): was develop/master branch, now therock-7.13 commit - lapack: was the lapack-3.7.1 branch, now the v3.7.1 commit - googletest: was the release-1.11.0 tag, now its commit Also convert rocSOLVER's rocm-cmake fallback from a hand-built archive URL download to git FetchContent, matching rocBLAS/rocSPARSE, so git verifies the commit by SHA. Each pin carries a "pinned-dep" comment to make future bumps easy to find. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
tfalders
approved these changes
Jun 24, 2026
tfalders
left a comment
tfalders
left a comment
Contributor
There was a problem hiding this comment.
Looks good! One minor comment.
Align the rocm-cmake fallback pin with the rest of the ROCm libraries (rocBLAS/rocSPARSE/hipBLAS/rocALUTION all track rocm-6.4.0) instead of a one-off therock tag, and reword the comment to reflect that this fetch is a rarely-hit fallback rather than something bumped every release. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8794 +/- ##
===========================================
+ Coverage 68.91% 68.93% +0.02%
===========================================
Files 2762 2776 +14
Lines 454696 457227 +2531
Branches 67057 67402 +345
===========================================
+ Hits 313322 315178 +1856
- Misses 118497 118998 +501
- Partials 22877 23051 +174
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
Add a reminder in bump_hipsolver_version.sh and bump_rocsolver_version.sh that build-time dependency fetches are pinned outside the script, with a grep "pinned-dep" pointer so a release-cycle bump can update them by hand. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a security scan finding by pinning build-time dependency fetches to immutable Git commits (instead of mutable branches/tags), improving reproducibility and reducing supply-chain risk in the HIP/ROC solver projects.
Changes:
- Switched rocm-cmake fallback acquisition to a pinned commit and FetchContent-based retrieval (rocSOLVER + hipSOLVER).
- Pinned googletest and LAPACK external dependency fetches in hipSOLVER to specific commits.
- Added notes to version-bump scripts clarifying that pinned dependency updates are handled elsewhere.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/rocsolver/cmake/get-rocm-cmake.cmake | Replaces mutable rocm-cmake fallback download with FetchContent + pinned commit. |
| projects/rocsolver/bump_rocsolver_version.sh | Adds guidance note about where pinned dependency commits are maintained. |
| projects/hipsolver/deps/external-lapack.cmake | Pins LAPACK fetch to an immutable commit (v3.7.1). |
| projects/hipsolver/deps/external-gtest.cmake | Pins googletest fetch to an immutable commit (release-1.11.0). |
| projects/hipsolver/cmake/get-rocm-cmake.cmake | Pins rocm-cmake fallback fetch to an immutable commit (replacing develop). |
| projects/hipsolver/bump_hipsolver_version.sh | Adds guidance note about where pinned dependency commits are maintained. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Contributor
|
This pull request has been inactive for 25 days and will be marked as stale. If you would like to keep this PR open, please:
This PR will be automatically closed in 5 days if no further activity occurs. |
Reword the pinned-dep note in both get-rocm-cmake.cmake files: the commit comes from the rocm-6.4.0 tag and is chosen for reproducible builds, rather than implying the hash is a shared in-monorepo pin. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
Refreshed against develop and pushed. Two remaining Copilot notes addressed:
|
mgates3
approved these changes
Sep 1, 2026
Contributor
TheRock Submodule Bump ActivityNewest first
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A security scan flagged that we should be pinning our external dependencies with hashes rather than relying on mutable URL or tags. This patch updates the solver libraries to reference specific commits and use more stable functionality for retrieving external dependencies.