Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JK] Portable/Relocatable Python Linking #275

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+33 −2
Open

[JK] Portable/Relocatable Python Linking #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions builders/ubuntu-python-builder.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class UbuntuPythonBuilder : NixPythonBuilder {

$pythonBinariesLocation = $this.GetFullPythonToolcacheLocation()

### To build Python with SO we must pass full path to lib folder to the linker
$env:LDFLAGS="-Wl,--rpath=${pythonBinariesLocation}/lib"
### To build Python with SO, passing relative path W.r.t to the binary location.
$env:LDFLAGS="-Wl,-rpath='`$`$ORIGIN/../lib'"
$configureString = "./configure"
$configureString += " --prefix=$pythonBinariesLocation"
$configureString += " --enable-shared"
Expand All @@ -43,6 +43,7 @@ class UbuntuPythonBuilder : NixPythonBuilder {

Write-Host "The passed configure options are: "
Write-Host $configureString
Write-Host "LDFLAGS: $env:LDFLAGS"

Execute-Command -Command $configureString
}
Expand Down
30 changes: 30 additions & 0 deletions tests/python-tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ BeforeAll {

return 0
}

function moveAssets([string] $source, [string] $destination) {
$parentDestDir = Split-Path -Path $destination -Parent
New-Item -Path $parentDestDir -ItemType Directory -Force
Move-Item -Path $source -Destination $parentDestDir -Force
}
}

Describe "Tests" {
Expand Down Expand Up @@ -88,6 +94,30 @@ Describe "Tests" {
It "Check if shared libraries are linked correctly" {
"bash ./sources/psutil-install-test.sh" | Should -ReturnZeroExitCode
}

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
}
}
Comment on lines +97 to +120
Copy link
Contributor

@mayeut mayeut Jun 1, 2024 •
edited
Loading

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

Copy link
Author

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.

Copy link
Contributor

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

}

# Pyinstaller 3.5 does not support Python 3.8.0. Check issue https://github.com/pyinstaller/pyinstaller/issues/4311
Expand Down