[Regression] in 15.5p1: Portable0.0 is skipped

natidea commented 7 years ago

From @onovotny on October 18, 2017 23:57

I have an SDK project that the MSBuild.Sdk.Extras to build a legacy PCL target. This works in 15.4 but fails in 15.5 preview 1.

In the IDE in 15.4, I see this:

In 15.5 p1 I see this:

Note that the Portable0.0 entry representing portable-net4+sl5+wp8+win8+wpa81 is missing.

This is a blocker.

Building in the IDE in 15.5 p1 also yields the following error:

Severity    Code    Description Project File    Line    Suppression State
Error       The TargetFramework value 'portable-net4+sl5+wp8+win8+wpa81' was not recognized. It may be misspelled. If not, then the TargetFrameworkIdentifier and/or TargetFrameworkVersion properties must be specified explicitly.    crypto  C:\Program Files\dotnet\sdk\2.0.2\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.TargetFrameworkInference.targets   96

This is incorrect as the TFI and TFV is set by the MSBuild.Sdk.Extras package.

Repro is https://github.com/onovotny/bc-csharp/tree/e55eb138bf5492769ec1cf2212b49f223e377145 Open the BouncyCastle.sln

/cc @nguerrera

Copied from original issue: dotnet/sdk#1666

Copied from original issue: dotnet/project-system#2910

natidea commented 7 years ago

From @livarcocc on October 19, 2017 19:21

@dsplaisted and @nguerrera can one of you take a look at this?

natidea commented 7 years ago

From @nguerrera on October 19, 2017 22:42

Looking

natidea commented 7 years ago

From @nguerrera on October 19, 2017 22:42

@davkean @emgarten I believe this is either a project system or nuget issue. There are no SDK changes between 15.4 and 15.5 preview 1 and things work as before on the command line. Something is causing the project to get evaluated without the MSBuildSdkExtras targets setting TFI and TFV, but only in the IDE. Moving to project system for next look.

natidea commented 7 years ago

From @Pilchie on October 23, 2017 23:4

Not sure what could be different here, as the error only appears on build, and does appear in the output. Can you take a look?

natidea commented 7 years ago

From @davkean on October 23, 2017 23:6

I would look at the configurations we fire up here - I suspect Lifeng's change to reduce evaluations might have affected this.

natidea commented 7 years ago

On 15.5 P2, I do see the Portable0.0 node, though it is consistently coming through as unresolved:

I also get the error above when building the project. Investigating...

natidea commented 7 years ago

Using the build logger, the TF in the configured project is: portable-net4+sl5+wp8+win8+wpa81

But the target framework on which things are conditioned in .nuget.g.props and .nuget.g.targets is: portable-net40+sl5+win8+wp8+wpa81

Hence the import of the targets that set TFI and TFV is failing. If I modify .nuget.g.props and .nuget.g.targets to include the first TF, the build succeeds. Did we make any 15.5 era changes to the way this TargetFramework is computed?

natidea commented 7 years ago

From @nguerrera on October 25, 2017 14:16

Configured projects need to respect the spelling in TargetFrameworks. Is that not happening here?

natidea commented 7 years ago

From @davkean on October 25, 2017 15:15

@natidea Can we make sure we're pushing the right value to NuGet? And if so, this is NuGet normalizing the value it writes to props, which it shouldn't be doing @emgarten

natidea commented 7 years ago

From @emgarten on October 25, 2017 17:2

👍 on checking the nomination value. Restore shouldn't be normalizing the TFM string unless this has regressed in NuGet.

natidea commented 7 years ago

NuGet is being nominated with incorrect target framework. ProjectConfigurationsService.GetKnownProjectConfigurationsAsync() is returning ProjectConfigurations with this value:

natidea commented 7 years ago

Actually, scratch that. The value we are sending to NuGet: portable-net4+sl5+wp8+win8+wpa81 is the value in the csproj file. But the conditions in the .nuget.g.props and .nuget.g.targets files are different: portable-net40+sl5+win8+wp8+wpa81. So this could be a bug in NuGet

natidea commented 7 years ago

Confirmed: if I change the targetframework in the csproj file to portable-net40+sl5+win8+wp8+wpa81 (matching the value emitted by NuGet), then everything loads and builds properly: