Skip to content
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

Jump to bottom

Remove .NETCore <3.0 as SupportedTargetFrameworks for WinForms and WPF #1567

Closed
vatsan-madhavan opened this issue Aug 8, 2019 · 2 comments
Closed

Remove .NETCore <3.0 as SupportedTargetFrameworks for WinForms and WPF #1567

vatsan-madhavan opened this issue Aug 8, 2019 · 2 comments
Assignees
@vatsan-madhavan
Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions tell-mode Issues and PR's that require notice to .NET Core Shiproom
Milestone
3.0

Comments

@vatsan-madhavan
Copy link
Member

vatsan-madhavan commented Aug 8, 2019 •
edited
Loading

This issue tracks WPF's portion of the work.

--

@etbyrd commented on Thu Jul 18 2019

The Project System needs a way to know the correct SupportedTargetFrameworks for WinForms and WPF projects so we can display the correct TFM's in the property pages.

Is it possible that this can be done on the SDK side?

dotnet/project-system#5010

cc @nguerrera

@nguerrera commented on Thu Jul 18 2019

cc @vatsan-madhavan @rladuca

@nguerrera commented on Thu Jul 18 2019

cc @dsplaisted

@nguerrera commented on Thu Jul 18 2019

Today we have a static list of supported TFMs here:

https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.SupportedTargetFrameworks.props

What I'm thinking is that the WindowsDesktop SDK could override this somehow to remove the target frameworks that it doesn't support.

Simplest form of that would be to do some Removes but then I think we want no versions of .NET Standard to be supported, and then we'd have a maintenance problem when say .NETStandard 2.2 came out and wasn't yet removed.

Another way would be for WindowsDesktop SDK to remove all and have its own fixed allow-list. But then it is in the business of taking an update for every version.

Finally, we could make the list above dynamic based on whether the WindowsDesktop SDK has set certain props.

What do you all think would be best?

@vatsan-madhavan commented on Thu Jul 18 2019

I don't understand the request for the SDK.

We already have logic for handling TFM < 3.0 && TargetFrameworkIdentifier==.NETCoreApp. What specifically is being requested here?

See _WindowsDesktopFrameworkRequiresVersion30 - currently there is a warning shown when someone tries to use WindowsDesktop SDK and sets TFM < netcoreapp3.0

@vatsan-madhavan commented on Thu Jul 18 2019

Today we have a static list of supported TFMs here:
https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.SupportedTargetFrameworks.props
Simplest form of that would be to do some Removes but then I think we want no versions of .NET Standard to be supported, and then we'd have a maintenance problem when say .NETStandard 2.2 came out and wasn't yet removed.

Spoke too soon.. thanks for the context.

@nguerrera commented on Thu Jul 18 2019

The request is that the list of SupportedTargetFramework: https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.SupportedTargetFrameworks.props

not include target frameworks that aren't supported by windows desktop projects. VS uses these items to populate a retargeting drop-down in property pages.

@vatsan-madhavan commented on Thu Jul 18 2019

Remove-ing these is easy, and doesn't pose maintainability problems.

<SupportedTargetFramework Include=".NETCoreApp,Version=v1.0" DisplayName=".NET Core 1.0" />
<SupportedTargetFramework Include=".NETCoreApp,Version=v1.1" DisplayName=".NET Core 1.1" />
<SupportedTargetFramework Include=".NETCoreApp,Version=v2.0" DisplayName=".NET Core 2.0" />
<SupportedTargetFramework Include=".NETCoreApp,Version=v2.1" DisplayName=".NET Core 2.1" />
<SupportedTargetFramework Include=".NETCoreApp,Version=v2.2" DisplayName=".NET Core 2.2" />

I would expect 3.0+ .NETCoreApp TFM's to be supported.

If we don't want any version of .NETStandard to be supported, is there a way we can Remove them all without having to run a Target? That would probably be the easiest thing to do... For e.g., if we could coordinate a change with dotnet/sdk and have it define @(_NetStandardSupportedTargets) in Microsoft.NET.SupportedTargetFrameworks.props, we could probably use that and easily eliminate .NET Standard TFM's.

Thoughts?

@vatsan-madhavan commented on Thu Jul 18 2019

How come this list doesn't include .NETFramework items? Is that because SDK-style projects "work with, but don't support" .NET Framework?

@nguerrera commented on Thu Jul 18 2019

Good question about .NET Framework. I suppose that is why. @davkean?

