From 0601fda18471ef9ca932bd8296d82fbe5ac76a6a Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Thu, 18 Jul 2024 14:44:18 +0200 Subject: [PATCH 01/23] feat: handle hardlinks --- internal/deb/extract.go | 8 +++++++- internal/deb/extract_test.go | 34 ++++++++++++++++++++++++++++++++++ internal/fsutil/create.go | 30 ++++++++++++++++++++++++++++-- internal/testutil/pkgdata.go | 13 +++++++++++++ 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 07f219bd..6cb67c1c 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -246,11 +246,17 @@ 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. + 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) diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 22a1fd18..91d8e9f3 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -352,6 +352,40 @@ var extractTests = []extractTest{{ }, }, error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`, +}, { + summary: "Hard link must be created if specified in the tarball", + pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file1.txt", "text for file1.txt"), + testutil.Hln(0644, "./file2.txt", "./file1.txt"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/*.txt": []deb.ExtractInfo{{ + Path: "/*.txt", + }}, + }, + }, + result: map[string]string{ + "/file1.txt": "file 0644 e926a7fb", + "/file2.txt": "file 0644 e926a7fb", + }, + notCreated: []string{}, +}, { + summary: "Dangling hard link must raise an error", + pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + // testutil.Reg(0644, "./file1.txt", "text for file1.txt"), + testutil.Hln(0644, "./file2.txt", "./file1.txt"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/*.txt": []deb.ExtractInfo{{ + Path: "/*.txt", + }}, + }, + }, + error: `cannot extract from package "test-package": the target file does not exist: .*/file1.txt`, }} func (s *S) TestExtract(c *C) { diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 48b12cb7..f135eef5 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -48,8 +48,12 @@ 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 != "" { + err = createHardLink(o) + } else { + err = createFile(o) + hash = hex.EncodeToString(rp.h.Sum(nil)) + } case fs.ModeDir: err = createDir(o) case fs.ModeSymlink: @@ -121,6 +125,28 @@ func createSymlink(o *CreateOptions) error { return os.Symlink(o.Link, o.Path) } +func createHardLink(o *CreateOptions) error { + debugf("Creating hard link: %s => %s", o.Path, o.Link) + targetInfo, err := os.Lstat(o.Link) + if err != nil && os.IsNotExist(err) { + return fmt.Errorf("the target file does not exist: %s", o.Link) + } 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 + } + return fmt.Errorf("the link already exists: %s", o.Path) + } 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 { diff --git a/internal/testutil/pkgdata.go b/internal/testutil/pkgdata.go index 11bc9028..d151cd6c 100644 --- a/internal/testutil/pkgdata.go +++ b/internal/testutil/pkgdata.go @@ -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". +func Hln(mode int64, path, target string) TarEntry { + return TarEntry{ + Header: tar.Header{ + Typeflag: tar.TypeLink, + Name: path, + Mode: mode, + Linkname: target, + }, + } +} From ffd99512f315c63893ae49055f3ad630d6238dd6 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Thu, 18 Jul 2024 17:13:09 +0200 Subject: [PATCH 02/23] test: hard link against symlink --- internal/deb/extract_test.go | 45 +++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 91d8e9f3..57c9b18a 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -353,39 +353,58 @@ var extractTests = []extractTest{{ }, error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`, }, { - summary: "Hard link must be created if specified in the tarball", + summary: "Hard link is extracted", pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file1.txt", "text for file1.txt"), - testutil.Hln(0644, "./file2.txt", "./file1.txt"), + testutil.Reg(0644, "./file.txt", "text for file.txt"), + testutil.Hln(0644, "./link-to-file.txt", "./file.txt"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ - "/*.txt": []deb.ExtractInfo{{ - Path: "/*.txt", + "/**": []deb.ExtractInfo{{ + Path: "/**", }}, }, }, result: map[string]string{ - "/file1.txt": "file 0644 e926a7fb", - "/file2.txt": "file 0644 e926a7fb", + "/file.txt": "file 0644 e940b71b", + "/link-to-file.txt": "file 0644 e940b71b", }, notCreated: []string{}, }, { - summary: "Dangling hard link must raise an error", + summary: "Dangling hard link", pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), - // testutil.Reg(0644, "./file1.txt", "text for file1.txt"), - testutil.Hln(0644, "./file2.txt", "./file1.txt"), + // 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"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ - "/*.txt": []deb.ExtractInfo{{ - Path: "/*.txt", + "/**": []deb.ExtractInfo{{ + Path: "/**", }}, }, }, - error: `cannot extract from package "test-package": the target file does not exist: .*/file1.txt`, + error: `cannot extract from package "test-package": the target file does not exist: .*\/file.txt`, +}, { + 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-file.txt", "./symlink-to-file.txt"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/**": []deb.ExtractInfo{{ + Path: "/**", + }}, + }, + }, + result: map[string]string{ + "/link-to-file.txt": "symlink ./file.txt", + "/symlink-to-file.txt": "symlink ./file.txt", + }, + notCreated: []string{}, }} func (s *S) TestExtract(c *C) { From 85a4cf90cc99950c89fafb834518cf97f07b6a56 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Thu, 18 Jul 2024 18:11:36 +0200 Subject: [PATCH 03/23] test: hard link testing for fsutil.create --- internal/fsutil/create_test.go | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 7288d878..4f97c5c4 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -15,10 +15,11 @@ import ( ) type createTest struct { - options fsutil.CreateOptions - hackdir func(c *C, dir string) - result map[string]string - error string + options fsutil.CreateOptions + hackdir func(c *C, dir string) + hackLinkPathPrefix func(c *C, dir string, options *fsutil.CreateOptions) + result map[string]string + error string } var createTests = []createTest{{ @@ -93,6 +94,24 @@ var createTests = []createTest{{ // mode is not updated. "/foo": "file 0666 d67e2e94", }, +}, { + options: fsutil.CreateOptions{ + Path: "dir/link-to-file", + Link: "file", + Mode: 0644, + MakeParents: true, + }, + hackdir: func(c *C, dir string) { + c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0644), IsNil) + }, + hackLinkPathPrefix: func(c *C, dir string, options *fsutil.CreateOptions) { + options.Link = filepath.Join(dir, options.Link) + }, + result: map[string]string{ + "/file": "file 0644 3a6eb079", + "/dir/": "dir 0755", + "/dir/link-to-file": "file 0644 3a6eb079", + }, }} func (s *S) TestCreate(c *C) { @@ -111,6 +130,9 @@ func (s *S) TestCreate(c *C) { if test.hackdir != nil { test.hackdir(c, dir) } + if test.hackLinkPathPrefix != nil { + test.hackLinkPathPrefix(c, dir, &test.options) + } options := test.options options.Path = filepath.Join(dir, options.Path) entry, err := fsutil.Create(&options) From 19fc4ff868e8bfd327c3ff8a4635ef4c8fc76070 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 10:42:36 +0200 Subject: [PATCH 04/23] test: make TreeDumpEntry handle hard link --- internal/testutil/treedump.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index 26aa164d..03633c7a 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -74,7 +74,17 @@ func TreeDumpEntry(entry *fsutil.Entry) string { return fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: return fmt.Sprintf("symlink %s", entry.Link) - case 0: // Regular + case 0: + // Hard link + if entry.Link != "" { + data, err := os.ReadFile(entry.Link) + if err != nil { + panic(err) + } + entry.Size = len(data) + entry.Hash = fmt.Sprintf("%.4x", sha256.Sum256(data)) + } + // Regular if entry.Size == 0 { return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) } else { From eefd7447c843615120cddcfd890c511a777e2cf3 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 10:45:34 +0200 Subject: [PATCH 05/23] test: combine into hackopt --- internal/fsutil/create_test.go | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 4f97c5c4..e2feb189 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -15,11 +15,10 @@ import ( ) type createTest struct { - options fsutil.CreateOptions - hackdir func(c *C, dir string) - hackLinkPathPrefix func(c *C, dir string, options *fsutil.CreateOptions) - result map[string]string - error string + options fsutil.CreateOptions + hackopt func(c *C, dir string, options *fsutil.CreateOptions) + result map[string]string + error string } var createTests = []createTest{{ @@ -73,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{ @@ -87,7 +86,7 @@ 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{ @@ -101,10 +100,8 @@ var createTests = []createTest{{ Mode: 0644, MakeParents: true, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, options *fsutil.CreateOptions) { c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0644), IsNil) - }, - hackLinkPathPrefix: func(c *C, dir string, options *fsutil.CreateOptions) { options.Link = filepath.Join(dir, options.Link) }, result: map[string]string{ @@ -127,11 +124,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.hackLinkPathPrefix != nil { - test.hackLinkPathPrefix(c, dir, &test.options) + if test.hackopt != nil { + test.hackopt(c, dir, &test.options) } options := test.options options.Path = filepath.Join(dir, options.Path) From 1237a08b3858690d5c8798992905f5e8f8df3426 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 10:52:25 +0200 Subject: [PATCH 06/23] test: extract all types of file --- internal/deb/extract_test.go | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 57c9b18a..14175616 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -391,7 +391,7 @@ var extractTests = []extractTest{{ pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Lnk(0644, "./symlink-to-file.txt", "./file.txt"), - testutil.Hln(0644, "./link-to-file.txt", "./symlink-to-file.txt"), + testutil.Hln(0644, "./link-to-symlink.txt", "./symlink-to-file.txt"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -401,10 +401,35 @@ var extractTests = []extractTest{{ }, }, result: map[string]string{ - "/link-to-file.txt": "symlink ./file.txt", + "/link-to-symlink.txt": "symlink ./file.txt", "/symlink-to-file.txt": "symlink ./file.txt", }, notCreated: []string{}, +}, { + summary: "Extract all types of files", + 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"), + testutil.Hln(0644, "./link-to-symlink.txt", "./symlink-to-file.txt"), + }), + 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) { From bc02824abcc83ecf4704e7b700a50c0b29a5bf41 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 11:00:57 +0200 Subject: [PATCH 07/23] chore: add comments for hard link assumptions --- internal/deb/extract.go | 5 +++++ internal/fsutil/create.go | 3 +++ internal/fsutil/create_test.go | 1 + 3 files changed, 9 insertions(+) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 6cb67c1c..b7197106 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -249,6 +249,11 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error { link := tarHeader.Linkname if tarHeader.Typeflag == tar.TypeLink { // The hard link requires the real path of the target file. + // 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) + // - the hard link is identified in `fsutil.CreateOptions` as a + // regular file but with a non-empty `Link` field. link = filepath.Join(options.TargetDir, link) } diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index f135eef5..06db4a96 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -49,6 +49,9 @@ func Create(options *CreateOptions) (*Entry, error) { switch o.Mode & fs.ModeType { case 0: 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. err = createHardLink(o) } else { err = createFile(o) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index e2feb189..5f4a3e86 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -94,6 +94,7 @@ var createTests = []createTest{{ "/foo": "file 0666 d67e2e94", }, }, { + // Create a hard link to `file` options: fsutil.CreateOptions{ Path: "dir/link-to-file", Link: "file", From a8bd58e14f97611cbda8d3bb02f7494c2c25daa5 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 11:39:55 +0200 Subject: [PATCH 08/23] test: comprehensive cases for hard link in create_test --- internal/fsutil/create_test.go | 44 ++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 5f4a3e86..95859bc1 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -110,6 +110,50 @@ var createTests = []createTest{{ "/dir/": "dir 0755", "/dir/link-to-file": "file 0644 3a6eb079", }, +}, { + // The target file for the hard link does not exist. + options: fsutil.CreateOptions{ + Path: "dir/link-to-file", + Link: "file-no-exist", + 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) + 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. + options: fsutil.CreateOptions{ + Path: "link-to-file", + 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. + options: fsutil.CreateOptions{ + Path: "link-to-file", + 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) { From 381f563b570bf2ff45523329e44e440435b4c76e Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 12:53:57 +0200 Subject: [PATCH 09/23] Revert "test: make TreeDumpEntry handle hard link" This reverts commit 19fc4ff868e8bfd327c3ff8a4635ef4c8fc76070. --- internal/testutil/treedump.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index 03633c7a..26aa164d 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -74,17 +74,7 @@ func TreeDumpEntry(entry *fsutil.Entry) string { return fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: return fmt.Sprintf("symlink %s", entry.Link) - case 0: - // Hard link - if entry.Link != "" { - data, err := os.ReadFile(entry.Link) - if err != nil { - panic(err) - } - entry.Size = len(data) - entry.Hash = fmt.Sprintf("%.4x", sha256.Sum256(data)) - } - // Regular + case 0: // Regular if entry.Size == 0 { return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) } else { From c8a79317db2984e19b09b5386932d03224de9de2 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 15:06:22 +0200 Subject: [PATCH 10/23] test: handle hard link test in createTest suite --- internal/fsutil/create_test.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 95859bc1..c30263d7 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -183,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 { + // 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. + 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]) } } From 265c49eb0aef1b33938c75a2880fb070ddfb5360 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 19 Jul 2024 17:42:28 +0200 Subject: [PATCH 11/23] chore: adjust error/test messages and test matching regex --- internal/deb/extract_test.go | 50 ++++++++-------------------- internal/fsutil/create.go | 4 +-- internal/fsutil/create_test.go | 61 +++++++++++++++++++--------------- 3 files changed, 51 insertions(+), 64 deletions(-) diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 14175616..beb474c7 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -352,31 +352,11 @@ 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"), - }), - 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"), + testutil.Hln(0644, "./link", "./non-existing-target"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -385,13 +365,13 @@ var extractTests = []extractTest{{ }}, }, }, - error: `cannot extract from package "test-package": the target file does not exist: .*\/file.txt`, + error: `cannot extract from package "test-package": link target does not exist: \/[^ ]*\/non-existing-target`, }, { - summary: "Hard link to symlink", + summary: "Hard link to symlink does not follow 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"), + testutil.Lnk(0644, "./symlink-to-file", "./file"), + testutil.Hln(0644, "./link-to-symlink", "./symlink-to-file"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -401,8 +381,8 @@ var extractTests = []extractTest{{ }, }, result: map[string]string{ - "/link-to-symlink.txt": "symlink ./file.txt", - "/symlink-to-file.txt": "symlink ./file.txt", + "/link-to-symlink": "symlink ./file", + "/symlink-to-file": "symlink ./file", }, notCreated: []string{}, }, { @@ -410,10 +390,9 @@ var extractTests = []extractTest{{ 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"), - testutil.Hln(0644, "./link-to-symlink.txt", "./symlink-to-file.txt"), + testutil.Reg(0644, "./dir/file", "text for file"), + testutil.Lnk(0644, "./symlink-to-file", "./dir/file"), + testutil.Hln(0644, "./link-to-file", "./dir/file"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -423,11 +402,10 @@ var extractTests = []extractTest{{ }, }, 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", + "/dir/": "dir 0755", + "/dir/file": "file 0644 28121945", + "/link-to-file": "file 0644 28121945", + "/symlink-to-file": "symlink ./dir/file", }, notCreated: []string{}, }} diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 06db4a96..2c66dbca 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -132,7 +132,7 @@ func createHardLink(o *CreateOptions) error { debugf("Creating hard link: %s => %s", o.Path, o.Link) targetInfo, err := os.Lstat(o.Link) if err != nil && os.IsNotExist(err) { - return fmt.Errorf("the target file does not exist: %s", o.Link) + return fmt.Errorf("link target does not exist: %s", o.Link) } else if err != nil { return err } @@ -142,7 +142,7 @@ func createHardLink(o *CreateOptions) error { if os.SameFile(targetInfo, linkInfo) { return nil } - return fmt.Errorf("the link already exists: %s", o.Path) + return fmt.Errorf("path %s already exists", o.Path) } else if !os.IsNotExist(err) { return err } diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index c30263d7..db1a56e3 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -15,13 +15,15 @@ import ( ) type createTest struct { + summary string options fsutil.CreateOptions - hackopt func(c *C, dir string, options *fsutil.CreateOptions) + hackopt func(c *C, targetDir string, options *fsutil.CreateOptions) result map[string]string error string } var createTests = []createTest{{ + summary: "Create a file under a new directory", options: fsutil.CreateOptions{ Path: "foo/bar", Data: bytes.NewBufferString("data1"), @@ -33,6 +35,7 @@ var createTests = []createTest{{ "/foo/bar": "file 0444 5b41362b", }, }, { + summary: "Create a symlink", options: fsutil.CreateOptions{ Path: "foo/bar", Link: "../baz", @@ -44,6 +47,7 @@ var createTests = []createTest{{ "/foo/bar": "symlink ../baz", }, }, { + summary: "Create a subdirectory", options: fsutil.CreateOptions{ Path: "foo/bar", Mode: fs.ModeDir | 0444, @@ -54,6 +58,7 @@ var createTests = []createTest{{ "/foo/bar/": "dir 0444", }, }, { + summary: "Create a directory with sticky bit", options: fsutil.CreateOptions{ Path: "tmp", Mode: fs.ModeDir | fs.ModeSticky | 0775, @@ -62,48 +67,52 @@ var createTests = []createTest{{ "/tmp/": "dir 01775", }, }, { + summary: "Cannot create a subdirectory without `MakeParents` set", options: fsutil.CreateOptions{ Path: "foo/bar", Mode: fs.ModeDir | 0775, }, - error: `.*: no such file or directory`, + error: `mkdir \/[^ ]*\/foo/bar: no such file or directory`, }, { + summary: "Re-creating an existing directory keeps the original mode", options: fsutil.CreateOptions{ Path: "foo", Mode: fs.ModeDir | 0775, }, - hackopt: func(c *C, dir string, options *fsutil.CreateOptions) { - c.Assert(os.Mkdir(filepath.Join(dir, "foo/"), fs.ModeDir|0765), IsNil) + hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { + c.Assert(os.Mkdir(filepath.Join(targetDir, "foo/"), fs.ModeDir|0765), IsNil) }, result: map[string]string{ // mode is not updated. "/foo/": "dir 0765", }, }, { + summary: "Re-creating an existing file keeps the original mode", options: fsutil.CreateOptions{ Path: "foo", // Mode should be ignored for existing entry. Mode: 0644, Data: bytes.NewBufferString("changed"), }, - hackopt: func(c *C, dir string, options *fsutil.CreateOptions) { - c.Assert(os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666), IsNil) + hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { + c.Assert(os.WriteFile(filepath.Join(targetDir, "foo"), []byte("data"), 0666), IsNil) }, result: map[string]string{ // mode is not updated. "/foo": "file 0666 d67e2e94", }, }, { - // Create a hard link to `file` + summary: "Create a hard link under to a file", options: fsutil.CreateOptions{ Path: "dir/link-to-file", 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) - options.Link = filepath.Join(dir, options.Link) + hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { + c.Assert(os.WriteFile(filepath.Join(targetDir, "file"), []byte("data"), 0644), IsNil) + // An absolute path is required to create a hard link. + options.Link = filepath.Join(targetDir, options.Link) }, result: map[string]string{ "/file": "file 0644 3a6eb079", @@ -111,49 +120,48 @@ var createTests = []createTest{{ "/dir/link-to-file": "file 0644 3a6eb079", }, }, { - // The target file for the hard link does not exist. + summary: "Cannot create a hard link if the link target does not exist", options: fsutil.CreateOptions{ Path: "dir/link-to-file", Link: "file-no-exist", 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) - options.Link = filepath.Join(dir, options.Link) + hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { + options.Link = filepath.Join(targetDir, options.Link) }, - error: `the target file does not exist: .*\/file-no-exist`, + error: `link target does not exist: \/[^ ]*\/file-no-exist`, }, { - // The hard link exists but is same as the target. + summary: "Re-creating a duplicated hard link keeps the original link", options: fsutil.CreateOptions{ Path: "link-to-file", 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) + hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { + c.Assert(os.WriteFile(filepath.Join(targetDir, "file"), []byte("data"), 0644), IsNil) + c.Assert(os.Link(filepath.Join(targetDir, "file"), filepath.Join(targetDir, "link-to-file")), IsNil) + options.Link = filepath.Join(targetDir, 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. + summary: "Cannot create a hard link if the link path exists and it is not a hard link to the target", options: fsutil.CreateOptions{ Path: "link-to-file", 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) + hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { + c.Assert(os.WriteFile(filepath.Join(targetDir, "file"), []byte("data"), 0644), IsNil) + c.Assert(os.WriteFile(filepath.Join(targetDir, "link-to-file"), []byte("data"), 0644), IsNil) + options.Link = filepath.Join(targetDir, options.Link) }, - error: `the link already exists: .*\/link-to-file`, + error: `path \/[^ ]*\/link-to-file already exists`, }} func (s *S) TestCreate(c *C) { @@ -163,6 +171,7 @@ func (s *S) TestCreate(c *C) { }() for _, test := range createTests { + c.Logf("Test: %s", test.summary) if test.result == nil { // Empty map for no files created. test.result = make(map[string]string) From 7b21a1e972fb05d70f6ac5a66093f74dd16693a9 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Mon, 22 Jul 2024 14:10:07 +0200 Subject: [PATCH 12/23] chore: fix comments --- internal/deb/extract.go | 5 ----- internal/fsutil/create.go | 3 ++- internal/fsutil/create_test.go | 4 ++-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index b7197106..6cb67c1c 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -249,11 +249,6 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error { link := tarHeader.Linkname if tarHeader.Typeflag == tar.TypeLink { // The hard link requires the real path of the target file. - // 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) - // - the hard link is identified in `fsutil.CreateOptions` as a - // regular file but with a non-empty `Link` field. link = filepath.Join(options.TargetDir, link) } diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 2c66dbca..e88e32f3 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -30,7 +30,8 @@ type Entry struct { } // Create creates a filesystem entry according to the provided options and returns -// the information about the created entry. +// the information about the created entry. Create treats a regular file with a +// non-empty `Link` field as a hard link. func Create(options *CreateOptions) (*Entry, error) { rp := &readerProxy{inner: options.Data, h: sha256.New()} // Use the proxy instead of the raw Reader. diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index db1a56e3..3b082183 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -193,6 +193,8 @@ 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. if entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 { // Entry is a hardlink. pathInfo, err := os.Lstat(entry.Path) @@ -201,8 +203,6 @@ func (s *S) TestCreate(c *C) { 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. entry.Path = strings.TrimPrefix(entry.Path, dir) // Add the slashes that TreeDump adds to the path. slashPath := "/" + test.options.Path From 06ccac53a0ddcdd7f4ef2ea6f39cdcb325eeed36 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Mon, 22 Jul 2024 16:41:01 +0200 Subject: [PATCH 13/23] test: TreeDumpEntry to handle hard links --- internal/fsutil/create_test.go | 17 ++++++++++++----- internal/testutil/treedump.go | 7 ++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 3b082183..9f6a3822 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -2,6 +2,7 @@ package fsutil_test import ( "bytes" + "fmt" "io/fs" "os" "path/filepath" @@ -193,17 +194,23 @@ 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. - if entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 { - // Entry is a hardlink. + isHardLink := entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 + // Special case for hard links: use the same file info to compare the + // path and the link. + if isHardLink { pathInfo, err := os.Lstat(entry.Path) c.Assert(err, IsNil) linkInfo, err := os.Lstat(entry.Link) c.Assert(err, IsNil) os.SameFile(pathInfo, linkInfo) + } + // [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) + if isHardLink { + result := fmt.Sprintf("hard link %#o %s", entry.Mode.Perm(), entry.Link) + c.Assert(testutil.TreeDumpEntry(entry), DeepEquals, result) } else { - 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() { diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index 26aa164d..c08dea03 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -74,7 +74,12 @@ func TreeDumpEntry(entry *fsutil.Entry) string { return fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: return fmt.Sprintf("symlink %s", entry.Link) - case 0: // Regular + case 0: + // Hard link + if entry.Link != "" { + return fmt.Sprintf("hard link %#o %s", fperm, entry.Link) + } + // Regular file if entry.Size == 0 { return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) } else { From 4090b4c4c27eda1bd386bb1757572a4529887ad9 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Tue, 23 Jul 2024 15:44:15 +0200 Subject: [PATCH 14/23] fix: multiple adjusts according to code review --- internal/fsutil/create.go | 7 ++++--- internal/fsutil/create_test.go | 28 ++++++++++------------------ internal/testutil/treedump.go | 4 ++-- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index e88e32f3..0dd96bce 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -15,6 +15,8 @@ type CreateOptions struct { Path string Mode fs.FileMode Data io.Reader + // If Link is set and the symlink flag is set in Mode, a symlink is + // created. Otherwise, a hard link is created. Link string // If MakeParents is true, missing parent directories of Path are // created with permissions 0755. @@ -50,9 +52,8 @@ func Create(options *CreateOptions) (*Entry, error) { switch o.Mode & fs.ModeType { case 0: 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. + // Creating the hard link does not occurs file reads. + // Therefore, its size and hash is not calculated here. err = createHardLink(o) } else { err = createFile(o) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 9f6a3822..bde915e4 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -2,7 +2,6 @@ package fsutil_test import ( "bytes" - "fmt" "io/fs" "os" "path/filepath" @@ -24,7 +23,7 @@ type createTest struct { } var createTests = []createTest{{ - summary: "Create a file under a new directory", + summary: "Create a file and its parent directory", options: fsutil.CreateOptions{ Path: "foo/bar", Data: bytes.NewBufferString("data1"), @@ -68,7 +67,7 @@ var createTests = []createTest{{ "/tmp/": "dir 01775", }, }, { - summary: "Cannot create a subdirectory without `MakeParents` set", + summary: "Cannot create a parent directory without MakeParents set", options: fsutil.CreateOptions{ Path: "foo/bar", Mode: fs.ModeDir | 0775, @@ -103,7 +102,7 @@ var createTests = []createTest{{ "/foo": "file 0666 d67e2e94", }, }, { - summary: "Create a hard link under to a file", + summary: "Create a hard link", options: fsutil.CreateOptions{ Path: "dir/link-to-file", Link: "file", @@ -179,11 +178,11 @@ func (s *S) TestCreate(c *C) { } c.Logf("Options: %v", test.options) dir := c.MkDir() - if test.hackopt != nil { - test.hackopt(c, dir, &test.options) - } options := test.options options.Path = filepath.Join(dir, options.Path) + if test.hackopt != nil { + test.hackopt(c, dir, &options) + } entry, err := fsutil.Create(&options) if test.error != "" { @@ -194,23 +193,16 @@ func (s *S) TestCreate(c *C) { c.Assert(err, IsNil) c.Assert(testutil.TreeDump(dir), DeepEquals, test.result) - isHardLink := entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 - // Special case for hard links: use the same file info to compare the - // path and the link. - if isHardLink { + // [fsutil.Create] does not return information about parent directories + // created implicitly. We only check for the requested path. + if entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 { pathInfo, err := os.Lstat(entry.Path) c.Assert(err, IsNil) linkInfo, err := os.Lstat(entry.Link) c.Assert(err, IsNil) os.SameFile(pathInfo, linkInfo) - } - // [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) - if isHardLink { - result := fmt.Sprintf("hard link %#o %s", entry.Mode.Perm(), entry.Link) - c.Assert(testutil.TreeDumpEntry(entry), DeepEquals, result) } else { + 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() { diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index c08dea03..dad3677a 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -75,11 +75,11 @@ func TreeDumpEntry(entry *fsutil.Entry) string { case fs.ModeSymlink: return fmt.Sprintf("symlink %s", entry.Link) case 0: - // Hard link + // Hard link. if entry.Link != "" { return fmt.Sprintf("hard link %#o %s", fperm, entry.Link) } - // Regular file + // Regular file. if entry.Size == 0 { return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) } else { From d3a7921df16337e984ea7fb89629d41eedc8da1d Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Tue, 23 Jul 2024 15:49:47 +0200 Subject: [PATCH 15/23] chore: adjust comments for fsutil.Create --- internal/fsutil/create.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 0dd96bce..1dd36407 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -32,8 +32,7 @@ type Entry struct { } // Create creates a filesystem entry according to the provided options and returns -// the information about the created entry. Create treats a regular file with a -// non-empty `Link` field as a hard link. +// the information about the created entry. func Create(options *CreateOptions) (*Entry, error) { rp := &readerProxy{inner: options.Data, h: sha256.New()} // Use the proxy instead of the raw Reader. From a798078fc8efac78c93ff3ded27a863a427277c4 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Tue, 23 Jul 2024 16:14:43 +0200 Subject: [PATCH 16/23] feat: drop handling hard link in TreeDumpEntry --- internal/testutil/treedump.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index dad3677a..90989c11 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -74,12 +74,7 @@ func TreeDumpEntry(entry *fsutil.Entry) string { return fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: return fmt.Sprintf("symlink %s", entry.Link) - case 0: - // Hard link. - if entry.Link != "" { - return fmt.Sprintf("hard link %#o %s", fperm, entry.Link) - } - // Regular file. + case 0: // Regular file. if entry.Size == 0 { return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) } else { From 2782eb811669f4b5fca578fe903f3f4c43c41ecd Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Wed, 24 Jul 2024 10:40:59 +0200 Subject: [PATCH 17/23] Revert "feat: drop handling hard link in TreeDumpEntry" This reverts commit a798078fc8efac78c93ff3ded27a863a427277c4. --- internal/testutil/treedump.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index 90989c11..dad3677a 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -74,7 +74,12 @@ func TreeDumpEntry(entry *fsutil.Entry) string { return fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: return fmt.Sprintf("symlink %s", entry.Link) - case 0: // Regular file. + case 0: + // Hard link. + if entry.Link != "" { + return fmt.Sprintf("hard link %#o %s", fperm, entry.Link) + } + // Regular file. if entry.Size == 0 { return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) } else { From 3e8d872c21ef3ed81e75c5319a052f872feb8b25 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Wed, 24 Jul 2024 11:15:04 +0200 Subject: [PATCH 18/23] chore: change commits and test case summaries --- internal/fsutil/create.go | 5 +++-- internal/fsutil/create_test.go | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 1dd36407..b4922adf 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -16,7 +16,8 @@ type CreateOptions struct { Mode fs.FileMode Data io.Reader // If Link is set and the symlink flag is set in Mode, a symlink is - // created. Otherwise, a hard link is created. + // created. If the Mode is not set to symlink, a hard link is created + // instead. Link string // If MakeParents is true, missing parent directories of Path are // created with permissions 0755. @@ -51,7 +52,7 @@ func Create(options *CreateOptions) (*Entry, error) { switch o.Mode & fs.ModeType { case 0: if o.Link != "" { - // Creating the hard link does not occurs file reads. + // Creating the hard link does not involve reading the file. // Therefore, its size and hash is not calculated here. err = createHardLink(o) } else { diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index bde915e4..481808c3 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -47,7 +47,7 @@ var createTests = []createTest{{ "/foo/bar": "symlink ../baz", }, }, { - summary: "Create a subdirectory", + summary: "Create a directory", options: fsutil.CreateOptions{ Path: "foo/bar", Mode: fs.ModeDir | 0444, @@ -196,6 +196,7 @@ func (s *S) TestCreate(c *C) { // [fsutil.Create] does not return information about parent directories // created implicitly. We only check for the requested path. if entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 { + // Entry is a hard link. pathInfo, err := os.Lstat(entry.Path) c.Assert(err, IsNil) linkInfo, err := os.Lstat(entry.Link) From 03ce4d322ccaff4f6ba8c00a32aee039de12c9d3 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Wed, 24 Jul 2024 14:23:32 +0200 Subject: [PATCH 19/23] chore: change variable naming --- internal/deb/extract.go | 2 +- internal/deb/extract_test.go | 20 ++++++++++---------- internal/fsutil/create.go | 6 +++--- internal/fsutil/create_test.go | 28 ++++++++++++++-------------- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 6cb67c1c..406c1991 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -248,7 +248,7 @@ 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. + // A hard link requires the real path of the target file. link = filepath.Join(options.TargetDir, link) } diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index beb474c7..1e78f641 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -370,8 +370,8 @@ var extractTests = []extractTest{{ summary: "Hard link to symlink does not follow symlink", pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), - testutil.Lnk(0644, "./symlink-to-file", "./file"), - testutil.Hln(0644, "./link-to-symlink", "./symlink-to-file"), + testutil.Lnk(0644, "./symlink", "./file"), + testutil.Hln(0644, "./hardlink", "./symlink"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -381,8 +381,8 @@ var extractTests = []extractTest{{ }, }, result: map[string]string{ - "/link-to-symlink": "symlink ./file", - "/symlink-to-file": "symlink ./file", + "/hardlink": "symlink ./file", + "/symlink": "symlink ./file", }, notCreated: []string{}, }, { @@ -391,8 +391,8 @@ var extractTests = []extractTest{{ testutil.Dir(0755, "./"), testutil.Dir(0755, "./dir/"), testutil.Reg(0644, "./dir/file", "text for file"), - testutil.Lnk(0644, "./symlink-to-file", "./dir/file"), - testutil.Hln(0644, "./link-to-file", "./dir/file"), + testutil.Lnk(0644, "./symlink", "./dir/file"), + testutil.Hln(0644, "./hardlink", "./dir/file"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -402,10 +402,10 @@ var extractTests = []extractTest{{ }, }, result: map[string]string{ - "/dir/": "dir 0755", - "/dir/file": "file 0644 28121945", - "/link-to-file": "file 0644 28121945", - "/symlink-to-file": "symlink ./dir/file", + "/dir/": "dir 0755", + "/dir/file": "file 0644 28121945", + "/hardlink": "file 0644 28121945", + "/symlink": "symlink ./dir/file", }, notCreated: []string{}, }} diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index b4922adf..06ab8e1d 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -132,16 +132,16 @@ func createSymlink(o *CreateOptions) error { func createHardLink(o *CreateOptions) error { debugf("Creating hard link: %s => %s", o.Path, o.Link) - targetInfo, err := os.Lstat(o.Link) + linkInfo, err := os.Lstat(o.Link) if err != nil && os.IsNotExist(err) { return fmt.Errorf("link target does not exist: %s", o.Link) } else if err != nil { return err } - linkInfo, err := os.Lstat(o.Path) + pathInfo, err := os.Lstat(o.Path) if err == nil || os.IsExist(err) { - if os.SameFile(targetInfo, linkInfo) { + if os.SameFile(linkInfo, pathInfo) { return nil } return fmt.Errorf("path %s already exists", o.Path) diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 481808c3..17fdd79c 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -104,7 +104,7 @@ var createTests = []createTest{{ }, { summary: "Create a hard link", options: fsutil.CreateOptions{ - Path: "dir/link-to-file", + Path: "dir/hardlink", Link: "file", Mode: 0644, MakeParents: true, @@ -115,53 +115,53 @@ var createTests = []createTest{{ options.Link = filepath.Join(targetDir, options.Link) }, result: map[string]string{ - "/file": "file 0644 3a6eb079", - "/dir/": "dir 0755", - "/dir/link-to-file": "file 0644 3a6eb079", + "/file": "file 0644 3a6eb079", + "/dir/": "dir 0755", + "/dir/hardlink": "file 0644 3a6eb079", }, }, { summary: "Cannot create a hard link if the link target does not exist", options: fsutil.CreateOptions{ - Path: "dir/link-to-file", - Link: "file-no-exist", + Path: "dir/hardlink", + Link: "missing-file", Mode: 0644, MakeParents: true, }, hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { options.Link = filepath.Join(targetDir, options.Link) }, - error: `link target does not exist: \/[^ ]*\/file-no-exist`, + error: `link target does not exist: \/[^ ]*\/missing-file`, }, { summary: "Re-creating a duplicated hard link keeps the original link", options: fsutil.CreateOptions{ - Path: "link-to-file", + Path: "hardlink", Link: "file", Mode: 0644, MakeParents: true, }, hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { c.Assert(os.WriteFile(filepath.Join(targetDir, "file"), []byte("data"), 0644), IsNil) - c.Assert(os.Link(filepath.Join(targetDir, "file"), filepath.Join(targetDir, "link-to-file")), IsNil) + c.Assert(os.Link(filepath.Join(targetDir, "file"), filepath.Join(targetDir, "hardlink")), IsNil) options.Link = filepath.Join(targetDir, options.Link) }, result: map[string]string{ - "/file": "file 0644 3a6eb079", - "/link-to-file": "file 0644 3a6eb079", + "/file": "file 0644 3a6eb079", + "/hardlink": "file 0644 3a6eb079", }, }, { summary: "Cannot create a hard link if the link path exists and it is not a hard link to the target", options: fsutil.CreateOptions{ - Path: "link-to-file", + Path: "hardlink", Link: "file", Mode: 0644, MakeParents: true, }, hackopt: func(c *C, targetDir string, options *fsutil.CreateOptions) { c.Assert(os.WriteFile(filepath.Join(targetDir, "file"), []byte("data"), 0644), IsNil) - c.Assert(os.WriteFile(filepath.Join(targetDir, "link-to-file"), []byte("data"), 0644), IsNil) + c.Assert(os.WriteFile(filepath.Join(targetDir, "hardlink"), []byte("data"), 0644), IsNil) options.Link = filepath.Join(targetDir, options.Link) }, - error: `path \/[^ ]*\/link-to-file already exists`, + error: `path \/[^ ]*\/hardlink already exists`, }} func (s *S) TestCreate(c *C) { From 5b2913884faafa446cb779b7b364ea8e03e01c5f Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Wed, 24 Jul 2024 15:47:37 +0200 Subject: [PATCH 20/23] feat: add test for same hardlink in slicer_test --- internal/slicer/slicer_test.go | 67 +++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 6 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 4f9dee55..32049332 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1051,6 +1051,54 @@ var slicerTests = []slicerTest{{ content.list("/foo-bar/") `, }, +}, { + summary: "Valid same hard link in two slices in the same package", + slices: []setup.SliceKey{ + {"test-package", "module1"}, + {"test-package", "module2"}}, + pkgs: map[string][]byte{ + "test-package": testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Dir(0755, "./dir/"), + testutil.Reg(0644, "./dir/file", "text for file"), + testutil.Reg(0644, "./dir/module1", "text for module1"), + testutil.Reg(0644, "./dir/module2", "text for module2"), + testutil.Hln(0644, "./hardlink", "./dir/file"), + }), + }, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + hardlink: + contents: + /dir/file: + /hardlink: + module1: + essential: + - test-package_hardlink + contents: + /dir/module1: + module2: + essential: + - test-package_hardlink + contents: + /dir/module2: + `, + }, + filesystem: map[string]string{ + "/dir/": "dir 0755", + "/dir/file": "file 0644 28121945", + "/dir/module1": "file 0644 60f84f1a", + "/dir/module2": "file 0644 3a35e2c7", + "/hardlink": "file 0644 28121945", + }, + report: map[string]string{ + "/dir/file": "file 0644 28121945 {test-package_hardlink}", + "/dir/module1": "file 0644 60f84f1a {test-package_module1}", + "/dir/module2": "file 0644 3a35e2c7 {test-package_module2}", + "/hardlink": "hardlink 0644 dir/file {test-package_hardlink}", + }, }} var defaultChiselYaml = ` @@ -1196,13 +1244,20 @@ func treeDumpReport(report *slicer.Report) map[string]string { fsDump = fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: fsDump = fmt.Sprintf("symlink %s", entry.Link) - case 0: // Regular - if entry.Size == 0 { - fsDump = fmt.Sprintf("file %#o empty", entry.Mode.Perm()) - } else if entry.FinalHash != "" { - fsDump = fmt.Sprintf("file %#o %s %s", fperm, entry.Hash[:8], entry.FinalHash[:8]) + case 0: + if entry.Link != "" { + // Hard link. + relLink := strings.TrimPrefix(entry.Link, report.Root) + fsDump = fmt.Sprintf("hardlink %#o %s", fperm, relLink) } else { - fsDump = fmt.Sprintf("file %#o %s", fperm, entry.Hash[:8]) + // Regular file. + if entry.Size == 0 { + fsDump = fmt.Sprintf("file %#o empty", entry.Mode.Perm()) + } else if entry.FinalHash != "" { + fsDump = fmt.Sprintf("file %#o %s %s", fperm, entry.Hash[:8], entry.FinalHash[:8]) + } else { + fsDump = fmt.Sprintf("file %#o %s", fperm, entry.Hash[:8]) + } } default: panic(fmt.Errorf("unknown file type %d: %s", entry.Mode.Type(), entry.Path)) From 0dca7a0bfab18d87d9fecdf36f40e8bd66f13b3c Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Thu, 25 Jul 2024 20:17:16 +0200 Subject: [PATCH 21/23] test: update duplicate hardlink in slicer_test --- internal/slicer/slicer_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 32049332..b2d53e20 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1070,20 +1070,16 @@ var slicerTests = []slicerTest{{ "slices/mydir/test-package.yaml": ` package: test-package slices: - hardlink: - contents: - /dir/file: - /hardlink: module1: - essential: - - test-package_hardlink contents: + /dir/file: /dir/module1: + /hardlink: module2: - essential: - - test-package_hardlink contents: + /dir/file: /dir/module2: + /hardlink: `, }, filesystem: map[string]string{ @@ -1094,10 +1090,10 @@ var slicerTests = []slicerTest{{ "/hardlink": "file 0644 28121945", }, report: map[string]string{ - "/dir/file": "file 0644 28121945 {test-package_hardlink}", + "/dir/file": "file 0644 28121945 {test-package_module1,test-package_module2}", "/dir/module1": "file 0644 60f84f1a {test-package_module1}", "/dir/module2": "file 0644 3a35e2c7 {test-package_module2}", - "/hardlink": "hardlink 0644 dir/file {test-package_hardlink}", + "/hardlink": "hardlink 0644 dir/file {test-package_module1,test-package_module2}", }, }} From acec1a72449a63996d5c9aefbcd121476bb3cbe8 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 26 Jul 2024 11:07:12 +0200 Subject: [PATCH 22/23] test: treeDumpReport hardlink target uses abs path --- internal/slicer/slicer_test.go | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index b2d53e20..22829601 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1054,15 +1054,13 @@ var slicerTests = []slicerTest{{ }, { summary: "Valid same hard link in two slices in the same package", slices: []setup.SliceKey{ - {"test-package", "module1"}, - {"test-package", "module2"}}, + {"test-package", "slice1"}, + {"test-package", "slice2"}}, pkgs: map[string][]byte{ "test-package": testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Dir(0755, "./dir/"), testutil.Reg(0644, "./dir/file", "text for file"), - testutil.Reg(0644, "./dir/module1", "text for module1"), - testutil.Reg(0644, "./dir/module2", "text for module2"), testutil.Hln(0644, "./hardlink", "./dir/file"), }), }, @@ -1070,30 +1068,24 @@ var slicerTests = []slicerTest{{ "slices/mydir/test-package.yaml": ` package: test-package slices: - module1: + slice1: contents: /dir/file: - /dir/module1: /hardlink: - module2: + slice2: contents: /dir/file: - /dir/module2: /hardlink: `, }, filesystem: map[string]string{ - "/dir/": "dir 0755", - "/dir/file": "file 0644 28121945", - "/dir/module1": "file 0644 60f84f1a", - "/dir/module2": "file 0644 3a35e2c7", - "/hardlink": "file 0644 28121945", + "/dir/": "dir 0755", + "/dir/file": "file 0644 28121945", + "/hardlink": "file 0644 28121945", }, report: map[string]string{ - "/dir/file": "file 0644 28121945 {test-package_module1,test-package_module2}", - "/dir/module1": "file 0644 60f84f1a {test-package_module1}", - "/dir/module2": "file 0644 3a35e2c7 {test-package_module2}", - "/hardlink": "hardlink 0644 dir/file {test-package_module1,test-package_module2}", + "/dir/file": "file 0644 28121945 {test-package_slice1,test-package_slice2}", + "/hardlink": "hardlink 0644 /dir/file {test-package_slice1,test-package_slice2}", }, }} @@ -1243,7 +1235,7 @@ func treeDumpReport(report *slicer.Report) map[string]string { case 0: if entry.Link != "" { // Hard link. - relLink := strings.TrimPrefix(entry.Link, report.Root) + relLink := filepath.Clean("/" + strings.TrimPrefix(entry.Link, report.Root)) fsDump = fmt.Sprintf("hardlink %#o %s", fperm, relLink) } else { // Regular file. From 4313070b5a0af67b4c1cdca323c0eb03d7f882b9 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Mon, 29 Jul 2024 12:25:00 +0200 Subject: [PATCH 23/23] chore: adjust tree dump hardlink string --- internal/slicer/slicer_test.go | 6 +++--- internal/testutil/treedump.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 22829601..cd40f3ce 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1052,7 +1052,7 @@ var slicerTests = []slicerTest{{ `, }, }, { - summary: "Valid same hard link in two slices in the same package", + summary: "Valid hard link in two slices in the same package", slices: []setup.SliceKey{ {"test-package", "slice1"}, {"test-package", "slice2"}}, @@ -1085,7 +1085,7 @@ var slicerTests = []slicerTest{{ }, report: map[string]string{ "/dir/file": "file 0644 28121945 {test-package_slice1,test-package_slice2}", - "/hardlink": "hardlink 0644 /dir/file {test-package_slice1,test-package_slice2}", + "/hardlink": "hardlink /dir/file {test-package_slice1,test-package_slice2}", }, }} @@ -1236,7 +1236,7 @@ func treeDumpReport(report *slicer.Report) map[string]string { if entry.Link != "" { // Hard link. relLink := filepath.Clean("/" + strings.TrimPrefix(entry.Link, report.Root)) - fsDump = fmt.Sprintf("hardlink %#o %s", fperm, relLink) + fsDump = fmt.Sprintf("hardlink %s", relLink) } else { // Regular file. if entry.Size == 0 { diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index dad3677a..7a0f55da 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -77,7 +77,7 @@ func TreeDumpEntry(entry *fsutil.Entry) string { case 0: // Hard link. if entry.Link != "" { - return fmt.Sprintf("hard link %#o %s", fperm, entry.Link) + return fmt.Sprintf("hardlink %s", entry.Link) } // Regular file. if entry.Size == 0 {