Skip to content

ExportChanges: use POSIX / Unix conventions for Tar operations - #41

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

ExportChanges: use POSIX / Unix conventions for Tar operations#41
thaJeztah merged 1 commit into
moby:mainfrom
thaJeztah:ExportChanges_posix

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

The Change.Path field holds a local path, but it was used to set the Tar.Name field.

Convert the path to a POSIX / Unix path before setting. Also explicitly convert the archive-path to a POSIX path when calling addTarFile, and explicitly strip a leading / (if present), instead of the first character.

The Change.Path field holds a local path, but it was used to set
the Tar.Name field.

Convert the path to a POSIX / Unix path before setting. Also explicitly
convert the archive-path to a POSIX path when calling addTarFile, and
explicitly strip a leading `/` (if present), instead of the first
character.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter

codecov-commenter commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.07%. Comparing base (2cd730e) to head (d108a5d).
⚠️ Report is 41 commits behind head on main.

Files with missing lines Patch % Lines
changes.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
- Coverage   66.35%   65.07%   -1.28%     
==========================================
  Files          42       42              
  Lines        2027     2033       +6     
==========================================
- Hits         1345     1323      -22     
- Misses        497      535      +38     
+ Partials      185      175      -10     

☔ 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.

Comment thread changes.go
path := filepath.Join(dir, change.Path)
if err := ta.addTarFile(path, change.Path[1:]); err != nil {
log.G(context.TODO()).Debugf("Can't add file %s to tar: %s", path, err)
srcPath := filepath.Join(dir, change.Path)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

GoDoc says;

// ExportChanges produces an Archive from the provided changes, relative to dir.

So ... relative to dir, but not sure if that should also mean "outside of dir", but that's something for a follow-up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Moby code uses it like this, so yeah, probably expected to be within dir

	parentFs, err := driver.Get(parent, "")
	if err != nil {
		return nil, err
	}
	defer driver.Put(parent)

	changes, err := archive.ChangesDirs(layerFs, parentFs)
	if err != nil {
		return nil, err
	}

	tarArchive, err := archive.ExportChanges(layerFs, changes, gdw.IDMap)
	if err != nil {
		return nil, err
	}

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 fixes tar archive path handling in ExportChanges by ensuring paths written into tar headers use POSIX (forward-slash) conventions and don’t start with an absolute /, which can be problematic for tar consumers and is especially incorrect for Windows-style paths.

Changes:

  • Normalize whiteout tar header names using filepath.ToSlash and strings.TrimPrefix(..., "/") instead of slicing off the first byte.
  • Compute a POSIX archivePath for addTarFile to ensure hardlink bookkeeping (SeenFiles / Linkname) uses consistent forward-slash paths.
  • Replace ambiguous path variable naming with clearer srcPath and archivePath.

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

@thaJeztah
thaJeztah merged commit b321c32 into moby:main Jul 15, 2026
12 checks passed
@thaJeztah
thaJeztah deleted the ExportChanges_posix branch July 15, 2026 14:26
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