dotnet / project-system

The .NET Project System for Visual Studio
MIT License
959 stars 385 forks source link

Consume newer Roslyn APIs for language services #9455

Closed drewnoakes closed 2 months ago

drewnoakes commented 2 months ago

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1881089

Roslyn recently introduced async API for obtaining and releasing "batch" objects for applying updates in https://github.com/dotnet/roslyn/pull/72424.

This change increases the package versions, uses the newer API, fixes some obsolete usages, and gets things building by adding a few package references in order to break the tie on some assembly version conflicts during build.

The package Microsoft.VisualStudio.Internal.MicroBuild.Swix was removed, as it conflicted with behaviour inherited via microsoft.visualstudioeng.microbuild.plugins.swixbuild, which now comes in transitively, resulting in errors running swc. Having both loaded was doubling up the items used to populate arguments, and it was throwing An item with the same key has already been added.

Microsoft Reviewers: Open in CodeFlow
drewnoakes commented 2 months ago

Looks like I'll need to find another workaround for the conflict between:

  1. microsoft.visualstudioeng.microbuild.plugins.swixbuild (1.1.286)
  2. microsoft.visualstudio.internal.microbuild.swix (2.0.115)
drewnoakes commented 2 months ago

Adding notes here as I investigate the SWIX conflicts that fall out of these package upgrades:

The error I see is:

  Unhandled Exception: System.ArgumentException: An item with the same key has already been added.
     at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
     at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
     at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
     at WixToolset.Simplified.SimplifiedWixCompiler.Run(Messaging messaging)
     at WixToolset.Simplified.SimplifiedWixCompiler.Main(String[] args)
D:\NuGetPackages\microsoft.devdiv.tasks2\17.4.0-preview-2-32803-137\build\swix.targets(154,5): error MSB6006: "swc.exe" exited with code -532462766.

I believe that exception is coming from here where "preprocessor defines" are being added.

Looking at the binlog shows the task does indeed have duplicates:

image

Task logs its CommandLineArguments as:

CompileVsix:
  D:\NuGetPackages\microsoft.visualstudio.internal.microbuild.setuptooling\2.0.115\build\..\tools\bin\swc.exe
   -arch neutral
   -ext D:\NuGetPackages\microsoft.visualstudio.internal.microbuild.setuptooling\2.0.115\build\..\ext\DevDivSwixExtension.dll
   -ext D:\NuGetPackages\microsoft.visualstudio.internal.microbuild.setuptooling\2.0.115\build\..\tools\VsixSwixExtension.dll
   -o d:\repos\project-system\artifacts\Debug\VSSetup\Insertion\Microsoft.VisualStudio.ProjectSystem.Managed.CommonFiles.vsix
   -d VisualStudioXamlRulesDir=d:\repos\project-system\artifacts\Debug\VSSetup\Rules\
   -d VisualStudioExtensionSetupDir=d:\repos\project-system\setup\
   -d Chip=AnyCPU
   -d MicrosoftPackageNamePrefix=Microsoft.
   -d "MicrosoftPublisher=CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US"
   -d "VSDefaultProgramPath=Software\Microsoft\VisualStudio_[InstanceId]\Capabilities"
   -d VsDdeApplication=VisualStudio.15.0
   -d Lang=enu
   -d Codepage=1252
   -d Culture=en
   -d LangCountry=en-US
   -d LCID=1033
   -d Chip=AnyCPU
   -d MicrosoftPackageNamePrefix=Microsoft.
   -d "MicrosoftPublisher=CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US"
   -d "VSDefaultProgramPath=Software\Microsoft\VisualStudio_[InstanceId]\Capabilities"
   -d VsDdeApplication=VisualStudio.15.0
   -d Lang=enu
   -d Codepage=1252
   -d Culture=en
   -d LangCountry=en-US
   -d LCID=1033
   d:\repos\project-system\artifacts\Debug\VSSetup.obj\Microsoft.VisualStudio.ProjectSystem.Managed.CommonFiles\swr\core.neutral.swr d:\repos\project-system\setup\Microsoft.VisualStudio.ProjectSystem.Managed.CommonFiles\CommonFiles.swr d:\repos\project-system\setup\Microsoft.VisualStudio.ProjectSystem.Managed.CommonFiles\ext.xproj.swr D:\NuGetPackages\microsoft.visualstudio.internal.microbuild.setuptooling\2.0.115\build\..\tools\VsixExternalFolders.swr
   -t vsix

