[fix] Harden binding redirect validation to catch missing DLLs - #15778
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens packaging validation for binding redirects so redirects to assemblies missing from the package layout are treated as validation errors instead of being silently skipped.
Changes:
- Detects binding redirects whose target DLL cannot be found in the extracted package layout.
- Records those cases as validation errors and emits explicit error output.
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 microsoft#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>
0c9bdca to
cb7d691
Compare
|
/iterate |
1 similar comment
|
/iterate |
|
Warning The Fixed the CI failure. The new validation in
Removed both redirects. These were silently skipped before; now the hardened script surfaces them as errors, which is exactly what this PR is designed to do.
|
….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>
|
pr is from fork, don't touch this anymore PR iterator. My bad. |
- 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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
eng/verify-binding-redirects.ps1:133
- Missing DLLs are reported through the same
$errorscollection as version mismatches, so the CI failure still tells contributors to run the local auto-fix even though this case cannot be auto-fixed and requires either shipping the DLL or removing the redirect. The missing-DLL error should carry actionable remediation instead of falling through to the generic version-mismatch instructions.
# failures (e.g. #15765). Fail so the redirect gets removed.
# This cannot be auto-fixed — the redirect must be manually removed or the DLL shipped.
$errors += "$($entry.ExeName): $assemblyName has a binding redirect but the DLL is not in the package layout"
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>
It ships in the package, so allowing it would mask a future packaging regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…soft#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 microsoft#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>
…soft#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 microsoft#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>
* 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>
* 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>
…ng redirect (#16083) Cherry-pick of the main fix (PR #16082) to unblock the rel/18.6 VMR build. testhost.x86 and datacollector neither reference Microsoft.Extensions.FileSystemGlobbing nor ship the DLL beside their EXE in the Microsoft.TestPlatform.CLI nupkg (TestHostNetFramework folder). Same anti-pattern as the DiagnosticSource fix (#15776). On main and rel/18.8 the _VerifyNuGetPackages target is skipped under DotNetBuild=true, so the latent issue is masked in the VMR. On rel/18.6 that skip is not present, so the hardened verifier (from #15778) running in the VMR pack catches the stale redirect and fails the build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#16082) testhost.x86 and datacollector neither reference Microsoft.Extensions.FileSystemGlobbing nor ship the DLL beside their EXE in the Microsoft.TestPlatform.CLI nupkg (TestHostNetFramework folder). The stale redirect is the same anti-pattern fixed for DiagnosticSource in #15776: a redirect to an assembly that isn't shipped can cause MissingMethodException on net462 with DisableAppDomain=true. Only vstest.console references the assembly (via FilePatternParser), so the redirect is kept in src/vstest.console/app.config where the DLL ships next to vstest.console.dll. This is also the latent issue surfaced by the hardened verifier added in #15778 - currently visible on the rel/18.6 VMR build because (1) Microsoft.TestPlatform.nupkg (which ships the EXEs together with FileSystemGlobbing.dll under tools/net462/Common7/IDE/Extensions/TestPlatform/) is gated off when DotNetBuild=true, leaving the verifier to look only inside Microsoft.TestPlatform.CLI's TestHostNetFramework folder; and (2) the DotNetBuild != 'true' skip on _VerifyNuGetPackages was not backported to rel/18.6. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…16090) The hardened binding-redirect verifier (#15778) is now catching four stale binding redirects in testhost.x86 and datacollector app.configs. The redirects point at versions that are higher than what actually ships beside the exe in the Microsoft.TestPlatform.CLI nupkg (TestHostNetFramework folder): | Assembly | Redirect | Shipped | |-----------------------------------------|----------|---------| | System.Runtime.CompilerServices.Unsafe | 6.0.3.0 | 6.0.0.0 | | System.Memory | 4.0.5.0 | 4.0.1.2 | | System.Buffers | 4.0.5.0 | 4.0.3.0 | | System.Collections.Immutable | 10.0.0.0 | 9.0.0.0 | These redirects were bumped by the auto-fix script in #15724 when those higher AV variants were present in the layout at the time, but the rel/18.6 package layout has since reverted to the older transitive versions. A redirect targeting a version that is not actually shipped causes FileLoadException at runtime in hosts without their own binding redirects (e.g. Azure DevOps Distributed Test Agent). Mirrors the auto-fix output of build.cmd -c Release: newVersion and the upper bound of oldVersion are narrowed to the actual shipped DLL version. vstest.console/app.config is intentionally left untouched - vstest.console.exe ships in Microsoft.TestPlatform.NonShipping packages that are not in the verifier's checked set and whose DLL versions are not verified here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
verify-binding-redirects.ps1silently skipped redirects whose DLL wasn't in the package layout. This allowed #15765 — the DiagnosticSource redirect shipped in testhost.exe.config but the DLL was excluded from the.nuspec.Now it's an error: if the package exists but a redirect's target DLL is missing, the script fails. The whole-package-missing case (e.g. VMR where the exe isn't found at all) is still a skip.