Skip to content

Commit bf4e415

Browse files
Point hipconfig's ROCM_PATH towards dev packages if develop packages are installed (#2870)
PR based on #2428. ## Motivation We'd like `hipconfig --rocmpath` to point towards `_rocm_sdk_devel` if the `rocm[devel]` packages are installed. This is useful for code that depends on `hipconfig --rocmpath` to resolve paths for ROCm files. Aimed as a fix for #1880 and other PyTorch issues that require the develop packages. <!-- Explain the purpose of this PR and the goals it aims to achieve. --> ## Technical Details - The PR at #2428 successfully directed `hipconfig --rocmpath` towards the `_rocm_sdk_devel` packages on Windows but was running into some issues on Linux as we were still using symlinks. - Added a change so Linux also uses hardlinks to avoid this issue. <!-- Explain the changes along with any relevant GitHub links. --> ## Test Plan Linux: - The unit test `testCLIUsesDevelRootPath` passes (`rocm-sdk test` passes) - Tested by installing without the devel packages and with the devel packages ``` pip install --index-url https://rocm.devreleases.amd.com/v2-staging/gfx1151/ "rocm[libraries]==7.11.0.dev0+5d4dfe88517275760144e58af2e70af3dff5d2bc" hipconfig --rocmpath # /home/rocm/darren/hipconfig-test/.venv/lib/python3.12/site-packages/_rocm_sdk_core pip install --index-url https://rocm.devreleases.amd.com/v2-staging/gfx1151/ "rocm[libraries,devel]==7.11.0.dev0+5d4dfe88517275760144e58af2e70af3dff5d2bc" hipconfig --rocmpath # /home/rocm/darren/hipconfig-test/.venv/lib/python3.12/site-packages/_rocm_sdk_devel ``` <!-- Explain any relevant testing done to verify this PR. --> ## Test Result On Linux, the unit test passes and without the develop packages `hipconfig --rocmpath` correctly points at the core packages. When the develop packages are installed, it correctly expands and points tot he develop packages. <!-- Briefly summarize test outcomes. --> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Scott Todd <scott.todd0@gmail.com> Co-authored-by: darren-amd <darren-amd@users.noreply.github.com>
1 parent 62ac8af commit bf4e415

5 files changed

Lines changed: 142 additions & 23 deletions

File tree

build_tools/packaging/python/templates/rocm-sdk-core/src/rocm_sdk_core/_cli.py

Lines changed: 90 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"""Trampoline for console scripts."""
55

66
import importlib
7+
import importlib.util
78
import os
89
import platform
910
import sys
@@ -12,17 +13,95 @@
1213
from ._dist_info import ALL_PACKAGES
1314

