Project-system: Multi-TFM: TargetFramework changes are not reflected in Nominate Data Flow

[natidea]

Repro Steps:

• Open Visual Studio with Logging (set PROJECTSYSTEM_PROJECTOUTPUTPANEENABLED=1)
• Create Multi-TFM Console App

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp2.0;net461</TargetFrameworks>
  </PropertyGroup>
</Project>

• Change Target Framework net461 -> net45

Actual:
Target Framework in nominate dataflow is not updated, resulting in a variety of problems (e.g. project.assets.json is not updated, Dependency node is unresolved). Observe incongruence with OriginalTargetFrameworks in logging:

BEGIN Nominate Restore for C:\Users\nayewah\source\repos\ConsoleApp46\ConsoleApp46\ConsoleApp46.csproj
    BaseIntermediatePath:     obj\
    OriginalTargetFrameworks: netcoreapp2.0;net45
    Target Frameworks (2)
        net461
            Project References
            Package References
            Target Framework Properties -- (RestoreAdditionalProjectFallbackFolders:;C:\Program Files\dotnet\sdk\15.5.0-preview-007044\Sdks\Microsoft.NET.Sdk\build\..\..\..\..\NuGetFallbackFolder | RestorePackagesPath: | MSBuildProjectDirectory:C:\Users\nayewah\source\repos\ConsoleApp46\ConsoleApp46 | RestoreAdditionalProjectSources: | PackageId:ConsoleApp46 | RuntimeIdentifier:win7-x86 | NoWarn:1701;1702;1705 | RestoreAdditionalProjectFallbackFoldersExcludes: | TargetFrameworkIdentifier:.NETFramework | VersionSuffix: | DotnetCliToolTargetFramework:netcoreapp2.0 | VersionPrefix:1.0.0 | BaseIntermediateOutputPath:obj\ | Version:1.0.0 | RestoreFallbackFolders: | MSBuildProjectFile:ConsoleApp46.csproj | TargetFrameworkMoniker:.NETFramework,Version=v4.6.1 | EmitAssetsLogMessages:true | TargetFramework:net461 | PackageTargetFallback: | TreatWarningsAsErrors:false | TargetFrameworkProfile: | RestoreSources: | TargetFrameworks:netcoreapp2.0;net45 | TargetFrameworkVersion:v4.6.1 | RuntimeIdentifiers: | PackageVersion:1.0.0 | WarningsAsErrors:NU1605 | AssetTargetFallback:)
        netcoreapp2.0
            Project References
            Package References
                Microsoft.NETCore.App -- (Description: | RuntimeIdentifier: | NoWarn: | OriginalItemSpec: | FrameworkName: | Version:2.0 | Visible: | TargetFramework: | IncludeAssets: | PrivateAssets: | ExcludeAssets: | IsImplicitlyDefined:true | FrameworkVersion: | Name: | Type: | Path:)
            Target Framework Properties -- (RestoreAdditionalProjectFallbackFolders:;C:\Program Files\dotnet\sdk\15.5.0-preview-007044\Sdks\Microsoft.NET.Sdk\build\..\..\..\..\NuGetFallbackFolder | RestorePackagesPath: | MSBuildProjectDirectory:C:\Users\nayewah\source\repos\ConsoleApp46\ConsoleApp46 | RestoreAdditionalProjectSources: | PackageId:ConsoleApp46 | RuntimeIdentifier: | NoWarn:1701;1702;1705 | RestoreAdditionalProjectFallbackFoldersExcludes: | TargetFrameworkIdentifier:.NETCoreApp | VersionSuffix: | DotnetCliToolTargetFramework:netcoreapp2.0 | VersionPrefix:1.0.0 | BaseIntermediateOutputPath:obj\ | Version:1.0.0 | RestoreFallbackFolders: | MSBuildProjectFile:ConsoleApp46.csproj | TargetFrameworkMoniker:.NETCoreApp,Version=v2.0 | EmitAssetsLogMessages:true | TargetFramework:netcoreapp2.0 | PackageTargetFallback: | TreatWarningsAsErrors:false | TargetFrameworkProfile: | RestoreSources: | TargetFrameworks:netcoreapp2.0;net45 | TargetFrameworkVersion:v2.0 | RuntimeIdentifiers: | PackageVersion:1.0.0 | WarningsAsErrors:NU1605 | AssetTargetFallback:;net461)
    Tool References

------------------------------------------
COMPLETED Nominate Restore for C:\Users\nayewah\source\repos\ConsoleApp46\ConsoleApp46\ConsoleApp46.csproj

Related Issues:

• https://github.com/dotnet/sdk/issues/1321
• https://github.com/dotnet/project-system/issues/2657

[natidea]

This does not repro in 15.5 so likely a regression. Perhaps ActiveConfigurationChanged event is not triggering all the necessary dataflow updates for nominate restore. Also note that in 15.6 switching from TargetFramework -> TargetFrameworks no longer triggers a project reload.

[natidea]

/cc @lifengl
This appears to be connected to the move to IActiveConfigurationGroupService.

After changing the TargetFrameworks property, I've seen at least one case in a debugger where our implementation of IProjectConfigurationDimensionsProvider.GetProjectConfigurationDimensionsAsync() returns the old value from the project file, after reading the evaluated property value here. Ultimately this dataflow in PackageRestoreInitiator returns an IConfigurationGroup<ConfiguredProject> that also contains the old value.

