-
Notifications
You must be signed in to change notification settings - Fork 189
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NativeAOT-LLVM] Build on Linux #2574
Changes from 15 commits
68c98bc
7f4de79
6198eee
91fa499
9ae54c2
ef6cdaa
b4df762
500a55d
6d3ca37
ba2d7af
d08e915
65ba66a
bc6f595
36250d2
d6e968b
14724d5
49a8d0e
88f45f8
e860a5d
f1d8712
e1adb5a
342e14e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -206,13 +206,23 @@ jobs: | |
df -h | ||
displayName: Disk Usage before Build | ||
|
||
# Install Wasm dependencies: emscripten, LLVM, NodeJS | ||
- ${{ if and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.archType, 'wasm')) }}: | ||
- script: call $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-emscripten.cmd $(Build.SourcesDirectory)\wasm-tools | ||
# Install Wasm dependencies on Windows: emscripten, LLVM, NodeJS | ||
- ${{ if and(eq(parameters.hostedOs, 'windows'), and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.archType, 'wasm'))) }}: | ||
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-emscripten.ps1 $(Build.SourcesDirectory)/wasm-tools | ||
displayName: Install/activate emscripten | ||
- script: call $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.cmd $(Build.SourcesDirectory)\wasm-tools $(Build.SourcesDirectory) ${{ parameters.buildConfig }} | ||
displayName: Install/build LLVM | ||
- script: call $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-nodejs.cmd $(Build.SourcesDirectory)\wasm-tools | ||
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-nodejs.ps1 $(Build.SourcesDirectory)\wasm-tools | ||
displayName: Install NodeJS | ||
|
||
# Install Wasm dependencies on Linux: emscripten, LLVM, NodeJS. | ||
- ${{ if and(eq(parameters.hostedOs, 'linux'), and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.archType, 'wasm'))) }}: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we collapse the Windows/Linux into one section now (will require putting Emscripten on the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, done |
||
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-emscripten.ps1 $(Build.SourcesDirectory)/wasm-tools | ||
displayName: Install/activate emscripten | ||
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 -Configs ${{ parameters.buildConfig }} -CI | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
workingDirectory: $(Build.SourcesDirectory)/wasm-tools | ||
displayName: Install/build LLVM | ||
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-nodejs.ps1 $(Build.SourcesDirectory)/wasm-tools | ||
displayName: Install NodeJS | ||
|
||
- ${{ if and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.platform, 'wasi_wasm_win')) }}: | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
param( | ||
[string]$InstallDir | ||
) | ||
|
||
New-Item -ItemType Directory -Force -ErrorAction SilentlyContinue -Path (Split-Path -Path $InstallDir -Parent) -Name (Split-Path -Path $InstallDir -Leaf) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this really need to be this elaborate? I just tried:
And it created the nested directory as expected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its ok on Linux, but with a drive letter it fails. We could strip the drive letter and assume we are "in" that drive, but it probably isn't much easier to read.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this should use
|
||
|
||
$ErrorActionPreference="Stop" | ||
|
||
Set-Location -Path $InstallDir | ||
|
||
git clone https://github.com/emscripten-core/emsdk.git | ||
|
||
Set-Location -Path emsdk | ||
|
||
# Checkout a specific commit to avoid unexpected issues | ||
git checkout 37b85e9 | ||
|
||
python ./emsdk.py install 3.1.47 | ||
|
||
./emsdk activate 3.1.47 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have Also, it is odd that the first line calls |
||
|
||
# Set a variable for later use (used in common/build.ps1) | ||
Write-Host "##vso[task.setvariable variable=NATIVEAOT_CI_WASM_BUILD_EMSDK_PATH]$PWD" |
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,7 +1,21 @@ | ||||||
$InstallPath = $Args[0] | ||||||
$NodeJSVersion = "v20.2.0" | ||||||
$NodeJSInstallName = "node-$NodeJSVersion-win-x64" | ||||||
$NodeJSZipName = "$NodeJSInstallName.zip" | ||||||
|
||||||
if (!(Test-Path variable:global:IsWindows)) | ||||||
{ | ||||||
$IsWindows=[environment]::OSVersion.Platform -eq [PlatformID]::Win32NT | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
|
||||||
if ($IsWIndows) | ||||||
{ | ||||||
$NodeJSInstallName = "node-$NodeJSVersion-win-x64" | ||||||
$NodeJSZipName = "$NodeJSInstallName.zip" | ||||||
} | ||||||
else | ||||||
{ | ||||||
$NodeJSInstallName = "node-$NodeJSVersion-linux-x64" | ||||||
$NodeJSZipName = "$NodeJSInstallName.tar.xz" | ||||||
} | ||||||
|
||||||
if (!(Test-Path $InstallPath)) | ||||||
{ | ||||||
|
@@ -33,9 +47,17 @@ if ($RetryCount -le 0) | |||||
exit 1 | ||||||
} | ||||||
|
||||||
Expand-Archive -LiteralPath "$InstallPath\$NodeJSInstallName.zip" -DestinationPath $InstallPath -Force | ||||||
if ($IsWindows) | ||||||
{ | ||||||
Expand-Archive -LiteralPath "$InstallPath\$NodeJSZipName" -DestinationPath $InstallPath -Force | ||||||
$NodeJSExePath = "$InstallPath\$NodeJSInstallName\node.exe" | ||||||
Comment on lines
+52
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does, but doesn't support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And those are our options for linux https://nodejs.org/dist/v9.9.0/ |
||||||
} | ||||||
else | ||||||
{ | ||||||
tar xJf $InstallPath/$NodeJSZipName -C $InstallPath | ||||||
$NodeJSExePath = "$InstallPath/$NodeJSInstallName/bin/node" | ||||||
} | ||||||
|
||||||
$NodeJSExePath = "$InstallPath\$NodeJSInstallName\node.exe" | ||||||
if (!(Test-Path $NodeJSExePath)) | ||||||
{ | ||||||
Write-Error "Did not find NodeJS at: '$NodeJSExePath'" | ||||||
|
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.
Are these only required for
NATIVEAOT_CI_WASM_BUILD_EMSDK_PATH
? It's not clear what they're for.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.
Its to avoid
${host_arch-''}
might be another way to solve it, or make it a mandatory parameter.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.
But the only uses of
host_arch
that I see look to be new (added in this change). My suggestion is delete them - or are they needed for something?