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.

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

Adjust properties to allow muti-target #2869

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,40 +1,6 @@
<Project>
<PropertyGroup>
Copy link
Author

Choose a reason for hiding this comment

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

Moved to target

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of moving the code, just add import into WinFX targets, if they are not already imported.

OR Is that pattern not preferred?

<_MicrosoftNetSdkWindowsDesktop>true</_MicrosoftNetSdkWindowsDesktop>
<EnableDefaultPageItems Condition="'$(EnableDefaultPageItems)' == ''">true</EnableDefaultPageItems>
<EnableDefaultApplicationDefinition Condition="'$(EnableDefaultApplicationDefinition)' == ''">true</EnableDefaultApplicationDefinition>
</PropertyGroup>

<PropertyGroup>
<!--
WindowsDesktop SDK supports WPF and WinForms on
- .NET Core 3.0 and greater
- .NET Framework 3.0 and greater

Note that on .NET Framework versions < 4.0, additional workarounds may be required to build applications
using the SDK style projects. For e.g., see https://github.com/microsoft/msbuild/issues/1333

Irrespective of whether '$(TargetFrameworkIdentifier)' is '.NETCoreApp' or '.NETFramework',
the minimum value of $(_TargetFrameworkVersionValue) we will be testing for is '3.0'

-->
<_WindowsDesktopSdkTargetFrameworkVersionFloor>3.0</_WindowsDesktopSdkTargetFrameworkVersionFloor>

<!--
Represents an undefined TFV value. This will be used in comparisons of _TargetFrameworkVersionValue (defined in Microsoft.NET.WindowsDesktop.targets)
to identify when a TFV is undefined
-->
<_UndefinedTargetFrameworkVersion>0.0</_UndefinedTargetFrameworkVersion>

<!--
Initial/Default value set to 'undefined'. Updated in Microsoft.NET.WindowsDesktop.targets
-->
<_TargetFrameworkVersionValue>$(_UndefinedTargetFrameworkVersion)</_TargetFrameworkVersionValue>
</PropertyGroup>

<ItemGroup Condition=" ('$(EnableDefaultItems)' == 'true') And ('$(UseWPF)' == 'true') And
('$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)') And
('$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)')">
<ItemGroup Condition=" '$(_EnableWindowsDesktopGlobbing)' == 'true' ">
Copy link
Author

Choose a reason for hiding this comment

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

Extract a new property in target to reduce the surface area, since props will always be imported

<ApplicationDefinition Include="App.xaml"
Condition="'$(EnableDefaultApplicationDefinition)' != 'false' And Exists('$(MSBuildProjectDirectory)/App.xaml') And '$(MSBuildProjectExtension)' == '.csproj'">
<Generator>MSBuild:Compile</Generator>
Expand Down Expand Up @@ -66,10 +32,7 @@
</ItemGroup>


<ItemGroup Condition="('$(DisableImplicitFrameworkReferences)' != 'true') And
('$(TargetFrameworkIdentifier)' == '.NETCoreApp') And
('$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)') And
('$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)')">
<ItemGroup Condition=" '$(_EnableWindowsDesktopNetCoreFrameworkReferences)' == 'true' ">

<FrameworkReference Include="Microsoft.WindowsDesktop.App" IsImplicitlyDefined="true"
Condition="('$(UseWPF)' == 'true') And ('$(UseWindowsForms)' == 'true')"/>
Expand Down Expand Up @@ -103,9 +66,7 @@
System.Windows.Controls.Ribbon

-->
<ItemGroup Condition="('$(TargetFrameworkIdentifier)' == '.NETFramework') And
('$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)') And
('$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)')">
<ItemGroup Condition=" '$(_EnableWindowsDesktopNETFrameworkImplicitReference)' == 'true' ">

<!--
The following 3 _WpfCommonNetFxReference items normally require Condition="'$(_TargetFrameworkVersionValue)' >= '3.0'", since
Expand All @@ -129,10 +90,7 @@
<_WpfCommonNetFxReference Include="System.Windows.Controls.Ribbon" Condition="'$(_TargetFrameworkVersionValue)' >= '4.5'" />
</ItemGroup>

