Skip to content

RebaseArchiveEntries: use POSIX / Unix paths - #42

Merged
vvoland merged 1 commit into
moby:mainfrom
thaJeztah:RebaseArchiveEntries_posix
Jul 15, 2026
Merged

vvoland merged 1 commit into
moby:mainfrom
thaJeztah:RebaseArchiveEntries_posix

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

This is mostly defense-in-depth; RebaseArchiveEntries is manipulating the Tar headers, which use POSIX / Unix paths; convert the given paths on the way in.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens RebaseArchiveEntries by normalizing oldBase/newBase to POSIX-style paths before rewriting tar headers, aligning with tar’s expectation of /-separated entry names.

Changes:

  • Normalize oldBase and newBase using filepath.ToSlash() before rebasing tar header names.
  • Treat oldBase == "/" as a special case (root) after normalization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread copy.go
@codecov-commenter

codecov-commenter commented Jul 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.33%. Comparing base (2cd730e) to head (ef0bff1).
⚠️ Report is 41 commits behind head on main.

Files with missing lines Patch % Lines
copy.go 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
- Coverage   66.35%   65.33%   -1.02%     
==========================================
  Files          42       42              
  Lines        2027     2034       +7     
==========================================
- Hits         1345     1329      -16     
- Misses        497      532      +35     
+ Partials      185      173      -12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

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 2 out of 2 changed files in this pull request and generated no new comments.

@thaJeztah
thaJeztah force-pushed the RebaseArchiveEntries_posix branch from 9ae153a to 14bda07 Compare July 15, 2026 13:32
@thaJeztah
thaJeztah requested a review from vvoland July 15, 2026 13:33
@thaJeztah

Copy link
Copy Markdown
Member Author

Heh; new test found a bug; I'll remove that test-case, and do a follow-up;

=== RUN   TestRebaseArchiveEntriesPlatformPaths/hardlink_rebase_from_root
    copy_test.go:104: assertion failed: prefix/link (tc.wantName string) != prefixlink (got.Name string)
    copy_test.go:105: assertion failed: prefix/target (tc.wantLinkName string) != prefixtarget (got.Linkname string)

This is mostly defense-in-depth; RebaseArchiveEntries is manipulating
the Tar headers, which use POSIX / Unix paths; convert the given paths
on the way in.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztah force-pushed the RebaseArchiveEntries_posix branch from 14bda07 to ef0bff1 Compare July 15, 2026 13:39
@vvoland
vvoland merged commit ea3e1e9 into moby:main Jul 15, 2026
12 checks passed
@thaJeztah
thaJeztah deleted the RebaseArchiveEntries_posix branch July 15, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants