Project-system: Conditionally multi-targeting a project causes VS memory to grow unbounded

[pranavkm]

Steps to reproduce:

1. Create a new project
2. Edit the project to multi-target where one of the TFMs is conditionally declared

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp1.0</TargetFrameworks>
    <TargetFrameworks Condition="'$(OS)'=='Windows_NT'">$(TargetFrameworks);net451</TargetFrameworks>
  </PropertyGroup>
</Project>

1. Open the project in VS

VS's memory grows unbounded and the UI is completely frozen. Removing the condition in the TFM specification causes the project to load normally.

VS version: 15.0.0-RC.3+26123.0.d15rel

I separately filed a feedback item for this but also creating this so we can track this via our work items.

[pranavkm]

cc @natemcmaster @davkean

[mavasani]

This is a dupe of https://github.com/dotnet/roslyn-project-system/issues/547, which I resolved as Won't fix as we didn't have an actual user scenario affected by it.

The issue here is that our TargetFramework project configuration dimension provider determines the set of TFMs by reading the project xml, because there is no way to do an msbuild project evaluation at unconfigured project level, and it only identifies netcoreapp1.0 as the set of TFMs. The language service loops infinitely to try to create an aggregate workpsace project context with 2 TFMs, but we end up with a single one and keep retrying. See this comment for the underlying issue. This piece of code executes before we create any configured project, so we cannot do any evaluation.

Couple of possible ways to fix this:

1. CPS/MSbuild allows us to do a project evaluation at unconfigured project level
2. We move away from implicitly inferring configurations from project file.

Tagging @davkean and @lifengl in case they have a solution for (1), i.e. https://github.com/dotnet/roslyn-project-system/issues/547.
Tagging @RaulPerez1, who was looking at (2).

[davkean]

Until we've figured 2), can we do something tactical here to not block VS and maybe just ignore the one that we can't resolve?

[mavasani]

I am wondering if even 2) will help, as we will eventually need an evaluation at unconfigured project level to compute configurations.

can we do something tactical here to not block VS and maybe just ignore the one that we can't resolve?

Yes, but is this a mainline scenario? Would it be better to just add this to release notes that TFMs cannot be conditioned for this release? If we ignore the conditioned properties, user would still be confused why it never worked as we don't generate any such diagnostic message.

[davkean]

Then add a diagnostic or fail the load. This really isn't ideal behavior to continue to consume memory until we crash.

[dsplaisted]

Yes, but is this a mainline scenario?

It's going to be pretty common for people that want to multi-target but also want to build on non-Windows OS's, since we don't support targeting .NET Framework when building outside of Windows right now. We do this in the SDK, for example.

[natemcmaster]

Follow up question to clarify what was fixed.

Since CPS still parses XML, does this mean only the unconditioned TargetFrameworks property is identified? If so, will this work better in VS?

    <TargetFrameworks>netcoreapp1.0;net451</TargetFrameworks>
    <TargetFrameworks Condition="'$(OS)'!='Windows_NT'">netcoreapp1.0</TargetFrameworks>

[mavasani]

Yes, that is correct.

[natemcmaster]

Ok, thanks. Should there be a follow-up issue to make conditional TargetFrameworks work?

[mavasani]

Yes, I re-opened and added comment to https://github.com/dotnet/roslyn-project-system/issues/547#issuecomment-277379863

[natemcmaster]

Thanks, I'll track that issue now.