-
Notifications
You must be signed in to change notification settings - Fork 720
Start the beginning of AOT'ing the Aspire.Cli. #9841
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
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.
Pull Request Overview
This PR initiates the work to AOT the Aspire.Cli by replacing tuple usage with strong, named types for RPC data structures and streamlining related dependencies.
- Replace tuples with strongly typed objects in both hosting and CLI backchannel areas.
- Remove the now unused CliRpcTarget dependency.
- Update configuration files and package versions to support AOT and improved RPC compatibility.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs | Removed CliRpcTarget dependency to match production code changes. |
| src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs | Changed RPC method return types from tuples to strong types. |
| src/Aspire.Cli/Program.cs | Removed CliRpcTarget registration from DI in CLI startup. |
| src/Aspire.Cli/Backchannel/AppHostBackchannel.cs | Updated RPC handling and conversion from strong types to tuples. |
| src/Aspire.Cli/Backchannel/BackchannelDataTypes.cs | Introduced strongly typed backchannel data objects for CLI. |
| src/Aspire.Cli/Aspire.Cli.csproj | Updated shared file references for consistency. |
| Directory.Packages.props | Upgraded StreamJsonRpc version to support latest functionality. |
Comments suppressed due to low confidence (4)
tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs:173
- The removal of CliRpcTarget from production code should be consistently reflected in tests. Ensure tests are updated to no longer rely on CliRpcTarget registration.
var logger = serviceProvider.GetRequiredService<ILogger<AppHostBackchannel>>();
src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs:26
- Ensure that all consumers of GetPublishingActivitiesAsync are updated to handle PublishingActivityState instead of tuple return values.
public async IAsyncEnumerable<PublishingActivityState> GetPublishingActivitiesAsync([EnumeratorCancellation] CancellationToken cancellationToken)
src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs:107
- Confirm that all RPC consumers are updated to use DashboardUrlsState in place of tuple-based dashboard URLs.
public Task<DashboardUrlsState> GetDashboardUrlsAsync()
src/Aspire.Cli/Backchannel/AppHostBackchannel.cs:168
- Ensure that converting PublishingActivityState to a tuple preserves type safety and that all consumers of this method are adjusted accordingly.
state.IsError);
| <PackageVersion Include="System.IO.Hashing" Version="9.0.4" /> | ||
| <PackageVersion Include="Yarp.ReverseProxy" Version="2.3.0" /> | ||
| <PackageVersion Include="StreamJsonRpc" Version="2.21.69" /> | ||
| <PackageVersion Include="StreamJsonRpc" Version="2.22.11" /> |
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.
Am I understanding correctly then that this new version is now AOT friendly? (meaning the branch that was WIP in their repo has now merged to main?)
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.
Not yet
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.
No. This new version doesn't include a bunch of changes in StreamJsonRpc that are needed. I'm working with @AArnott to get all the necessary changes in main and then ship a "preview" package with them in.
We will use the "preview" package just from the CLI. The AppHost will continue to reference the latest released stable version. The two versions will be compatible. And the CLI can use a "preview" package because we package the code and will AOT it in our released executable. We don't need to reference a stable version like we do in our libraries.
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.
This is awesome! thanks @eerhardt!
This is the initial work to getting the Aspire.Cli AOT'd.
Don't use Tuples on the wire. Instead always use strong, named types. This does not require a baseline capability version bump becuase we already bumped it for 9.4 in #9400.
With more work and depending on microsoft/vs-streamjsonrpc#1196 I got this working locally:
The
aspire.exeis about 12 MB today and starts basically immediately.