1415
CORE_PACKAGE = ALL_PACKAGES["core"]
15-
PLATFORM_NAME = CORE_PACKAGE.get_py_package_name()
16-
PLATFORM_MODULE = importlib.import_module(PLATFORM_NAME)
17-
# NOTE: dependent on there being an __init__.py in the platform package.
18-
PLATFORM_PATH = Path(PLATFORM_MODULE.__file__).parent
16+
CORE_PY_PACKAGE_NAME = CORE_PACKAGE.get_py_package_name()
17+
18+
19+
def _get_core_module_path():
20+
# NOTE: dependent on there being an __init__.py in the core package.
21+
core_module = importlib.import_module(CORE_PY_PACKAGE_NAME)
22+
return Path(core_module.__file__).parent
23+
24+
25+
DEVEL_PACKAGE = ALL_PACKAGES["devel"]
26+
DEVEL_PURE_PY_PACKAGE_NAME = DEVEL_PACKAGE.pure_py_package_name
27+
DEVEL_PY_PACKAGE_NAME = DEVEL_PACKAGE.get_py_package_name()
28+
29+
30+
def _has_devel_module():
31+
return importlib.util.find_spec(DEVEL_PURE_PY_PACKAGE_NAME) is not None
32+
33+
34+
def _is_devel_module_expanded():
35+
return importlib.util.find_spec(DEVEL_PY_PACKAGE_NAME) is not None
36+
37+
38+
def _expand_devel_module():
39+
import subprocess
40+
41+
try:
42+
subprocess.check_call([sys.executable, "-m", "rocm_sdk", "init", "--quiet"])
43+
except subprocess.CalledProcessError:
44+
print(
45+
"ERROR: Failed to expand rocm[devel] package. "
46+
"Try running `rocm-sdk init` manually for details.",
47+
file=sys.stderr,
48+
)
49+
sys.exit(1)
50+
51+
52+
def _get_devel_module_path():
53+
# NOTE: dependent on there being an __init__.py in the devel package.
54+
try:
55+
devel_module = importlib.import_module(DEVEL_PY_PACKAGE_NAME)
56+
except ImportError:
57+
print(
58+
"WARNING: Failed to import devel module, falling back to core.",
59+
file=sys.stderr,
60+
)
61+
return _get_core_module_path()
62+
if devel_module.__file__ is None:
63+
print(
64+
"WARNING: Devel module has no __file__, falling back to core.",
65+
file=sys.stderr,
66+
)
67+
return _get_core_module_path()
68+
return Path(devel_module.__file__).parent
69+
70+
71+
def _get_module_path(expand_devel: bool) -> Path:
72+
"""Gets the module path, either from 'core' or 'devel'.
73+
74+
If the 'devel' package IS NOT installed then 'core' is used, ignoring the input of `expand_devel`.
75+
If the 'devel' package IS installed AND already expanded then it is used.
76+
If the 'devel' package IS installed AND NOT already expanded then either
77+
A) System information tools like amd-smi can choose to run more quickly
78+
with 'core' by skipping the (compute-intensive) 'devel' expansion.
79+
These tools should pass `expand_devel=False`.
80+
B) Other tools that benefit from the extra files in the 'devel' package
81+
will expand expand it by passing `expand_devel=True`.
82+
"""
83+
if _has_devel_module():
84+
if _is_devel_module_expanded():
85+
return _get_devel_module_path()
86+
elif expand_devel:
87+
_expand_devel_module()
88+
return _get_devel_module_path()
89+
else:
90+
# Passthrough. Fallback to core module.
91+
pass
92+
93+
return _get_core_module_path()
94+
1995

2096
is_windows = platform.system() == "Windows"
2197
exe_suffix = ".exe" if is_windows else ""
2298

2399

24-
def _exec(relpath: str):
25-
full_path = PLATFORM_PATH / (relpath + exe_suffix)
100+
def _exec(relpath: str, expand_devel=True):
101+
# Default is True because most CLI tools are compiler/build tools that
102+
# need the devel files. System info tools (amd-smi, rocminfo, etc.)
103+
# override with expand_devel=False to avoid the expansion cost.
104+
full_path = _get_module_path(expand_devel) / (relpath + exe_suffix)
26105
os.execv(full_path, [str(full_path)] + sys.argv[1:])
27106

28107

@@ -51,7 +130,7 @@ def amdlld():
51130

52131

53132
def amd_smi():
54-
_exec("bin/amd-smi")
133+
_exec("bin/amd-smi", expand_devel=False)
55134

56135

57136
def hipcc():
@@ -71,23 +150,23 @@ def hipify_perl():
71150

72151

73152
def hipInfo():
74-
_exec("bin/hipInfo")
153+
_exec("bin/hipInfo", expand_devel=False)
75154

76155

77156
def offload_arch():
78157
_exec("lib/llvm/bin/offload-arch")
79158

80159

81160
def rocm_agent_enumerator():
82-
_exec("bin/rocm_agent_enumerator")
161+
_exec("bin/rocm_agent_enumerator", expand_devel=False)
83162

84163

85164
def rocm_info():
86-
_exec("bin/rocminfo")
165+
_exec("bin/rocminfo", expand_devel=False)
87166

88167

89168
def rocm_smi():
90-
_exec("bin/rocm-smi")
169+
_exec("bin/rocm-smi", expand_devel=False)
91170

92171

93172
def roccoremerge():

build_tools/packaging/python/templates/rocm/src/rocm_sdk/__main__.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ def _do_init(args: argparse.Namespace):
4545
# if contents for development were not yet unpacked.
4646
root_path = _devel.get_devel_root()
4747
except ModuleNotFoundError as e:
48-
print(f"ERROR: {e}", file=sys.stderr)
48+
print(f"ERROR running init: {e}", file=sys.stderr)
4949
sys.exit(1)
50-
print(f"Devel contents expanded to '{root_path}'")
50+
if not args.quiet:
51+
print(f"Devel contents expanded to '{root_path}'")
5152

5253

