- Notifications
You must be signed in to change notification settings - Fork126
[JK] Portable/Relocatable Python Linking#275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| It"Relocatable Python" { | ||
| $semversion= [semver]$Version | ||
| $pyfilename="python$($semversion.Major).$($semversion.Minor)" | ||
| $artifactPath=Join-Path"Python"$Version|Join-Path-ChildPath$Architecture | ||
| $relocatedPython=Join-Path$HOME"relocated_python" | ||
| $relocatedPythonTool=Join-Path-Path$relocatedPython-ChildPath$artifactPath | ||
| $relocatedFullPath=Join-Path$relocatedPythonTool"bin"|Join-Path-ChildPath$pyfilename | ||
| # copy the current build to relocated_python | ||
| $toolCacheArtifact=Join-Path$env:RUNNER_TOOL_CACHE$artifactPath | ||
| moveAssets-source$toolCacheArtifact-destination$relocatedPythonTool | ||
| try { | ||
| # Verify that relocated Python works | ||
| $relocatedFullPath| Should-Exist | ||
| "$relocatedFullPath --version"| Should-ReturnZeroExitCode | ||
| "sudo$relocatedFullPath --version"| Should-ReturnZeroExitCode | ||
| } | ||
| finally { | ||
| # Revert the changes for other tests | ||
| moveAssets-source$relocatedPythonTool-destination$toolCacheArtifact | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this might fail on macOS.
@jaswanthikolla, can you run the full build in your fork to make sure this passes ?
It might save some time if it requires some modifications before maintainers validate the workflow / review the PR.
It might only fail when using a version not using the universal2 installer so worth checking a build of python < 3.11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is Ubuntu/Linux Specific code, So this test doesn't get executed for macOS. May be, I can simplify by removing powershell code and write bash instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It does fail on macOS which is an UNIX (it's UNIX specific code, not Linux):https://github.com/mayeut/python-versions/actions/runs/11766830107/job/32774981352
klalumiere left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks!
navarro967 left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Builds working on my forks thanks to this PR.
jaswanthikolla commentedOct 31, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Any update on this PR? Can it be reviewed and merged? |
WTPOptAxe commentedNov 7, 2024
Just adding a +1 here. This is a real issue for many users,@jaswanthikolla has put in the work to resolve it for many use cases, and it's been going stale for 5 months. |
stolson commentedNov 21, 2024
Also echoing the need for this fix to be merged. |
leofang commentedJan 7, 2025
What is needed to get this PR merged? |
Uh oh!
There was an error while loading.Please reload this page.
What's the Issue?
Python binary iscompiled with
rpaththat's/opt/hostedtoolcache/Python. Now, latest github runners defineRUNNER_TOOL_CACHE/AGENT_TOOLSDIRECTORYdifferently than/opt/hostedtoolcacheand that installs python at/home/runner/_work/_tool/Python/. So, with this, there are 2 issues.sudo python --versiondoesn't work ( See Error section) because most systems's doesn't allow passing LD_LIBRARY_PATH due to security issues.python --versiondoesn't work without setting environment variableLD_LIBRARY_PATHoutput of ldd :
Error:
Solution:
$ORIGIN is used to as reference to the binary path. So, we can use rpath that references relative path instead of absolute compile time path. With this relative path, Python binaries become relocatable.
Change:
--rpathchanged to-rpathas it's thestandard documented one.How this is tested?
Why bother about this issue?
LD_LIBRARY_PATH can't be used with
sudo pythondue to security concerns. But, more details are hereactions/setup-python#871