Remove unused dependencies and clean-up & Update dependencies from roslyn and arcade#1812
Conversation
0876716 to
3f261e2
Compare
|
Questions for @mmitche and @MichaelSimons: Should dotnet/format add a subscription to runtime and msbuild so that dependencies are kept up-to-date? We could also add a toolset dependency for nuget which is currently brought in just for test purposes. |
f030a2d to
f1f6063
Compare
| <!-- In order tests against the same version of NuGet as the SDK. We have to set this to match. --> | ||
| <NuGetVersion>6.6.0-preview.3.61</NuGetVersion> |
There was a problem hiding this comment.
This is the concerning part in this repository. The tests currently require the version of NuGet assemblies referenced (transitively via Microsoft.CodeAnalysis.Analyzer.Testing but bumped up directly in the project file) and the local SDK to match. We should try to remove this dependency somehow as a follow-up. Otherwise we would put a high pressure on SBRP or not be able to regularly update to latest preview SDK pushed by Arcade. cc @mmitche
There was a problem hiding this comment.
Yeah, avoiding such an exact match would be good.
c67135f to
6d61c63
Compare
| if ($stage -eq "format-workspace") { | ||
| Write-Output "$(Get-Date) - $solutionFile - Formatting Workspace" | ||
| $output = dotnet.exe "$currentLocation/artifacts/bin/dotnet-format/Release/net7.0/dotnet-format.dll" $solution --no-restore -v diag --verify-no-changes | Out-String | ||
| $output = dotnet.exe "$currentLocation/artifacts/bin/dotnet-format/Release/net8.0/dotnet-format.dll" $solution --no-restore -v diag --verify-no-changes | Out-String |
There was a problem hiding this comment.
Opened an issue for the hardcoded TFM values: #1814
1. Clean-up package dependencies that aren't needed. 2. The "<?xml" tags aren't required anymore by msbuild 3. Define versions in Versions.props consistently. Consider later adding dependabot support for external dependencies. 4. Move common properties into Directory.Build.props (ie LangVersion). 5. Remove unused dotnet7 feed. 6. Use reference assemblies package used to compile the tests for running the tests.
| @@ -30,9 +37,19 @@ private static async Task<IEnumerable<MetadataReference>> GetReferencesAsync() | |||
| MetadataReference.CreateFromFile(typeof(CodeFixProvider).Assembly.Location), | |||
| }; | |||
There was a problem hiding this comment.
📝 All of these should be removed and resolved from NuGet packages instead of from assemblies on disk. The only time MetadataReference.CreateFromFile(typeof(T).Assembly.Location) is valid is when the test assembly is defined in the same repository as T.
There was a problem hiding this comment.
If you don't mind, let's do that in a follow-up as this is already existent in main. Would you mind filing an issue?
2218631 to
cbea6d1
Compare
| netCurrentReferenceAssemblies = netCurrentReferenceAssemblies.WithNuGetConfigFilePath(nugetConfigPath); | ||
|
|
||
| var netcoreMetadataReferences = await netCurrentReferenceAssemblies.ResolveAsync(LanguageNames.CSharp, CancellationToken.None); | ||
| references.AddRange(netcoreMetadataReferences.Where(reference => Path.GetFileName(reference.Display) != "System.Collections.Immutable.dll")); |
There was a problem hiding this comment.
❔ Do we still need this line?
There was a problem hiding this comment.
That code is still valid as System.Collections.Immutable.dll is part of the Microsoft.NETCore.App.Ref targeting pack. I have no context here but I assume the assembly is filtered out as it's coming in via an external dependency and this is a lazy attempt to replicate msbuild's conflict resolution.
This should probably be removed together with your above feedback
All of these should be removed and resolved from NuGet packages instead of from assemblies on disk. The only time MetadataReference.CreateFromFile(typeof(T).Assembly.Location) is valid is when the test assembly is defined in the same repository as T.
[main] Update dependencies from dotnet/arcade - Update Versions.props - Update NuGet version to 6.6.0-preview.3.57 - Merge branch 'main' into darc-main-3d6ee651-0e5b-4cab-b906-1bc0244ff60f
dependabot support for external dependencies.
running the tests.
with the now updated repo local 8.0 P3 SDK.
This fixes the observed issues in the "Code velocity chat" and cleans up the repository.
Version changes
Commit 1 and 2 upgrade dependencies (Arcade + Roslyn)
Commit 3 updates the following versions:
MicrosoftBuildVersion -> 17.3.2
Microsoft.Extensions.FileSystemGlobbing -> 7.0.0
Microsoft.Extensions.Logging -> 7.0.0
BenchmarkDotNet -> 0.13.5
BenchmarkDotNet.Annotations -> 0.13.5
Microsoft.DiaSymReader -> 2.0.0-beta-22503-02 (transitive dependency)
Microsoft.VisualBasic -> 10.3.0 (transitive test dependency)
NETStandard.Library -> 2.0.3 (transitive test dependency)
NuGetVersion -> 6.6.0-preview.3.61 (transitive test dependency)
Microsoft.VisualStudio.Composition -> 17.4.16 (transitive test dependency)