Match in-tree order of targets with workloads and fix some WinUI issues by mattleibow · Pull Request #5923 · dotnet/maui

mattleibow · 2022-04-08T19:47:02Z

Description of Change

The order of the props/targets in the maui repo solution don't quite match the order of the targets once the workload is installed.

This PR Fixes some things:

Correctly ordering the props/targets in the repo to match the installed workloads
Fixed the AppxManifest inclusion logic:
- Remove the logic that skipped including AppxManifest as this cannot work because the properties that are checked come AFTER this line
- Added logic to remove the AppxManifest when it is about to be validated/used if this is an unpackaged app
Fixed the SDKReference warnings/errors
- This is a UWP feature and due to a ordering issue in the WinUI targets, the check comes before the property to remove it
- If this is a modern WinUI app, we set the WinUISDKReference to false to instruct the WinUI targets to not include them

Details

Also fixes some poor choices in #5896
And Fixes #5881 properly

This PR tries to get even closer to this. Not an exact match as there is no way to get the AutoImport.props in the right place, but it can be simulated by adding it to the end of the .csproj. And then we can use the Directory.Build.targets to simulate after all the things are ready.

It still does not quite match, but here is the orders of things:

Workloads	Maui Repo
Directory.Build.props	Directory.Build.props
.nuget.g.props	.nuget.g.props
AutoImport.props
.csproj	.csproj
	Maui.InTree.props (by .csproj)
	--> AutoImport.InTree.props
Maui.Controls.props	--> Maui.Controls.props
--> Maui.Controls.SingleProject.props	--> --> Maui.Controls.SingleProject.props
.nuget.g.targets	.nuget.g.targets
	Directory.Build.targets (local)
	--> Maui.InTree.targets
Maui.Controls.targets	--> --> Maui.Controls.targets
Maui.Controls.DefaultItems.targets	--> --> Maui.Controls.DefaultItems.targets
--> Maui.Controls.SingleProject.targets	--> --> --> Maui.Controls.SingleProject.targets
Directory.Build.targets	--> Directory.Build.targets (actual)

mattleibow · 2022-04-08T22:30:01Z

.nuspec/AutoImport.InTree.props

+  <ItemDefinitionGroup>
+    <MauiXaml>
+      <SubType>Designer</SubType>
+    </MauiXaml>
+  </ItemDefinitionGroup>
+
+  <ItemGroup Condition="'$(EnableDefaultItems)'=='True' And '$(EnableDefaultXamlItems)'=='True' And '$(EnableDefaultEmbeddedResourceItems)'=='True'">
+    <MauiXaml Include="**\*.xaml" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder);$(DefaultWebContentItemExcludes)"/>
+  </ItemGroup>
+
+  <ItemGroup Condition="'$(EnableDefaultItems)'=='True' And '$(EnableDefaultCssItems)'=='True' And '$(EnableDefaultEmbeddedResourceItems)'=='True'">
+    <MauiCss Include="**\*.css" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder);$(DefaultWebContentItemExcludes)" />
+  </ItemGroup>


Moved this in from DefaultItems.props as that was just a file in the maui repo and was confusing. Also, this chunk of stuff is in the AutoImport.props that the workload uses. This way it is all matching.

mattleibow · 2022-04-08T22:30:51Z

.nuspec/Microsoft.Maui.Controls.SingleProject.props

  <PropertyGroup Condition=" '$(SingleProject)' == 'true' and '$([MSBuild]::GetTargetPlatformIdentifier($(TargetFramework)))' == 'windows' ">
    <OutputType Condition="'$(OutputType)' == 'Exe'">WinExe</OutputType>
    <EnablePreviewMsixTooling Condition="'$(EnablePreviewMsixTooling)' == '' and ('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe')">true</EnablePreviewMsixTooling>
+    <WindowsPackageType Condition=" '$(WindowsPackageType)' == '' and '$(EnablePreviewMsixTooling)' == 'true' and ('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe') ">MSIX</WindowsPackageType>


This needs to be here to come before the nugets. Fixes a bug in #5896

