Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: handle hardlinks #17

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0601fda
feat: handle hardlinks
zhijie-yang Jul 18, 2024
ffd9951
test: hard link against symlink
zhijie-yang Jul 18, 2024
85a4cf9
test: hard link testing for fsutil.create
zhijie-yang Jul 18, 2024
19fc4ff
test: make TreeDumpEntry handle hard link
zhijie-yang Jul 19, 2024
eefd744
test: combine into hackopt
zhijie-yang Jul 19, 2024
1237a08
test: extract all types of file
zhijie-yang Jul 19, 2024
bc02824
chore: add comments for hard link assumptions
zhijie-yang Jul 19, 2024
a8bd58e
test: comprehensive cases for hard link in create_test
zhijie-yang Jul 19, 2024
381f563
Revert "test: make TreeDumpEntry handle hard link"
zhijie-yang Jul 19, 2024
c8a7931
test: handle hard link test in createTest suite
zhijie-yang Jul 19, 2024
265c49e
chore: adjust error/test messages and test matching regex
zhijie-yang Jul 19, 2024
7b21a1e
chore: fix comments
zhijie-yang Jul 22, 2024
06ccac5
test: TreeDumpEntry to handle hard links
zhijie-yang Jul 22, 2024
4090b4c
fix: multiple adjusts according to code review
zhijie-yang Jul 23, 2024
d3a7921
chore: adjust comments for fsutil.Create
zhijie-yang Jul 23, 2024
a798078
feat: drop handling hard link in TreeDumpEntry
zhijie-yang Jul 23, 2024
2782eb8
Revert "feat: drop handling hard link in TreeDumpEntry"
zhijie-yang Jul 24, 2024
3e8d872
chore: change commits and test case summaries
zhijie-yang Jul 24, 2024
03ce4d3
chore: change variable naming
zhijie-yang Jul 24, 2024
5b29138
feat: add test for same hardlink in slicer_test
zhijie-yang Jul 24, 2024
0dca7a0
test: update duplicate hardlink in slicer_test
zhijie-yang Jul 25, 2024
acec1a7
test: treeDumpReport hardlink target uses abs path
zhijie-yang Jul 26, 2024
4313070
chore: adjust tree dump hardlink string
zhijie-yang Jul 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion internal/deb/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,22 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
}
}
// Create the entry itself.
link := tarHeader.Linkname
if tarHeader.Typeflag == tar.TypeLink {
// The hard link requires the real path of the target file.
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
// For a hard link, we assume:
// - the target file is already created (the target file should
// exist before the hard link in a valid tarball)
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
// - the hard link is identified in `fsutil.CreateOptions` as a
// regular file but with a non-empty `Link` field.
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
link = filepath.Join(options.TargetDir, link)
}

