Skip to content
Open
Changes from all commits
Commits
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
52 changes: 50 additions & 2 deletions utils/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -413,11 +413,19 @@ $KnownPythons = @{
URL = "https://www.nuget.org/api/v2/package/python/3.10.1";
SHA256 = "987a0e446d68900f58297bc47dc7a235ee4640a49dace58bc9f573797d3a8b33";
};
AMD64_Embedded = @{
URL = "https://www.python.org/ftp/python/3.10.1/python-3.10.1-embed-amd64.zip";
SHA256 = "502670dcdff0083847abf6a33f30be666594e7e5201cd6fccd4a523b577403de";
};
ARM64 = @{
URL = "https://www.nuget.org/api/v2/package/pythonarm64/3.10.1";
SHA256 = "16becfccedf1269ff0b8695a13c64fac2102a524d66cecf69a8f9229a43b10d3";
};
};
ARM64_Embedded = @{
URL = "https://www.python.org/ftp/python/3.10.1/python-3.10.1-embed-arm64.zip";
SHA256 = "1f9e215fe4e8f22a8e8fba1859efb1426437044fb3103ce85794630e3b511bc2";
};
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets leave the definitions for 3.9, allowing to optionally upgrade to 3.10 while we iterate to get this working. That will allow you to make progress and get the changes merged earlier rather than trying to keep up to date with the changes in this script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason that we need both the regular and the embedded URLs? Could we not get away with just the embedded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, embeddable python does not come with pip. We could download and run get-pip.py, but I don't think we would gain a lot of CI time in doing so. I have not tried it out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reasoning for the version selection here is not CI times but rather to match what llvm.org distributes. I think that we don't need to inject pip unless we are building the installer for distribution (i.e. nightlies/releases).

}

