Skip to content

Update Aspire.AppHost.Sdk to support simpler file-based app AppHosts #11445

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Sep 18, 2025
Merged

Update Aspire.AppHost.Sdk to support simpler file-based app AppHosts #11445

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
153e3a3
Auto-import base .NET SDK from the Aspire.AppHost.Sdk if not already …
DamianEdwards Sep 16, 2025
65f8586
Default to disable publishaot
DamianEdwards Sep 17, 2025
8c4ba11
Auto-add the AppHost package if not referenced
DamianEdwards Sep 17, 2025
c584212
Update Sdk.in.targets
DamianEdwards Sep 17, 2025
2d2d0cb
Fix typo
DamianEdwards Sep 17, 2025
c97e648
Update Sdk.in.props
DamianEdwards Sep 17, 2025
9dfa9e9
Move defaulting PublishAot to SDK targets
DamianEdwards Sep 17, 2025
112e4e9
rebase on single-file stuff & simplify template
DamianEdwards Sep 17, 2025
1c48da7
Set PublishAot earlier to ensure it's REALLY set
DamianEdwards Sep 17, 2025
d4dde9f
MOAR!
DamianEdwards Sep 17, 2025
96e0065
Fix typo
DamianEdwards Sep 17, 2025
7e3c467
Once more with feeling
DamianEdwards Sep 17, 2025
979496f
There's still hope
DamianEdwards Sep 17, 2025
46b90c5
Restrict implicit SDK, package reference, & AOT/trimming defaults to …
DamianEdwards Sep 18, 2025
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
15 changes: 13 additions & 2 deletions src/Aspire.AppHost.Sdk/SDK/Sdk.in.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
<Project>
<PropertyGroup>
<AspireHostingSDKVersion>@VERSION@</AspireHostingSDKVersion>
<!-- Do not import the workload targets as this project is instead using
the Aspire.AppHost.Sdk -->
<!-- Do not import the workload targets as this project is instead using the Aspire.AppHost.Sdk -->
<SkipAspireWorkloadManifest>true</SkipAspireWorkloadManifest>
<IsAspireHost>true</IsAspireHost>
<PublishAot>false</PublishAot>
<PublishTrimmed>false</PublishTrimmed>
<EnableAotAnalyzer>false</EnableAotAnalyzer>
Copy link
Member

Choose a reason for hiding this comment

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

Instead of setting all of these it may be easier to set IsAotCompatible and IsTrimmable to false - these are an implicit condition of all 3 of the analyzers, and they also tie into disabling the ProjectCapability Items you remove later on:

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me try.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I think still need to set these explicitly don't I because they'll be set an evaluation due to the file-based apps virtual project setting PublishAot to true?

Copy link
Member Author

Choose a reason for hiding this comment

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

Also AppHost projects aren't publishable so the publish targets shouldn't matter in this case.

Copy link
Member

Choose a reason for hiding this comment

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

mm not sure I love setting these in the SDK, where other projects that are not file-based apps would be impacted. Is there a way to fix this via the virtual project somehow? For instance, if it sees that the aspire sdk is set then set these to false inside the virtual project?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I'm not a fan of hard-coding stuff like that in file-based apps directly, that's backwards :)
SDKs should be able to impart changes on projects that reference them, that's not an issue. This specific implementation of doing it in the targets is a temp workaround for file-based apps which we can remove once that's fixed. Then we'll just be setting the default values of these in our Sdk.in.props file.

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 property that we can chain of only defined in virtual projects so we can add a condition to these? If not, I think at the very least we should add conditions to all of these to check if they were not already defined (meaning only set them if they are not set already)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think at the very least we should add conditions to all of these to check if they were not already defined (meaning only set them if they are not set already)

This specifically won't work because of where they have to run, that's the issue.