5354
def _do_test(args: argparse.Namespace):
@@ -147,6 +148,11 @@ def main(argv: list[str] | None = None):
147148
init_p = sub_p.add_parser(
148149
"init", help="Expand devel contents to initialize rocm[devel]"
149150
)
151+
init_p.add_argument(
152+
"--quiet",
153+
action="store_true",
154+
help="Run without producing output",
155+
)
150156
init_p.set_defaults(func=_do_init)
151157

152158
args = p.parse_args(argv)

build_tools/packaging/python/templates/rocm/src/rocm_sdk/_devel.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,17 +198,25 @@ def _clean_dir(dir: Path):
198198
# hash (empty)
199199
# size (empty)
200200
record_file.write(f"{ti.name},,\n")
201-
if _is_windows() and ti.issym():
202-
# Convert symlinks into hardlinks on Windows.
203-
# This saves disk space while improving compatibility
204-
# on systems without as robust symlink support.
201+
if ti.issym():
202+
# Convert file symlinks into hardlinks on all platforms.
203+
# This saves disk space while improving compatibility.
204+
# On Windows: symlinks require admin privileges.
205+
# On Linux: native binaries that use readlink(/proc/self/exe)
206+
# to determine their location will resolve symlinks and
207+
# report the wrong path (e.g., _rocm_sdk_core instead of
208+
# _rocm_sdk_devel). Hardlinks avoid this issue.
205209
# As needed, we could also generate tarfiles with
206210
# copies instead of symlinks, at the cost of disk space.
207-
# Creating symlinks on Windows also requires admin privileges.
208211
parent_path.mkdir(parents=True, exist_ok=True)
209212
symlink_target = ti.linkname
210213
hardlink_target = dest_path.parent / symlink_target
211-
dest_path.hardlink_to(hardlink_target)
214+
# Only create hardlinks for files, not directories
215+
if hardlink_target.is_file():
216+
dest_path.hardlink_to(hardlink_target)
217+
else:
218+
# For directory symlinks, extract as normal
219+
tf.extract(ti, path=site_lib_path)
212220
else:
213221
tf.extract(ti, path=site_lib_path)
214222
elif ti.isdir():

build_tools/packaging/python/templates/rocm/src/rocm_sdk/tests/devel_test.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,30 @@ def testCLIPathRoot(self):
6666
bin_path = path / "bin"
6767
self.assertTrue(bin_path.exists(), msg=f"Expected bin path {bin_path} to exist")
6868

69+
def testCLIUsesDevelRootPath(self):
70+
root_path_output = (
71+
utils.run_command(
72+
[sys.executable, "-m", "rocm_sdk", "path", "--root"], capture=True
73+
)
74+
.decode()
75+
.strip()
76+
)
77+
root_path = Path(root_path_output)
78+
79+
# CLI scripts by default run from _rocm_sdk_core.
80+
# When the devel package is installed they should run from _rocm_sdk_devel.
81+
rocmpath_output = (
82+
utils.run_command(["hipconfig", "--rocmpath"], capture=True)
83+
.decode()
84+
.strip()
85+
)
86+
rocmpath = Path(rocmpath_output)
87+
self.assertEqual(
88+
root_path,
89+
rocmpath,
90+
msg=f"Expected `hipconfig --rocmpath` to return {root_path}, not {rocmpath}",
91+
)
92+
6993
@unittest.skipIf(
7094
platform.system() == "Windows", "root LLVM symlink only exists on Linux"
7195
)

docs/packaging/python_packaging.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ We generate three types of packages:
2525
a relative binary).
2626
- Devel package: The `rocm-sdk-devel` package is the catch-all for everything.
2727
For any file already populated in a runtime package, it will include it as
28-
a relative symlink (also rewriting shared library soname links as needed).
29-
Since symlinks and non-standard attributes cannot be included in a wheel file,
30-
the platform contents are stored in a `_devel.tar` or `_devel.tar.xz` file.
31-
The installed package is extended in response to requesting a path to it
28+
a relative symlink in the tarball. During extraction, file symlinks are
29+
converted to hardlinks to improve compatibility, while directory symlinks
30+
remain as symlinks. Shared library soname links are also rewritten as needed.
31+
Since symlinks/hardlinks and non-standard attributes cannot be included in a
32+
wheel file, the platform contents are stored in a `_devel.tar` or `_devel.tar.xz`
33+
file. The installed package is extended in response to requesting a path to it
3234
via the `rocm-sdk` tool.
3335

3436
Runtime packages can either be target neutral or target specific. Target specific

0 commit comments

Comments
 (0)