-
Notifications
You must be signed in to change notification settings - Fork 707
Aspire cli: Add AOT builds for Linux, and macOS #10275
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
Changes from 7 commits
82c6823
b96b8d8
b30d97a
6247e1f
729e7f2
6c635a5
fe48334
a6bfce8
dc535d2
ef3b855
01d7720
bfbb8a3
12bc107
271c0aa
d6dab11
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 |
|---|---|---|
| @@ -1,9 +1,24 @@ | ||
| <Project> | ||
| <ItemGroup Condition="'$(DotNetBuildFromSource)' != 'true' and '$(DotNetBuild)' != 'true'"> | ||
| <Project TreatAsLocalProperty="TargetRids"> | ||
| <PropertyGroup> | ||
| <_AspireOnlyBuild Condition="'$(DotNetBuildFromSource)' != 'true' and '$(DotNetBuild)' != 'true'">true</_AspireOnlyBuild> | ||
radical marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| <BuildNative>true</BuildNative> | ||
radical marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| <BuildRid>$([System.Runtime.InteropServices.RuntimeInformation]::RuntimeIdentifier)</BuildRid> | ||
|
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. Will this always be correct? I think there are more RIDs than the ones we currently 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. For instance, take a look at this issue: #5486. I think if someone is building in those similar environments then they would now fail to find the .csproj and therefore fail to build. One thing we could do is to condition the ProjectToBuild addition to only in the case the project exists. Another alternative is to use the logic we have here: https://github.com/dotnet/aspire/blob/main/src/Aspire.AppHost.Sdk/Aspire.RuntimeIdentifier.Tool/Program.cs to pick the best rid (we support) based on the rid you are currently on. 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. Hm this needs 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'm fine with waiting for a follow up PR, but I believe that the way you have it now it would actually break the build in some of those distros, so at the very least we should add some defensive code here for those cases. For instance, if you are in one of those distros like rhel, I suspect the build would fail trying to find a project called Aspire.Cli.rhel.....csproj which obviously won't exist. 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. While we personally might not build in those distros, we don't want to prevent folks working in those distros from building. 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. For this PR I added a check so we skip if the correct Cli project cannot be found. |
||
|
|
||
| <!-- this property is meant only for CI use. Use a colon separate list of RIDs here, like osx-x64:osx-arm64 . | ||
radical marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| It is helpful to allow building the non-aot cli builds on a single CI job --> | ||
| <TargetRids Condition="'$(TargetRids)' == ''">$(BuildRid)</TargetRids> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup Condition="'$(_AspireOnlyBuild)' == 'true' and '$(BuildNativeOnly)' != 'true'"> | ||
| <ProjectToBuild Include="$(RepoRoot)src\**\*.csproj" Exclude="$(RepoRoot)src\Aspire.ProjectTemplates\templates\**\*.csproj" /> | ||
| <ProjectToBuild Include="$(RepoRoot)eng\dcppack\**\*.csproj" /> | ||
| <ProjectToBuild Include="$(RepoRoot)eng\dashboardpack\**\*.csproj" /> | ||
| <ProjectToBuild Include="$(RepoRoot)eng\clipack\**\*.csproj" /> | ||
|
|
||
| <_TargetRidItem Include="$(TargetRids.Split(':'))" /> | ||
| <ProjectToBuild Condition="'$(BuildNative)' == 'true'" Include="@(_TargetRidItem -> '$(RepoRoot)eng\clipack\Aspire.Cli.%(Identity).csproj')" /> | ||
|
|
||
| <ProjectToBuild Include="$(RepoRoot)playground\**\*.csproj" /> | ||
|
|
||
| <!-- `$(SkipTestProjects)` allows skipping test projects from being | ||
|
|
@@ -12,6 +27,14 @@ | |
| <ProjectToBuild Include="$(RepoRoot)tests\**\*.csproj" Condition="'$(SkipTestProjects)' != 'true'" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Native build only --> | ||
| <ItemGroup Condition="'$(_AspireOnlyBuild)' == 'true' and '$(BuildNativeOnly)' == 'true'"> | ||
| <ProjectToBuild Include="$(RepoRoot)src\Aspire.Cli\Aspire.Cli.csproj" /> | ||
|
|
||
| <_TargetRidItem Include="$(TargetRids.Split(':'))" /> | ||
| <ProjectToBuild Include="@(_TargetRidItem -> '$(RepoRoot)eng\clipack\Aspire.Cli.%(Identity).csproj')" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- When building from source, we want to use the live repo contents as opposed to cloning the repo. --> | ||
| <PropertyGroup Condition="'$(ArcadeBuildFromSource)' == 'true' or '$(DotNetBuildRepo)' == 'true'"> | ||
| <CopySrcInsteadOfClone>true</CopySrcInsteadOfClone> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,11 +112,43 @@ extends: | |
|
|
||
| stages: | ||
|
|
||
| - stage: build_sign_native | ||
| displayName: Build+Sign native packages | ||
|
|
||
| jobs: | ||
| - template: /eng/pipelines/templates/build_sign_native.yml@self | ||
| parameters: | ||
| agentOs: macos | ||
| targetRidsForSameOS: | ||
| - osx-arm64 | ||
| - osx-x64 | ||
| codeSign: true | ||
| teamName: $(_TeamName) | ||
| extraBuildArgs: >- | ||
| /p:Configuration=$(_BuildConfig) | ||
| $(_SignArgs) | ||
| $(_OfficialBuildIdArgs) | ||
|
|
||
| - template: /eng/pipelines/templates/build_sign_native.yml@self | ||
| parameters: | ||
| agentOs: linux | ||
| targetRidsForSameOS: | ||
| - linux-x64 | ||
|
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. Isn't this missing arm64 and musl? 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 don't AOT those yet, so they are being built in the main Windows job instead of having separate agents to produce self-contained executables. 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 you help me understand why? Why don't we aot those yet? I thought all we needed was to match the OS, so I would have expected that this same queue (the linux-x64 one) to be able to aot for those other two. 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 need to set the machine up for cross compliation. See Publish NativeAOT on Ubuntu doesn't work with -r linux-musl-x64 (dotnet/runtime#92294) and https://learn.microsoft.com/en-us/dotnet/core/deploying/native-aot/cross-compile#linux 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 see, we should probably link those here where we say that a few of these are non aot'ed yet, but I don't think we should hold off this PR for that. 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. The scripts PR has a README where we mention this - https://github.com/dotnet/aspire/pull/10254/files#diff-e2e2432bd7a67b477d58cdba86311ddd7d77d117ba1805bf70545131e380e0dd . 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 in the PR description. |
||
| # no need to sign ELF binaries on linux | ||
| codeSign: false | ||
| teamName: $(_TeamName) | ||
| extraBuildArgs: >- | ||
| /p:Configuration=$(_BuildConfig) | ||
| $(_SignArgs) | ||
| $(_OfficialBuildIdArgs) | ||
|
|
||
| # ---------------------------------------------------------------- | ||
| # This stage performs build, test, packaging | ||
| # ---------------------------------------------------------------- | ||
| - stage: build | ||
| displayName: Build | ||
| dependsOn: | ||
| - build_sign_native | ||
| jobs: | ||
| - template: /eng/common/templates-official/jobs/jobs.yml@self | ||
| parameters: | ||
|
|
@@ -160,6 +192,21 @@ extends: | |
| clean: true | ||
|
|
||
| steps: | ||
| - task: DownloadPipelineArtifact@2 | ||
| displayName: 🟣Download All Native Archives | ||
| inputs: | ||
| itemPattern: | | ||
| **/aspire-cli-*.zip | ||
| **/aspire-cli-*.tar.gz | ||
| targetPath: '$(Build.SourcesDirectory)/artifacts/packages/$(_BuildConfig)' | ||
|
|
||
| - task: PowerShell@2 | ||
| displayName: 🟣List artifacts packages contents | ||
| inputs: | ||
| targetType: 'inline' | ||
| script: | | ||
| Get-ChildItem -Path "$(Build.SourcesDirectory)\artifacts\packages" -File -Recurse | Select-Object FullName, @{Name="Size(MB)";Expression={[math]::Round($_.Length/1MB,2)}} | Format-Table -AutoSize | ||
|
|
||
| - template: /eng/pipelines/templates/BuildAndTest.yml | ||
| parameters: | ||
| dotnetScript: $(Build.SourcesDirectory)/dotnet.cmd | ||
|
|
@@ -169,40 +216,14 @@ extends: | |
| repoLogPath: $(Build.Arcade.LogsPath) | ||
| repoTestResultsPath: $(Build.Arcade.TestResultsPath) | ||
| isWindows: true | ||
|
|
||
| - ${{ if eq(variables._RunAsPublic, True) }}: | ||
| - job: Linux | ||
| ${{ if or(startswith(variables['Build.SourceBranch'], 'refs/heads/release/'), startswith(variables['Build.SourceBranch'], 'refs/heads/internal/release/'), eq(variables['Build.Reason'], 'Manual')) }}: | ||
| # If the build is getting signed, then the timeout should be increased. | ||
| timeoutInMinutes: 120 | ||
| ${{ else }}: | ||
| # timeout accounts for wait times for helix agents up to 30mins | ||
| timeoutInMinutes: 90 | ||
|
|
||
| pool: | ||
| name: NetCore1ESPool-Internal | ||
| image: 1es-mariner-2 | ||
| os: linux | ||
|
|
||
| variables: | ||
| - name: _buildScript | ||
| value: $(Build.SourcesDirectory)/build.sh --ci | ||
|
|
||
| preSteps: | ||
| - checkout: self | ||
| fetchDepth: 1 | ||
| clean: true | ||
|
|
||
| steps: | ||
| - template: /eng/pipelines/templates/BuildAndTest.yml | ||
| parameters: | ||
| dotnetScript: $(Build.SourcesDirectory)/dotnet.sh | ||
| buildScript: $(_buildScript) | ||
| buildConfig: $(_BuildConfig) | ||
| repoArtifactsPath: $(Build.Arcade.ArtifactsPath) | ||
| repoLogPath: $(Build.Arcade.LogsPath) | ||
| repoTestResultsPath: $(Build.Arcade.TestResultsPath) | ||
| isWindows: false | ||
| targetRids: | ||
|
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 you help me understand this? Why do we need to run build in all platforms when build native is happening in a previous step? Also, why are we removing the things like pool, etc? Finally, shouldn't now the 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.
re:pools, that is dropping the old un-used job that ran on Linux. -
- - ${{ if eq(variables._RunAsPublic, True) }}:
- - job: Linux
- ${{ if or(startswith(variables['Build.SourceBranch'], 'refs/heads/release/'), startswith(variables['Bui>
- # If the build is getting signed, then the timeout should be increased.
- timeoutInMinutes: 120
- ${{ else }}:
- # timeout accounts for wait times for helix agents up to 30mins
- timeoutInMinutes: 90
-
- pool:
- name: NetCore1ESPool-Internal
- image: 1es-mariner-2
- os: linux
-
- variables:
- - name: _buildScript
- value: $(Build.SourcesDirectory)/build.sh --ci
-
- preSteps:
- - checkout: self
- fetchDepth: 1
- clean: true
-
- steps:
- - template: /eng/pipelines/templates/BuildAndTest.yml
- parameters:
- dotnetScript: $(Build.SourcesDirectory)/dotnet.sh
- buildScript: $(_buildScript)
- buildConfig: $(_BuildConfig)
- repoArtifactsPath: $(Build.Arcade.ArtifactsPath)
- repoLogPath: $(Build.Arcade.LogsPath)
- repoTestResultsPath: $(Build.Arcade.TestResultsPath)
- isWindows: falseThere 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 you expand why win-x86, linux-arm64 and linux-musl are not aot? We of course would care for those to be aot as well. 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. NativeAOT is not supported for win-x86. |
||
| # aot | ||
| - win-x64 | ||
| - win-arm64 | ||
| # non-aot - single file builds | ||
| - win-x86 | ||
| - linux-arm64 | ||
| - linux-musl-x64 | ||
|
|
||
| - ${{ if and(notin(variables['Build.Reason'], 'PullRequest'), eq(variables['Build.SourceBranch'], 'refs/heads/main')) }}: | ||
| - template: /eng/common/templates-official/job/onelocbuild.yml@self | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.