FrameworkReference support

[drewnoakes]

Relates to #4362 / #4762 / #4444.

Work in progress.

Outstanding questions:

• Do framework references expand to show child dependencies? (deferred until later, tracked by Show assemblies within shared framework targeting packs #4444)
• Should the NuGet package used to deliver the framework be hidden, as is done for SDKs today?
• Does it make sense to show both the Microsoft.WindowsDesktop.App SDK and Framework?
• Is there a better icon for framework references?
• Should we create a new ResolvedFrameworkReference rather than re-using CollectedFrameworkReference?

Outstanding tasks:

• Update .swr files
• Determine additional metadata for FrameworkReference item browse objects

[davidwengier]

I only reviewed the one commit you mentioned to review in this.

[cartermp]

Do framework references expand to show child dependencies?

Is there a notable performance implication for this? I'd rather not pile onto any existing issues if possible here, and if there's nothing actionable to be done about seeing the child dependencies, I'd be fine with not showing them. If we get feedback about this later, we could revisit.

Does it make sense to show both the Microsoft.WindowsDesktop.App SDK and Framework?

I think showing Framework is sufficient here, since calling it an SDK is a bit of a misnomer (yes, this is a part of the literal SDK attribute on the project file).

I think there was a desire to be rid of SDK as a note altogether, since all it contained was items that were already shown elsewhere, and its contents were really just shared frameworks. Seems like we could put 'em both under Frameworks.

Should the NuGet package used to deliver the framework be hidden, as is done for SDKs today?

I think so, yes.

Is there a better icon for framework references?

Perhaps. But we can iterate on that and chat with some UX folks separately 🙂

[drewnoakes]

@cartermp

Is there a notable performance implication for [showing child dependencies]?

I'm looking into this. I think it'd be useful and educational to see what's inside. Unless there's a strong reason not to, I'd like to allow this. Though it's lower priority and can slip beyond 16.3 if need be.

I think showing Framework is sufficient here

I'll do this.

I think there was a desire to be rid of SDK as a note altogether, since all it contained was items that were already shown elsewhere, and its contents were really just shared frameworks.

@davkean pointed out that "Extension SDKs" can appear in this node, and so we cannot lose the node altogether. We can, however, look to remove MSBuild SDKs from it, though this will require changes within the SDK itself to stop reporting them, as I don't think the project system can differentiate between the different kinds of SDK in any way. @davkean please correct me if I'm mistaken here.

[drewnoakes]

Framework dependencies are now nested underneath, and the NuGet package is now hidden:

[cartermp]

Gotcha, makes sense to show Extension SDKs if we have them. I think the desire I was referring to was for first-party "SDKs" (that were really shared framework references).

[dsplaisted]

As @nguerrera has mentioned, it looks like you're using a very old version of the .NET Core SDK. Current 3.0 previews shouldn't have any PackageReference items for the WindowsDesktop FrameworkReference. Targeting and runtime packs are downloaded via PackageDownload items, which should not show up in the tree. Currently, we do still have a PackageReference for Microsoft.NETCore.App if you have any other PackageReference items, but that will go away as part of dotnet/sdk#3325.

Do framework references expand to show child dependencies?

I think it would be a good idea if they expand to show the reference assemblies (mscorlib, System, System.Core, PresentationFramework, etc) coming from the targeting pack. Here's an example of the metadata currently applied to the references resolved from targeting packs:

C:\Program Files\dotnet\packs\Microsoft.WindowsDesktop.App.Ref\3.0.0-preview7-27817-03\ref\netcoreapp3.0\PresentationFramework.dll
    AssemblyVersion = 4.0.0.0
    ExternallyResolved = true
    FileVersion = 4.800.19.31411
    FrameworkName = Microsoft.WindowsDesktop.App.WPF
    FrameworkVersion = 3.0.0-preview7-27817-03
    IsSystemReference = true
    NuGetPackageId = Microsoft.WindowsDesktop.App.Ref
    NuGetPackageVersion = 3.0.0-preview7-27817-03
    Private = false
    PublicKeyToken = 31bf3856ad364e35
    ReferenceGrouping = Microsoft.WindowsDesktop.App.WPF
    ReferenceGroupingDisplayName = Microsoft.WindowsDesktop.App.WPF
    ResolvedFrom = TargetingPack
    Visible = false
    WinMDFile = false

These FrameworkName or ReferenceGrouping metadata could be used to determine which shared framework to put these under. We can also update the metadata if something else is needed.

Should the NuGet package used to deliver the framework be hidden, as is done for SDKs today?

As mentioned, I don't think you should need to do anything special to hide this with the latest SDKs.

Does it make sense to show both the Microsoft.WindowsDesktop.App SDK and Framework?

No, I think we should not have anything in the SDKs node for .NET Core 3.0 projects. Personally, I think it would be ideal that for prior versions, we would show the implicit PackageReference items under the Frameworks node instead of under SDKs.

Is there a better icon for framework references?

No opinion from me here.

Can we use evaluated FrameworkReference items in PackageRestoreConfiguredDataSource?
Should we create a new ResolvedFrameworkReference rather than re-using CollectedFrameworkReference?

I don't know about the details of these.

We also haven't accounted for the profiles of the WindowsDesktop shared framework. The following are valid FrameworkReferences:

• Microsoft.WindowsDesktop.App.WPF - Only references assemblies needed for WPF
• Microsoft.WindowsDesktop.App.WindowsForms - Only references assemblies needed for Windows Forms
• Microsoft.WindowsDesktop.App - References both Windows Forms and WPF assemblies, as well as the integration DLL

Generally, which of these is referenced will depend on whether the UseWPF and/or UseWindowsForms properties are set to true in the project. However, these references flow transitively, so if you are getting the FrameworkReference items via a design time build (which runs AddTransitiveFrameworkReferences), you may have more than one of these FrameworkReferences in the same project.

It might be nice to show subnodes for WPF and WindowsForms under the Microsoft.WindowsDesktop.App node. On the other hand, it may be a lot simpler if you just get the FrameworkReferences from evaluation, in which case you would normally not have multiple WindowsDesktop FrameworkReferences, and you can just show the full name including WPF or WindowsForms under the Frameworks node.

[drewnoakes]

using a very old version of the .NET Core SDK

Indeed. My PC just arrived from the UK and I didn't realise how old the SDK was on it. Updating gives better results.

For project:

<Project Sdk="Microsoft.NET.Sdk.WindowsDesktop">
  <PropertyGroup>
    <OutputType>WinExe</OutputType>
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <UseWPF>true</UseWPF>
  </PropertyGroup>
</Project>

The whole tree looks like:

The tree doesn't currently change based upon combinations of UseWPF and UseWindowsForms.

These FrameworkName or ReferenceGrouping metadata could be used to determine which shared framework to put these under

That sounds good, though in the example you show the framework is Microsoft.WindowsDesktop.App.WPF not Microsoft.WindowsDesktop.App. I'll be looking into this today but at first glance it seems like it might be an issue.

...if you are getting the FrameworkReference items via a design time build

The intention is to populate all top-level items via evaluation, and disallow adding top-level items in design time builds.

What additional data would you expect to be provided by a design time build about a FrameworkReference? Currently I assume:

• Contained assemblies (via the mechanism described above)
• Unresolved state (by absence from design time build items)

...if you just get the FrameworkReferences from evaluation, in which case you would normally not have multiple WindowsDesktop FrameworkReferences, and you can just show the full name including WPF or WindowsForms under the Frameworks node.

How would this work exactly? The evaluated item will be Microsoft.WindowsDesktop.App. How should the dependencies node know to change its display name? Is there a general mechanism in the design for this, or would it involve hard coding special knowledge of the frameworks into the project system?

[davkean]

I think there was a desire to be rid of SDK as a note altogether, since all it contained was items that were already shown elsewhere, and its contents were really just shared frameworks.

We can, however, look to remove MSBuild SDKs from it,

We're mixing concepts here, MSBuild SDKs are a completely different concept. We don't show them anywhere or do anything with them. You are referring to what I'm going to call "NuGet-based SDKs". We should not touch these at all, we do not show items that they represent elsewhere, the contents are only shown under SDKs. Let's treat this as a very separate concept to lighting up FrameworkReference.

[drewnoakes]

This is now ready for review.

[davidwengier]

Looks like there is an erroneous file here: https://github.com/drewnoakes/project-system/blob/framework-references/new

Also TIL GitHub doesn't allow file level comments

[davidwengier]

[drewnoakes]

Looks like there is an erroneous file here

Fixed.

[davkean]

We're expecting both of these properties (IsImplicitlyDefined/PrivateAssets) to be copied to the ResolvedFrameworkReference, but they aren't currently: https://github.com/dotnet/sdk/pull/3355/files#r297966595.

[drewnoakes]

dotnet/sdk#3355 was updated to include IsImplicitlyDefined but not PrivateAssets. It seems like your conversation with @dsplaisted was unfinished in this regard, and I'm not sure what the better path forward here is. I'll make a note on #4762 to track this.

[cartermp]

🎉