<ItemGroup Condition="('$(DisableImplicitFrameworkReferences)' != 'true') And
('$(TargetFrameworkIdentifier)' == '.NETFramework') And
('$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)') And
('$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)')">
<ItemGroup Condition=" '$(_EnableWindowsDesktopNETFrameworkImplicitReference)' == 'true' ">

<_SDKImplicitReference Include="@(_WpfCommonNetFxReference)"
Condition="'$(UseWPF)' == 'true'"/>
Expand Down Expand Up @@ -204,6 +162,4 @@

<SupportedTargetFramework Remove="@(_UnsupportedNETCoreAppTargetFramework);@(_UnsupportedNETStandardTargetFramework);@(_UnsupportedNETFrameworkTargetFramework)" />
</ItemGroup>

<Import Project="Microsoft.WinFX.props" />
</Project>
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
<Project>


<PropertyGroup>
<_MicrosoftNetSdkWindowsDesktop>true</_MicrosoftNetSdkWindowsDesktop>
<EnableDefaultPageItems Condition="'$(EnableDefaultPageItems)' == ''">true</EnableDefaultPageItems>
<EnableDefaultApplicationDefinition Condition="'$(EnableDefaultApplicationDefinition)' == ''">true</EnableDefaultApplicationDefinition>
</PropertyGroup>

<PropertyGroup>
<!--
WindowsDesktop SDK supports WPF and WinForms on
- .NET Core 3.0 and greater
- .NET Framework 3.0 and greater

Note that on .NET Framework versions < 4.0, additional workarounds may be required to build applications
using the SDK style projects. For e.g., see https://github.com/microsoft/msbuild/issues/1333

Irrespective of whether '$(TargetFrameworkIdentifier)' is '.NETCoreApp' or '.NETFramework',
the minimum value of $(_TargetFrameworkVersionValue) we will be testing for is '3.0'

-->
<_WindowsDesktopSdkTargetFrameworkVersionFloor>3.0</_WindowsDesktopSdkTargetFrameworkVersionFloor>

<!--
Represents an undefined TFV value. This will be used in comparisons of _TargetFrameworkVersionValue (defined in Microsoft.NET.WindowsDesktop.targets)
to identify when a TFV is undefined
-->
<_UndefinedTargetFrameworkVersion>0.0</_UndefinedTargetFrameworkVersion>
</PropertyGroup>


<!--
$(TargetFrameworkVersion), $(_TargetFrameworkVersionWithoutV) etc. are defined in
Microsoft.NET.Sdk\targets\Microsoft.NET.TargetFrameworkInference.targets.
Expand All @@ -22,6 +51,25 @@
<PropertyGroup>
<_TargetFrameworkVersionValue>$([MSBuild]::ValueOrDefault('$(_TargetFrameworkVersionWithoutV)', '$(_UndefinedTargetFrameworkVersion)'))</_TargetFrameworkVersionValue>
</PropertyGroup>

<!-- PropertyGroup flags control if Itemgroups in Microsoft.NET.Sdk.WindowsDesktop.props file is being imported -->
<PropertyGroup>
<_EnableWindowsDesktopGlobbing Condition=" ('$(EnableDefaultItems)' == 'true') And ('$(UseWPF)' == 'true') And
Copy link
Author

Choose a reason for hiding this comment

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

Please make sure the name looks good @dsplaisted