createOptions := &fsutil.CreateOptions{
Path: filepath.Join(options.TargetDir, targetPath),
Mode: tarHeader.FileInfo().Mode(),
Data: pathReader,
Link: tarHeader.Linkname,
Link: link,
MakeParents: true,
}
err := options.Create(extractInfos, createOptions)
Expand Down
78 changes: 78 additions & 0 deletions internal/deb/extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,84 @@ var extractTests = []extractTest{{
},
},
error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`,
}, {
summary: "Hard link is extracted",
pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(0755, "./"),
testutil.Reg(0644, "./file.txt", "text for file.txt"),
testutil.Hln(0644, "./link-to-file.txt", "./file.txt"),
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
}),
options: deb.ExtractOptions{
Extract: map[string][]deb.ExtractInfo{
"/**": []deb.ExtractInfo{{
Path: "/**",
}},
},
},
result: map[string]string{
"/file.txt": "file 0644 e940b71b",
"/link-to-file.txt": "file 0644 e940b71b",
},
notCreated: []string{},
}, {
summary: "Dangling hard link",
pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(0755, "./"),
// The file.txt is left out to create a dangling hard link link-to-file.txt
testutil.Hln(0644, "./link-to-file.txt", "./file.txt"),
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
}),
options: deb.ExtractOptions{
Extract: map[string][]deb.ExtractInfo{
"/**": []deb.ExtractInfo{{
Path: "/**",
}},
},
},
error: `cannot extract from package "test-package": the target file does not exist: .*\/file.txt`,
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
}, {
summary: "Hard link to symlink",
pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(0755, "./"),
testutil.Lnk(0644, "./symlink-to-file.txt", "./file.txt"),
testutil.Hln(0644, "./link-to-symlink.txt", "./symlink-to-file.txt"),
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
}),
options: deb.ExtractOptions{
Extract: map[string][]deb.ExtractInfo{
"/**": []deb.ExtractInfo{{
Path: "/**",
}},
},
},
result: map[string]string{
"/link-to-symlink.txt": "symlink ./file.txt",
"/symlink-to-file.txt": "symlink ./file.txt",
},
notCreated: []string{},
}, {
summary: "Extract all types of files",
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(0755, "./"),
testutil.Dir(0755, "./dir/"),
testutil.Reg(0644, "./dir/file.txt", "text for file.txt"),
testutil.Lnk(0644, "./symlink-to-file.txt", "./dir/file.txt"),
testutil.Hln(0644, "./link-to-file.txt", "./dir/file.txt"),
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
testutil.Hln(0644, "./link-to-symlink.txt", "./symlink-to-file.txt"),
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
}),
options: deb.ExtractOptions{
Extract: map[string][]deb.ExtractInfo{
"/**": []deb.ExtractInfo{{
Path: "/**",
}},
},
},
result: map[string]string{
"/dir/": "dir 0755",
"/dir/file.txt": "file 0644 e940b71b",
"/link-to-file.txt": "file 0644 e940b71b",
"/link-to-symlink.txt": "symlink ./dir/file.txt",
"/symlink-to-file.txt": "symlink ./dir/file.txt",
},
notCreated: []string{},
}}

func (s *S) TestExtract(c *C) {
Expand Down
33 changes: 31 additions & 2 deletions internal/fsutil/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,15 @@ func Create(options *CreateOptions) (*Entry, error) {

switch o.Mode & fs.ModeType {
case 0:
err = createFile(o)
hash = hex.EncodeToString(rp.h.Sum(nil))
if o.Link != "" {
// Creating the hard link does not occurs file reads. Therefore,
// its size and hash is not calculated here but in
// `testutil.TreeDumpEntry` for testing purpose instead.
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
err = createHardLink(o)
} else {
err = createFile(o)
hash = hex.EncodeToString(rp.h.Sum(nil))
}
case fs.ModeDir:
err = createDir(o)
case fs.ModeSymlink:
Expand Down Expand Up @@ -121,6 +128,28 @@ func createSymlink(o *CreateOptions) error {
return os.Symlink(o.Link, o.Path)
}

func createHardLink(o *CreateOptions) error {
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
debugf("Creating hard link: %s => %s", o.Path, o.Link)
targetInfo, err := os.Lstat(o.Link)
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
if err != nil && os.IsNotExist(err) {
return fmt.Errorf("the target file does not exist: %s", o.Link)
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
} else if err != nil {
return err
}

linkInfo, err := os.Lstat(o.Path)
if err == nil || os.IsExist(err) {
if os.SameFile(targetInfo, linkInfo) {
return nil
}
Copy link
Owner

Choose a reason for hiding this comment

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

I did some more tests and ln does not respect this, let's remove it. Probably previous comment about naming does not apply as we no longer need those variables.

Copy link
Author

Choose a reason for hiding this comment

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

Bookkeeping: As per the offline discussion, this has to exist since there can be different slices in one package that create the same hard link multiple times.

return fmt.Errorf("the link already exists: %s", o.Path)
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
} else if !os.IsNotExist(err) {
return err
}

return os.Link(o.Link, o.Path)
}

// readerProxy implements the io.Reader interface proxying the calls to its
// inner io.Reader. On each read, the proxy keeps track of the file size and hash.
type readerProxy struct {
Expand Down
97 changes: 84 additions & 13 deletions internal/fsutil/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

type createTest struct {
options fsutil.CreateOptions
hackdir func(c *C, dir string)
hackopt func(c *C, dir string, options *fsutil.CreateOptions)
result map[string]string
error string
}
Expand Down Expand Up @@ -72,7 +72,7 @@ var createTests = []createTest{{
Path: "foo",
Mode: fs.ModeDir | 0775,
},
hackdir: func(c *C, dir string) {
hackopt: func(c *C, dir string, options *fsutil.CreateOptions) {
c.Assert(os.Mkdir(filepath.Join(dir, "foo/"), fs.ModeDir|0765), IsNil)
},
result: map[string]string{
Expand All @@ -86,13 +86,74 @@ var createTests = []createTest{{
Mode: 0644,
Data: bytes.NewBufferString("changed"),
},
hackdir: func(c *C, dir string) {
hackopt: func(c *C, dir string, options *fsutil.CreateOptions) {
c.Assert(os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666), IsNil)
},
result: map[string]string{
// mode is not updated.
"/foo": "file 0666 d67e2e94",
},
}, {
// Create a hard link to `file`
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
options: fsutil.CreateOptions{
Path: "dir/link-to-file",
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
Link: "file",
Mode: 0644,
MakeParents: true,
},
hackopt: func(c *C, dir string, options *fsutil.CreateOptions) {
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0644), IsNil)
options.Link = filepath.Join(dir, options.Link)
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
},
result: map[string]string{
"/file": "file 0644 3a6eb079",
"/dir/": "dir 0755",
"/dir/link-to-file": "file 0644 3a6eb079",
},
}, {
// The target file for the hard link does not exist.
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
options: fsutil.CreateOptions{
Path: "dir/link-to-file",
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
Link: "file-no-exist",
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
Mode: 0644,
MakeParents: true,
},
hackopt: func(c *C, dir string, options *fsutil.CreateOptions) {
c.Assert(os.WriteFile(filepath.Join(dir, "file-exist"), []byte("data"), 0644), IsNil)
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
options.Link = filepath.Join(dir, options.Link)
},
error: `the target file does not exist: .*\/file-no-exist`,
}, {
// The hard link exists but is same as the target.
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
options: fsutil.CreateOptions{
Path: "link-to-file",
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
Link: "file",
Mode: 0644,
MakeParents: true,
},
hackopt: func(c *C, dir string, options *fsutil.CreateOptions) {
c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0644), IsNil)
c.Assert(os.Link(filepath.Join(dir, "file"), filepath.Join(dir, "link-to-file")), IsNil)
options.Link = filepath.Join(dir, options.Link)
},
result: map[string]string{
"/file": "file 0644 3a6eb079",
"/link-to-file": "file 0644 3a6eb079",
},
}, {
// The hard link exists but is not the same as the target.
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
options: fsutil.CreateOptions{
Path: "link-to-file",
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
Link: "file",
Mode: 0644,
MakeParents: true,
},
hackopt: func(c *C, dir string, options *fsutil.CreateOptions) {
c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0644), IsNil)
c.Assert(os.WriteFile(filepath.Join(dir, "link-to-file"), []byte("data"), 0644), IsNil)
options.Link = filepath.Join(dir, options.Link)
},
error: `the link already exists: .*\/link-to-file`,
}}

func (s *S) TestCreate(c *C) {
Expand All @@ -108,8 +169,8 @@ func (s *S) TestCreate(c *C) {
}
c.Logf("Options: %v", test.options)
dir := c.MkDir()
if test.hackdir != nil {
test.hackdir(c, dir)
if test.hackopt != nil {
test.hackopt(c, dir, &test.options)
}
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
options := test.options
options.Path = filepath.Join(dir, options.Path)
Expand All @@ -122,14 +183,24 @@ func (s *S) TestCreate(c *C) {

c.Assert(err, IsNil)
c.Assert(testutil.TreeDump(dir), DeepEquals, test.result)
// [fsutil.Create] does not return information about parent directories
// created implicitly. We only check for the requested path.
entry.Path = strings.TrimPrefix(entry.Path, dir)
// Add the slashes that TreeDump adds to the path.
slashPath := "/" + test.options.Path
if test.options.Mode.IsDir() {
slashPath = slashPath + "/"

if entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 {
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
// Entry is a hardlink.
pathInfo, err := os.Lstat(entry.Path)
c.Assert(err, IsNil)
linkInfo, err := os.Lstat(entry.Link)
c.Assert(err, IsNil)
os.SameFile(pathInfo, linkInfo)
} else {
// [fsutil.Create] does not return information about parent directories
// created implicitly. We only check for the requested path.
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
entry.Path = strings.TrimPrefix(entry.Path, dir)
// Add the slashes that TreeDump adds to the path.
slashPath := "/" + test.options.Path
if test.options.Mode.IsDir() {
slashPath = slashPath + "/"
}
c.Assert(testutil.TreeDumpEntry(entry), DeepEquals, test.result[slashPath])
}
c.Assert(testutil.TreeDumpEntry(entry), DeepEquals, test.result[slashPath])
}
}
13 changes: 13 additions & 0 deletions internal/testutil/pkgdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,16 @@ func Lnk(mode int64, path, target string) TarEntry {
},
}
}

// Hln is a shortcut for creating a hard link TarEntry structure (with
// tar.Typeflag set to tar.TypeLink). Hln stands for "Hard LiNk".
zhijie-yang marked this conversation as resolved.
Show resolved Hide resolved
func Hln(mode int64, path, target string) TarEntry {
return TarEntry{
Header: tar.Header{
Typeflag: tar.TypeLink,
Name: path,
Mode: mode,
Linkname: target,
},
}
}
Loading