mattleibow · 2022-04-08T22:33:55Z

.nuspec/Microsoft.Maui.Controls.SingleProject.targets

+  <Target Name="_MauiRemoveRemoveAppxManifestForUnpackaged"
+          Condition="'$(WindowsPackageType)' == 'None' and '@(AppxManifest)' != ''">
+
+    <ItemGroup>
+      <AppxManifest Remove="@(AppxManifest)" />
+    </ItemGroup>
+
+  </Target>


The issue that #5896 fixes is when the manifest AND unpackaged is specified. That PR tried to fix it by not including the manifest, however, this is wrong because the way the PR did it was to check after the nuget had already defaulted to None and thus never included it.

Technically, this is actually an invalid state with unpackaged AND a manifest, however since we auto-include we also need to un-include. And secondly, what if this is a multi-mode project? Instead of requiring the user to do weird conditions, just not exclude it for the user.

mattleibow · 2022-04-08T22:34:36Z

src/BlazorWebView/tests/MauiDeviceTests/Directory.Build.targets

-  <Import Project="..\..\..\Maui.InTree.targets" />
-  <Import Project="$(MauiNuSpecDirectory)Microsoft.Maui.TestUtils.DeviceTests.Runners.targets" />
-  <Import Project="..\..\..\..\Directory.Build.targets" />
+  <Import Project="$(MauiSrcDirectory)Maui.InTree.targets" Condition=" '$(UseMaui)' != 'true' " />
+  <Import Project="$(_MauiBuildTasksLocation)Microsoft.Maui.TestUtils.DeviceTests.Runners.targets" />
+  <Import Project="$(MauiRootDirectory)Directory.Build.targets" />


A big chunk of removing the ../ as this is terribly confusing.

mattleibow · 2022-04-08T22:35:24Z

src/Compatibility/ControlGallery/src/Android/Compatibility.ControlGallery.Android.csproj

    <Compile Include="Properties\MapsKey.cs" Condition="Exists('Properties\MapsKey.cs')" />
  </ItemGroup>

+  <Import Project="$(MauiSrcDirectory)Maui.InTree.props" Condition=" '$(UseMaui)' != 'true' " />


All projects that need maui things, like xaml compilation, need to add Maui.InTree.props to the bottom of the .csproj in order to set values that are used by the nugets being imported.

mattleibow · 2022-04-08T22:37:12Z

src/Compatibility/ControlGallery/src/Directory.Build.targets

+  <Import Project="$(MauiSrcDirectory)Maui.InTree.targets" Condition=" '$(UseMaui)' != 'true' " />
+  <Import Project="$(MauiRootDirectory)Directory.Build.targets" />


Add the Maui.InTree.targets riiiight at the end of the list, but before the parent Directory.Build.targets. The only way to do this is to include the Maui.InTree.targets in a local Directory.Build.targets, and then include the parent afterwards.

mattleibow · 2022-04-08T22:38:43Z

src/Workload/Microsoft.Maui.Sdk/Sdk/AutoImport.props

    <AppxManifest
        Include="$(PackageManifest)"
-        Condition="'$(WindowsPackageType)' != 'None' and Exists('$(PackageManifest)')" />
+        Condition="Exists('$(PackageManifest)')" />


The other half of the fix in #5896 that ran before the .csproj or nugets. This means the only way to set this was via Directory.Build.props or via the CLI - which is not a valid requirement.

mattleibow · 2022-04-08T22:45:06Z

.nuspec/Microsoft.Maui.Controls.SingleProject.props

    <OutputType Condition="'$(OutputType)' == 'Exe'">WinExe</OutputType>
    <EnablePreviewMsixTooling Condition="'$(EnablePreviewMsixTooling)' == '' and ('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe')">true</EnablePreviewMsixTooling>
+    <WindowsPackageType Condition=" '$(WindowsPackageType)' == '' and '$(EnablePreviewMsixTooling)' == 'true' and ('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe') ">MSIX</WindowsPackageType>
+    <WinUISDKReferences Condition=" '$(WinUISDKReferences)' == '' and '$(EnablePreviewMsixTooling)' == 'true' and ('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe') ">false</WinUISDKReferences>


