Skip to content

archive: replace tarName() with filepath.ToSlash()#7344

Merged
kzys merged 1 commit intocontainerd:mainfrom
thaJeztah:archive_cleanup
Aug 30, 2022
Merged

archive: replace tarName() with filepath.ToSlash()#7344
kzys merged 1 commit intocontainerd:mainfrom
thaJeztah:archive_cleanup

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

This code was copied from github.com/moby/moby/pkg/archive; moby/moby@28842d3, which got later simplified in
moby/moby@a5aed69

This patch aligns the containerd implementation with those changes, and uses filepath.ToSlash() unconditionally on all platforms, as it's a no-op on platforms that use a forward-slash; https://github.com/golang/go/blob/go1.19/src/path/filepath/path.go#L175-L183

This code was copied from github.com/moby/moby/pkg/archive;
moby/moby@28842d3,
which got later simplified in
moby/moby@a5aed69

This patch aligns the containerd implementation with those changes, and uses
filepath.ToSlash() unconditionally on all platforms, as it's a no-op on platforms
that use a forward-slash; https://github.com/golang/go/blob/go1.19/src/path/filepath/path.go#L175-L183

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Comment thread archive/tar_windows.go
Comment on lines -33 to -39
// windows: convert windows style relative path with backslashes
// into forward slashes. Since windows does not allow '/' or '\'
// in file names, it is mostly safe to replace however we must
// check just in case
if strings.Contains(p, "/") {
return "", fmt.Errorf("windows path contains forward slash: %s", p)
}
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.

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@kzys kzys merged commit 1d6d5b4 into containerd:main Aug 30, 2022
@thaJeztah thaJeztah deleted the archive_cleanup branch August 30, 2022 16:49
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.

3 participants