I speculated that perhaps this was another manifestation of the capabilities context race condition. But changing the dataflow above to use ProjectCapabilitiesContext.CreateIsolatedContext does not fix the issue because the dataflow already contains the incorrect value before its action block is executed. Perhaps, this is another case of version mismatch in the dataflow results?

The dependency tree is not yet using IActiveConfigurationGroupService, and it returns the correct values.

[natidea]

Curiously even after changing the TargetFrameworks twice, (e.g. net461 -> net46 -> net45), the dataflow still returns net461

[NickCraver]

I just hit this as well doing some open source work, happened after adding netcoreapp2.0 to a freshly added project. If I can help debug, let me know.

[Pilchie]

Pulling back to 15.6 as this is a regression we'd like to investigate.

[natidea]

CPS confirmed this is a missing feature of the new IActiveConfigurationGroupService
I've filed 562913 to track this. Feature work is not planned until 15.7

[NickCraver]

Are we saying this is broken until 15.7? We're supposed to close and restart Visual Studio every time we change TFMs on a project?

That's one hell of a regression, please tell me the regression isn't actually going to ship with 15.6...

[clairernovotny]

@natidea This is a really bad regression, IMO. I don't think this can wait for 15.7.

[NickCraver]

I think it'd help to describe what developers actually see here:

I have hit this personally 4 times in the past 3 days across multiple solutions. The result of this bug is what appears to be random build errors. Mine manifested as runtime type identifiers not declared for a framework I wasn't even targeting, packages that didn't restore (e.g. netstandard2.0 packages not restoring to net451 I was no longer targeting...), tests that indefinitely hung (not sure what confused the runner), and APIs not existing when the command line compiled fine.

This doesn't manifest as a bug. It manifests as many from the user point of view.

[natidea]

@NickCraver are these errors occurring because of changes to target frameworks or some other related scenario?

/cc @lifengl @Pilchie to consider pulling back to 15.6

From Project System perspective, our only recourse may be to roll back IActiveConfigurationGroupService (PR https://github.com/dotnet/project-system/pull/3093) /cc @davkean on that.

[NickCraver]

@natidea all my issues were changing target frameworks - things just go nuts until you restart VS. After that, everything is okay, but that's not my first instinct (nor should it be). The assumption that the compiler is always right is baked into developers, they're going to eat a lot of collective time on this one and then just be justifiably angry that they did afterwards :-/

[lifengl]

Ok, if it causes serious user experience regression, we will take a look in 15.6

Sent from my phone

On Feb 6, 2018, at 3:51 AM, Nick Craver <[email protected][e-mail]> wrote:

@natideahttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnatidea&data=02%7C01%7C%7C6904ae96c5494a1e9e4508d56d57ec4c%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636535146742826884&sdata=t2Tnkf%2Bs5%2FGeUHt48IW7PJjerT9o%2FMOE28RcyMk3rec%3D&reserved=0 all my issues were changing target frameworks - things just go nuts until you restart VS. After that, everything is okay, but that's not my first instinct (nor should it be). The assumption that the compiler is always right is baked into developers, they're going to eat a lot of collective time on this one and then just be justifiably angry that they did afterwards :-/

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fproject-system%2Fissues%2F3182%23issuecomment-363399669&data=02%7C01%7C%7C6904ae96c5494a1e9e4508d56d57ec4c%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636535146742826884&sdata=QdZrVP2x3ZNOYpnAneam2R%2BxIcWHOWBMswjIxurPO2k%3D&reserved=0, or mute the threadhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FALGWwvwBAL9IdlBbNvFa6hfFhkzSGTFuks5tSDywgaJpZM4RsI5f&data=02%7C01%7C%7C6904ae96c5494a1e9e4508d56d57ec4c%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636535146742826884&sdata=CUAmTu80e0CxUryfZICUYaiUzXJ5SpbiGMpDevsGtWo%3D&reserved=0.

[clairernovotny]

Thanks @lifengl! I agree with @NickCraver and believe it will.

[Pilchie]

Updating milestone to match. @lifengl let @natidea and I know if there is anything we can do to help here...

[lifengl]

Is this scenario ever worked? I looked into it, and think this is a large feature area which have never been designed or done in the product. We can chained the dataflow to publish TPM changes, which may work if you happened to rename the second or third TPM, but what about the first one. What if you suddenly remove debug/release configuration completely and replace with something else? The VS COM layer never expects to handle project file changes to update configurations, especially the current active one. updating the second might happen to work because most VS feature doesn't know it exists. Once it touches configurations or first TPM, I guess we can run into one problem or another one. I think a possible solution in 15.6 is to add a ReloadIntercept and reload the project if project configuration related properties are changed inside the project model, just like changing the project from a single TPM to multiple one. We can enable those features when we really have implemented and tested the product to work with those changes.

[natidea]

Prior to 15.6 we set this up to work specifically for changes to TargetFrameworks by setting up a brand new subscription each time: NuGetRestorer.cs#L87-L90. So it probably didn't work well for other configuration changes.

[Pilchie]

@lifengl just merged a change here for 15.6 Preview 6.

Thanks @lifengl