Now that we have the order correct, we can also properly exclude the SDKReferences items because they are not something WinUI actually needs. This is a UWP thing. This fixes #5881

Eilon · 2022-04-11T18:07:05Z

Works on my machine! I first built main, which failed with the gosh darn missing appxrecipe. I switched to the PR branch and it works!

solomonfried · 2022-10-07T14:59:30Z

I am trying to publish my first MAUI app and am getting this error..
Improper project configuration: WindowsPackageType is set to None, but PublishAppxPackage is set to true.

According to these posts the issue was resolved. I created the project on September 5th on a brand new VS2022 installation.
Are there any instructions on how to update the project file to eliminate this?
Thank you.

Eilon · 2022-10-07T16:24:10Z

Hi @solomonfried , can you please file a new issue with detailed steps to reproduce?

solomonfried · 2022-10-09T16:51:11Z

Hi @solomonfried , can you please file a new issue with detailed steps to reproduce?

I have created an issue
#10576

Thanks

McoreD · 2023-02-01T11:06:18Z

As soon as I add <WindowsPackageType>None</WindowsPackageType> I cannot compile the solution anymore. I hope Microsoft fixes this.

rmarinho · 2023-02-01T11:29:43Z

@McoreD what's the error? Do you have spaces in your user profile?

Match in-tree order of targets with workloads

9c22dab

mattleibow requested a review from a team as a code owner April 8, 2022 19:47

mattleibow added 4 commits April 8, 2022 23:09

use a consistent path

d927f88

move the props file

02842c3

This runs before the csproj, so we cannot do this

e485215

Have to do it this way

02df1d5

Eilon added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Apr 8, 2022

mattleibow added 3 commits April 9, 2022 00:14

needs more power

3918e9c

less power

cb01c3f

comment

2e6b20c

mattleibow requested review from PureWeen, Redth, jonathanpeppers and rmarinho April 8, 2022 22:27

mattleibow added t/bug Something isn't working p/0 Current heighest priority issues that we are targeting for a release. fatal labels Apr 8, 2022

mattleibow self-assigned this Apr 8, 2022

mattleibow added this to the 6.0.300-rc.2 milestone Apr 8, 2022

mattleibow commented Apr 8, 2022

View reviewed changes

Correctly disable the WinUISDKReferences items

995694c

mattleibow commented Apr 8, 2022

View reviewed changes

mattleibow changed the title ~~Match in-tree order of targets with workloads~~ Match in-tree order of targets with workloads and fix some WinUI issues Apr 8, 2022

rmarinho approved these changes Apr 11, 2022

View reviewed changes

jsuarezruiz approved these changes Apr 11, 2022

View reviewed changes

mattleibow merged commit 87cc187 into main Apr 11, 2022

mattleibow deleted the dev/maui-intree branch April 11, 2022 20:19

jsuarezruiz mentioned this pull request Apr 12, 2022

Unable to run Windows application from command line #5975

Open

4 tasks

github-actions bot locked and limited conversation to collaborators Dec 21, 2023

samhouts added the fixed-in-6.0.300-rc.2 Look for this fix in 6.0.300-rc.2! label Aug 2, 2024

		<Import Project="$(MauiSrcDirectory)Maui.InTree.targets" Condition=" '$(UseMaui)' != 'true' " />
		<Import Project="$(MauiRootDirectory)Directory.Build.targets" />

Conversation

mattleibow commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Details

Uh oh!

mattleibow Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

mattleibow Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

mattleibow Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

mattleibow Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

mattleibow Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

mattleibow Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

mattleibow Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

mattleibow Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eilon commented Apr 11, 2022

Uh oh!

solomonfried commented Oct 7, 2022

Uh oh!

Eilon commented Oct 7, 2022

Uh oh!

solomonfried commented Oct 9, 2022

Uh oh!

McoreD commented Feb 1, 2023

Uh oh!

rmarinho commented Feb 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mattleibow commented Apr 8, 2022 •

edited

Loading

mattleibow Apr 8, 2022 •

edited

Loading