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

Move some of the require props into Core and fix #6529 #6767

Merged
merged 1 commit into from
May 5, 2022
Merged

Conversation

mattleibow
Copy link
Member

Description of Change

Previously we just assumed that when the project was a maui project (that always brings in WASDK) and the TFM is windows (always the WASDK) that we can use things. However, the app will lie to us and ALWAYS tell us we are a windows library - even if we are net6.0.

This PR fixes some things, but mainly makes sure that when it is importing the WinUI workarounds it is also referencing WASDK. So, when the app lies to us, we can see the truth!

Issues Fixed

Fixes #6529

@mattleibow mattleibow self-assigned this May 3, 2022
@mattleibow mattleibow changed the title Move some of the require props into Core Move some of the require props into Core and fix #6529 May 3, 2022
As a result, if you have a net6.0 class library, the app will call MSBuild on that library - with the Windows TFM!
This results in the $(TargetPlatformIdentifier) condition being met - even though there are no WASK targets to run!
-->
<Import Project="WinUI.targets" Condition=" '$(TargetPlatformIdentifier)' == 'windows' and '$(WindowsAppSDKWinUI)' == 'true'" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Make sure that when this target is imported it is BOTH a windows project AND it is referencing the WASDK nuget. If MSBuild is run with a TargetFramework of windows, the first condition will become true, but the second one will not because the real project will not have referenced the WASDK nuget.

And this is fine because all the targets in the WinUI.targets are workarounds for windows AND WASDK nuget together.

Comment on lines +8 to +13
<PropertyGroup Condition=" '$([MSBuild]::GetTargetPlatformIdentifier($(TargetFramework)))' == 'windows' ">
<EnableMsixTooling Condition=" '$(EnableMsixTooling)' == '' ">true</EnableMsixTooling>
<EnablePreviewMsixTooling Condition=" '$(EnablePreviewMsixTooling)' == '' ">$(EnableMsixTooling)</EnablePreviewMsixTooling>
<GenerateLibraryLayout Condition=" '$(GenerateLibraryLayout)' == '' and '$(EnableMsixTooling)' == 'true' and '$(OutputType)' != 'Exe' and '$(OutputType)' != 'WinExe' ">true</GenerateLibraryLayout>
<WinUISDKReferences Condition=" '$(WinUISDKReferences)' == '' and '$(EnableMsixTooling)' == 'true' ">false</WinUISDKReferences>
</PropertyGroup>
Copy link
Member Author

@mattleibow mattleibow May 3, 2022

Choose a reason for hiding this comment

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

Moved these properties to here to reduce the required properties in class libraries that use maui but do not set single project and target windows:

  • we mainly require EnableMsixTooling for dotnet build, but it also fixes some bugs
  • GenerateLibraryLayout is required for all libraries... not sure why it is not set
  • WinUISDKReferences is a compatibility feature and should be avoided in new projects

@mattleibow mattleibow added area-single-project Splash Screen, Multi-Targeting, MauiFont, MauiImage, MauiAsset, Resizetizer platform/windows 🪟 p/0 Work that we can't release without labels May 3, 2022
@mattleibow mattleibow added this to the 6.0.300 milestone May 3, 2022
<WindowsPackageType Condition=" '$(WindowsPackageType)' == '' and '$(EnableMsixTooling)' == 'true' and ('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe') ">MSIX</WindowsPackageType>
<WinUISDKReferences Condition=" '$(WinUISDKReferences)' == '' and '$(EnableMsixTooling)' == 'true' and ('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe') ">false</WinUISDKReferences>
<GenerateLibraryLayout Condition=" '$(GenerateLibraryLayout)' == '' and '$(EnableMsixTooling)' == 'true' and '$(OutputType)' != 'Exe' and '$(OutputType)' != 'WinExe' ">true</GenerateLibraryLayout>
<WindowsAppSdkBootstrapInitialize Condition=" '$(WindowsAppSdkBootstrapInitialize)' == '' and '$(EnableMsixTooling)' == 'true' and '$(OutputType)' != 'Exe' and '$(OutputType)' != 'WinExe' ">false</WindowsAppSdkBootstrapInitialize>
Copy link
Member Author

@mattleibow mattleibow May 3, 2022

Choose a reason for hiding this comment

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

This might also need to be moved as a result of microsoft/WindowsAppSDK#2456 - along with WindowsPackageType

@Redth Redth added p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint and removed p/0 Work that we can't release without labels May 3, 2022
@mattleibow mattleibow merged commit 306a7fc into main May 5, 2022
@mattleibow mattleibow deleted the dev/winui branch May 5, 2022 07:07
mattleibow added a commit that referenced this pull request May 5, 2022
As part of #6767, I did not restart VS enough times so some changes did not apply in the IDE.
mattleibow added a commit that referenced this pull request May 5, 2022
As part of #6767, I did not restart VS enough times so some changes did not apply in the IDE.
@pauldendulk
Copy link

The rd.3 release tag is set 12 days ago:
https://github.com/dotnet/maui/tags
While the MR was merged 10 days ago.
#6767

So perhaps the conclusion is that the fix is just not part of the release.

What was confusing for my is that the actual release of rc.3 is 5 days ago. I assumed it was released from master on that same day but apparently it was released 7 days before. Not sure what happens in those 7 days.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@samhouts samhouts added the fixed-in-6.0.312 Look for this fix in 6.0.312! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-single-project Splash Screen, Multi-Targeting, MauiFont, MauiImage, MauiAsset, Resizetizer fixed-in-6.0.312 Look for this fix in 6.0.312! p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/windows 🪟
Projects
None yet
Development

Successfully merging this pull request may close these issues.

With UseMaui = true in class library I get a '"CopyLocalFilesOutputGroup" does not exist' error
4 participants