Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable pre-built detection #6610

Merged
merged 12 commits into from
Jun 8, 2023

Conversation

oleksandr-didyk
Copy link
Contributor

@oleksandr-didyk oleksandr-didyk commented Apr 25, 2023

Resolves #6606

@dotnet/roslyn-analysis there are several exceptions that we ran into and would require help with. I will mention these in individual comments in the PR. Thanks!

Context for what and why of this PR and the linked issue - https://github.com/dotnet/source-build/blob/main/Documentation/eliminating-pre-builts.md

CC: @MichaelSimons @mmitche

@oleksandr-didyk oleksandr-didyk requested a review from a team as a code owner April 25, 2023 16:31
<SystemCollectionsImmutableExecutableVersion Condition="'$(SystemCollectionsImmutableExecutableVersion)' == ''">5.0.0</SystemCollectionsImmutableExecutableVersion>
<SystemReflectionMetadataExecutableVersion>$(SystemReflectionMetadataVersion)</SystemReflectionMetadataExecutableVersion>
<SystemReflectionMetadataExecutableVersion Condition="'$(SystemReflectionMetadataExecutableVersion)' == ''">5.0.0</SystemReflectionMetadataExecutableVersion>
<MicrosoftCodeAnalysisExecutableVersion>$(MicrosoftCodeAnalysisVersion)</MicrosoftCodeAnalysisExecutableVersion>
<MicrosoftCodeAnalysisExecutableVersion Condition="'$(MicrosoftCodeAnalysisExecutableVersion)' == ''">3.8.0</MicrosoftCodeAnalysisExecutableVersion>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the linked issue, for source-build we cannot utilize a ref pack for Microsoft.CodeAnalysis dependency versions.
Bumping even the minor version from 3.8.X to 3.9.X (or 3.7.X to 3.8.X which declared below) would result in build exceptions:

  /datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/AbstractRunIterations`1.cs(39,32): error CS8600: Converting null literal or possible null value to non-nullable type. [/datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/Roslyn.Diagnostics.Analyzers.csproj]
  /datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/AbstractRunIterations`1.cs(40,18): error CS8604: Possible null reference argument for parameter 'method' in 'bool IMethodSymbolExtensions.IsBenchmarkOrXUnitTestMethod(IMethodSymbol method, ConcurrentDictionary<INamedTypeSymbol, bool> knownTestAttributes, INamedTypeSymbol? benchmarkAttribute, INamedTypeSymbol? xunitFactAttribute)'. [/datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/Roslyn.Diagnostics.Analyzers.csproj]
  /datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/AbstractApplyTraitToClass`1.cs(83,41): error CS8602: Dereference of a possibly null reference. [/datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/Roslyn.Diagnostics.Analyzers.csproj]
  /datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/AbstractApplyTraitToClass`1.cs(103,51): error CS8602: Dereference of a possibly null reference. [/datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Roslyn.Diagnostics.Analyzers/Core/Roslyn.Diagnostics.Analyzers.csproj]

<Dependency Name="Microsoft.DotNet.XliffTasks" Version="1.0.0-beta.23211.1" CoherentParentDependency="Microsoft.DotNet.Arcade.Sdk">
<Uri>https://github.com/dotnet/xliff-tasks</Uri>
<Sha>519d565b45c46ac452fe5a77ab63295138992e07</Sha>
<SourceBuild RepoName="xliff-tasks" ManagedOnly="true" />
</Dependency>
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="4.5.0">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package is marked as pre-built and we cannot utilize a ref assembly for it. Bumping the package to the latest version causes a built-time exception:

/datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs(462,101): error CS8601: Possible null reference assignment. [/datadrive/source/roslyn-analyzers/artifacts/source-build/self/src/src/NetAnalyzers/Core/Microsoft.CodeAnalysis.NetAnalyzers.csproj]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to fix the compile error. Because these 3 packages contain functionality that has to execute, them must either come from previously source-built, or from roslyn's build. Pinning to a stable version here works, but will probably mean more instability in product source build. Updating regularly from roslyn means more dependency flow and channel/subscription management.

