[ROCm] Use -rpath-link to fix libtinfo conflict by jithunnair-amd · Pull Request #83552 · pytorch/pytorch

jithunnair-amd · 2022-08-16T22:26:44Z

Fixes issue building PyTorch for ROCm5.3 and above on Ubuntu20.04 because libtinfo6 from conda conflicts with the one from the distro causing symbol not found errors.

Some more context here: ROCm#1058 (comment)

cc @jeffdaily @sunway513 @ROCmSupport

facebook-github-bot · 2022-08-16T22:26:49Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/83552
📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓Need help or want to give feedback on the CI? Visit our office hours

✅ No Failures (0 Pending)

As of commit 836b07cb23 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

jithunnair-amd · 2022-08-16T22:26:52Z

cc @jaglinux

jithunnair-amd · 2022-09-12T22:21:23Z

@pytorchbot rebase

pytorchmergebot · 2022-09-12T22:23:08Z

@pytorchbot successfully started a rebase job. Check the current status here

pytorchmergebot · 2022-09-12T22:23:15Z

Successfully rebased fix_for_libtinfo6 onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via git checkout fix_for_libtinfo6 && git pull --rebase)

pytorch-bot · 2022-09-12T22:23:20Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/83552

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Failures, 6 Pending

As of commit ef86b50:

The following jobs have failed:

linux-bionic-py3_7-clang8-xla / test (xla, 1, 1, linux.2xlarge)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

malfet

Looks fine, though it would be nice to use python (or even CMAKE command)

cmake/Dependencies.cmake

facebook-github-bot · 2022-10-04T00:39:05Z

/easycla

As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details.

This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign.

linux-foundation-easycla · 2022-10-04T00:39:08Z

The committers listed above are authorized under a signed CLA.

✅ login: jithunnair-amd / name: Jithun Nair (0930c0ce326cf4666bb72b0b68661aefe5fe2787, 956ade57e16f893a556408620949a24931510958)

jithunnair-amd · 2022-10-21T22:33:41Z

@pytorchbot rebase

pytorchmergebot · 2022-10-22T00:49:33Z

@pytorchbot successfully started a rebase job. Check the current status here

pytorchmergebot · 2022-10-22T00:49:36Z

Tried to rebase and push PR #83552, but it was already up to date

kit1980 · 2022-10-25T01:37:41Z

@pytorchbot rebase

kit1980 · 2022-10-25T01:38:54Z

@jithunnair-amd should we merge this?

pytorchmergebot · 2022-10-25T02:26:02Z

@pytorchbot successfully started a rebase job. Check the current status here

pytorchmergebot · 2022-10-25T02:26:06Z

Successfully rebased fix_for_libtinfo6 onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout fix_for_libtinfo6 && git pull --rebase)

jithunnair-amd · 2022-10-26T14:38:52Z

@pytorchbot merge

pytorchmergebot · 2022-10-26T14:40:25Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

This reverts commit a10446c. Reverted #83552 on behalf of https://github.com/kit1980 due to Broke ios/macos builds https://github.com/pytorch/pytorch/actions/runs/3329991911/jobs/5507911292

malfet · 2022-10-26T17:09:42Z

The error on ios/macos build is

2022-10-26T14:58:27.6760830Z CMake Error at cmake/Dependencies.cmake:1276 (file):
2022-10-26T14:58:27.6761450Z   file STRINGS file "/etc/os-release" cannot be read.