('$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)') And
('$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)')">true</_EnableWindowsDesktopGlobbing>
<_EnableWindowsDesktopNetCoreFrameworkReferences Condition="('$(DisableImplicitFrameworkReferences)' != 'true') And
('$(TargetFrameworkIdentifier)' == '.NETCoreApp') And
('$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)') And
('$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)')">true</_EnableWindowsDesktopNetCoreFrameworkReferences>
<_EnableWindowsDesktopNETFrameworkImplicitReference Condition="('$(DisableImplicitFrameworkReferences)' != 'true') And
('$(TargetFrameworkIdentifier)' == '.NETFramework') And
('$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)') And
('$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)')">true</_EnableWindowsDesktopNETFrameworkImplicitReference>
<_EnableWindowsDesktopSupportUnsupportedTargetFramework Condition="('$(UseWPF)' == 'true' Or '$(UseWindowsForms)' == 'true') And
'$(_TargetFrameworkVersionValue)' != '$(_UndefinedTargetFrameworkVersion)' And
'$(_TargetFrameworkVersionValue)' >= '$(_WindowsDesktopSdkTargetFrameworkVersionFloor)'">true</_EnableWindowsDesktopSupportUnsupportedTargetFramework>
</PropertyGroup>


<Import Project="Microsoft.WinFX.targets" />

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,2 @@
<Project>
<PropertyGroup>
Copy link
Author

Choose a reason for hiding this comment

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

Moved to WinFx.target

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of moving the code, just add import into WinFX targets, if they are not already imported.

OR Is that pattern not preferred?

Copy link
Author

Choose a reason for hiding this comment

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

In this case, props will be imported regardless if the workload is installed or not. So we need to move everything that could be moved.

For more see the description, linked issue. As well as the doc related to the net5.0 design.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could keep the logic in the props file and import it in the Sdk targets. No need to move the logic and risk losing history. -- Just a suggestion

<_PresentationBuildTasksTfm Condition="'$(MSBuildRuntimeType)' == 'Core'">netcoreapp2.1</_PresentationBuildTasksTfm>
<_PresentationBuildTasksTfm Condition="'$(MSBuildRuntimeType)' != 'Core'">net472</_PresentationBuildTasksTfm>
<_PresentationBuildTasksAssembly Condition="'$(_PresentationBuildTasksAssembly)'==''">$([MSBuild]::Unescape($([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\tools\$(_PresentationBuildTasksTfm)\PresentationBuildTasks.dll'))))</_PresentationBuildTasksAssembly>
</PropertyGroup>

<UsingTask TaskName="Microsoft.Build.Tasks.Windows.MarkupCompilePass1" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.UidManager" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.ResourcesGenerator" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.FileClassifier" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.MarkupCompilePass2" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.GenerateTemporaryTargetAssembly" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.MergeLocalizationDirectives" AssemblyFile="$(_PresentationBuildTasksAssembly)" />

</Project>
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<PropertyGroup>
<_PresentationBuildTasksTfm Condition="'$(MSBuildRuntimeType)' == 'Core'">netcoreapp2.1</_PresentationBuildTasksTfm>
<_PresentationBuildTasksTfm Condition="'$(MSBuildRuntimeType)' != 'Core'">net472</_PresentationBuildTasksTfm>
<_PresentationBuildTasksAssembly Condition="'$(_PresentationBuildTasksAssembly)'==''">$([MSBuild]::Unescape($([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\tools\$(_PresentationBuildTasksTfm)\PresentationBuildTasks.dll'))))</_PresentationBuildTasksAssembly>
</PropertyGroup>

<UsingTask TaskName="Microsoft.Build.Tasks.Windows.MarkupCompilePass1" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.UidManager" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.ResourcesGenerator" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.FileClassifier" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.MarkupCompilePass2" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.GenerateTemporaryTargetAssembly" AssemblyFile="$(_PresentationBuildTasksAssembly)" />
<UsingTask TaskName="Microsoft.Build.Tasks.Windows.MergeLocalizationDirectives" AssemblyFile="$(_PresentationBuildTasksAssembly)" />

<PropertyGroup>
<AlwaysCompileMarkupFilesInSeparateDomain Condition="'$(BuildingInsideVisualStudio)' == 'true' and '$(AlwaysCompileMarkupFilesInSeparateDomain)' == ''">true</AlwaysCompileMarkupFilesInSeparateDomain>
<AlwaysCompileMarkupFilesInSeparateDomain Condition="'$(AlwaysCompileMarkupFilesInSeparateDomain)' == '' ">true</AlwaysCompileMarkupFilesInSeparateDomain>
Expand Down