Poison leak of Microsoft.CodeAnalysis.AnalyzerUtilities.dll

[mthalman]

The Microsoft.CodeAnalysis.AnalyzerUtilities.dll file is showing up as a poison leak for the VMR in the following paths:

• dotnet-format.8.0.435601/tools/net8.0/any/Microsoft.CodeAnalysis.AnalyzerUtilities.dll
• dotnet-sdk-8.0.100-preview.7.23367.1-fedora.38-x64.tar/sdk/8.0.100-preview.7.23367.1/DotnetTools/dotnet-format/Microsoft.CodeAnalysis.AnalyzerUtilities.dll
• dotnet-sdk-8.0.100-preview.7.23367.1-fedora.38-x64.tar/sdk/8.0.100-preview.7.23367.1/DotnetTools/dotnet-watch/8.0.100-preview.7.23364.43/tools/net8.0/any/Microsoft.CodeAnalysis.AnalyzerUtilities.dll

This regressed as a result of the changes in dotnet/roslyn-analyzers#6701.

[mthalman]

This poison leak in source-build is caused by several factors:

• Prior to Bump up the Microsoft.CodeAnalysis version used by packages in the repo roslyn-analyzers#6701, the previous version of Microsoft.CodeAnalysis.AnalyzerUtilities was 3.3.5. That's the version that exists in previously source-built artifacts (PSBA).
• Bump up the Microsoft.CodeAnalysis version used by packages in the repo roslyn-analyzers#6701 has upgraded the current version of Microsoft.CodeAnalysis.AnalyzerUtilities to 3.11.0.
• There is a cyclic dependency between the roslyn and roslyn-analyzers repos with roslyn being built first.
• The Microsoft.CodeAnalysis.AnalyzerUtilities.dll is included in the SDK for dotnet-format and dotnet-watch.

So here's how all this fits together...

When the roslyn repo builds, it outputs the Microsoft.CodeAnalysis.Features package. This package has a dependency on Microsoft.CodeAnalysis.AnalyzerUtilities, which comes from the roslyn-analyzers repo. Because Microsoft.CodeAnalysis.AnalyzerUtilities hasn't been built yet, it is retrieved from PSBA which has version 3.3.5 of that package.

Later, the roslyn-analyzers repo builds and produces the current version of Microsoft.CodeAnalysis.AnalyzerUtilities, which has version 3.11.0.

During the format repo's build, there's a dependency on the Microsoft.CodeAnalysis.Features package produced by roslyn. It retrieves that package but remember that it contains a reference to Microsoft.CodeAnalysis.AnalyzerUtilities.3.3.5. The dotnet-format package bundles together its dependencies, including the 3.3.5 version of Microsoft.CodeAnalysis.AnalyzerUtilities.dll.

The build of the sdk extracts the bundled content of dotnet-format. At this point, the version 3.3.5 of Microsoft.CodeAnalysis.AnalyzerUtilities.dll ends up in the final SDK output.

Since version 3.3.5 of Microsoft.CodeAnalysis.AnalyzerUtilities ends up in the final output and was not built by the current build (it came from a previous build via PSBA), it's considered to be a poison leak and not allowed by the source-build constraints.

So why wasn't this an issue prior to the change in dotnet/roslyn-analyzers#6701? Let's walk through that scenario.

In that case, the build of roslyn-analyzers produces a 3.3.5 version of Microsoft.CodeAnalysis.AnalyzerUtilities. Then when the format repo builds, it retrieves the Microsoft.CodeAnalysis.Features package and its dependency on Microsoft.CodeAnalysis.AnalyzerUtilities.3.3.5. Since Microsoft.CodeAnalysis.AnalyzerUtilities.3.3.5 was produced earlier by roslyn-analyzers, that's the version which takes precedence over PSBA because of feed ordering. So the SDK ends up with the version of Microsoft.CodeAnalysis.AnalyzerUtilities that was produced by the current build rather than from PSBA.

In the short term, we can allow Preview 7 to ship with this poison leak. That will cause Microsoft.CodeAnalysis.AnalyzerUtilities.3.11.0 to be included in PSBA. Once that version is in PSBA, then future builds will not have poison leaks (so long as roslyn-analyzers continues to produce 3.11.0 packages).

Any future version change in roslyn-analyzers will lead us right back to this situation.

Long term, we need to investigate ways to prevent this from happening:

• Do roslyn and roslyn-analyzers need to be separate repos? This cyclic dependency is problematic for source-build.
• Does this Microsoft.CodeAnalysis.AnalyzerUtilities.dll need to be included in the bundled output of dotnet-format at all?

cc @MichaelSimons, @NikolaMilosavljevic, @mmitche, @jaredpar

[mmitche]

@sharwell Do you have a recommendation on how to proceed? This is another case where the layering between the repos causes issues in source-build.

[jaredpar]

What is a poison leak?

[mthalman]

What is a poison leak?

From above:

Since version 3.3.5 of Microsoft.CodeAnalysis.AnalyzerUtilities ends up in the final output and was not built by the current build (it came from a previous build via PSBA), it's considered to be a poison leak and not allowed by the source-build constraints.

Essentially, anything that ends up in the final output that wasn't directly produced by the build is a poison leak.

[sharwell]

@mthalman Why are we building dotnet/format in source build? When we do build dotnet/format, why is it being allowed to located a pre-built during its packaging process?

[mthalman]

Why are we building dotnet/format in source build?

Because dotnet/sdk depends on it. Since we source-build sdk, we have to source-build its dependencies.

When we do build dotnet/format, why is it being allowed to located a pre-built during its packaging process?

It's not a prebuilt. A prebuilt is a package that hasn't come from the current build, PSBA (previously source build artifacts), or SBRP (source-build reference packages). Builds are allowed to reference artifacts from PSBA because it's necessary in a lot of cases. They are not allowed to cause them to be included in the output of the SDK. That's exactly what this poison validation is checking for.

[MichaelSimons]

Why are we building dotnet/format in source build?

Because dotnet/sdk depends on it. Since we source-build sdk, we have to source-build its dependencies.

Because dotnet-format is a bundled global tool in the sdk, it must be source built.

[sharwell]

My recommendation here is to modify the source build to do one of the following:

1. Permanently allow this specific version of MS.CA.AnalyzerUtilities to appear in the build output, with a reference to the commit used to rebuild it
2. Detect and build a second commit from the dotnet/roslyn-analyzers repository as part of source build, allowing both versions of this assembly to be available
3. Exclude dotnet/format (an optional tool that just happens to appear in the same package) from source build versions of the SDK
4. At the end of the build, overwrite the MS.CA.AnalyzerUtilities binary in the output directory with the one from the current build (they are binary compatible, so by definition this substitution is required to work)
5. Override the version of MS.CA.AnalyzerUtilities referenced by dotnet/format and dotnet/roslyn based on information derived at build time

I think my preference is (4) since it's the closest match to a real build.

[mthalman]

Does this Microsoft.CodeAnalysis.AnalyzerUtilities.dll need to be included in the bundled output of dotnet-format at all?

Yes. Both dotnet-format and dotnet-watch bundle this in their output. They are the only sources of this file contained by the SDK. The SDK does not include Microsoft.CodeAnalysis.AnalyzerUtilities.dll outside the context of these two tools.