I propose that we keep as-is for now, and if we see frequent breaks, we should move to change this to an actively updated dependency.

@@ -33,7 +33,7 @@
</None>
</ItemGroup>

<ItemGroup>
<ItemGroup Condition="'$(DotNetBuildFromSource)' != 'true'" >
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not building Analyzers in source-build, hence this conditional

@@ -13,7 +13,6 @@

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis" Version="$(MicrosoftCodeAnalysisExecutableVersion)" />
<PackageReference Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutableExecutableVersion)" />
<PackageReference Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataExecutableVersion)" />
<PackageReference Include="System.Composition" Version="8.0.0-preview.4.23220.1" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporary change to make the build consume source-build-reference-package dependency until Microsoft.CodeAnalysis issue is resolved (described in #6606 (comment))

The package is loaded during the build and an SBRP for it exists. When a dependency on SBRP is added, the ref assembly for the package is picked up, crashing the build. I circumvented this by forcing the latest version of the package, which might not be desirable

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Merging #6610 (397fab4) into main (2b6ab8d) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6610      +/-   ##
==========================================
- Coverage   96.40%   96.40%   -0.01%     
==========================================
  Files        1379     1379              
  Lines      322253   322253              
  Branches    10460    10460              
==========================================
- Hits       310657   310654       -3     
- Misses       9103     9107       +4     
+ Partials     2493     2492       -1     

@oleksandr-didyk oleksandr-didyk marked this pull request as draft May 2, 2023 09:01
@oleksandr-didyk
Copy link
Contributor Author

@mavasani thank you for the approval, but unfortunately we would need some help from you and rolsyn-analyzers maintainers before this PR can be merged due to issues described here and here (as well as in the issue linked to the PR).

This might not have been conveyed properly initially, my bad. I converted the PR to Draft to better indicate its status

@mmitche
Copy link
Member

mmitche commented May 10, 2023

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 6610 in repo dotnet/roslyn-analyzers

@mmitche mmitche marked this pull request as ready for review May 23, 2023 21:24
@mmitche mmitche requested review from sharwell and a team May 23, 2023 21:24
<PropertyGroup Condition="'$(DotNetBuildFromSource)' == 'true'">
<!-- When building in source build mode, treat this set of warnings not as errors. Source build is
using a newer version of Microsoft.CodeAnalysis than the default build -->
<NoWarn>$(NoWarn);CS8600;CS0618;CS8601;CS8602;CS8603;CS8604</NoWarn>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are thousands of CA warnings when the version of Microsoft.CodeAnalysis is upgraded. These are set to nowarn in full source-build mode. Not sure whether it's worth taking a look at these.

@mmitche
Copy link
Member

mmitche commented May 24, 2023

@sharwell can you take a look at this PR?

@MichaelSimons
Copy link
Member

@mmitche - Can you please validate in the VMR prior to merging?

@mmitche
Copy link
Member

mmitche commented May 25, 2023

@mmitche - Can you please validate in the VMR prior to merging?

Will do

@mmitche
Copy link
Member

mmitche commented May 26, 2023

Local VMR build was green

@@ -48,7 +47,7 @@ private async Task<Document> ConvertKindToIsKindAsync(Document document, TextSpa

protected abstract void FixDiagnostic(DocumentEditor editor, SyntaxNode nodeToFix);

private sealed class CustomFixAllProvider : DocumentBasedFixAllProvider
private sealed class CustomFixAllProvider : Analyzer.Utilities.DocumentBasedFixAllProvider
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Note to self: review this unexpected change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 It appears this library is building against the wrong set of reference assemblies. This should be corrected, at which point no changes are required to this file.

@@ -2,7 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>2.9.0</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForBannedApiAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

@sharwell sharwell May 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 2.9.0).

@@ -7,7 +7,7 @@
Restore would conclude that there is a cyclic dependency between us and the Microsoft.CodeAnalysis.BannedApiAnalyzer package.
-->
<PackageId>*$(MSBuildProjectFile)*</PackageId>
<MicrosoftCodeAnalysisVersion>2.9.0</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForBannedApiAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 2.9.0).

