novotnyllc / MSBuildSdkExtras

Extra properties for MSBuild SDK projects
MIT License
347 stars 42 forks source link

VS 2017 15.6+ breaks UAP builds #43

Closed GeertvanHorrik closed 6 years ago

GeertvanHorrik commented 6 years ago

I just wrote a post with the following example repository:

https://github.com/GeertvanHorrik/MultiTargetingProject/blob/master/src/Ghk.MultiTargeting/Ghk.MultiTargeting.csproj

Then I read about the new 15.6 method to include the references, so I made the following changes.

Changes

  1. Replace <Project Sdk="Microsoft.NET.Sdk"> with <Project Sdk="MSBuild.Sdk.Extras/1.2.2">
  2. Remove <Import Project="$(MSBuildSDKExtrasTargets)" Condition="Exists('$(MSBuildSDKExtrasTargets)')" />
  3. Remove the package reference to MSBuild.Sdk.Extras

Result

========================================
Build
========================================
Microsoft (R) Build Engine version 15.6.82.30579 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

C:\Source\MultiTargetingProject\src\Ghk.MultiTargeting\Platforms\uap10.0\Views\MyView.xaml(1,23): error MC3074: The tag 'UserControl' does not exist in XML
namespace 'using:Catel.Windows.Controls'. Line 1 Position 23. [C:\Source\MultiTargetingProject\src\Ghk.MultiTargeting\Ghk.MultiTargeting.csproj]
C:\Source\MultiTargetingProject\src\Ghk.MultiTargeting\Platforms\uap10.0\Views\MyView.xaml(1,23): error MC3074: The tag 'UserControl' does not exist in XML
namespace 'using:Catel.Windows.Controls'. Line 1 Position 23. [C:\Source\MultiTargetingProject\src\Ghk.MultiTargeting\Ghk.MultiTargeting.csproj]
An error occurred when executing task 'Build'.
Error: One or more errors occurred.
        MSBuild: Process returned an error (exit code 1).
GeertvanHorrik commented 6 years ago

Could this be the result of a different order because of the SDK usage?

clairernovotny commented 6 years ago

Will take a look. Also @kzu, any thoughts as you rearranged a few things to support SDK's :)

kzu commented 6 years ago

Is this reproducible both in sdk usage as well as packgeref usage?

--

-- /kzu from mobile

GeertvanHorrik commented 6 years ago

In package ref, it works fine. It only breaks when using it as SDK.

GeertvanHorrik commented 6 years ago

See this repository:

https://github.com/GeertvanHorrik/MultiTargetingProject

Here is the csproj:

https://github.com/GeertvanHorrik/MultiTargetingProject/blob/master/src/Ghk.MultiTargeting/Ghk.MultiTargeting.csproj

Whenever I make the changes, it breaks.

clairernovotny commented 6 years ago

@GeertvanHorrik Still investigating, but it looks like something in the WPF target defaults is interfering:

Setting <EnableDefaultWpfItems>false</EnableDefaultWpfItems> in your properties seems to get it compiling using the SDK syntax.

Still not sure what's different in the ordering. Would love any thoughts @kzu

clairernovotny commented 6 years ago

For testing/creating a simple repro, I added a new test project in this repo in tests\ClasslibWithNuGetSdkRef. make sure to build msbuild.sdk.extras first as it creates the temp package needed by the other one project.

Would be great if we can get an isolated repro there.

clairernovotny commented 6 years ago

I suspect what's going on is in here: https://github.com/onovotny/MSBuildSdkExtras/blob/aa3e822ea43a24222ea6c230011c8b327d431782/MSBuild.Sdk.Extras/build/platforms/languageTargets/Wpf.targets#L21-L28

The WPF targets are including everything as a page by default.

I think the proper fix is two-fold:

  1. Setting EnableDefaultWpfItems to false as I suggest above.
  2. Updating those patterns to work with #47. The trick there is knowing the difference if we should include files in the root instead of just the Platforms folder in case it's "just" a sdk-style WPF app without multi-targeting (like NuGet Package Explorer).

I'm still not sure why it worked as a PackageReference since based on the above, it shouldn't have?

clairernovotny commented 6 years ago

@GeertvanHorrik did that help your issue?

GeertvanHorrik commented 6 years ago

Sorry for my late reply, the "focused inbox" got me on this. Will look into it.

GeertvanHorrik commented 6 years ago

Trying with 1.3.1, but that doesn't work yet. Trying the fix you proposed.

GeertvanHorrik commented 6 years ago

See commit:

https://github.com/GeertvanHorrik/MultiTargetingProject/commit/2fa12402a810fec83d99c17ca281ef37bd9d49e3

Only setting EnableDefaultWpfItems to false seems to fix this

clairernovotny commented 6 years ago

Yeah, the issue is with the default Page glob. Not sure what the right balance is for this, getting a single WPF target vs multi-target where different project types share the XAML file extension.

GeertvanHorrik commented 6 years ago

Are more people using the extras for multitargeting? I think so, but you now the stats better…

Nirmal4G commented 6 years ago

@GeertvanHorrik Can you make your template work with the package built using the recent commit on my PR?

You can put your project folder into TestProjects folder in the repo and make your changes, it'll automatically picks up latest extras when you build the repo solution first and then yours.

GeertvanHorrik commented 6 years ago

Just verified that it works with a few changes. Here is the full commit ensuring compatibility with the PR:

https://github.com/GeertvanHorrik/MultiTargetingProject/commit/0b7a8dedb557cf31ff721bf8800915fa4a49ec79