Point hipconfig's ROCM_PATH towards dev packages if develop packages are installed#2870
Point hipconfig's ROCM_PATH towards dev packages if develop packages are installed#2870darren-amd merged 4 commits intomainfrom
Conversation
|
Have not tested on Windows yet, will do so after build completes. |
7e9ba03 to
3077ce8
Compare
5d4dfe8 to
3aad9ba
Compare
|
Thanks, just verified with build |
| if _is_windows() and ti.issym(): | ||
| # Convert symlinks into hardlinks on Windows. | ||
| # This saves disk space while improving compatibility | ||
| # on systems without as robust symlink support. | ||
| if ti.issym(): | ||
| # Convert file symlinks into hardlinks on all platforms. | ||
| # This saves disk space while improving compatibility. | ||
| # On Windows: symlinks require admin privileges. | ||
| # On Linux: native binaries that use readlink(/proc/self/exe) | ||
| # to determine their location will resolve symlinks and | ||
| # report the wrong path (e.g., _rocm_sdk_core instead of | ||
| # _rocm_sdk_devel). Hardlinks avoid this issue. |
There was a problem hiding this comment.
Thanks, this change looks good to me by I would also like to hear @stellaraccident 's opinion. I was on the fence about making it and discussed a bit with @marbre before and the comment helps justify it, at least to me.
We should probably also update the documentation at https://github.com/ROCm/TheRock/blob/main/docs/packaging/python_packaging.md, specifically this section:
- Devel package: The
rocm-sdk-develpackage is the catch-all for everything.
For any file already populated in a runtime package, it will include it as
a relative symlink (also rewriting shared library soname links as needed).
Since symlinks and non-standard attributes cannot be included in a wheel file,
the platform contents are stored in a_devel.taror_devel.tar.xzfile.
The installed package is extended in response to requesting a path to it
via therocm-sdktool.
There was a problem hiding this comment.
Thanks Scott, I will update the docs
ScottTodd
left a comment
There was a problem hiding this comment.
LGTM, but I'm a bit worried this may have some unintended side effects on Linux. We have been using hardlinks on Windows though, so 🤷
…stalled (#2428) Fixes #1880. Many projects (such as llama.cpp) look up paths to ROCm files using `hipconfig --rocmpath`. Previously, our CLI scripts would always trampoline to e.g. `_rocm_sdk_core/bin/hipconfig`, even if the development files were available in `_rocm_sdk_devel` (which is a superset of `_rocm_sdk_core`). This trampolines to `_rocm_sdk_devel` if the devel package is installed. * Added a new unit test * Ran various local tests like: ```bash py -V:3.12 -m venv 3.12.venv && .\3.12.venv\Scripts\activate.bat pip install --index-url https://rocm.nightlies.amd.com/v2/gfx110X-all/ --pre rocm[libraries] hipconfig --rocmpath # D:\scratch\therock\3.12.venv\Lib\site-packages\_rocm_sdk_core pip install --index-url https://rocm.nightlies.amd.com/v2/gfx110X-all/ --pre rocm[libraries,devel] hipconfig --rocmpath (Initialized automatically) hipconfig --rocmpath shows no redundant init) ``` - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
Makes Linux use hardlinks like Windows does. Fixes hipconfig binary path detection.
c68a677 to
e2907e7
Compare
stellaraccident
left a comment
There was a problem hiding this comment.
I'm good with this. We probably should have done this from the get-go as symlink indirection can cause many problems.

PR based on #2428.
Motivation
We'd like
hipconfig --rocmpathto point towards_rocm_sdk_develif therocm[devel]packages are installed. This is useful for code that depends onhipconfig --rocmpathto resolve paths for ROCm files. Aimed as a fix for #1880 and other PyTorch issues that require the develop packages.Technical Details
rocm[devel]is installed #2428 successfully directedhipconfig --rocmpathtowards the_rocm_sdk_develpackages on Windows but was running into some issues on Linux as we were still using symlinks.Test Plan
Linux:
testCLIUsesDevelRootPathpasses (rocm-sdk testpasses)Test Result
On Linux, the unit test passes and without the develop packages
hipconfig --rocmpathcorrectly points at the core packages. When the develop packages are installed, it correctly expands and points tot he develop packages.Submission Checklist