We can condition this so that it only applies to file-based apps (there's a property) if we really want to isolate it, but I'm not convinced it matters. What's the concern?

Copy link
Member

Choose a reason for hiding this comment

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

put the other way - are there actual valid use cases for an Aspire AppHost to be trimmed/AOT? If not, why even allow it?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are not, it's dev time only and we block publishing.

<EnableTrimAnalyzer>false</EnableTrimAnalyzer>
<EnableSingleFileAnalyzer>false</EnableSingleFileAnalyzer>
Comment on lines +10 to +14
Copy link
Preview

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

These property defaults are duplicated in both Sdk.in.props (lines 7-11) and in the DisablePublishAotForAspireHost target in Sdk.in.targets (lines 56-60). This duplication could lead to inconsistencies. Consider consolidating these settings in one location, preferably in the target where they can be conditionally applied.

Suggested change
<PublishAot>false</PublishAot>
<PublishTrimmed>false</PublishTrimmed>
<EnableAotAnalyzer>false</EnableAotAnalyzer>
<EnableTrimAnalyzer>false</EnableTrimAnalyzer>
<EnableSingleFileAnalyzer>false</EnableSingleFileAnalyzer>

Copilot uses AI. Check for mistakes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, doing it in both places until dotnet/sdk#50885 is merged and flows through to rc.2 builds.

<_ImportMicrosoftNETSdkFromAspireSdk>false</_ImportMicrosoftNETSdkFromAspireSdk>
</PropertyGroup>

<PropertyGroup Condition=" '$(UsingMicrosoftNETSdk)' == '' ">
<_ImportMicrosoftNETSdkFromAspireSdk>true</_ImportMicrosoftNETSdkFromAspireSdk>
</PropertyGroup>

<Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" Condition=" '$(_ImportMicrosoftNETSdkFromAspireSdk)' == 'true' " />
</Project>
28 changes: 27 additions & 1 deletion src/Aspire.AppHost.Sdk/SDK/Sdk.in.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
This means they cannot be overridden in the csproj, and may cause ordering issues, particularly StaticWebAssets.
-->

<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" Condition=" '$(_ImportMicrosoftNETSdkFromAspireSdk)' == 'true' " />

<ItemGroup>
<ProjectCapability Include="DynamicFileNesting" />
<ProjectCapability Include="DynamicFileNestingEnabled" />
Expand Down Expand Up @@ -49,11 +51,35 @@
<AspireRidToolExecutable>$(AspireRidToolDirectory)Aspire.RuntimeIdentifier.Tool.dll</AspireRidToolExecutable>
</PropertyGroup>

<Target Name="DisablePublishAotForAspireHost" BeforeTargets="_ComputeToolPackInputsToProcessFrameworkReferences;_CollectTargetFrameworkForTelemetry;ProcessFrameworkReferences;Restore;CollectPackageReferences" Condition="'$(IsAspireHost)' == 'true' and '$(DisableAspirePublishAotDefaults)' != 'true'">
Copy link
Preview

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

[nitpick] The target name 'DisablePublishAotForAspireHost' doesn't accurately reflect what the target does - it sets multiple AOT and trimming properties, not just PublishAot. Consider renaming to 'SetAspireHostAotDefaults' or 'ConfigureAspireHostPublishSettings' for better clarity.

Suggested change
<Target Name="DisablePublishAotForAspireHost" BeforeTargets="_ComputeToolPackInputsToProcessFrameworkReferences;_CollectTargetFrameworkForTelemetry;ProcessFrameworkReferences;Restore;CollectPackageReferences" Condition="'$(IsAspireHost)' == 'true' and '$(DisableAspirePublishAotDefaults)' != 'true'">
<Target Name="SetAspireHostAotDefaults" BeforeTargets="_ComputeToolPackInputsToProcessFrameworkReferences;_CollectTargetFrameworkForTelemetry;ProcessFrameworkReferences;Restore;CollectPackageReferences" Condition="'$(IsAspireHost)' == 'true' and '$(DisableAspirePublishAotDefaults)' != 'true'">

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

There's a part of me that wants to try and trim this dependency list - e.g. Restore is implied by CollectPackageReferences, so it could be safe to drop Restore directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

The intent is for this to go away completely once dotnet/sdk#50885 is in anyway.

<PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this target if these are defined in Sdk.props already statically?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because file-based apps resets the PublishAot property (and thus everything that is evaluated off of its value) directly in the virtual project today, so setting it in our props gets overwritten. Doing it in this target is required to ensure it overrides what is set in the project. This whole target will be removed once dotnet run is fixed to only set the implicit project-level stuff if a value isn't already explicitly set (dotnet/sdk#50885).

<PublishAot>false</PublishAot>
<PublishTrimmed>false</PublishTrimmed>
<EnableAotAnalyzer>false</EnableAotAnalyzer>
<EnableTrimAnalyzer>false</EnableTrimAnalyzer>
<EnableSingleFileAnalyzer>false</EnableSingleFileAnalyzer>
</PropertyGroup>
<ItemGroup>
<ProjectCapability Remove="NativeAOT;PublishAot;PublishTrimmed" />
</ItemGroup>
</Target>

<Target Name="AddImplicitAspireAppHostPackage" BeforeTargets="Restore;CollectPackageReferences" Condition="'$(IsAspireHost)' == 'true' And '$(SkipAddAspireDefaultReferences)' != 'true'">
<PropertyGroup>
<_ImplicitAppHostVersion>@VERSION@</_ImplicitAppHostVersion>
</PropertyGroup>
<ItemGroup>
<!-- Add the reference to the Aspire.Hosting.AppHost package if not already referenced -->
<PackageReference Include="Aspire.Hosting.AppHost" Version="$(_ImplicitAppHostVersion)" IsImplicitlyDefined="true"
Copy link
Member

Choose a reason for hiding this comment

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

One thing I don't like about this change, is that it may cause people to remove the PackageReference from their project, and therefore never finding out if there is a newer version of Aspire given Msbuild SDKs don't have update notifications. If possible, I'd also condition this to only happen for file-based apps.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't have to advertise it for now and the templates aren't being changed. Plus we have aspire update (assuming that still works 😄). If we're really super concerned about this, we can limit this to file-based AppHosts only.

Condition="'@(PackageReference->WithMetadataValue('Identity','Aspire.Hosting.AppHost'))' == '' And '@(PackageVersion->WithMetadataValue('Identity','Aspire.Hosting.AppHost'))' == ''" />
Copy link
Member

Choose a reason for hiding this comment

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

NIT: why do you need to check PackageVersion too? Even if using CPM, wouldn't we want to do this even if there was a PackageVersion for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's copilot being helpful 😄
I think it's safe to remove the check for PackageVersion like you say.

Copy link
Member Author

Choose a reason for hiding this comment

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

Although the challenge there is, if using CPM, we don't want to implicitly add the package with a version right? Because that version should now be controlled by CPM, not the PackageReference we add. Or does the IsImplicitlyDefined="true" effectively turn off CPM for that PackageReference?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, when you mark the package as implicitly defined then you can pass a version.

</ItemGroup>
</Target>

<!-- This target extracts the version of Aspire.Hosting.AppHost referenced by the project, and adds
a reference to Aspire.Dashboard.Sdk and Aspire.Hosting.Orchestration for the build-time platform using
the same version. This is done here dynamically to avoid having to pull in DCP and Dashboard packages
for all of the platforms. This mechanism can be disabled by setting `SkipAddAspireDefaultReferences` to `true` -->
<Target Name="AddReferenceToDashboardAndDCP" BeforeTargets="Restore;CollectPackageReferences" Condition="'$(SkipAddAspireDefaultReferences)' != 'true'">
<Target Name="AddReferenceToDashboardAndDCP" BeforeTargets="Restore;CollectPackageReferences" AfterTargets="AddImplicitAspireAppHostPackage" Condition="'$(SkipAddAspireDefaultReferences)' != 'true'">

<!-- First, we assume project is not using Central Package Management, and we try to extract the version
from PackageReference Items. -->
Expand Down
3 changes: 0 additions & 3 deletions src/Aspire.ProjectTemplates/templates/aspire-apphost-singlefile/9.5/apphost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
#:sdk Microsoft.NET.Sdk
#:sdk Aspire.AppHost.Sdk@!!REPLACE_WITH_LATEST_VERSION!!
#:package Aspire.Hosting.AppHost@!!REPLACE_WITH_LATEST_VERSION!!
#:property PublishAot=false

var builder = DistributedApplication.CreateBuilder(args);

Expand Down
Loading