$PythonModules = @{
Expand Down Expand Up @@ -741,6 +749,10 @@ function Get-PythonPath([Hashtable] $Platform) {
return [IO.Path]::Combine("$BinaryCache\", "Python$($Platform.Architecture.CMakeName)-$PythonVersion")
}

function Get-EmbeddedPythonPath([Hashtable] $Platform) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, is there a bigger picture around the embedded vs non-embedded python that the extraction needs special logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason is pip missing in embeddedable python. We build with the regular Python, but we bundle the embeddable python in the installer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There-in lies the trick! We can inject pip into the packaging :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think injecting pip would work well because of this SO comment, especially because the Python documentation explicitly states:

Using pip to manage dependencies as for a regular Python installation is not supported with this distribution [embeddable Python]

Given that, and the extra steps it would take to manually install pip with the embeddable python, I think it's simpler to:

  • Build with the regular Python
  • Test with the regular Python (easier to install dependencies)
  • Package the embeddable Python.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should just use a boolean state on the known dictionary rather than extract this. Overall, I'd like to see the python distribution to be complete enough for use in scripting (i.e. has pip).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should just use a boolean state on the known dictionary rather than extract this.

I'm not sure I understand what that boolean state would look like: would it be something like hasPip?

Overall, I'd like to see the python distribution to be complete enough for use in scripting (i.e. has pip).

I agree that it would be more convenient, however in that case, why not package a complete Python install? pip is not supported by embeddable Python: https://github.com/swiftlang/swift/pull/83488/files#r2254566330.

I see the Embeddable Python in the installer feature as a way for users to get a functional lldb whether they have the correct Python version installed or not. This is the main source of confusion for users: llvm/llvm-project#74073 (comment)

I think it's reasonable to expect power users who want to have scripting features to install Python themselves and have proper pip support rather than to risk using pip in an unsupported way with Embeddable Python.

return [IO.Path]::Combine("$BinaryCache\", "EmbeddedPython$($Platform.Architecture.CMakeName)-$PythonVersion")
}

function Get-PythonExecutable {
return [IO.Path]::Combine((Get-PythonPath $BuildPlatform), "tools", "python.exe")
}
Expand All @@ -749,6 +761,10 @@ function Get-PythonScriptsPath {
return [IO.Path]::Combine((Get-PythonPath $BuildPlatform), "tools", "Scripts")
}

function Get-EmbeddedPythonInstallDir() {
return [IO.Path]::Combine("$ImageRoot\", "Program Files", "Swift", "Python-$PythonVersion")
}

function Get-Syft {
return $KnownSyft[$SyftVersion][$BuildArchName]
}
Expand Down Expand Up @@ -1250,11 +1266,33 @@ function Get-Dependencies {
return $KnownPythons[$PythonVersion].$ArchName
}

function Get-KnownEmbeddedPython([string] $ArchName) {
if (-not $KnownPythons.ContainsKey($PythonVersion)) {
throw "Unknown python version: $PythonVersion"
}
if (-not $KnownPythons[$PythonVersion].ContainsKey("${ArchName}_Embedded")) {
return $null
}
return $KnownPythons[$PythonVersion]["${ArchName}_Embedded"]
}

function Install-Python([string] $ArchName) {
$Python = Get-KnownPython $ArchName
DownloadAndVerify $Python.URL "$BinaryCache\Python$ArchName-$PythonVersion.zip" $Python.SHA256
if (-not $ToBatch) {
Expand-ZipFile Python$ArchName-$PythonVersion.zip "$BinaryCache" Python$ArchName-$PythonVersion
Expand-ZipFile "Python$ArchName-$PythonVersion.zip" "$BinaryCache" "Python$ArchName-$PythonVersion"
}
}

function Install-EmbeddedPython([string] $ArchName) {
$Python = Get-KnownEmbeddedPython $ArchName
if ($null -eq $Python) {
Write-Output "Python $PythonVersion does not have an embeddable version."
return
}
Comment on lines +1289 to +1292
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should handle this similarly to Install-Python.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I'm not sure I understand: the 2 functions are identical to me apart from the if ($null -eq $Python) { check.

DownloadAndVerify $Python.URL "$BinaryCache\EmbeddedPython$ArchName-$PythonVersion.zip" $Python.SHA256
if (-not $ToBatch) {
Expand-ZipFile "EmbeddedPython$ArchName-$PythonVersion.zip" "$BinaryCache" "EmbeddedPython$ArchName-$PythonVersion"
}
}

Expand Down Expand Up @@ -1305,6 +1343,7 @@ function Get-Dependencies {
}

Install-Python $HostArchName
Install-EmbeddedPython $HostArchName
if ($IsCrossCompiling) {
Install-Python $BuildArchName
}
Expand Down Expand Up @@ -2163,6 +2202,7 @@ function Get-CompilersDefines([Hashtable] $Platform, [string] $Variant, [switch]
LLDB_PYTHON_EXE_RELATIVE_PATH = "python.exe";
LLDB_PYTHON_EXT_SUFFIX = ".pyd";
LLDB_PYTHON_RELATIVE_PATH = "lib/site-packages";
LLDB_PYTHON_DLL_RELATIVE_PATH = "../../../../Python-$PythonVersion";
LLDB_TABLEGEN = (Join-Path -Path $BuildTools -ChildPath "lldb-tblgen.exe");
LLDB_TEST_MAKE = "$BinaryCache\GnuWin32Make-4.4.1\bin\make.exe";
LLVM_CONFIG_PATH = (Join-Path -Path $BuildTools -ChildPath "llvm-config.exe");
Expand All @@ -2187,6 +2227,7 @@ function Get-CompilersDefines([Hashtable] $Platform, [string] $Variant, [switch]
Python3_INCLUDE_DIR = "$PythonRoot\include";
Python3_LIBRARY = "$PythonRoot\libs\$PythonLibName.lib";
Python3_ROOT_DIR = $PythonRoot;
Python3_VERSION = $PythonVersion;
SWIFT_TOOLCHAIN_VERSION = "${ToolchainIdentifier}";
SWIFT_BUILD_SWIFT_SYNTAX = "YES";
SWIFT_CLANG_LOCATION = (Get-PinnedToolchainToolsDir);
Expand Down Expand Up @@ -3831,6 +3872,12 @@ function Install-HostToolchain() {
Copy-Item -Force `
-Path $SwiftDriver `
-Destination "$($HostPlatform.ToolchainInstallRoot)\usr\bin\swiftc.exe"

# Copy embeddable Python
New-Item -Type Directory -Path "$(Get-EmbeddedPythonInstallDir)" -ErrorAction Ignore | Out-Null
Copy-Item -Force -Recurse `
-Path "$(Get-EmbeddedPythonPath $HostPlatform)\*" `
-Destination "$(Get-EmbeddedPythonInstallDir)"
}

function Build-Inspect([Hashtable] $Platform) {
Expand Down Expand Up @@ -3903,6 +3950,7 @@ function Build-Installer([Hashtable] $Platform) {
INCLUDE_SWIFT_DOCC = $INCLUDE_SWIFT_DOCC;
SWIFT_DOCC_BUILD = "$(Get-ProjectBinaryCache $HostPlatform DocC)\release";
SWIFT_DOCC_RENDER_ARTIFACT_ROOT = "${SourceCache}\swift-docc-render-artifact";
PythonVersion = $PythonVersion
}

Invoke-IsolatingEnvVars {
Expand Down