Skip to content

Replace VSSDK-sourced DLLs with proper package references#15567

Merged
nohwnd merged 3 commits into
microsoft:mainfrom
nohwnd:fix-vssdk-deps
Mar 26, 2026
Merged

Replace VSSDK-sourced DLLs with proper package references#15567
nohwnd merged 3 commits into
microsoft:mainfrom
nohwnd:fix-vssdk-deps

Conversation

@nohwnd
Copy link
Copy Markdown
Member

@nohwnd nohwnd commented Mar 24, 2026

Closes #15504

Source \System.Diagnostics.DiagnosticSource\ from its own NuGet package instead of from the \Microsoft.VSSDK.BuildTools\ package. Add a \PackageReference\ with \GeneratePathProperty\ and reference the DLL via the generated path property.

Remove the redundant VSSDK-sourced \System.Runtime.CompilerServices.Unsafe\ entry since it is already included from the TestPlatform build output.

Remove the now-unused \VSSDKBuildToolsFolder\ property.

Copilot AI review requested due to automatic review settings March 24, 2026 08:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the VSIX packaging project to source System.Diagnostics.DiagnosticSource from its own NuGet package (instead of VSSDK build tools) and removes redundant VSSDK-sourced DLL references/properties.

Changes:

  • Add PackageReference to System.Diagnostics.DiagnosticSource with GeneratePathProperty enabled.
  • Update VSIX file inclusion to pull System.Diagnostics.DiagnosticSource.dll from the restored NuGet package path and remove the VSSDK-sourced Unsafe inclusion.
  • Remove the now-unused VSSDKBuildToolsFolder MSBuild property and introduce a central version property for the new package.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/package/Microsoft.VisualStudio.TestTools.TestPlatform.V2.CLI/Microsoft.VisualStudio.TestTools.TestPlatform.V2.CLI.csproj Switches VSIX DLL sourcing from VSSDK folder to a NuGet package path generated by GeneratePathProperty.
eng/Versions.props Adds a central version property for System.Diagnostics.DiagnosticSource.

nohwnd and others added 2 commits March 24, 2026 10:53
Source System.Diagnostics.DiagnosticSource from its own NuGet package
instead of from the Microsoft.VSSDK.BuildTools package. Add a
PackageReference with GeneratePathProperty and reference the DLL via
the generated path property.

Remove the redundant VSSDK-sourced System.Runtime.CompilerServices.Unsafe
entry since it is already included from the TestPlatform build output.

Remove the now-unused VSSDKBuildToolsFolder property.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add binding redirect for System.Diagnostics.DiagnosticSource (8.0.0.1)
to vstest.console, testhost.x86, and datacollector app.config files.
This is needed after replacing the VSSDK-sourced DLL with the 8.0.1
NuGet package.

Also fix vstest.console/app.config which had a duplicate
System.Runtime.CompilerServices.Unsafe entry, and widen the Unsafe
oldVersion range from 1.0.0.0 to 0.0.0.0 in all three config files
to cover all possible version requests.

Verified System.Runtime.CompilerServices.Unsafe assembly version
(6.0.0.0) matches the existing newVersion in binding redirects.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 24, 2026 09:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread src/vstest.console/app.config
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 24, 2026 11:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@nohwnd nohwnd added the 🚢 Ship it! Add to PRs where owner approves automated PR, but cannot approve because they "wrote it". label Mar 25, 2026
@nohwnd nohwnd merged commit ba547a3 into microsoft:main Mar 26, 2026
6 checks passed
nohwnd added a commit that referenced this pull request May 14, 2026
* Remove DiagnosticSource binding redirect from app.configs

The binding redirect for System.Diagnostics.DiagnosticSource (8.0.0.1)
was added in #15567 but the DLL is excluded from the CLI package during
packaging. The redirect points to a version that doesn't ship, causing
MissingMethodException on net462 with DisableAppDomain=true when a test
triggers assembly resolution for DiagnosticSource.

Fixes #15765

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* [fix] Harden binding redirect validation to catch missing DLLs (#15778)

* Error on binding redirects pointing to missing DLLs

Change verify-binding-redirects.ps1 to fail when a binding redirect
references a DLL that isn't in the package layout. Previously this was
silently skipped, which allowed #15765 — a redirect for
DiagnosticSource 8.0.0.1 shipped in the config but the DLL was
excluded from the package.

The whole-package-missing case (VMR) is still a skip.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove stale DiagnosticSource and Encodings.Web redirects from vstest.console

The hardened validation script caught two more orphaned binding redirects
in vstest.console/app.config — both DLLs are explicitly excluded from
the CLI package nuspec.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix review feedback: fail locally on missing-DLL errors

- Missing-DLL errors now Write-Error instead of reporting success
- CI message distinguishes version mismatches (auto-fixable) from
  missing-DLL errors (manual removal needed)
- Addresses Copilot review comments on lines 133 and 138

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix em-dash encoding in verify-binding-redirects.ps1

Replace Unicode em-dash (U+2014) with ASCII hyphen in comments and
strings. The em-dash gets corrupted to multi-byte garbage on CI's
PowerShell, causing a parse error that breaks the entire validation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove FileSystemGlobbing from allow-list

It ships in the package, so allowing it would mask a future
packaging regression.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nohwnd added a commit that referenced this pull request May 14, 2026
* Remove DiagnosticSource binding redirect from app.configs

The binding redirect for System.Diagnostics.DiagnosticSource (8.0.0.1)
was added in #15567 but the DLL is excluded from the CLI package during
packaging. The redirect points to a version that doesn't ship, causing
MissingMethodException on net462 with DisableAppDomain=true when a test
triggers assembly resolution for DiagnosticSource.

Fixes #15765

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* [fix] Harden binding redirect validation to catch missing DLLs (#15778)

* Error on binding redirects pointing to missing DLLs

Change verify-binding-redirects.ps1 to fail when a binding redirect
references a DLL that isn't in the package layout. Previously this was
silently skipped, which allowed #15765 — a redirect for
DiagnosticSource 8.0.0.1 shipped in the config but the DLL was
excluded from the package.

The whole-package-missing case (VMR) is still a skip.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove stale DiagnosticSource and Encodings.Web redirects from vstest.console

The hardened validation script caught two more orphaned binding redirects
in vstest.console/app.config — both DLLs are explicitly excluded from
the CLI package nuspec.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix review feedback: fail locally on missing-DLL errors

- Missing-DLL errors now Write-Error instead of reporting success
- CI message distinguishes version mismatches (auto-fixable) from
  missing-DLL errors (manual removal needed)
- Addresses Copilot review comments on lines 133 and 138

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix em-dash encoding in verify-binding-redirects.ps1

Replace Unicode em-dash (U+2014) with ASCII hyphen in comments and
strings. The em-dash gets corrupted to multi-byte garbage on CI's
PowerShell, causing a parse error that breaks the entire validation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove FileSystemGlobbing from allow-list

It ships in the package, so allowing it would mask a future
packaging regression.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚢 Ship it! Add to PRs where owner approves automated PR, but cannot approve because they "wrote it".

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some files are taken from VSSDK tools

2 participants