Improve usage of ProjectReferences in the AppHost

[davidfowl]

We use project references from the app host to service projects for 2 reasons:

1. dotnet run just works. We get an orchestrated build and run experience for free from the command line.
2. We use these P2Ps to create a class for each referenced project for a type-safe experience for adding project references.

There are 2 major things that have come up recently with respect to the app host and orchestration:

• Dependencies in the apps affect the app host (see https://github.com/dotnet/aspire/issues/948)
• The app host's TFM shouldn't affect the application's TFM. People are starting to ask for support for older framework versions. While I don't think it's important to support < .NET 8 right now, this drives some architectural changes into the product that are good.

[danegsta]

We should be able to control these transitive dependency behaviors via ProjectReference metadata. If I remember correctly, <SkipGetTargetFrameworkProperties>true</SkipGetTargetFrameworkProperties> and <ReferenceOutputAssembly>false</ReferenceOutputAssembly> will give us the behavior we want, but we should definitely test to be sure (see here for the list of ProjectReference metadata).

In order to guard against an unforeseen future scenario where a user does want a ProjectReference with full transitive dependency behavior in their AppHost project, we could update our scaffolding to append a custom metadata value to the service ProjectReference items. Something along the lines of <IsAspireService>true</IsAspireService> that we can key off of to automatically apply the above dependency metadata values.

[DamianEdwards]

@baronfel thoughts?

[DamianEdwards]

and <ReferenceOutputAssembly>false</ReferenceOutputAssembly>

When I tried that it broke our current code-generation logic. Likely it can be fixed though by altering our approach.

[baronfel]

@baronfel thoughts?

Oh boy, do I!

I did a lot of this in this commit of a branch with the goal of keeping ProjectReferences for describing the relationship between components. It works quite well! You can go even further by removing the requirement to even build.

The ProjectReference metadata changes that @danegsta mentions could be handled by the Aspire.Hosting.Targets themselves if you have the custom metadata hook he's describing - in this case you could have an <ItemGroup> that does something like this in your targets:

<ItemGroup>
  <ProjectReference Update="@(ProjectReference)" Condition="'%(ProjectReference.IsAspireService)' == 'true'">
    <SkipGetTargetFrameworkProperties>true</SkipGetTargetFrameworkProperties>
    <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
  <ProjectReference>
</ItemGroup>

That's an established and fine pattern - the only potential concern could be if a referenced project exposes multiple TFMs. The branch I have above kind of works like that, but can't really be used in the aspire samples themselves because they take a project reference on aspire itself, instead of a PackageReference. That gap would be solved if you were ok with needing to add a piece of metadata.

If you wanted to go further then you could have the Aspire Host not even build the projects by setting BuildReference=false - this would speed up operations like building the Aspire AppHost and generating the Aspire manifest massively, but at the cost of making the AppHost need to orchestrate building the applications after the AppHost was launched (e.g. docker-compose up building containers on-demand).

[baronfel]

When I tried that it broke our current code-generation logic. Likely it can be fixed though by altering our approach.

Yes, you get around this by changing the code-generation logic to operate on the ProjectReferences themselves, which you can see in this diff. You ask each project reference to compute its output path (a very fast operation) and then go from there.

[rainersigwald]

I'm surprised to see an explicit <MSBuild task call -- could you instead use OutputItemType to hook on the "normal" infrastructure? I guess that'd require TF negotiation if the service was multitargeted but that doesn't seem horrible . . .

[baronfel]

Do you mean on my branch @rainersigwald? That could work if the other projects were built normally first, which currently is the case, but that's not strictly required just to generate the codegen'd structures.

[eerhardt]

A problem with blindly setting ReferenceOutputAssembly=false on all the AppHost's ProjectReferences is the case where a user has a set of AppHost extensions in a class library, and references that class library from the AppHost (i.e. the "normal" case when you want to use ProjectReference). This would break that scenario.

A thought I've been throwing around is to create a new type of Reference item, maybe AspireProjectReference, and then behind the scenes we could turn those into <ProjectReference ReferenceOutputAssembly=false SkipGetTargetFrameworkProperties=true /> items. (Note this is just a minor tweak on @danegsta's thoughts above on <IsAspireService>true</IsAspireService>.)

[DamianEdwards]

I don't mind that suggestion @eerhardt but I think it's worth pointing it means we lose a built-in CLI story for adding references to the AppHost as one can't just use dotnet add reference anymore.

[danegsta]

@eerhardt yeah, that would really just come down to whether we use <ProjectReference Include="..."> or <ProjectReference Update="..."> to generate/update the needed ProjectReference items. It's mostly down to the preferred UX for the references. @DamianEdwards if there's not a way to add custom metadata with dotnet add reference, the CLI story may be broken either way.

[rainersigwald]

If that's a high priority the inverse of IsAspireService may be what you want -- IsAppHostExtension. Then dotnet add reference works for services and you're an edit away for extensions.

[DamianEdwards]

Wasn't trying to convey a priority, just wanted to ensure it was entered into the conversation as we figure this out.

[danegsta]

@rainersigwald only downside with the inverse property is that we could break existing projects (the Aspire tests, for example, would probably be broken without adding IsAppHostExtension to some project references). But it does say "Preview" on the box.

[DamianEdwards]

The inverse property idea seems like a bad precedent to set, i.e. making regular looking ProjectReference items behave totally differently. If anything I think we'd want a new item type or metadata on an existing item type.

[eerhardt]

I don't mind that suggestion @eerhardt but I think it's worth pointing it means we lose a built-in CLI story for adding references to the AppHost as one can't just use dotnet add reference anymore.

dotnet add aspire-reference 😄

[DamianEdwards]

I don't mind that suggestion @eerhardt but I think it's worth pointing it means we lose a built-in CLI story for adding references to the AppHost as one can't just use dotnet add reference anymore.

dotnet add aspire-reference 😄

😬

[danegsta]

Regardless of the approach, we should make sure to add a comment to the AppHost project template(s) to explain the behavior for anyone looking to manually add a service project reference.

[mitchdenny]

Extending dotnet add [something] is probably something that would need to wait until .NET 9.0? We probably need an interim solution.