@nguerrera commented on Thu Jul 18 2019

I think I like where you were going we could have this:

<SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v1.0" DisplayName=".NET Core 1.0" />
<SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v1.1" DisplayName=".NET Core 1.1" />
<SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v2.0" DisplayName=".NET Core 2.0" />
<!-- etc. -->

<SupportedNETStandardTargetFramework Include=".NETStandard,Version=1.0" DisplayName=".NET Standard 1.0" />
<! -- etc. -->

<SupportedTargetFramework Include="@(SupportedNETCoreTargetFramework);@(SupportedNETStandardTargetFramework)" />

Another idea would be to set the metadata with the TargetFrameworkIdentifier and Version separated. I think this would let you do the removes with conditions that match all .NET Standard, though I have to check what is possible in evaluation here. We can't use a target for any of this.

@vatsan-madhavan commented on Thu Jul 18 2019

I don't think metadata will work - though I wish that it did.

   <!-- Microsoft.NET.Build.Tasks/targets/Microsoft.NET.SupportedTargetFrameworks.props --> 

    <!-- .NET Standard -->
    <ItemGroup>
        <SupportedTargetFramework Include=".NETStandard,Version=v1.0" DisplayName=".NET Standard 1.0" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v1.1" DisplayName=".NET Standard 1.1" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v1.2" DisplayName=".NET Standard 1.2" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v1.3" DisplayName=".NET Standard 1.3" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v1.4" DisplayName=".NET Standard 1.4" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v1.5" DisplayName=".NET Standard 1.5" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v1.6" DisplayName=".NET Standard 1.6" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v2.0" DisplayName=".NET Standard 2.0" TargetFrameworkIdentifier=".NETStandard" />
        <SupportedTargetFramework Include=".NETStandard,Version=v2.1" DisplayName=".NET Standard 2.1" TargetFrameworkIdentifier=".NETStandard" />
    </ItemGroup>
<!-- WindowsDesktop SDK props -->
  <ItemGroup>
    <SupportedTargetFramework Remove="@(SupportedTargetFramework0)" Condition="'%(TargetFrameworkIdentifier)'=='.NETStandard'" />
  </ItemGroup>
error MSB4191: The reference to custom metadata "TargetFrameworkIdentifier" at position 1 is not allowed in this condition "'%(SupportedTargetFramework.TargetFrameworkIdentifier)'=='.NETStandard'"

@vatsan-madhavan commented on Thu Jul 18 2019

We should probably do this, IMO, like you described above:

Microsoft.NET.Build.Tasks/targets/Microsoft.NET.SupportedTargetFrameworks.props

    <!-- .NET Core -->
    <ItemGroup>
        <SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v1.0" DisplayName=".NET Core 1.0" />
        <SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v1.1" DisplayName=".NET Core 1.1" />
        <SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v2.0" DisplayName=".NET Core 2.0" />
        <SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v2.1" DisplayName=".NET Core 2.1" />
        <SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v2.2" DisplayName=".NET Core 2.2" />
        <SupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v3.0" DisplayName=".NET Core 3.0" />
    </ItemGroup>

    <!-- .NET Standard -->
    <ItemGroup>
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v1.0" DisplayName=".NET Standard 1.0" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v1.1" DisplayName=".NET Standard 1.1" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v1.2" DisplayName=".NET Standard 1.2" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v1.3" DisplayName=".NET Standard 1.3" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v1.4" DisplayName=".NET Standard 1.4" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v1.5" DisplayName=".NET Standard 1.5" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v1.6" DisplayName=".NET Standard 1.6" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v2.0" DisplayName=".NET Standard 2.0" />
        <SupportedNETtandardTargetFramework Include=".NETStandard,Version=v2.1" DisplayName=".NET Standard 2.1" />
    </ItemGroup>


    <ItemGroup>
        <SupportedTargetFramework Include="@(SupportedNETCoreAppTargetFramework);@(SupportedNETtandardTargetFramework)" />
    </ItemGroup>

Microsoft.NET.Sdk.WindowsDesktop.props

  <ItemGroup>

    <SupportedNETCoreAppTargetFramework Remove=".NETCoreApp,Version=v1.0" />
    <SupportedNETCoreAppTargetFramework Remove=".NETCoreApp,Version=v1.1" />
    <SupportedNETCoreAppTargetFramework Remove=".NETCoreApp,Version=v2.0" />
    <SupportedNETCoreAppTargetFramework Remove=".NETCoreApp,Version=v2.1" />
    <SupportedNETCoreAppTargetFramework Remove=".NETCoreApp,Version=v2.2" />
    
    <SupportedTargetFramework0 Remove="@(SupportedNETtandardTargetFramework)" />
  </ItemGroup>

