Repository navigation
[release/2.15] [ROCm] Keep inherited LD_LIBRARY_PATH in TestAmdSmiImport (#200650) - #3845
Merged
jeffdaily merged 1 commit intoOct 19, 2026
Conversation
…0650) ## Linked issue or supporting maintainer Supported by @jeffdaily Related: ROCm/TheRock#8888 (the CI lane where the test fails) and pytorch#197482, which added the test. ## BC-breaking? No. ## Summary (human written only) `test_amdsmi_bare_unversioned_path_uses_loader_first` (added in pytorch#197482) fails with `Expected 0 but got 127` when the Python running the tests needs `LD_LIBRARY_PATH` to find libpython, as with an interpreter installed by actions/setup-python. `_run_amdsmi_import` replaced `LD_LIBRARY_PATH` with its temp loader directory before spawning `sys.executable`, so the child died in the dynamic loader before running any Python. This prepends the loader directory to the inherited value instead. It is still first on `LD_LIBRARY_PATH`, and the test's assertion (the bare name loads, the SDK fallback is not used) is unchanged. ## Agent details (if applicable) Validation on gfx942 at 6c3af02, done with Claude; the helper and the amdsmi hook are unchanged between that commit and this branch's base. With the stock interpreter all five TestAmdSmiImport tests pass before and after. Under a locally built launcher that reaches libpython only through `LD_LIBRARY_PATH` (it exits 127 without it), `test_amdsmi_bare_unversioned_path_uses_loader_first` fails with `Expected 0 but got 127` before the change and all five pass after. The patched test also passes with a non-loadable `libamd_smi.so` in an inherited `LD_LIBRARY_PATH` directory. This reproduces the failure mode, not the TheRock CI lane itself. lintrunner clean. Pull Request resolved: pytorch#200650 Approved by: https://github.com/jeffdaily Co-authored-by: Jeff Daily <jeff.daily@amd.com> (cherry picked from commit 1ed0e94)
jeffdaily
deleted the
jeffdaily/release-2.15-amdsmi-keep-ld-library-path
branch
October 19, 2026 15:56
jithunnair-amd
pushed a commit
that referenced
this pull request
Nov 4, 2026
…ort (pytorch#200650) (#3845) ## Linked issue or supporting maintainer Supported by @jeffdaily Related: ROCm/TheRock#8888 (the CI lane where the test fails) and pytorch#197482, which added the test. ## BC-breaking? No. ## Summary (human written only) `test_amdsmi_bare_unversioned_path_uses_loader_first` (added in pytorch#197482) fails with `Expected 0 but got 127` when the Python running the tests needs `LD_LIBRARY_PATH` to find libpython, as with an interpreter installed by actions/setup-python. `_run_amdsmi_import` replaced `LD_LIBRARY_PATH` with its temp loader directory before spawning `sys.executable`, so the child died in the dynamic loader before running any Python. This prepends the loader directory to the inherited value instead. It is still first on `LD_LIBRARY_PATH`, and the test's assertion (the bare name loads, the SDK fallback is not used) is unchanged. ## Agent details (if applicable) Validation on gfx942 at 6c3af02, done with Claude; the helper and the amdsmi hook are unchanged between that commit and this branch's base. With the stock interpreter all five TestAmdSmiImport tests pass before and after. Under a locally built launcher that reaches libpython only through `LD_LIBRARY_PATH` (it exits 127 without it), `test_amdsmi_bare_unversioned_path_uses_loader_first` fails with `Expected 0 but got 127` before the change and all five pass after. The patched test also passes with a non-loadable `libamd_smi.so` in an inherited `LD_LIBRARY_PATH` directory. This reproduces the failure mode, not the TheRock CI lane itself. lintrunner clean. Pull Request resolved: pytorch#200650 Approved by: https://github.com/jeffdaily Co-authored-by: Jeff Daily <jeff.daily@amd.com>
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.
Linked issue or supporting maintainer
Supported by @jeffdaily
Related: ROCm/TheRock#8888 (the CI lane where the test fails) and pytorch#197482, which added the test.
BC-breaking?
No.
Summary (human written only)
test_amdsmi_bare_unversioned_path_uses_loader_first(added in pytorch#197482) fails withExpected 0 but got 127when the Python running the tests needsLD_LIBRARY_PATHto find libpython, as with an interpreter installed by actions/setup-python._run_amdsmi_importreplacedLD_LIBRARY_PATHwith its temp loader directory before spawningsys.executable, so the child died in the dynamic loader before running any Python. This prepends the loader directory to the inherited value instead. It is still first onLD_LIBRARY_PATH, and the test's assertion (the bare name loads, the SDK fallback is not used) is unchanged.Agent details (if applicable)
Validation on gfx942 at 6c3af02, done with Claude; the helper and the amdsmi hook are unchanged between that commit and this branch's base. With the stock interpreter all five TestAmdSmiImport tests pass before and after. Under a locally built launcher that reaches libpython only through
LD_LIBRARY_PATH(it exits 127 without it),test_amdsmi_bare_unversioned_path_uses_loader_firstfails withExpected 0 but got 127before the change and all five pass after. The patched test also passes with a non-loadablelibamd_smi.soin an inheritedLD_LIBRARY_PATHdirectory. This reproduces the failure mode, not the TheRock CI lane itself. lintrunner clean.Pull Request resolved: pytorch#200650
Approved by: https://github.com/jeffdaily
Co-authored-by: Jeff Daily jeff.daily@amd.com