@@ -2,7 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>2.9.0</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForBannedApiAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 2.9.0).

@@ -7,7 +7,7 @@

<!-- Avoid ID conflicts with the package project. -->
<PackageId>*$(MSBuildProjectFile)*</PackageId>
<MicrosoftCodeAnalysisVersion>4.0.1</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForResxSourceGenerators)</MicrosoftCodeAnalysisVersion>
Copy link
Member

@sharwell sharwell May 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 4.0.1).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that goes for all the ResX projects?

@@ -7,7 +7,7 @@

<!-- Avoid ID conflicts with the package project. -->
<PackageId>*$(MSBuildProjectFile)*</PackageId>
<MicrosoftCodeAnalysisVersion>4.0.1</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForResxSourceGenerators)</MicrosoftCodeAnalysisVersion>
Copy link
Member

@sharwell sharwell May 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 4.0.1).

@@ -7,7 +7,7 @@

<!-- Avoid ID conflicts with the package project. -->
<PackageId>*$(MSBuildProjectFile)*</PackageId>
<MicrosoftCodeAnalysisVersion>4.0.1</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForResxSourceGenerators)</MicrosoftCodeAnalysisVersion>
Copy link
Member

@sharwell sharwell May 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 4.0.1).

@@ -2,7 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>2.9.0</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForPerfSensitiveAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 2.9.0).

<ReleaseTrackingOptOut>true</ReleaseTrackingOptOut>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForBannedApiAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This project produces a library only. It should always build against a consistent set of reference assemblies (currently 2.9.0).

<ReleaseTrackingOptOut>true</ReleaseTrackingOptOut>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForBannedApiAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForBannedApiAnalyzers)</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForPerfSensitiveAnalyzers)</MicrosoftCodeAnalysisVersion>

@@ -10,7 +11,7 @@
<InternalsVisibleTo Include="Roslyn.Diagnostics.Analyzers.UnitTests" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisForRoslynDiagnosticsAnalyzersVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisVersion)" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ Did this need to change? It's not following the same pattern I see in other files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is following the same pattern as the rest of the files now (set property at the top PropertyGroup and use later)

@@ -3,9 +3,10 @@
<PropertyGroup>
<TargetFramework>$(NetCurrent)</TargetFramework>
<ServerGarbageCollection>true</ServerGarbageCollection>
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true'">$(MicrosoftCodeAnalysisVersionForTests)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ Are we expecting to build unit tests in source build? Are we expecting those tests to pass when executed? Is anything executing the tests currently?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the unit tests are executing in source build currently. Should build though.

- NoWarn a couple things
- Update SBRPs
@mmitche
Copy link
Member

mmitche commented Jun 2, 2023

@sharwell Alright I made another significant iteration here. Goal was to ensure that product source build and repo source build always use the correct surface area when building the libraries here. Think I'm getting closer.

This repo tended to use MicrosoftCodeAnalysisVersion as a default and did not separate out when it was for surface area or executables. This presents a problem for source build, since it will just override that with the latest version coming from roslyn.

What I did was remove the global MicrosoftCodeAnalysisVersion property, and instead set it in each project that wants to use it, setting the value based on how it's used in the project. This means that source-build, which sets MicrosoftCodeAnalysisVersion globally, will not transparently reset the surface area for library projects. It also means that for non-source build, the version in use is much more obvious.

I still need to add one more reference package, but I think it's close now. PTAL.

NuGet.config Outdated
@@ -4,8 +4,10 @@
<packageSources>
<clear />
<add key="dotnet5" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json" />
<add key="general-testing" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/general-testing/nuget/v3/index.json" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore for now, will remove when the SBRP update is done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be removed now?

