dotnet / installer

.NET SDK Installer
https://github.com/dotnet/sdk
1.27k stars 446 forks source link

Composition of SDKs / SDK Extras / Multi-targeting #181

Closed clairernovotny closed 8 months ago

clairernovotny commented 5 years ago

I was looking to see how I can incorporate the MSBuild.Sdk.Extras with the new WindowsDesktop SDK and hit a few challenges. The overall scenario is getting multi-targeting to work, where iOS, UWP, Android, Tizen, are handled by the Extras, while WPF/WinForms is handled by the WindowsDesktop SDK.

Here’s where I hit some issues (numbered for reference):

  1. The Microsoft.NET.Sdk props do not define a base directory or a version. I need a hack to see if WindowsDesktop is available like this: https://github.com/onovotny/MSBuildSdkExtras/blob/master/Source/MSBuild.Sdk.Extras/Sdk/Sdk.props#L16-L17
  2. Once calculated, the WindowsDesktop\targets\props (sorry, no public source, need to refer to files in the sdk dir), has a FrameworkReference for Microsoft.WindowsDesktop.App without a condition on UseWPF/UseWindowsForms. It applies to any .NET Core App, but someone using the extras may not want that desktop app for their .NET Core App target. I need a way to turn that implicit reference off. Can there be a new property it checks (like IsDesktopProject or ProjectType = Desktop) to “light up” that stuff? The other item groups are keyed off of either UseWpf or UseWindowsForms, so they’re okay. I have a temporary workaround here, but this seems hacky. Need to know when to bring it in discreetly: https://github.com/onovotny/MSBuildSdkExtras/blob/master/Source/MSBuild.Sdk.Extras/Sdk/Sdk.props#L29
  3. In WindowsDesktop\targets\targets, The import for the WinFX targets has the same issue as the FrameworkReference. I need a way to conditionally import those (or turn that off unless the ProjectType = Desktop)…something. The other ItemGroup and Target are conditioned on UseWPF, so that’s okay too. I worked around this, for now, by essentially keying off of UseWPF since all the items/targets/imports were tied to it. https://github.com/onovotny/MSBuildSdkExtras/blob/master/Source/MSBuild.Sdk.Extras/Sdk/Sdk.targets#L17. This could be brittle though, especially if other things get added. Essentially need a way to turn pieces on/off as needed (like the WinFX imports).

I think that should cover it? If a couple of conditionals are added, I can have people use the MSBuild.Sdk.Extras as the SDK and pull in the WindowsDestop props/targets. The TFM-specific conditions would add all the items/targets.

What scenarios are covered by the Microsoft.WindowsDesktop.App FrameworkReference where neither UseWpf nor UseWindowsForms would be defined?

--

Overall, I wound up having to bypass the WindowsDesktop SDK itself and go straight to its targets, since I couldn't compose things the way I needed to in the right order. I'm hoping for ideas/suggestions/improvements going forward.

vatsan-madhavan commented 5 years ago

@nguerrera, thoughts on changing these?

In Microsoft.NET.Sdk.WindowsDesktop.props, we have:

  <ItemGroup Condition="'$(DisableImplicitFrameworkReferences)' != 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
    <FrameworkReference Include="Microsoft.WindowsDesktop.App" IsImplicitlyDefined="true" />
  </ItemGroup>

Perhaps this could be condition on either UseWpf Or UseWindowsForms

  <ItemGroup Condition="'$(DisableImplicitFrameworkReferences)' != 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And ('$(UseWpf)'=='true' Or $(UseWindowsForms)=='true')">
    <FrameworkReference Include="Microsoft.WindowsDesktop.App" IsImplicitlyDefined="true" />
  </ItemGroup>

Similarly, in Microsoft.NET.Sdk.WindowsDesktop.targets, we have:

 <Import Project="$(MSBuildToolsPath)\Microsoft.WinFX.targets" 
         Condition ="'$(MSBuildRuntimeType)' != 'Core' and '$(UseLegacyPresentationBuildTasks)' == 'true'"/>

 <Import Project="Microsoft.WinFX.targets" 
         Condition="'$(MSBuildRuntimeType)' == 'Core' or '$(UseLegacyPresentationBuildTasks)' != 'true'" />

This could be conditioned on UseWpf:

 <Import Project="$(MSBuildToolsPath)\Microsoft.WinFX.targets" 
         Condition ="'$(MSBuildRuntimeType)' != 'Core' and '$(UseLegacyPresentationBuildTasks)' == 'true' and '$(UseWpf)'=='true'"/>

 <Import Project="Microsoft.WinFX.targets" 
         Condition="'$(MSBuildRuntimeType)' == 'Core' or '$(UseLegacyPresentationBuildTasks)' != 'true' and '$(UseWpf)'=='true'" />

cc @rladuca

nguerrera commented 5 years ago

I think the conditions proposed make sense.

Aside: I think we can remove UseLegacyPresentationBuildTasks now. I put that there when switching over to new PBT to have an escape hatch if folks were blocked. It wasn't intended to be permanent. I don't think it even works anymore because it has a corresponding hack in the microsoft.net.windows.desktop.app targets that aren't imported after targeting pack work. cc @dsplaisted

davidfowl commented 5 years ago

cc @DamianEdwards

marcpopMSFT commented 8 months ago

Old issue triage: We assume a solution was found for this or people have moved on so closing. We can reactivate if there is still a needed scenario for this.