Remove external tar process workaround for hardlink signing#16994
Open
MichaelSimons wants to merge 4 commits into
Open
Remove external tar process workaround for hardlink signing#16994MichaelSimons wants to merge 4 commits into
MichaelSimons wants to merge 4 commits into
Conversation
The workaround (#16484) used external tar.exe on Windows to handle hardlinks because System.Formats.Tar lacked proper support (dotnet/runtime#74404). Now that the runtime issue is fixed and arcade targets .NET 10+, hardlinks are handled the same way as symlinks: skipped during reading (only the target regular file is signed) and preserved as-is during repack. Changes: - Remove ReadTarGZipEntriesWithExternalTar and RepackTarGZipWithExternalTar private methods from ZipData.cs - Remove Windows-conditional branches in ReadEntries and RepackTarGZip - Add TarEntryType.HardLink to the skip filter in ReadEntries (same treatment as symlinks) - Change SignTarGZipFileWithHardlinks test from [WindowsOnlyFact] to [Fact] - Update test expectations: only the regular file entry is signed; hardlinks are preserved and validated via new helper - Add ValidateProducedTarGZipHardlinks helper to verify hardlinks survive the repack round-trip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the Windows-only external tar.exe workaround used by SignTool for .tar.gz archives containing hardlinks, relying instead on System.Formats.Tar now that Arcade targets .NET 10+. It updates tar entry filtering and adjusts/expands test coverage to validate that hardlinks are preserved through the signing + repack flow.
Changes:
- Remove external
tar.exe-based read/repack paths and Windows-conditional branches for.tar.gz. - Skip
TarEntryType.HardLinkduring tar entry enumeration (same handling as symlinks) so only the target regular file is signed. - Update the hardlink signing test to run cross-platform and validate hardlinks are preserved in the repacked archive.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Microsoft.DotNet.SignTool/src/ZipData.cs | Removes the external-tar workaround; updates tar entry filtering to skip hardlinks. |
| src/Microsoft.DotNet.SignTool.Tests/SignToolTests.cs | Makes the hardlink test cross-platform; adds validation helper to assert hardlinks survive repack. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
Please submit a real signed build for validation, i.e. in the VMR to avoid potential disruption. |
ellahathaway
previously approved these changes
Jun 9, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The workaround (#16484) used external tar.exe on Windows to handle hardlinks because System.Formats.Tar lacked proper support (dotnet/runtime#74404). Now that the runtime issue is fixed and arcade targets .NET 10+, hardlinks are handled the same way as symlinks: skipped during reading (only the target regular file is signed) and preserved as-is during repack. Changes: - Remove ReadTarGZipEntriesWithExternalTar and RepackTarGZipWithExternalTar private methods from ZipData.cs - Remove Windows-conditional branches in ReadEntries and RepackTarGZip - Add TarEntryType.HardLink to the skip filter in ReadEntries (same treatment as symlinks) - Change SignTarGZipFileWithHardlinks test from [WindowsOnlyFact] to [Fact] - Update test expectations: only the regular file entry is signed; hardlinks are preserved and validated via new helper - Add ValidateProducedTarGZipHardlinks helper to verify hardlinks survive the repack round-trip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mmitche
reviewed
Jun 16, 2026
|
|
||
| foreach ((string path, string linkName) in expectedHardlinks) | ||
| { | ||
| hardlinkEntries.Should().Contain((Name: path, LinkName: linkName), |
Member
There was a problem hiding this comment.
Nit: If possible I'd make this stronger and check that no additional hard links are present. I think that awesomeassertions has a nice primitive for this kind of assert.
mmitche
approved these changes
Jun 16, 2026
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 workaround (dotnet/arcade#16484) used external
tar.exeon Windows to handle hardlinks becauseSystem.Formats.Tarlacked proper support (dotnet/runtime#74404). Now that the runtime issue is fixed and arcade targets .NET 10+, hardlinks are handled the same way as symlinks: skipped during reading (only the target regular file is signed) and preserved as-is during repack.Changes
ReadTarGZipEntriesWithExternalTarandRepackTarGZipWithExternalTarprivate methods fromZipData.csReadEntriesandRepackTarGZipTarEntryType.HardLinkto the skip filter inReadEntries(same treatment as symlinks)SignTarGZipFileWithHardlinkstest from[WindowsOnlyFact]to[Fact]ValidateProducedTarGZipHardlinkshelper to verify hardlinks survive the repack round-tripFixes #16484