Make debug repro packing resilient to repeated input paths - #2502
Open
Sergio0694 wants to merge 2 commits into
Open
Make debug repro packing resilient to repeated input paths#2502Sergio0694 wants to merge 2 commits into
Sergio0694 wants to merge 2 commits into
Conversation
The CsWinRT build tools all package their inputs into a self-contained debug
repro '.zip' when '--debug-repro-directory' is set. Files are staged under a
name derived from a hash of their full original path, and the original path is
then recorded in a map keyed by that name.
MSBuild item lists are not deduplicated, and several targets contribute to the
same reference sets (e.g. '@(ReferencePathWithRefAssemblies)'), so the same path
can reach a generator more than once. That made 'originalPaths.Add' throw, and
the whole build failed with an unhandled 'CSWINRT*GEN9999' error during the
'save-debug-repro' phase. Staging is now deduplicated per destination folder
(the hashed name is path-derived, so a file already staged there came from the
exact same source), while the returned name list still mirrors the original
argument list.
Two related issues are fixed at the same time:
- 'File.Copy' propagates the read-only attribute, so staging a read-only input
(normal in enlistment-style builds) left a read-only copy behind: staging it
twice failed with 'UnauthorizedAccessException', and the staging directory
could not be deleted afterwards. The attribute is now cleared on each copy.
- On unpack, the single top-level file (the input/output assembly) was matched
by entry name before the category subfolder was checked. When the same file
was staged both at the top level and inside a subfolder, the subfolder entry
was mis-claimed and dropped from its category, producing an archive that
saved but could not be replayed. Category subfolders now win.
The interop generator's local variant now only validates that a reserved .dll
was passed with a consistent path, and defers staging to the shared helper.
Adds regression tests to 'WinMDGeneratorTest' (each one fails without the fix)
and wires that project into CI, where it was not being run.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8e084ca2-e552-4f3c-82dd-cf30a5970335
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Split the reserved-.dll validation back into separate steps: return early for a null input path, then compute the hashed name once, then check it. This keeps the shape it had before and avoids the stacked conditions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e084ca2-e552-4f3c-82dd-cf30a5970335
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.
Problem
All five CsWinRT build tools support a debug repro: with
--debug-repro-directoryset they package every input file plus a faithful.rspinto a self-contained.zipthat can be replayed by passing the.zipback to the tool. Files are staged under a name derived from a SHA-256 of their full original path, and that name is then used as the key of the original-path map.MSBuild item lists are not deduplicated, and several targets contribute to the same reference sets, so the same path routinely reaches a generator more than once.
@(ReferencePathWithRefAssemblies)really does end up with e.g.Microsoft.CSharp.dlllisted twice.originalPaths.Add(...)then threw, and the whole build failed:This is reproducible on
staging/3.0today by building any project with-p:CsWinRTGeneratorDebugReproDirectory=<dir>. It is how I found it: I could not capture aninterop-debug-repro.zipto attach to an upstream issue, because asking for one broke the build.Fix
Staging is now deduplicated per destination folder. The hashed name is path-derived, so a file already staged at a given destination necessarily came from the exact same source and staging it again is a no-op. The check is deliberately per-folder rather than "already in the map", because the same file can legitimately be staged under two folders of the same repro (e.g. as both the input assembly and a reference assembly), and both copies are load-bearing. The returned name list still mirrors the original argument list, so the packed
.rspstays faithful.Two related issues surfaced while writing the tests and are fixed here too:
File.Copypropagates the read-only attribute, so a read-only input (normal in enlistment-style builds) left a read-only staged copy behind. Staging it twice failed withUnauthorizedAccessException, andDirectory.Delete(recursive: true)on the staging directory failed as well. The attribute is now cleared on each staged copy.CSWINRTWINMDGEN0003: Failed to parse argument 'ReferenceAssemblyPaths'. Category subfolders now win; the top-level entry is still validated by name.As a side effect the interop generator's local
CopyHashedFileToDirectoryshrinks to just the reserved-.dllpath validation and defers staging to the shared helper, which also removes the same latent skip bug from it.Tests
Three new tests in
src/Tests/WinMDGeneratorTest/Test_DebugRepro.cs, driving the realcswinrtwinmdgenend-to-end. Each saves an archive and replays it, so a repro that saves but cannot be replayed still fails. Each one fails onstaging/3.0with a distinct symptom:DebugRepro_IsSavedAndCanBeReplayedArgumentException: Argument_AddingDuplicateWithKeyDebugRepro_WithRepeatedInputPaths_IsSavedAndCanBeReplayedArgumentException: Argument_AddingDuplicateWithKeyDebugRepro_WithRepeatedReadOnlyInputPaths_IsSavedAndCanBeReplayedUnauthorizedAccessExceptionon the staged copyReverting only the unpack-ordering change instead makes the first two fail with
CSWINRTWINMDGEN0003on replay, so that half is load-bearing too.WinMDGeneratorTesthas never been run in CI (it was added in #2443 and only ever built), so it is also wired intoCsWinRT-Test-Steps.yml, x64-only, mirroring the source generator tests.Validation
WinMDGeneratorTest: 17/17 pass with the fix, 14/17 without it (the 3 new ones fail).ObjectLifetimeTests.Liftedwith-p:CsWinRTGeneratorDebugReproDirectory=<dir>fails onstaging/3.0and succeeds here, producing bothinterop-debug-repro.zip(21 MB) andprojection-debug-repro.zip(16 MB).cswinrtinteropgen.exe/cswinrtprojectiongen.exe, each emitting its output assembly.