But how? Is USE_ROCM true on MacOS/iOS somehow? Or a merge conflict? (Also, I've realized that PR is old enough not to have an automated ciflow/trunk label on review)

kit1980 · 2022-10-26T17:49:48Z

The error on ios/macos build is
2022-10-26T14:58:27.6760830Z CMake Error at cmake/Dependencies.cmake:1276 (file):
2022-10-26T14:58:27.6761450Z   file STRINGS file "/etc/os-release" cannot be read.
But how? Is USE_ROCM true on MacOS/iOS somehow? Or a merge conflict? (Also, I've realized that PR is old enough not to have an automated ciflow/trunk label on review)

I'm curious how also. Maybe cmake does some kind of static analysis before running, don't know...

…os-release" cannot be read.'

malfet · 2022-10-27T00:23:57Z

I'm curious how also. Maybe cmake does some kind of static analysis before running, don't know...
No, cmake is a very simple tool, answer is that ROCM is always enabled unless disabled explicitly:

pytorch/CMakeLists.txt

Line 193 in 2cc624c

option(USE_ROCM "Use ROCm" ON)

kit1980 · 2022-10-27T00:31:27Z

Should it be disabled by default instead?

jithunnair-amd · 2022-10-27T07:51:47Z

Should it be disabled by default instead?

@malfet @kit1980 I don't think that should be necessary. My latest change (simple file-exists check) seems to have fixed the macOS/iOS failures. The 1 failure for linux-bionic-py3_7-clang8-xla / test seems unrelated.

pruthvistony

This change is backward compatible with older versions of ROCm.

kit1980 · 2022-10-28T00:25:29Z

@jithunnair-amd - PR is merged #87833
So your last commit ef86b50 is no longer needed. You can remove it and merge again.

malfet · 2022-10-28T03:46:37Z

@pytorchbot merge -f "XLA failures are unrelated"

malfet · 2022-10-28T03:48:24Z

cmake/Dependencies.cmake

 if(USE_ROCM)
+  # This prevents linking in the libtinfo from /opt/conda/lib which conflicts with ROCm libtinfo.
+  # Currently only active for Ubuntu 20.04 and greater versions.
+  if(UNIX AND EXISTS "/etc/os-release")


Didn't know that LINUX environment variable has only been added in cmake-3.25, though in PyTorch CMake file it has long been defined (as UNIX check would return true even for CygWin)

pytorchmergebot · 2022-10-28T03:50:39Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

github-actions · 2022-10-28T03:51:33Z

Hey @jithunnair-amd.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Fixes issue building PyTorch for ROCm5.3 and above on Ubuntu20.04 because libtinfo6 from conda conflicts with the one from the distro causing symbol not found errors. Pull Request resolved: pytorch#83552 Approved by: https://github.com/malfet

This reverts commit a10446c. Reverted pytorch#83552 on behalf of https://github.com/kit1980 due to Broke ios/macos builds https://github.com/pytorch/pytorch/actions/runs/3329991911/jobs/5507911292

@jeffdaily

Fixes issue building PyTorch for ROCm5.3 and above on Ubuntu20.04 because libtinfo6 from conda conflicts with the one from the distro causing symbol not found errors. cc @jeffdaily @sunway513 @ROCmSupport Pull Request resolved: pytorch#83552 Approved by: https://github.com/malfet, https://github.com/pruthvistony

@jeffdaily

Fixes issue building PyTorch for ROCm5.3 and above on Ubuntu20.04 because libtinfo6 from conda conflicts with the one from the distro causing symbol not found errors. cc @jeffdaily @sunway513 @ROCmSupport Pull Request resolved: pytorch#83552 Approved by: https://github.com/malfet, https://github.com/pruthvistony

Fixes issue building PyTorch for ROCm5.3 and above on Ubuntu20.04 because libtinfo6 from conda conflicts with the one from the distro causing symbol not found errors. Pull Request resolved: pytorch#83552 Approved by: https://github.com/malfet

This reverts commit a10446c. Reverted pytorch#83552 on behalf of https://github.com/kit1980 due to Broke ios/macos builds https://github.com/pytorch/pytorch/actions/runs/3329991911/jobs/5507911292

@jeffdaily

Fixes issue building PyTorch for ROCm5.3 and above on Ubuntu20.04 because libtinfo6 from conda conflicts with the one from the distro causing symbol not found errors. cc @jeffdaily @sunway513 @ROCmSupport Pull Request resolved: pytorch#83552 Approved by: https://github.com/malfet, https://github.com/pruthvistony

@jeffdaily

Fixes issue building PyTorch for ROCm5.3 and above on Ubuntu20.04 because libtinfo6 from conda conflicts with the one from the distro causing symbol not found errors. cc @jeffdaily @sunway513 @ROCmSupport Pull Request resolved: pytorch#83552 Approved by: https://github.com/malfet, https://github.com/pruthvistony

pytorch-bot bot added the module: rocm AMD GPU support for Pytorch label Aug 16, 2022

facebook-github-bot added the cla signed label Aug 16, 2022

pytorchbot added the open source label Aug 16, 2022

ngimel requested a review from malfet August 24, 2022 20:13

ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 24, 2022

pytorchmergebot force-pushed the fix_for_libtinfo6 branch from 836b07c to 0930c0c Compare September 12, 2022 22:23

malfet approved these changes Sep 13, 2022

View reviewed changes

malfet reviewed Sep 13, 2022

View reviewed changes

cmake/Dependencies.cmake Outdated Show resolved Hide resolved

jithunnair-amd force-pushed the fix_for_libtinfo6 branch from 956ade5 to bb1269b Compare October 21, 2022 22:56

jithunnair-amd added 2 commits October 25, 2022 02:26

Use -rpath-link to fix libtinfo conflict

ef3d1b2

Use CMake primitives

5d89b4b

pytorchmergebot force-pushed the fix_for_libtinfo6 branch from bb1269b to 5d89b4b Compare October 25, 2022 02:26

pytorchmergebot added the Merged label Oct 26, 2022

pytorchmergebot added the Reverted label Oct 26, 2022

malfet reopened this Oct 26, 2022

malfet added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 26, 2022

Try to workaround CMake error on MacOS/iOS: 'file STRINGS file "/etc/…

ef86b50

…os-release" cannot be read.'

pruthvistony approved these changes Oct 27, 2022

View reviewed changes

malfet reviewed Oct 28, 2022

View reviewed changes

pytorchmergebot closed this in 2e48b47 Oct 28, 2022

kit1980 removed the Reverted label Oct 29, 2022

Conversation

jithunnair-amd commented Aug 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Aug 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

✅ No Failures (0 Pending)

Uh oh!

jithunnair-amd commented Aug 16, 2022

Uh oh!

jithunnair-amd commented Sep 12, 2022

Uh oh!

pytorchmergebot commented Sep 12, 2022

Uh oh!

pytorchmergebot commented Sep 12, 2022

Uh oh!

pytorch-bot bot commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/83552

❌ 1 Failures, 6 Pending

Uh oh!

malfet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Oct 4, 2022

Uh oh!

linux-foundation-easycla bot commented Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jithunnair-amd commented Oct 21, 2022

Uh oh!

pytorchmergebot commented Oct 22, 2022

Uh oh!

pytorchmergebot commented Oct 22, 2022

Uh oh!

kit1980 commented Oct 25, 2022

Uh oh!

kit1980 commented Oct 25, 2022

Uh oh!

pytorchmergebot commented Oct 25, 2022

Uh oh!

pytorchmergebot commented Oct 25, 2022

Uh oh!

jithunnair-amd commented Oct 26, 2022

Uh oh!

pytorchmergebot commented Oct 26, 2022

Merge started

Uh oh!

malfet commented Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kit1980 commented Oct 26, 2022

Uh oh!

malfet commented Oct 27, 2022

Uh oh!

kit1980 commented Oct 27, 2022

Uh oh!

jithunnair-amd commented Oct 27, 2022

Uh oh!

pruthvistony left a comment

Choose a reason for hiding this comment

Uh oh!

kit1980 commented Oct 28, 2022

Uh oh!

malfet commented Oct 28, 2022

Uh oh!

malfet Oct 28, 2022

Choose a reason for hiding this comment

Uh oh!

pytorchmergebot commented Oct 28, 2022

Merge started

Uh oh!

github-actions bot commented Oct 28, 2022

Uh oh!

Reviewers

Assignees

Labels

jithunnair-amd commented Aug 16, 2022 •

edited

Loading

facebook-github-bot commented Aug 16, 2022 •

edited

Loading

pytorch-bot bot commented Sep 12, 2022 •

edited

Loading

linux-foundation-easycla bot commented Oct 4, 2022 •

edited

Loading

malfet commented Oct 26, 2022 •

edited

Loading