Where -d is a define, and we see the duplicates.

These defines are coming from PackagePreprocessorDefinitions items in the build. Digging through the binlogs shows that two build files are reassigning the property:

  1. D:\NuGetPackages\microsoft.visualstudio.internal.microbuild.swix\2.0.115\build\Microsoft.Wix4.Swix.Tools.cleaned.targets
  2. D:\NuGetPackages\microsoft.visualstudioeng.microbuild.plugins.swixbuild\1.1.286\build\Microsoft.Wix4.Swix.Tools.targets

They have near-identical code. Indeed one appears to have been copied from the other. My assumption is that they're not intended to be used together.

I had previously tried removing microsoft.visualstudio.internal.microbuild.swix, which worked locally but did not work in CI, giving error:

error MSB4057: The target "CreateManifestResourceNames" does not exist in the project. [D:\a_work\1\s\setup\Microsoft.VisualStudio.ProjectSystem.Managed.CommonFiles\Microsoft.VisualStudio.ProjectSystem.Managed.CommonFiles.swixproj]

So now my plan is to find out why we're bringing in the other (microsoft.visualstudioeng.microbuild.plugins.swixbuild) and see if we can prevent that.

drewnoakes commented 2 months ago

microsoft.visualstudioeng.microbuild.plugins.swixbuild is imported from a NuGet package:

image

There's no mention of that package in the dependencies tree though. Looking in the assets file (D:\repos\project-system\artifacts\Debug\obj\Microsoft.VisualStudio.ProjectSystem.Managed.CommonFiles\project.assets.json) also shows no direct mention of microsoft.visualstudioeng.microbuild.plugins.swixbuild, so I'm unclear on how this is being added and still ended up in the .g.props file.

Pushed another experimental commit.

drewnoakes commented 2 months ago

That's making progress.

Now we have test failures. I assume this is due to updates to Microsoft.VisualStudio.Threading.

Microsoft.VisualStudio.ProjectSystem.Properties.AssemblyInfoPropertiesProviderTests.SourceFileProperties_SetPropertyValueAsync line 184

Microsoft.VisualStudio.Threading.JoinableTaskContextException : An attempt to switch to the main thread failed to reach the expected thread. Was the JoinableTaskContext initialized on the wrong thread or with a SynchronizationContext whose Post method does not execute its delegate on the main thread?

Stack Trace: 

    MainThreadAwaiter.GetResult()
    SourceAssemblyAttributePropertyValueProvider.SetPropertyValueAsync(String value) line 102
    AssemblyInfoProperties.SetPropertyValueAsync(String propertyName, String unevaluatedPropertyValue, IReadOnlyDictionary`2 dimensionalConditions) line 102
    InterceptedProjectProperties.SetPropertyValueAsync(String propertyName, String unevaluatedPropertyValue, IReadOnlyDictionary`2 dimensionalConditions) line 80
    AssemblyInfoPropertiesProviderTests.SourceFileProperties_SetPropertyValueAsync(String code, String propertyName, String propertyValue, String expectedCode) line 191

There are test failures with RenamerTests too, along the same lines.

drewnoakes commented 2 months ago

That exception is thrown here:

https://github.com/microsoft/vs-threading/blob/e4417b768162ac74b43e251320d11de771e7653b/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs#L854

I was able to fix these issues by suppressing the synchronization context for the tests in question.

drewnoakes commented 2 months ago

@tmeschter @ToddGrun this is ready for review now.

ToddGrun commented 2 months ago

The Roslyn interaction part LGTM

drewnoakes commented 1 month ago

We are reverting this because it's broken build signing and is blocking other work. Will revisit.