@nguerrera commented on Thu Jul 18 2019

That works for me.

(I assume the 0 in SupportedTargetFramework0 is a typo.)

@vatsan-madhavan commented on Thu Jul 18 2019

That works for me.

(I assume the 0 in SupportedTargetFramework0 is a typo.)

yeah, typo ...

@nguerrera commented on Thu Jul 18 2019

@davkean Any concerns here? Shall we go ahead?

@vatsan-madhavan commented on Thu Jul 18 2019

Do we need a separate issue to puzzle out whether to add .NETFramework to SupportedTargetFramework items in Microsoft.NET.Build.Tasks/targets/Microsoft.NET.SupportedTargetFrameworks.props ?

Unless we have documented it as unsupported, I'm starting to think that it ought to be supported.
The very first example at How to: Use MSBuild project SDKs shows how to target .NET 4.6 using SDK style project. Additions to the csproj format for .NET Core also describes how to leverage it for .NET Framework. These seem to suggest strongly that the SDK style projects are both supported and encouraged for use by .NET Framework projects.

@nguerrera commented on Thu Jul 18 2019

I assume the project system doesn't use this data for .NET Framework and instead looks in a more classical place for versions of .NET Framework to offer. I'm fine adding it, but if it's not going to be used, it doesn't seem to be high priority.

@vatsan-madhavan commented on Thu Jul 18 2019

I assume the project system doesn't use this data for .NET Framework and instead looks in a more classical place for versions of .NET Framework to offer.

If the project system doesn't use it, then I agree that it doesn't matter. We should probably add a comment that .NET Framework is missing because the project system doesn't use it...

@nguerrera commented on Thu Jul 18 2019

.NET Framework is supported by us. In fact, ASP.NET Core 1.x, 2.x projects could even target .NET Framework from File -> New Project.

@nguerrera commented on Thu Jul 18 2019

It already shows the full list:

image

It is quite possible that the oldest ones in that list don't work so maybe we should get them out of there?

@vatsan-madhavan commented on Thu Jul 18 2019

It is quite possible that the oldest ones in that list don't work so maybe we should get them out of there?

If the project system used SupportedTargetFrameworks, this would be so easy ;-)

@davkean commented on Wed Jul 24 2019

We can use it for desktop, assuming you won't return < .NET Framework 3.0 for WPF/WInForms? It would be great if you return all of the TFMs from all frameworks so that we can switch between .NET Framework -> .NET Core as we've had quite a few bugs on this.

@nguerrera commented on Wed Jul 24 2019

@davkean OK, we can add supported .NET Framework TFMs with the same pattern and WindowsDesktop can remove accordingly there too.

So you're good with this plan?

@davkean commented on Wed Jul 24 2019

Yep, does this overlap with the existing infrastructure we already use to retrieve the frameworks?

@vatsan-madhavan commented on Wed Jul 24 2019

We can use it for desktop, assuming you won't return < .NET Framework 3.0 for WPF/WInForms?

Why not WinForms?

@davkean commented on Wed Jul 24 2019

WinForms is supported on .NET Framework 2.0.

@vatsan-madhavan commented on Wed Jul 24 2019

Ah, got it. We have the right rules in place for WinForms for that in WindowsDesktop SDK.

@nguerrera We should see if we can do this correctly for WindowsDesktop.WindowsForms (netfx 2.0+) vs. WindowsDesktop.WPF vs. WindowsDesktop (same as WPF).

@davkean commented on Mon Aug 05 2019

What's the state of this? Is this ready to be consumed?

@nguerrera commented on Mon Aug 05 2019

Not yet. We have approval to go ahead. I'm a bit slammed on more critical issues. @livarcocc can you possibly see if someone else can pick this up?

@peterhuene commented on Wed Aug 07 2019

I'll go ahead with implementing this in the SDK.

@peterhuene commented on Wed Aug 07 2019

I assume WindowsDesktop would want this in their props (for completeness):