@@ -14,6 +14,7 @@

<!-- RS0026: Avoid public API overloads with differences in optional parameters -->
<NoWarn>$(NoWarn);RS0026</NoWarn>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in version, 3.3.1

@@ -2,6 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in version, 3.3.1

@@ -8,6 +8,7 @@
Restore would conclude that there is a cyclic dependency between Microsoft.CodeAnalysis and Microsoft.CodeAnalysis.Analyzers.
-->
<PackageId>*$(MSBuildProjectFile)*</PackageId>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in version, 3.3.1

@@ -2,6 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in version, 3.3.1

@@ -2,6 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForNetAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in version, 3.3.1. Added new property for this.

@@ -9,6 +9,7 @@
<PackageId>*$(MSBuildProjectFile)*</PackageId>
<RootNamespace></RootNamespace>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForNetAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in version, 3.3.1. Added new property for this.

@@ -2,6 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForNetAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in version, 3.3.1. Added new property for this.


<!-- Executable code, so set this in non-source build or repo source-build. Full
source build will use the incoming, pre-set MicrosoftCodeAnalysisVersion. -->
<MicrosoftCodeAnalysisVersion Condition="'$(DotNetBuildFromSource)' != 'true' or '$(DotNetBuildFromSourceFlavor)' != 'Product'">$(MicrosoftCodeAnalysisVersionForTests)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sharwell I want to verify that in these these unit test cases it is desired to build against the correct surface area (e.g. 2.9.0 for perf sensitive analyzers), but run against the test version (4.6.0*)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these fail when not executed against the 3.3.1 version. My bet is that the failures are just test issues (more diagnostics shown than expected). Reverting for now and marking these two as not building from source with an appropriate comment.

@@ -2,6 +2,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForTextAnalyzers)</MicrosoftCodeAnalysisVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added new property for this, set to 3.3.1

@mmitche
Copy link
Member

mmitche commented Jun 6, 2023

Woop Woop! All green and good to go. @dotnet/source-build-internal for SB signoff and @sharwell for any other comments on the PR.

NuGet.config Outdated
@@ -4,8 +4,10 @@
<packageSources>
<clear />
<add key="dotnet5" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json" />
<add key="general-testing" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/general-testing/nuget/v3/index.json" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be removed now?

<UsagePattern IdentityGlob="*/*" />
<UsagePattern IdentityGlob="Microsoft.SourceBuild.Intermediate.*" />

<!-- This packages contain functionality that is executed in the build -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<!-- This packages contain functionality that is executed in the build -->
<!-- These packages contain functionality that is executed in the build -->

its dependencies. This is not an issue in full SB mode. -->
<UsagePattern IdentityGlob="Microsoft.CodeAnalysis.Analyzers*/*" />

<!-- This is the version of Microsoft.CodeAnalysis executed and built against in source build.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify what source build you are referring to here? The repo leg or product build?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<!-- This is the version of Microsoft.CodeAnalysis executed and built against in source build.
When updating that verison, update the version here. -->
<UsagePattern IdentityGlob="Microsoft.CodeAnalysis*/*4.6.0-1.final*" />
<UsagePattern IdentityGlob="System.Threading.Channels/*6.0.0*" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a transitive dependency from M.CA?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out it's not needed. Removed.

@mmitche
Copy link
Member

mmitche commented Jun 7, 2023

Addressed feedback.

@mmitche
Copy link
Member

mmitche commented Jun 7, 2023

Running one last full VMR build to verify. I'll merge after unless @sharwell has additional comments

@mmitche
Copy link
Member

mmitche commented Jun 8, 2023

Looking good.

@mmitche mmitche merged commit 127dd35 into dotnet:main Jun 8, 2023
11 checks passed
@github-actions github-actions bot added this to the vNext milestone Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable source-build pre-built detection
6 participants