Remove duplicate NuGet packages from packaging projects and no-op binding redirects#15731
Draft
nohwnd wants to merge 3 commits intomicrosoft:mainfrom
Draft
Remove duplicate NuGet packages from packaging projects and no-op binding redirects#15731nohwnd wants to merge 3 commits intomicrosoft:mainfrom
nohwnd wants to merge 3 commits intomicrosoft:mainfrom
Conversation
Packaging projects (TestPlatform, Portable, CLI) explicitly referenced NuGet packages that were already transitive dependencies, then used CopyFiles targets to overwrite build output with copies from NuGet cache. This caused version mismatches — notably System.Collections.Immutable 9.0.0.0 was shipped while the code compiled against 8.0.0.0. Changes: - Remove explicit PackageReferences and CopyFiles entries for: - System.Collections.Immutable (transitive via SRM) - System.Reflection.Metadata (direct dep of ObjectModel/CoreUtilities) - Microsoft.Extensions.FileSystemGlobbing (transitive via vstest.console) - Microsoft.Diagnostics.NETCore.Client (transitive via BlameDataCollector) - Add CopyLocalLockFileAssemblies=true to src/package/Directory.Build.props so transitive NuGet DLLs are copied to output for library-style projects - Keep DependencyModel as explicit ref (not transitive for net48/.NET Framework where TestHostProvider uses Jsonite, and pruned by NuGet on .NET 10) - Keep FileSystemGlobbing as explicit ref in Portable (netstandard2.0 has no project ref to vstest.console) - Update nuspec file paths from subfolder to flat layout where CopyFiles targets were removed (DependencyModel, FileSystemGlobbing, DiagnosticsNETCoreClient) - Update V2.CLI VSIX property paths to match flat layout - Binding redirects auto-fixed: SCI 9.0.0.0 -> 8.0.0.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove binding redirects that were no-ops (all assembly references in the dependency graph already target the shipped version): - System.Collections.Immutable (8.0.0.0) - System.Reflection.Metadata (8.0.0.0) - System.Memory (4.0.1.2) - Microsoft.Extensions.FileSystemGlobbing (8.0.0.0) - System.Diagnostics.DiagnosticSource (8.0.0.0) - System.Text.Encodings.Web (8.0.0.0) - System.Runtime.CompilerServices.Unsafe (6.0.0.0) [vstest.console only] Remaining redirects are genuinely needed: - ObjectModel 11-15 -> 15 (backward compat with old adapters) - CompilerServices.Unsafe 1-6 -> 6 (System.Memory 4.5.5 refs 4.0.4.1) - TestWindow.Interfaces 11-18 -> 18 (VS integration, testhost.x86) - UnitTestFramework 10.1 -> 10.0 (MSTest v1 compat, testhost.x86) Validated with assembly reference checker tool (PEReader/MetadataReader) and acceptance tests (372 passed, 104 skipped due to pre-existing test asset build issue, 23 skipped). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR simplifies VSTest packaging by removing redundant explicit NuGet package references and related “copy from NuGet” steps, ensuring the packaged outputs match what the projects actually build against. It also cleans up .NET Framework binding redirects by removing redirects that are now no-ops.
Changes:
- Removed explicit (duplicate/transitive) PackageReferences and the corresponding “copy extracted NuGet DLLs over build outputs” behavior in packaging projects.
- Enabled
CopyLocalLockFileAssemblies=truefor packaging projects so transitive NuGet DLLs are present in output for.nuspec/VSIX file inclusion. - Pruned no-op binding redirects from
app.configfiles (keeping only redirects that are still required for compatibility).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vstest.console/app.config | Removes no-op binding redirects; adds comment explaining the remaining Unsafe redirect. |
| src/testhost.x86/app.config | Removes no-op binding redirects; adds comment explaining the remaining Unsafe redirect. |
| src/datacollector/app.config | Removes no-op binding redirects; adds comment explaining the remaining Unsafe redirect. |
| src/package/Directory.Build.props | Sets CopyLocalLockFileAssemblies=true to make transitive NuGet DLLs available in packaging outputs. |
| src/package/Microsoft.TestPlatform/Microsoft.TestPlatform.csproj | Removes redundant package refs and NuGet-extraction copy steps; keeps only required explicit refs. |
| src/package/Microsoft.TestPlatform/Microsoft.TestPlatform.nuspec | Updates file paths to reflect DLLs now being present at output root (not under package subfolders). |
| src/package/Microsoft.TestPlatform.Portable/Microsoft.TestPlatform.Portable.csproj | Removes redundant package refs/copies; keeps explicit refs only where not transitive. |
| src/package/Microsoft.TestPlatform.CLI/Microsoft.TestPlatform.CLI.csproj | Removes redundant package refs and NuGet-extraction copy steps; keeps explicit DependencyModel ref. |
| src/package/Microsoft.TestPlatform.CLI/Microsoft.TestPlatform.CLI.nuspec | Updates file paths to match new output layout (root-level DLLs). |
| src/package/Microsoft.TestPlatform.CLI/Microsoft.TestPlatform.CLI.sourcebuild.nuspec | Updates source-build file paths to match new output layout (root-level DLLs). |
| src/package/Microsoft.TestPlatform.CLI/Microsoft.TestPlatform.CLI.sourcebuild.product.nuspec | Updates source-build product file paths to match new output layout (root-level DLLs). |
| src/package/Microsoft.VisualStudio.TestTools.TestPlatform.V2.CLI/Microsoft.VisualStudio.TestTools.TestPlatform.V2.CLI.csproj | Updates VSIX inclusion paths to consume root-level DependencyModel/FileSystemGlobbing DLLs. |
| <assemblyIdentity name="Microsoft.Extensions.FileSystemGlobbing" publicKeyToken="adb9793829ddae60" culture="neutral" /> | ||
| <bindingRedirect oldVersion="1.0.0.0-2.0.0.0" newVersion="2.0.0.0" /> | ||
| </dependentAssembly> | ||
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
There was a problem hiding this comment.
The comment refers to "CompilerServices.Unsafe", but the actual assembly being redirected is "System.Runtime.CompilerServices.Unsafe". Please update the comment to use the full assembly name to avoid confusion when troubleshooting binding redirects.
Suggested change
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> | |
| <!-- System.Memory 4.5.5 references System.Runtime.CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
| <assemblyIdentity name="Microsoft.Extensions.FileSystemGlobbing" publicKeyToken="adb9793829ddae60" culture="neutral" /> | ||
| <bindingRedirect oldVersion="1.0.0.0-2.0.0.0" newVersion="2.0.0.0" /> | ||
| </dependentAssembly> | ||
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
There was a problem hiding this comment.
The comment refers to "CompilerServices.Unsafe", but the bindingRedirect is for "System.Runtime.CompilerServices.Unsafe". Consider updating the comment to the full assembly name so it’s clear what the redirect applies to.
Suggested change
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> | |
| <!-- System.Memory 4.5.5 references System.Runtime.CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
| <assemblyIdentity name="Microsoft.Extensions.FileSystemGlobbing" publicKeyToken="adb9793829ddae60" culture="neutral" /> | ||
| <bindingRedirect oldVersion="1.0.0.0-2.0.0.0" newVersion="2.0.0.0" /> | ||
| </dependentAssembly> | ||
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
There was a problem hiding this comment.
The comment mentions "CompilerServices.Unsafe", but the assembly identity in the redirect is "System.Runtime.CompilerServices.Unsafe". Updating the comment to the full assembly name would make this easier to understand/maintain.
Suggested change
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> | |
| <!-- System.Memory 4.5.5 references System.Runtime.CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
Don't dismiss test failures as pre-existing without investigating. Document that acceptance tests need --test-parameter BuildCompatibility=true to build compatibility matrix test assets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @@ -24,26 +24,11 @@ | |||
| <assemblyIdentity name="Microsoft.VisualStudio.QualityTools.UnitTestFramework" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> | |||
| <bindingRedirect oldVersion="10.1.0.0" newVersion="10.0.0.0" /> | |||
| @@ -24,26 +24,11 @@ | |||
| <assemblyIdentity name="Microsoft.VisualStudio.QualityTools.UnitTestFramework" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> | |||
| <bindingRedirect oldVersion="10.1.0.0" newVersion="10.0.0.0" /> | |||
| </dependentAssembly> | |||
| @@ -24,26 +24,11 @@ | |||
| <assemblyIdentity name="Microsoft.VisualStudio.QualityTools.UnitTestFramework" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> | |||
| <Copy SourceFiles="@(MicrosoftExtensionsFileSystemGlobbing)" DestinationFiles="$(OutDir)\%(RecursiveDir)%(Filename)%(Extension)" /> | ||
| <Copy SourceFiles="@(SystemCollectionsImmutable)" DestinationFiles="$(OutDir)\%(RecursiveDir)%(Filename)%(Extension)" /> | ||
| <Copy SourceFiles="@(SystemReflectionMetadata)" DestinationFiles="$(OutDir)\%(RecursiveDir)%(Filename)%(Extension)" /> | ||
| <Copy SourceFiles="@(MicrosoftInternalDia)" DestinationFiles="$(OutDir)\Microsoft.Internal.Dia\%(RecursiveDir)%(Filename)%(Extension)" /> |
| <assemblyIdentity name="Microsoft.Extensions.FileSystemGlobbing" publicKeyToken="adb9793829ddae60" culture="neutral" /> | ||
| <bindingRedirect oldVersion="1.0.0.0-2.0.0.0" newVersion="2.0.0.0" /> | ||
| </dependentAssembly> | ||
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
| <assemblyIdentity name="Microsoft.Extensions.FileSystemGlobbing" publicKeyToken="adb9793829ddae60" culture="neutral" /> | ||
| <bindingRedirect oldVersion="1.0.0.0-2.0.0.0" newVersion="2.0.0.0" /> | ||
| </dependentAssembly> | ||
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> |
| <dependentAssembly> | ||
| <assemblyIdentity name="System.Runtime.CompilerServices.Unsafe" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> | ||
| <bindingRedirect oldVersion="1.0.0.0-6.0.0.0" newVersion="6.0.0.0" /> | ||
| </dependentAssembly> |
| <!-- System.Memory 4.5.5 references CompilerServices.Unsafe 4.0.4.1 but we ship 6.0.0.0 (from SCI/SRM). --> | ||
| <dependentAssembly> | ||
| <assemblyIdentity name="System.Runtime.CompilerServices.Unsafe" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> | ||
| <bindingRedirect oldVersion="1.0.0.0-6.0.0.0" newVersion="6.0.0.0" /> |
Member
Author
|
comment in english please . @copilot, can you translate the comments for me? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The packaging projects (Microsoft.TestPlatform, Portable, CLI) explicitly reference NuGet packages that already flow transitively through project references. The CopyFiles targets then overwrite build-produced DLLs with ones extracted from NuGet — potentially different versions. We end up shipping different things than we build with.
Removed packages (truly transitive):
Kept as explicit refs:
Also added \CopyLocalLockFileAssemblies=true\ in \src/package/Directory.Build.props\ so transitive NuGet DLLs actually get copied to output for these library-style packaging projects.
Binding redirects cleaned up:
Removed all no-op redirects where every assembly reference in the dependency graph already targets the shipped version. Verified with a PEReader/MetadataReader-based tool that reads AssemblyRef tables from all DLLs.
Remaining redirects are genuinely needed: