-
Notifications
You must be signed in to change notification settings - Fork 696
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
Conversation
…t to the host os and arch
…the main Windows job, and publish the archives
…. Arcade signing with MacDeveloperHardenWithNotarization is enough
|
||
<BuildNative>true</BuildNative> | ||
|
||
<BuildRid>$([System.Runtime.InteropServices.RuntimeInformation]::RuntimeIdentifier)</BuildRid> |
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.
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Hm this needs NuGet.ProjectModel
, and will need a custom task too. I can explore this in a follow up PR.
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'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 comment
The 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 comment
The 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.
repoLogPath: $(Build.Arcade.LogsPath) | ||
repoTestResultsPath: $(Build.Arcade.TestResultsPath) | ||
isWindows: false | ||
targetRids: |
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.
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 isWindows
property get conditioned based on the target rid? Same for the dotnet script.
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.
- The
- stage: build_sign_native
runs the jobs to AOT on various os/arch combinations. But it does not cover all the archs under macos and linux (eg.linux-arm64
). And it skips Windows.- This stage then uploads the archives as artifacts which the main Windows job in
- stage: build
downloads.- And this Windows job produces the self-contained cli executables for the missing combinations (like
linux-arm64
) and Windows.
- And this Windows job produces the self-contained cli executables for the missing combinations (like
- This stage then uploads the archives as artifacts which the main Windows job in
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: false
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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
NativeAOT is not supported for win-x86.
For the rest, in this PR they are not AOT because I haven't yet figured out the correct build images to use for the agents. And the aim was to get this ready for merging on the past Monday. I do plan to add AOT for these though.
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
The Azdo failures are unrelated, and I'm looking at those separately. |
eng/Build.props
Outdated
<!-- Use a colon separate list of RIDs here, like osx-x64:osx-arm64 . | ||
It is helpful to allow building the non-aot cli builds on a single CI job --> | ||
<TargetRids Condition="'$(TargetRids)' == ''">$(BuildRid)</TargetRids> | ||
<SkipManagedBuild Condition="'$(SkipManagedBuild)' == ''">false</SkipManagedBuild> |
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.
NIT: No need to include any of these two here, you can fully remove as nothing is checking that they are set to false. In other words, if they are not set that should be the default, since you are only validating they are not true. Does that make sense?
parameters: | ||
agentOs: linux | ||
targetRidsForSameOS: | ||
- linux-x64 |
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.
Isn't this missing arm64 and musl?
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.
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 comment
The 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 comment
The 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 comment
The 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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
.. and in the PR description.
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.
…ith msbuild logic to validate the list of packages
Internal build to validate the new changes - https://dev.azure.com/dnceng/internal/_build/results?buildId=2747492&view=results |
/backport to release/9.4 |
Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16225253898 |
Add AOT builds for Linux, and macOS
What?
This PR adds AOT builds for Linux, and macOS on the internal builds. On macOS the binaries are appropriately signed, and notarized making them usable after downloading.
How?
This is done by having separate OS-specific jobs just to build the AOTed aspire cli. And these upload the archives for use by a "joining" Windows job which builds windows binaries, and publishes all the archives. Checksum (
sha512
) files are also generated for the archives which can be used to verify the archive when downloading.This is the current supported matrix:
win-x64
win-arm64
win-x86
linux-x64
linux-arm64
linux-musl-x64
osx-x64
osx-arm64
win-x86
- Native AOT is not supported for thislinux-arm64
,linux-musl-x64
- will be added in a follow up PRFor the non-aot cases a self-contained executable is generated.
Tests
To catch missing archives due to any reason on the CI build