<ItemGroup>
    <_UnsupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v1.0" />
    <_UnsupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v1.1" />
    <_UnsupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v2.0" />
    <_UnsupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v2.1" />
    <_UnsupportedNETCoreAppTargetFramework Include=".NETCoreApp,Version=v2.2" />

    <_UnsupportedNETStandardTargetFramework Include="@(SupportedNETStandardTargetFramework)" />
    
    <SupportedNETCoreAppTargetFramework Remove="@(_UnsupportedNETCoreAppTargetFramework)" />
    <SupportedNETStandardTargetFramework Remove="@(_UnsupportedNETStandardTargetFramework)" />
    <SupportedTargetFramework Remove="@(_UnsupportedNETCoreAppTargetFramework);@(_UnsupportedNETStandardTargetFramework)" />
</ItemGroup>

@peterhuene commented on Wed Aug 07 2019

I've created PR #3516 for the SDK side of the changes.

@vatsan-madhavan commented on Wed Aug 07 2019

Nice!

We have to obviously wait until an SDK containing this change flows into dotnet/wpf - which can be a few hours or a few days...

@peterhuene commented on Wed Aug 07 2019

I've verified my suggestion for the Windows Desktop props above with VS (16.3 preview). With both the SDK change and that one, Visual Studio only shows netcoreapp3.0 in the drop down.

@nguerrera commented on Wed Aug 07 2019

There was some discussion of adding supported .NET Framework to this above. But given that VS doesn't use that today, I think it is fine to do later. Should we open a separate issue to track that?

@peterhuene commented on Wed Aug 07 2019

Ah I missed that part. I can add that to this change now rather than deferring that work.

Do we have a list of the expected supported .NET Framework TFs?

@nguerrera commented on Wed Aug 07 2019

I don't know. I'm pretty sure older ones and client platform don't work. @dsplaisted Do we have a list?

@nguerrera commented on Wed Aug 07 2019

I'm guessing we should use the same set as what was used for the full framework targeting packs as nupkgs? Where is that?

@vatsan-madhavan commented on Wed Aug 07 2019

For WPF and WinForms, the .NET Framework support list can be found indirectly at

wpf/packaging/Microsoft.NET.Sdk.WindowsDesktop/targets/Microsoft.NET.Sdk.WindowsDesktop.props

Line 77 in 8a23402

<!--
.

In short,

  • WPF: .NET Framework 3.0+
  • WinForms: .NET Framework 3.0 +

The exact reference set varies from net30, net35 to net40, net45 etc. , but they are all supported all the way to net48.

  • I can build a WPF and WinForms app in Visual Studio for any of the following targets:
    • net30, net35, net40, net45, net46, net472, net48
    • edit: the ones I haven't listed here in-between these values would also surely work.
  • I can build the same projects using dotnet for TFM net40+

There are corner cases that need some special handling. For e.g., if you try to use SplashScreen with net30, you'll get an build-error (unsupported), but those are small issues.
@vatsan-madhavan vatsan-madhavan added the Enhancement Requested Product code improvement that does NOT require public API changes/additions label Aug 8, 2019
@vatsan-madhavan vatsan-madhavan added this to the 3.0 milestone Aug 8, 2019
vatsan-madhavan added a commit that referenced this issue Aug 8, 2019
@vatsan-madhavan
- Addresses #1567
733a460 
- Completes work started in dotnet/sdk#3438

Removes unsupported TFM's from SupportedTargetFramework.
@vatsan-madhavan vatsan-madhavan mentioned this issue Aug 8, 2019
Merged
@vatsan-madhavan vatsan-madhavan added the tell-mode Issues and PR's that require notice to .NET Core Shiproom label Aug 9, 2019
vatsan-madhavan added a commit that referenced this issue Aug 9, 2019
@vatsan-madhavan
Remove unsupported TFM's from SupportedTargetFramework. (#1578)
52484fe 
* - Addresses #1567
- Completes work started in dotnet/sdk#3438

Removes unsupported TFM's from SupportedTargetFramework.

* Add netfx exclusions

* Update comment describing supported version of WPF and WinForms (for netfx) better.
@vatsan-madhavan
Copy link
Member Author

@rladuca @grubioe this can be closed after tell-mode reviews.

@grubioe
Copy link
Contributor

grubioe commented Aug 13, 2019

Reviewed during 8/13 .NET Tactics

@grubioe grubioe closed this as completed Aug 13, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Apr 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@vatsan-madhavan vatsan-madhavan

Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions tell-mode Issues and PR's that require notice to .NET Core Shiproom
Projects
None yet
Milestone
3.0
Development

No branches or pull requests
2 participants
@vatsan-madhavan @grubioe