msbuild icon indicating copy to clipboard operation
msbuild copied to clipboard

ResolveSdkReferences complains about referencing UAP SDKs in a Desktop app

Open stevenbrix opened this issue 3 years ago • 8 comments

Issue Description

When you try to reference either Microsoft.VCLibs or Microsoft.UniversalCRT.Debug when the TargetPlatformIdentifier == Windows, you get errors MSB3842 and MSB3843.

This is a new scenario that is uncovered due to Project Reunion, where we are making efforts to remove all differences at the project system layer between Desktop and UWP Windows.

Expected Behavior

Build succeeds

Actual Behavior

Build fails.

Analysis

This can be worked around by adding this:

  <PropertyGroup>
    <!-- Suppress warnings -->
    <LogSDKReferenceResolutionErrorsAsWarnings>true</LogSDKReferenceResolutionErrorsAsWarnings>
    <MSBuildWarningsAsMessages>
      $(MSBuildWarningsAsMessages);
      MSB3842;
      MSB3843;
    </MSBuildWarningsAsMessages>
  </PropertyGroup>

But this seems like an unnecessary big hammer. Since Desktop Windows is a complete superset of UWP Windows, it should be perfectly valid to have an SDK Reference to a UAP SDK when building Desktop, but not the other way around.

Versions & Configurations

c:\dev\winui>msbuild -version Microsoft (R) Build Engine version 16.9.0-preview-21065-09+34bbbedaf for .NET Framework Copyright (C) Microsoft Corporation. All rights reserved.

16.9.0.6509

Attach a binlog

msbuild.zip

stevenbrix avatar Feb 11 '21 16:02 stevenbrix

Notes for investigation: UAP appears to be an alias for UWP.

benvillalobos avatar Feb 17 '21 16:02 benvillalobos

@stevenbrix can you help us set a priority on this? Is it something you think we must fix for 17.0? Just trying to figure out how to rank it in our giant list-o-work.

rainersigwald avatar Jun 30 '21 15:06 rainersigwald

@stevenbrix Can you point me to the repo you used for this repro scenario?

benvillalobos avatar Jun 30 '21 23:06 benvillalobos

Hey all, I'm no longer on this team. @alwu-msft would better know the priority of this issue.

stevenbrix avatar Jun 30 '21 23:06 stevenbrix

I don't have any context around this, but I'll try my best to infer something. :)

According to the earlier referenced https://github.com/microsoft/microsoft-ui-xaml/issues/4351, this isn't directly affecting us anymore as we've implemented workarounds although there might be some doubt as to their correctness. I think you can repro this using a C++ Win32 console application that includes an <SDKReference /> to a UWP SDK (e.g. Microsoft.VCLibs or Microsoft.UniversalCRT.Debug).

@Scottj1s may remember more as he was more closely involved in that investigation.

evelynwu-msft avatar Jul 01 '21 00:07 evelynwu-msft

@alwu-msft Thanks for the tip, I was able to repro by creating a cpp console app and adding

  <ItemGroup>
    <SdkReference Include="Microsoft.UniversalCRT.Debug, Version=10.0.10240.0"/>
  </ItemGroup>

to the project.

this isn't directly affecting us anymore as we've implemented workarounds

What are those workarounds?

Some brief investigation notes:

It boils down to this check where 'Windows' (targetPlatformIdentifier) doesn't match 'UAP' (TargetPlatform).

                if (!String.IsNullOrEmpty(TargetPlatform) && !String.Equals(targetPlatformIdentifier, TargetPlatform))
                {
                    AddResolutionErrorOrWarning("ResolveSDKReference.TargetPlatformIdentifierDoesNotMatch", projectName, DisplayName, Version, targetPlatformIdentifier, TargetPlatform);
                }

What happens in the proper case? Does TargetPlatform and targetFrameworkIdentifier resolve to the same string? What is that string? UWP? I'll try other SDKReference's and see what happens


benvillalobos avatar Jul 01 '21 23:07 benvillalobos

My apologies, I'd specified the wrong project when linking the issue describing our workaround (https://github.com/microsoft/microsoft-ui-xaml/issues/4351); @Scott1js is the person who can describe the exact nature of the workaround (I'm not familiar with what he did beyond what is described in the issue).

I don't know where TargetPlatform comes from (the SDKReference?), but TargetPlatformIdentifier is tied to the overall project. The argument we're making is that UAP should be considered a subset of Windows, so a UAP SDKReference should be usable by a project that sets TargetPlatformIdentifier = Windows.

evelynwu-msft avatar Jul 02 '21 01:07 evelynwu-msft

Looking at this briefly, it sounds like we're saying we should treat UAP the same as "windows" as far as this check is concerned, right? So it is reasonable to just specialcase UAP for the check BenVillalobos mentioned and add && (!String.Equals("UAP", TargetPlatform) || !String.Equals(targetPlatformIdentifier, "Windows"))? Potentially with an OrdinalIgnoreCase comparator mixed in.

Forgind avatar Jun 15 '22 23:06 Forgind

@evelynwu-msft Double checking the status of the workaround here. IIt sounds like this workaround won't be sufficient in the future. Is that the case?

benvillalobos avatar Aug 18 '22 16:08 benvillalobos