From 3031298a5a30d8808e70f12b5205b43b7293fc48 Mon Sep 17 00:00:00 2001 From: zhijie-yang Date: Thu, 5 Dec 2024 11:23:58 +0100 Subject: [PATCH 01/11] feat: add support for hard links Hard links can now be extracted and they are correctly reflected in the manifest. --------- Co-authored-by: Rafid Bin Mostofa Co-authored-by: Alberto Carretero --- internal/archive/archive.go | 6 +- internal/cache/cache.go | 2 +- internal/deb/extract.go | 234 ++++++++++++++++++++----- internal/deb/extract_test.go | 91 +++++++++- internal/fsutil/create.go | 57 ++++-- internal/fsutil/create_test.go | 129 +++++++++++--- internal/manifest/manifest.go | 29 ++++ internal/manifest/manifest_test.go | 131 ++++++++++++++ internal/manifest/report.go | 54 ++++-- internal/manifest/report_test.go | 97 ++++++++++- internal/setup/setup.go | 2 +- internal/slicer/slicer.go | 16 +- internal/slicer/slicer_test.go | 269 +++++++++++++++++++++++++++++ internal/testutil/archive.go | 4 +- internal/testutil/nopcloser.go | 17 ++ internal/testutil/pkgdata.go | 13 ++ internal/testutil/treedump.go | 40 ++++- 17 files changed, 1077 insertions(+), 114 deletions(-) create mode 100644 internal/testutil/nopcloser.go diff --git a/internal/archive/archive.go b/internal/archive/archive.go index 170e3bd5..b1e091f4 100644 --- a/internal/archive/archive.go +++ b/internal/archive/archive.go @@ -18,7 +18,7 @@ import ( type Archive interface { Options() *Options - Fetch(pkg string) (io.ReadCloser, *PackageInfo, error) + Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, error) Exists(pkg string) bool Info(pkg string) (*PackageInfo, error) } @@ -123,7 +123,7 @@ func (a *ubuntuArchive) selectPackage(pkg string) (control.Section, *ubuntuIndex return selectedSection, selectedIndex, nil } -func (a *ubuntuArchive) Fetch(pkg string) (io.ReadCloser, *PackageInfo, error) { +func (a *ubuntuArchive) Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, error) { section, index, err := a.selectPackage(pkg) if err != nil { return nil, nil, err @@ -363,7 +363,7 @@ func (index *ubuntuIndex) checkComponents(components []string) error { return nil } -func (index *ubuntuIndex) fetch(suffix, digest string, flags fetchFlags) (io.ReadCloser, error) { +func (index *ubuntuIndex) fetch(suffix, digest string, flags fetchFlags) (io.ReadSeekCloser, error) { reader, err := index.archive.cache.Open(digest) if err == nil { return reader, nil diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 09d6df03..1dabc0d1 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -132,7 +132,7 @@ func (c *Cache) Write(digest string, data []byte) error { return err2 } -func (c *Cache) Open(digest string) (io.ReadCloser, error) { +func (c *Cache) Open(digest string) (io.ReadSeekCloser, error) { if c.Dir == "" || digest == "" { return nil, MissErr } diff --git a/internal/deb/extract.go b/internal/deb/extract.go index d9e84875..02cbfb02 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -62,7 +62,7 @@ func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { return options, nil } -func Extract(pkgReader io.Reader, options *ExtractOptions) (err error) { +func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { defer func() { if err != nil { err = fmt.Errorf("cannot extract from package %q: %w", options.Package, err) @@ -83,43 +83,15 @@ func Extract(pkgReader io.Reader, options *ExtractOptions) (err error) { return err } - arReader := ar.NewReader(pkgReader) - var dataReader io.Reader - for dataReader == nil { - arHeader, err := arReader.Next() - if err == io.EOF { - return fmt.Errorf("no data payload") - } - if err != nil { - return err - } - switch arHeader.Name { - case "data.tar.gz": - gzipReader, err := gzip.NewReader(arReader) - if err != nil { - return err - } - defer gzipReader.Close() - dataReader = gzipReader - case "data.tar.xz": - xzReader, err := xz.NewReader(arReader) - if err != nil { - return err - } - dataReader = xzReader - case "data.tar.zst": - zstdReader, err := zstd.NewReader(arReader) - if err != nil { - return err - } - defer zstdReader.Close() - dataReader = zstdReader - } - } - return extractData(dataReader, validOpts) + return extractData(pkgReader, validOpts) } -func extractData(dataReader io.Reader, options *ExtractOptions) error { +func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { + dataReader, err := getDataReader(pkgReader) + if err != nil { + return err + } + defer dataReader.Close() oldUmask := syscall.Umask(0) defer func() { @@ -136,6 +108,15 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error { } } + // Store the hard links that we cannot extract when we first iterate over + // the tarball. + // + // This happens because the tarball only stores the contents once in the + // first entry and the rest of them point to the first one. Therefore, we + // cannot tell whether we need to extract the content until after we get to + // a hard link. In this case, we need a second pass. + pendingHardLinks := make(map[string][]pendingHardLink) + // When creating a file we will iterate through its parent directories and // create them with the permissions defined in the tarball. // @@ -153,11 +134,7 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error { return err } - sourcePath := tarHeader.Name - if len(sourcePath) < 3 || sourcePath[0] != '.' || sourcePath[1] != '/' { - continue - } - sourcePath = sourcePath[1:] + sourcePath := sanitizeTarPath(tarHeader.Name) if sourcePath == "" { continue } @@ -245,22 +222,50 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error { return err } } + link := tarHeader.Linkname + if tarHeader.Typeflag == tar.TypeLink { + // A hard link requires the real path of the target file. + link = filepath.Join(options.TargetDir, link) + } + // Create the entry itself. createOptions := &fsutil.CreateOptions{ Path: filepath.Join(options.TargetDir, targetPath), Mode: tarHeader.FileInfo().Mode(), Data: pathReader, - Link: tarHeader.Linkname, + Link: link, MakeParents: true, OverrideMode: true, } err := options.Create(extractInfos, createOptions) - if err != nil { + if err != nil && os.IsNotExist(err) && tarHeader.Typeflag == tar.TypeLink { + // The hard link could not be created because the content + // was not extracted previously. Add this hard link entry + // to the pending list to extract later. + relLinkPath := sanitizeTarPath(tarHeader.Linkname) + info := pendingHardLink{ + path: targetPath, + extractInfos: extractInfos, + } + pendingHardLinks[relLinkPath] = append(pendingHardLinks[relLinkPath], info) + } else if err != nil { return err } } } + if len(pendingHardLinks) > 0 { + // Go over the tarball again to textract the pending hard links. + extractHardLinkOptions := &extractHardLinkOptions{ + ExtractOptions: options, + pendingLinks: pendingHardLinks, + } + err = extractHardLinks(pkgReader, extractHardLinkOptions) + if err != nil { + return err + } + } + if len(pendingPaths) > 0 { pendingList := make([]string, 0, len(pendingPaths)) for pendingPath := range pendingPaths { @@ -277,6 +282,140 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error { return nil } +type pendingHardLink struct { + path string + extractInfos []ExtractInfo +} + +type extractHardLinkOptions struct { + *ExtractOptions + pendingLinks map[string][]pendingHardLink +} + +// extractHardLinks iterates through the tarball a second time to extract the +// hard links that were not extracted in the first pass. +func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { + offset, err := pkgReader.Seek(0, io.SeekStart) + if err != nil { + return err + } + if offset != 0 { + return fmt.Errorf("internal error: cannot seek to the beginning of the package") + } + dataReader, err := getDataReader(pkgReader) + if err != nil { + return err + } + defer dataReader.Close() + + tarReader := tar.NewReader(dataReader) + for { + tarHeader, err := tarReader.Next() + if err == io.EOF { + break + } + if err != nil { + return err + } + + sourcePath := sanitizeTarPath(tarHeader.Name) + if sourcePath == "" { + continue + } + + links, ok := opts.pendingLinks[sourcePath] + if !ok || len(links) == 0 { + continue + } + + // For a target path, the first hard link will be created as a file with + // the content of the target path. If there are more pending hard links, + // the remaining ones will be created as hard links with the newly + // created file as their target. + absLink := filepath.Join(opts.TargetDir, links[0].path) + // Extract the content to the first hard link path. + createOptions := &fsutil.CreateOptions{ + Path: absLink, + Mode: tarHeader.FileInfo().Mode(), + Data: tarReader, + } + err = opts.Create(links[0].extractInfos, createOptions) + if err != nil { + return err + } + + // Create the remaining hard links. + for _, link := range links[1:] { + createOptions := &fsutil.CreateOptions{ + Path: filepath.Join(opts.TargetDir, link.path), + Mode: tarHeader.FileInfo().Mode(), + // Link to the first file extracted for the hard links. + Link: absLink, + } + err := opts.Create(link.extractInfos, createOptions) + if err != nil { + return err + } + } + delete(opts.pendingLinks, sourcePath) + } + + // If there are pending links, that means the link targets do not come from + // this package. + if len(opts.pendingLinks) > 0 { + var errs []string + for target, links := range opts.pendingLinks { + for _, link := range links { + errs = append(errs, fmt.Sprintf("cannot create hard link %s: no content at %s", + link.path, target)) + } + } + if len(errs) == 1 { + return fmt.Errorf("%s", errs[0]) + } + sort.Strings(errs) + return fmt.Errorf("\n- %s", strings.Join(errs, "\n- ")) + } + + return nil +} + +func getDataReader(pkgReader io.ReadSeeker) (io.ReadCloser, error) { + arReader := ar.NewReader(pkgReader) + var dataReader io.ReadCloser + for dataReader == nil { + arHeader, err := arReader.Next() + if err == io.EOF { + return nil, fmt.Errorf("no data payload") + } + if err != nil { + return nil, err + } + switch arHeader.Name { + case "data.tar.gz": + gzipReader, err := gzip.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = gzipReader + case "data.tar.xz": + xzReader, err := xz.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = io.NopCloser(xzReader) + case "data.tar.zst": + zstdReader, err := zstd.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = zstdReader.IOReadCloser() + } + } + + return dataReader, nil +} + func parentDirs(path string) []string { path = filepath.Clean(path) parents := make([]string, strings.Count(path, "/")) @@ -289,3 +428,12 @@ func parentDirs(path string) []string { } return parents } + +// sanitizeTarPath removes the leading "./" from the source path in the tarball, +// and verifies that the path is not empty. +func sanitizeTarPath(path string) string { + if len(path) < 3 || path[0] != '.' || path[1] != '/' { + return "" + } + return path[1:] +} diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index db73df86..36f71b94 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -354,6 +354,93 @@ var extractTests = []extractTest{{ }, }, error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`, +}, { + summary: "Single hard link entry can be extracted with the content entry", + pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "text for file"), + testutil.Hlk(0644, "./hardlink", "./file"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/**": []deb.ExtractInfo{{ + Path: "/**", + }}, + }, + }, + result: map[string]string{ + "/file": "file 0644 28121945 <1>", + "/hardlink": "file 0644 28121945 <1>", + }, + notCreated: []string{}, +}, { + summary: "Single hard link entry can be extracted without the content entry", + pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "text for file"), + testutil.Hlk(0644, "./hardlink", "./file"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/hardlink": []deb.ExtractInfo{{ + Path: "/hardlink", + }}, + }, + }, + result: map[string]string{ + "/hardlink": "file 0644 28121945", + }, + notCreated: []string{}, +}, { + summary: "Dangling hard link", + pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Hlk(0644, "./hardlink", "./non-existing-target"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/hardlink": []deb.ExtractInfo{{ + Path: "/hardlink", + }}, + }, + }, + error: `cannot extract from package "test-package": cannot create hard link /hardlink: no content at /non-existing-target`, +}, { + summary: "Multiple dangling hard links", + pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Hlk(0644, "./hardlink1", "./non-existing-target"), + testutil.Hlk(0644, "./hardlink2", "./non-existing-target"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/**": []deb.ExtractInfo{{ + Path: "/**", + }}, + }, + }, + error: `cannot extract from package "test-package": ` + + `\n- cannot create hard link /hardlink1: no content at /non-existing-target` + + `\n- cannot create hard link /hardlink2: no content at /non-existing-target`, +}, { + summary: "Hard link does not follow the symlink", + pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Lnk(0644, "./symlink", "./file"), + testutil.Hlk(0644, "./hardlink", "./symlink"), + }), + options: deb.ExtractOptions{ + Extract: map[string][]deb.ExtractInfo{ + "/**": []deb.ExtractInfo{{ + Path: "/**", + }}, + }, + }, + result: map[string]string{ + "/hardlink": "symlink ./file <1>", + "/symlink": "symlink ./file <1>", + }, + notCreated: []string{}, }, { summary: "Explicit extraction overrides existing file", pkgdata: testutil.PackageData["test-package"], @@ -398,7 +485,7 @@ func (s *S) TestExtract(c *C) { test.hackopt(c, &options) } - err := deb.Extract(bytes.NewBuffer(test.pkgdata), &options) + err := deb.Extract(bytes.NewReader(test.pkgdata), &options) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue @@ -509,7 +596,7 @@ func (s *S) TestExtractCreateCallback(c *C) { return nil } - err := deb.Extract(bytes.NewBuffer(test.pkgdata), &options) + err := deb.Extract(bytes.NewReader(test.pkgdata), &options) c.Assert(err, IsNil) c.Assert(createExtractInfos, DeepEquals, test.calls) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 76561b77..2d9a112f 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 not empty and the symlink flag is set in Mode, a symlink is + // created. If the symlink flag is not set in Mode, a hard link is created. Link string // If MakeParents is true, missing parent directories of Path are // created with permissions 0755. @@ -25,15 +27,18 @@ type CreateOptions struct { } type Entry struct { - Path string - Mode fs.FileMode - SHA256 string - Size int - Link string + Path string + Mode fs.FileMode + SHA256 string + Size int + Link string + HardLink bool } // Create creates a filesystem entry according to the provided options and returns // the information about the created entry. +// +// Create can return errors from the os package. func Create(options *CreateOptions) (*Entry, error) { rp := &readerProxy{inner: options.Data, h: sha256.New()} // Use the proxy instead of the raw Reader. @@ -43,6 +48,7 @@ func Create(options *CreateOptions) (*Entry, error) { var err error var hash string + var hardLink bool if o.MakeParents { if err := os.MkdirAll(filepath.Dir(o.Path), 0755); err != nil { return nil, err @@ -51,8 +57,13 @@ 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) + hardLink = true + } else { + err = createFile(o) + hash = hex.EncodeToString(rp.h.Sum(nil)) + } case fs.ModeDir: err = createDir(o) case fs.ModeSymlink: @@ -69,7 +80,7 @@ func Create(options *CreateOptions) (*Entry, error) { return nil, err } mode := s.Mode() - if o.OverrideMode && mode != o.Mode && o.Mode&fs.ModeSymlink == 0 { + if o.OverrideMode && mode != o.Mode && o.Link == "" { err := os.Chmod(o.Path, o.Mode) if err != nil { return nil, err @@ -78,11 +89,12 @@ func Create(options *CreateOptions) (*Entry, error) { } entry := &Entry{ - Path: o.Path, - Mode: mode, - SHA256: hash, - Size: rp.size, - Link: o.Link, + Path: o.Path, + Mode: mode, + SHA256: hash, + Size: rp.size, + Link: o.Link, + HardLink: hardLink, } return entry, nil } @@ -162,6 +174,25 @@ 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) + err := os.Link(o.Link, o.Path) + if err != nil && os.IsExist(err) { + linkInfo, serr := os.Lstat(o.Link) + if serr != nil { + return serr + } + pathInfo, serr := os.Lstat(o.Path) + if serr != nil { + return serr + } + if os.SameFile(linkInfo, pathInfo) { + return nil + } + } + return err +} + // 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/fsutil/create_test.go b/internal/fsutil/create_test.go index be086ea5..a87b2b0d 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 - hackdir func(c *C, dir string) + hackopt func(c *C, dir string, opts *fsutil.CreateOptions) result map[string]string error string } var createTests = []createTest{{ + summary: "Create a file and its parent 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 directory", 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,17 +67,19 @@ var createTests = []createTest{{ "/tmp/": "dir 01775", }, }, { + summary: "Cannot create a parent directory 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, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { c.Assert(os.Mkdir(filepath.Join(dir, "foo/"), fs.ModeDir|0765), IsNil) }, result: map[string]string{ @@ -80,13 +87,14 @@ var createTests = []createTest{{ "/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"), }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { c.Assert(os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666), IsNil) }, result: map[string]string{ @@ -94,12 +102,73 @@ var createTests = []createTest{{ "/foo": "file 0666 d67e2e94", }, }, { + summary: "Create a hard link", + options: fsutil.CreateOptions{ + Path: "hardlink", + Link: "file", + Mode: 0644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0644), IsNil) + // An absolute path is required to create a hard link. + opts.Link = filepath.Join(dir, opts.Link) + }, + result: map[string]string{ + "/file": "file 0644 3a6eb079 <1>", + "/hardlink": "file 0644 3a6eb079 <1>", + }, +}, { + summary: "Cannot create a hard link if the link target does not exist", + options: fsutil.CreateOptions{ + Path: "hardlink", + Link: "missing-file", + Mode: 0644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { + opts.Link = filepath.Join(dir, opts.Link) + }, + error: `link /[^ ]*/missing-file /[^ ]*/hardlink: no such file or directory`, +}, { + summary: "No error if hard link already exists", + options: fsutil.CreateOptions{ + Path: "hardlink", + Link: "file", + Mode: 0644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *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, "hardlink")), IsNil) + opts.Link = filepath.Join(dir, opts.Link) + }, + result: map[string]string{ + "/file": "file 0644 3a6eb079 <1>", + "/hardlink": "file 0644 3a6eb079 <1>", + }, +}, { + summary: "Cannot create a hard link if file exists but differs", + options: fsutil.CreateOptions{ + Path: "hardlink", + Link: "file", + Mode: 0644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0644), IsNil) + c.Assert(os.WriteFile(filepath.Join(dir, "hardlink"), []byte("data"), 0644), IsNil) + opts.Link = filepath.Join(dir, opts.Link) + }, + error: `link /[^ ]*/file /[^ ]*/hardlink: file exists`, +}, { + summary: "The mode of a dir can be overridden", options: fsutil.CreateOptions{ Path: "foo", Mode: fs.ModeDir | 0775, OverrideMode: true, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { c.Assert(os.Mkdir(filepath.Join(dir, "foo/"), fs.ModeDir|0765), IsNil) }, result: map[string]string{ @@ -107,13 +176,14 @@ var createTests = []createTest{{ "/foo/": "dir 0775", }, }, { + summary: "The mode of a file can be overridden", options: fsutil.CreateOptions{ Path: "foo", Mode: 0775, Data: bytes.NewBufferString("whatever"), OverrideMode: true, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { err := os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666) c.Assert(err, IsNil) }, @@ -122,12 +192,13 @@ var createTests = []createTest{{ "/foo": "file 0775 85738f8f", }, }, { + summary: "The mode of a symlink cannot be overridden", options: fsutil.CreateOptions{ Path: "foo", Link: "./bar", Mode: 0666 | fs.ModeSymlink, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { err := os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666) c.Assert(err, IsNil) }, @@ -135,13 +206,14 @@ var createTests = []createTest{{ "/foo": "symlink ./bar", }, }, { + summary: "OverrideMode does not follow symlink", options: fsutil.CreateOptions{ Path: "foo", Link: "./bar", Mode: 0776 | fs.ModeSymlink, OverrideMode: true, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { err := os.WriteFile(filepath.Join(dir, "bar"), []byte("data"), 0666) c.Assert(err, IsNil) err = os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666) @@ -153,13 +225,14 @@ var createTests = []createTest{{ "/bar": "file 0666 3a6eb079", }, }, { + summary: "The target of an existing symlink can be overridden", options: fsutil.CreateOptions{ Path: "bar", // Existing link with different target. Link: "other", Mode: 0666 | fs.ModeSymlink, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { err := os.Symlink("foo", filepath.Join(dir, "bar")) c.Assert(err, IsNil) }, @@ -167,13 +240,14 @@ var createTests = []createTest{{ "/bar": "symlink other", }, }, { + summary: "No error if symlink already exists", options: fsutil.CreateOptions{ Path: "bar", // Existing link with same target. Link: "foo", Mode: 0666 | fs.ModeSymlink, }, - hackdir: func(c *C, dir string) { + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { err := os.Symlink("foo", filepath.Join(dir, "bar")) c.Assert(err, IsNil) }, @@ -189,17 +263,18 @@ 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) } c.Logf("Options: %v", test.options) dir := c.MkDir() - if test.hackdir != nil { - test.hackdir(c, dir) - } 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 != "" { @@ -209,15 +284,27 @@ 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.HardLink { + // We should test hard link entries differently because + // fsutil.Create does not return hash or size when it creates hard + // links. + 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]) } } diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 273d3106..4f3b35fd 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -39,6 +39,7 @@ type Path struct { FinalSHA256 string `json:"final_sha256,omitempty"` Size uint64 `json:"size,omitempty"` Link string `json:"link,omitempty"` + HardLinkID uint64 `json:"hard_link_id,omitempty"` } type Content struct { @@ -291,6 +292,7 @@ func manifestAddReport(dbw *jsonwall.DBWriter, report *Report) error { FinalSHA256: entry.FinalSHA256, Size: uint64(entry.Size), Link: entry.Link, + HardLinkID: entry.HardLinkID, }) if err != nil { return err @@ -332,6 +334,7 @@ func fastValidate(options *WriteOptions) (err error) { } sliceExist[slice.String()] = true } + hardLinkGroups := make(map[uint64][]*ReportEntry) for _, entry := range options.Report.Entries { err := validateReportEntry(&entry) if err != nil { @@ -342,7 +345,33 @@ func fastValidate(options *WriteOptions) (err error) { return fmt.Errorf("path %q refers to missing slice %s", entry.Path, slice.String()) } } + if entry.HardLinkID != 0 { + // TODO remove the following line after upgrading to Go 1.22 or higher. + e := entry + hardLinkGroups[e.HardLinkID] = append(hardLinkGroups[e.HardLinkID], &e) + } } + // Entries within a hard link group must have same content. + for id := 1; id <= len(hardLinkGroups); id++ { + entries, ok := hardLinkGroups[uint64(id)] + if !ok { + return fmt.Errorf("cannot find hard link id %d", id) + } + if len(entries) == 1 { + return fmt.Errorf("hard link group %d has only one path: %s", id, entries[0].Path) + } + sort.Slice(entries, func(i, j int) bool { + return entries[i].Path < entries[j].Path + }) + e0 := entries[0] + for _, e := range entries[1:] { + if e.Link != e0.Link || e.Mode != e0.Mode || e.SHA256 != e0.SHA256 || + e.Size != e0.Size || e.FinalSHA256 != e0.FinalSHA256 { + return fmt.Errorf("hard linked paths %q and %q have diverging contents", e0.Path, e.Path) + } + } + } + return nil } diff --git a/internal/manifest/manifest_test.go b/internal/manifest/manifest_test.go index 1259d819..53b28993 100644 --- a/internal/manifest/manifest_test.go +++ b/internal/manifest/manifest_test.go @@ -35,13 +35,17 @@ var readManifestTests = []struct { {"jsonwall":"1.0","schema":"1.0","count":13} {"kind":"content","slice":"pkg1_manifest","path":"/manifest/manifest.wall"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/file"} + {"kind":"content","slice":"pkg1_myslice","path":"/dir/file2"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/foo/bar/"} + {"kind":"content","slice":"pkg1_myslice","path":"/dir/hardlink"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/link/file"} {"kind":"content","slice":"pkg2_myotherslice","path":"/dir/foo/bar/"} {"kind":"package","name":"pkg1","version":"v1","sha256":"hash1","arch":"arch1"} {"kind":"package","name":"pkg2","version":"v2","sha256":"hash2","arch":"arch2"} {"kind":"path","path":"/dir/file","mode":"0644","slices":["pkg1_myslice"],"sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","final_sha256":"8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6","size":21} + {"kind":"path","path":"/dir/file2","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"hard_link_id":1} {"kind":"path","path":"/dir/foo/bar/","mode":"01777","slices":["pkg2_myotherslice","pkg1_myslice"]} + {"kind":"path","path":"/dir/hardlink","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"hard_link_id":1} {"kind":"path","path":"/dir/link/file","mode":"0644","slices":["pkg1_myslice"],"link":"/dir/file"} {"kind":"path","path":"/manifest/manifest.wall","mode":"0644","slices":["pkg1_manifest"]} {"kind":"slice","name":"pkg1_manifest"} @@ -51,7 +55,9 @@ var readManifestTests = []struct { mfest: &manifestContents{ Paths: []*manifest.Path{ {Kind: "path", Path: "/dir/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", FinalSHA256: "8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6", Size: 0x15, Link: ""}, + {Kind: "path", Path: "/dir/file2", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", Size: 0x03, Link: "", HardLinkID: 0x01}, {Kind: "path", Path: "/dir/foo/bar/", Mode: "01777", Slices: []string{"pkg2_myotherslice", "pkg1_myslice"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""}, + {Kind: "path", Path: "/dir/hardlink", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", Size: 0x03, Link: "", HardLinkID: 0x01}, {Kind: "path", Path: "/dir/link/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: "/dir/file"}, {Kind: "path", Path: "/manifest/manifest.wall", Mode: "0644", Slices: []string{"pkg1_manifest"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""}, }, @@ -67,7 +73,9 @@ var readManifestTests = []struct { Contents: []*manifest.Content{ {Kind: "content", Slice: "pkg1_manifest", Path: "/manifest/manifest.wall"}, {Kind: "content", Slice: "pkg1_myslice", Path: "/dir/file"}, + {Kind: "content", Slice: "pkg1_myslice", Path: "/dir/file2"}, {Kind: "content", Slice: "pkg1_myslice", Path: "/dir/foo/bar/"}, + {Kind: "content", Slice: "pkg1_myslice", Path: "/dir/hardlink"}, {Kind: "content", Slice: "pkg1_myslice", Path: "/dir/link/file"}, {Kind: "content", Slice: "pkg2_myotherslice", Path: "/dir/foo/bar/"}, }, @@ -539,6 +547,129 @@ var generateManifestTests = []struct { }, }, error: `internal error: invalid manifest: path "/dir" has invalid options: size set for directory`, +}, { + summary: "Basic hard link", + selection: []*setup.Slice{slice1}, + report: &manifest.Report{ + Root: "/", + Entries: map[string]manifest.ReportEntry{ + "/file": { + Path: "/file", + Mode: 0456, + SHA256: "hash", + Size: 1234, + Slices: map[*setup.Slice]bool{slice1: true}, + FinalSHA256: "final-hash", + HardLinkID: 1, + }, + "/hardlink": { + Path: "/hardlink", + Mode: 0456, + SHA256: "hash", + Size: 1234, + Slices: map[*setup.Slice]bool{slice1: true}, + FinalSHA256: "final-hash", + HardLinkID: 1, + }, + }, + }, + packageInfo: []*archive.PackageInfo{{ + Name: "package1", + Version: "v1", + Arch: "a1", + SHA256: "s1", + }}, + expected: &manifestContents{ + Paths: []*manifest.Path{{ + Kind: "path", + Path: "/file", + Mode: "0456", + Slices: []string{"package1_slice1"}, + Size: 1234, + SHA256: "hash", + FinalSHA256: "final-hash", + HardLinkID: 1, + }, { + Kind: "path", + Path: "/hardlink", + Mode: "0456", + Slices: []string{"package1_slice1"}, + Size: 1234, + SHA256: "hash", + FinalSHA256: "final-hash", + HardLinkID: 1, + }}, + Packages: []*manifest.Package{{ + Kind: "package", + Name: "package1", + Version: "v1", + Digest: "s1", + Arch: "a1", + }}, + Slices: []*manifest.Slice{{ + Kind: "slice", + Name: "package1_slice1", + }}, + Contents: []*manifest.Content{{ + Kind: "content", + Slice: "package1_slice1", + Path: "/file", + }, { + Kind: "content", + Slice: "package1_slice1", + Path: "/hardlink", + }}, + }, +}, { + summary: "Skipped hard link id", + report: &manifest.Report{ + Root: "/", + Entries: map[string]manifest.ReportEntry{ + "/file": { + Path: "/file", + Slices: map[*setup.Slice]bool{slice1: true}, + HardLinkID: 2, + }, + }, + }, + error: `internal error: invalid manifest: cannot find hard link id 1`, +}, { + summary: "Hard link group has only one path", + report: &manifest.Report{ + Root: "/", + Entries: map[string]manifest.ReportEntry{ + "/file": { + Path: "/file", + Slices: map[*setup.Slice]bool{slice1: true}, + HardLinkID: 1, + }, + }, + }, + error: `internal error: invalid manifest: hard link group 1 has only one path: /file`, +}, { + summary: "Hard linked paths differ", + report: &manifest.Report{ + Root: "/", + Entries: map[string]manifest.ReportEntry{ + "/file": { + Path: "/file", + Mode: 0456, + SHA256: "hash", + Size: 1234, + Slices: map[*setup.Slice]bool{slice1: true}, + HardLinkID: 1, + }, + "/hardlink": { + Path: "/hardlink", + Mode: 0456, + SHA256: "different-hash", + Size: 1234, + Slices: map[*setup.Slice]bool{slice1: true}, + HardLinkID: 1, + }, + }, + }, + error: `internal error: invalid manifest: hard linked paths "/file" and "/hardlink" have diverging contents`, }, { summary: "Invalid package: missing name", packageInfo: []*archive.PackageInfo{{ diff --git a/internal/manifest/report.go b/internal/manifest/report.go index 092ed823..77ef0a0a 100644 --- a/internal/manifest/report.go +++ b/internal/manifest/report.go @@ -5,6 +5,7 @@ import ( "io/fs" "path/filepath" "strings" + "sync/atomic" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/setup" @@ -18,6 +19,8 @@ type ReportEntry struct { Slices map[*setup.Slice]bool Link string FinalSHA256 string + // If HardLinkID is greater than 0, all entries with the same id represent hard links to the same inode. + HardLinkID uint64 } // Report holds the information about files and directories created when slicing @@ -27,6 +30,9 @@ type Report struct { Root string // Entries holds all reported content, indexed by their path. Entries map[string]ReportEntry + // lastHardLinkID is used internally to allocate unique HardLinkID for hard + // links. + lastHardLinkID atomic.Uint64 } // NewReport returns an empty report for content that will be based at the @@ -52,26 +58,46 @@ func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { return fmt.Errorf("cannot add path to report: %s", err) } + var hardLinkID uint64 + fsEntryCpy := *fsEntry + if fsEntry.HardLink { + relLinkPath, _ := r.sanitizeAbsPath(fsEntry.Link, false) + entry, ok := r.Entries[relLinkPath] + if !ok { + return fmt.Errorf("cannot add hard link %s to report: target %s not previously added", relPath, relLinkPath) + } + if entry.HardLinkID == 0 { + entry.HardLinkID = r.lastHardLinkID.Add(1) + r.lastHardLinkID.Store(entry.HardLinkID) + r.Entries[relLinkPath] = entry + } + hardLinkID = entry.HardLinkID + fsEntryCpy.SHA256 = entry.SHA256 + fsEntryCpy.Size = entry.Size + fsEntryCpy.Link = entry.Link + } + if entry, ok := r.Entries[relPath]; ok { - if fsEntry.Mode != entry.Mode { - return fmt.Errorf("path %s reported twice with diverging mode: 0%03o != 0%03o", relPath, fsEntry.Mode, entry.Mode) - } else if fsEntry.Link != entry.Link { - return fmt.Errorf("path %s reported twice with diverging link: %q != %q", relPath, fsEntry.Link, entry.Link) - } else if fsEntry.Size != entry.Size { - return fmt.Errorf("path %s reported twice with diverging size: %d != %d", relPath, fsEntry.Size, entry.Size) - } else if fsEntry.SHA256 != entry.SHA256 { - return fmt.Errorf("path %s reported twice with diverging hash: %q != %q", relPath, fsEntry.SHA256, entry.SHA256) + if fsEntryCpy.Mode != entry.Mode { + return fmt.Errorf("path %s reported twice with diverging mode: 0%03o != 0%03o", relPath, fsEntryCpy.Mode, entry.Mode) + } else if fsEntryCpy.Link != entry.Link { + return fmt.Errorf("path %s reported twice with diverging link: %q != %q", relPath, fsEntryCpy.Link, entry.Link) + } else if fsEntryCpy.Size != entry.Size { + return fmt.Errorf("path %s reported twice with diverging size: %d != %d", relPath, fsEntryCpy.Size, entry.Size) + } else if fsEntryCpy.SHA256 != entry.SHA256 { + return fmt.Errorf("path %s reported twice with diverging hash: %q != %q", relPath, fsEntryCpy.SHA256, entry.SHA256) } entry.Slices[slice] = true r.Entries[relPath] = entry } else { r.Entries[relPath] = ReportEntry{ - Path: relPath, - Mode: fsEntry.Mode, - SHA256: fsEntry.SHA256, - Size: fsEntry.Size, - Slices: map[*setup.Slice]bool{slice: true}, - Link: fsEntry.Link, + Path: relPath, + Mode: fsEntry.Mode, + SHA256: fsEntryCpy.SHA256, + Size: fsEntryCpy.Size, + Slices: map[*setup.Slice]bool{slice: true}, + Link: fsEntryCpy.Link, + HardLinkID: hardLinkID, } } return nil diff --git a/internal/manifest/report_test.go b/internal/manifest/report_test.go index 9c001509..90398c8f 100644 --- a/internal/manifest/report_test.go +++ b/internal/manifest/report_test.go @@ -40,14 +40,21 @@ var sampleFile = fsutil.Entry{ Link: "", } -var sampleLink = fsutil.Entry{ +var sampleSymLink = fsutil.Entry{ Path: "/base/example-link", - Mode: 0777, + Mode: fs.ModeSymlink | 0777, SHA256: "example-file_hash", Size: 5678, Link: "/base/example-file", } +var sampleHardLink = fsutil.Entry{ + Path: "/base/example-hard-link", + Mode: sampleFile.Mode, + Link: "/base/example-file", + HardLink: true, +} + var sampleFileMutated = fsutil.Entry{ Path: sampleFile.Path, SHA256: sampleFile.SHA256 + "_changed", @@ -104,11 +111,11 @@ var reportTests = []struct { }}, }, { summary: "Regular file link", - add: []sliceAndEntry{{entry: sampleLink, slice: oneSlice}}, + add: []sliceAndEntry{{entry: sampleSymLink, slice: oneSlice}}, expected: map[string]manifest.ReportEntry{ "/example-link": { Path: "/example-link", - Mode: 0777, + Mode: fs.ModeSymlink | 0777, SHA256: "example-file_hash", Size: 5678, Slices: map[*setup.Slice]bool{oneSlice: true}, @@ -267,6 +274,88 @@ var reportTests = []struct { add: []sliceAndEntry{{entry: sampleDir, slice: oneSlice}}, mutate: []*fsutil.Entry{&sampleDir}, err: `cannot mutate path in report: /example-dir/ is a directory`, +}, { + summary: "Hard link to regular file", + add: []sliceAndEntry{ + {entry: sampleFile, slice: oneSlice}, + {entry: sampleHardLink, slice: oneSlice}}, + expected: map[string]manifest.ReportEntry{ + "/example-file": { + Path: "/example-file", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + HardLinkID: 1, + }, + "/example-hard-link": { + Path: "/example-hard-link", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + HardLinkID: 1, + }, + }, +}, { + summary: "Multiple hard links groups", + add: []sliceAndEntry{{ + entry: sampleFile, + slice: oneSlice, + }, { + entry: sampleHardLink, + slice: oneSlice, + }, { + entry: fsutil.Entry{ + Path: "/base/another-file", + Mode: 0777, + SHA256: "another-file_hash", + Size: 5678, + }, + slice: otherSlice, + }, { + entry: fsutil.Entry{ + Path: "/base/another-hard-link", + Mode: 0777, + Link: "/base/another-file", + HardLink: true, + }, + slice: otherSlice, + }}, + expected: map[string]manifest.ReportEntry{ + "/example-file": { + Path: "/example-file", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + HardLinkID: 1, + }, + "/example-hard-link": { + Path: "/example-hard-link", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + HardLinkID: 1, + }, + "/another-file": { + Path: "/another-file", + Mode: 0777, + SHA256: "another-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{otherSlice: true}, + HardLinkID: 2, + }, + "/another-hard-link": { + Path: "/another-hard-link", + Mode: 0777, + SHA256: "another-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{otherSlice: true}, + HardLinkID: 2, + }, + }, }} func (s *S) TestReport(c *C) { diff --git a/internal/setup/setup.go b/internal/setup/setup.go index db9fe8cc..66a3d56d 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -113,7 +113,7 @@ func (s SliceKey) String() string { return s.Package + "_" + s.Slice } // Selection holds the required configuration to create a Build for a selection // of slices from a Release. It's still an abstract proposal in the sense that -// the real information coming from pacakges is still unknown, so referenced +// the real information coming from packages is still unknown, so referenced // paths could potentially be missing, for example. type Selection struct { Release *Release diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 2d4c45e4..464de4f2 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -32,8 +32,9 @@ type RunOptions struct { } type pathData struct { - until setup.PathUntil - mutable bool + until setup.PathUntil + mutable bool + hardLink bool } type contentChecker struct { @@ -44,6 +45,9 @@ func (cc *contentChecker) checkMutable(path string) error { if !cc.knownPaths[path].mutable { return fmt.Errorf("cannot write file which is not mutable: %s", path) } + if cc.knownPaths[path].hardLink { + return fmt.Errorf("cannot mutate a hard link: %s", path) + } return nil } @@ -134,7 +138,7 @@ func Run(options *RunOptions) error { } // Fetch all packages, using the selection order. - packages := make(map[string]io.ReadCloser) + packages := make(map[string]io.ReadSeekCloser) var pkgInfos []*archive.PackageInfo for _, slice := range options.Selection.Slices { if packages[slice.Package] != nil { @@ -205,7 +209,11 @@ func Run(options *RunOptions) error { } if inSliceContents { - data := pathData{mutable: mutable, until: until} + data := pathData{ + mutable: mutable, + until: until, + hardLink: entry.HardLink, + } addKnownPath(knownPaths, relPath, data) } return nil diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index a49fdd97..4e20ddd5 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1478,6 +1478,270 @@ var slicerTests = []slicerTest{{ `, }, error: `cannot find package "test-package" in archive\(s\)`, +}, { + summary: "Valid hard link in two slices in the same package", + slices: []setup.SliceKey{ + {"test-package", "slice1"}, + {"test-package", "slice2"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hlk(0644, "./hardlink", "./file"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + slice1: + contents: + /file: + /hardlink: + slice2: + contents: + /hardlink: + `, + }, + filesystem: map[string]string{ + "/file": "file 0644 2c26b46b <1>", + "/hardlink": "file 0644 2c26b46b <1>", + }, + manifestPaths: map[string]string{ + "/file": "file 0644 2c26b46b <1> {test-package_slice1}", + "/hardlink": "file 0644 2c26b46b <1> {test-package_slice1,test-package_slice2}", + }, +}, { + summary: "Hard link entries can be extracted without extracting the regular file", + slices: []setup.SliceKey{ + {"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hlk(0644, "./hardlink1", "./file"), + testutil.Hlk(0644, "./hardlink2", "./file"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /hardlink*: + `, + }, + filesystem: map[string]string{ + "/hardlink1": "file 0644 2c26b46b <1>", + "/hardlink2": "file 0644 2c26b46b <1>", + }, + manifestPaths: map[string]string{ + "/hardlink1": "file 0644 2c26b46b <1> {test-package_myslice}", + "/hardlink2": "file 0644 2c26b46b <1> {test-package_myslice}", + }, +}, { + summary: "Hard link identifier for different groups", + slices: []setup.SliceKey{ + {"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file1", "text for file1"), + testutil.Reg(0644, "./file2", "text for file2"), + testutil.Hlk(0644, "./hardlink1", "./file1"), + testutil.Hlk(0644, "./hardlink2", "./file2"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /**: + `, + }, + filesystem: map[string]string{ + "/file1": "file 0644 df82bbbd <1>", + "/file2": "file 0644 dcddda2e <2>", + "/hardlink1": "file 0644 df82bbbd <1>", + "/hardlink2": "file 0644 dcddda2e <2>", + }, + manifestPaths: map[string]string{ + "/file1": "file 0644 df82bbbd <1> {test-package_myslice}", + "/file2": "file 0644 dcddda2e <2> {test-package_myslice}", + "/hardlink1": "file 0644 df82bbbd <1> {test-package_myslice}", + "/hardlink2": "file 0644 dcddda2e <2> {test-package_myslice}", + }, +}, { + summary: "Single hard link entry can be extracted without regular file and no hard links are created", + slices: []setup.SliceKey{ + {"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hlk(0644, "./hardlink", "./file"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /hardlink: + `, + }, + filesystem: map[string]string{ + "/hardlink": "file 0644 2c26b46b", + }, + manifestPaths: map[string]string{ + "/hardlink": "file 0644 2c26b46b {test-package_myslice}", + }, +}, { + summary: "Hard link to symlink does not follow symlink", + slices: []setup.SliceKey{ + {"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Lnk(0644, "./symlink", "./file"), + testutil.Hlk(0644, "./hardlink", "./symlink"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /hardlink: + /symlink: + `, + }, + filesystem: map[string]string{ + "/hardlink": "symlink ./file <1>", + "/symlink": "symlink ./file <1>", + }, + manifestPaths: map[string]string{ + "/symlink": "symlink ./file <1> {test-package_myslice}", + "/hardlink": "symlink ./file <1> {test-package_myslice}", + }, +}, { + summary: "Hard link identifiers are unique across packages", + slices: []setup.SliceKey{ + {"test-package1", "myslice"}, + {"test-package2", "myslice"}, + }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package1", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file1", "foo"), + testutil.Hlk(0644, "./hardlink1", "./file1"), + }), + }, { + Name: "test-package2", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file2", "foo"), + testutil.Hlk(0644, "./hardlink2", "./file2"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package1.yaml": ` + package: test-package1 + slices: + myslice: + contents: + /file1: + /hardlink1: + `, + "slices/mydir/test-package2.yaml": ` + package: test-package2 + slices: + myslice: + contents: + /file2: + /hardlink2: + `, + }, + filesystem: map[string]string{ + "/file1": "file 0644 2c26b46b <1>", + "/hardlink1": "file 0644 2c26b46b <1>", + "/file2": "file 0644 2c26b46b <2>", + "/hardlink2": "file 0644 2c26b46b <2>", + }, + manifestPaths: map[string]string{ + "/file1": "file 0644 2c26b46b <1> {test-package1_myslice}", + "/hardlink1": "file 0644 2c26b46b <1> {test-package1_myslice}", + "/file2": "file 0644 2c26b46b <2> {test-package2_myslice}", + "/hardlink2": "file 0644 2c26b46b <2> {test-package2_myslice}", + }, +}, { + summary: "Mutations for hard links are forbidden", + slices: []setup.SliceKey{ + {"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hlk(0644, "./hardlink", "./file"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /file: + /hardlink: {mutable: true} + mutate: | + content.write("/hardlink", "new content") + `, + }, + error: `slice test-package_myslice: cannot mutate a hard link: /hardlink`, +}, { + summary: "Hard links can be marked as mutable, but not mutated", + slices: []setup.SliceKey{ + {"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hlk(0644, "./hardlink", "./file"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /file: + /hardlink: {mutable: true} + `, + }, + filesystem: map[string]string{ + "/file": "file 0644 2c26b46b <1>", + "/hardlink": "file 0644 2c26b46b <1>", + }, + manifestPaths: map[string]string{ + "/file": "file 0644 2c26b46b <1> {test-package_myslice}", + "/hardlink": "file 0644 2c26b46b <1> {test-package_myslice}", + }, }} var defaultChiselYaml = ` @@ -1646,6 +1910,11 @@ func treeDumpManifestPaths(mfest *manifest.Manifest) (map[string]string, error) } } + if path.HardLinkID != 0 { + // Append to the end of the path dump. + fsDump = fmt.Sprintf("%s <%d>", fsDump, path.HardLinkID) + } + // append {slice1, ..., sliceN} to the end of the path dump. slicesStr := make([]string, 0, len(path.Slices)) for _, slice := range path.Slices { diff --git a/internal/testutil/archive.go b/internal/testutil/archive.go index ae8258c2..d06fd1b0 100644 --- a/internal/testutil/archive.go +++ b/internal/testutil/archive.go @@ -26,7 +26,7 @@ func (a *TestArchive) Options() *archive.Options { return &a.Opts } -func (a *TestArchive) Fetch(pkgName string) (io.ReadCloser, *archive.PackageInfo, error) { +func (a *TestArchive) Fetch(pkgName string) (io.ReadSeekCloser, *archive.PackageInfo, error) { pkg, ok := a.Packages[pkgName] if !ok { return nil, nil, fmt.Errorf("cannot find package %q in archive", pkgName) @@ -37,7 +37,7 @@ func (a *TestArchive) Fetch(pkgName string) (io.ReadCloser, *archive.PackageInfo SHA256: pkg.Hash, Arch: pkg.Arch, } - return io.NopCloser(bytes.NewBuffer(pkg.Data)), info, nil + return ReadSeekNopCloser(bytes.NewReader(pkg.Data)), info, nil } func (a *TestArchive) Exists(pkg string) bool { diff --git a/internal/testutil/nopcloser.go b/internal/testutil/nopcloser.go new file mode 100644 index 00000000..8e00eefa --- /dev/null +++ b/internal/testutil/nopcloser.go @@ -0,0 +1,17 @@ +package testutil + +import ( + "io" +) + +// readSeekNopCloser is an io.ReadSeeker that does nothing on Close. +// It is an extension of io.NopCloser that also implements io.Seeker. +type readSeekNopCloser struct { + io.ReadSeeker +} + +func (readSeekNopCloser) Close() error { return nil } + +func ReadSeekNopCloser(r io.ReadSeeker) io.ReadSeekCloser { + return readSeekNopCloser{r} +} diff --git a/internal/testutil/pkgdata.go b/internal/testutil/pkgdata.go index 11bc9028..587d5dd9 100644 --- a/internal/testutil/pkgdata.go +++ b/internal/testutil/pkgdata.go @@ -197,3 +197,16 @@ func Lnk(mode int64, path, target string) TarEntry { }, } } + +// Hlk is a shortcut for creating a hard link TarEntry structure (with +// tar.Typeflag set to tar.TypeLink). Hlk stands for "Hard LinK". +func Hlk(mode int64, path, target string) TarEntry { + return TarEntry{ + Header: tar.Header{ + Typeflag: tar.TypeLink, + Name: path, + Mode: mode, + Linkname: target, + }, + } +} diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index 83b275e3..f208311a 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -6,11 +6,14 @@ import ( "io/fs" "os" "path/filepath" + "syscall" "github.com/canonical/chisel/internal/fsutil" ) func TreeDump(dir string) map[string]string { + var inodes []uint64 + pathsByInodes := make(map[uint64][]string) result := make(map[string]string) dirfs := os.DirFS(dir) err := fs.WalkDir(dirfs, ".", func(path string, d fs.DirEntry, err error) error { @@ -20,25 +23,28 @@ func TreeDump(dir string) map[string]string { if path == "." { return nil } + fpath := filepath.Join(dir, path) finfo, err := d.Info() if err != nil { - return fmt.Errorf("cannot get stat info for %q: %w", path, err) + return fmt.Errorf("cannot get stat info for %q: %w", fpath, err) } fperm := finfo.Mode() & fs.ModePerm ftype := finfo.Mode() & fs.ModeType if finfo.Mode()&fs.ModeSticky != 0 { fperm |= 01000 } - fpath := filepath.Join(dir, path) + var resultEntry string switch ftype { case fs.ModeDir: - result["/"+path+"/"] = fmt.Sprintf("dir %#o", fperm) + path = "/" + path + "/" + resultEntry = fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: lpath, err := os.Readlink(fpath) if err != nil { return err } - result["/"+path] = fmt.Sprintf("symlink %s", lpath) + path = "/" + path + resultEntry = fmt.Sprintf("symlink %s", lpath) case 0: // Regular data, err := os.ReadFile(fpath) if err != nil { @@ -51,15 +57,36 @@ func TreeDump(dir string) map[string]string { sum := sha256.Sum256(data) entry = fmt.Sprintf("file %#o %.4x", fperm, sum) } - result["/"+path] = entry + path = "/" + path + resultEntry = entry default: return fmt.Errorf("unknown file type %d: %s", ftype, fpath) } + result[path] = resultEntry + if ftype != fs.ModeDir { + stat, ok := finfo.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("cannot get syscall stat info for %q", fpath) + } + inode := stat.Ino + if len(pathsByInodes[inode]) == 1 { + inodes = append(inodes, inode) + } + pathsByInodes[inode] = append(pathsByInodes[inode], path) + } return nil }) if err != nil { panic(err) } + + // Append identifiers to paths who share an inode e.g. hard links. + for i := 0; i < len(inodes); i++ { + paths := pathsByInodes[inodes[i]] + for _, path := range paths { + result[path] = fmt.Sprintf("%s <%d>", result[path], i+1) + } + } return result } @@ -74,7 +101,8 @@ 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: + // Regular file. if entry.Size == 0 { return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) } else { From 8a65d8aa0ae1d4442ee8241534795fc05e050e94 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Thu, 5 Dec 2024 11:27:01 +0100 Subject: [PATCH 02/11] correct small typos left from review --- internal/manifest/report_test.go | 4 ++-- internal/testutil/nopcloser.go | 3 ++- internal/testutil/treedump.go | 13 +++++-------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/internal/manifest/report_test.go b/internal/manifest/report_test.go index 90398c8f..9195d864 100644 --- a/internal/manifest/report_test.go +++ b/internal/manifest/report_test.go @@ -40,7 +40,7 @@ var sampleFile = fsutil.Entry{ Link: "", } -var sampleSymLink = fsutil.Entry{ +var sampleSymlink = fsutil.Entry{ Path: "/base/example-link", Mode: fs.ModeSymlink | 0777, SHA256: "example-file_hash", @@ -111,7 +111,7 @@ var reportTests = []struct { }}, }, { summary: "Regular file link", - add: []sliceAndEntry{{entry: sampleSymLink, slice: oneSlice}}, + add: []sliceAndEntry{{entry: sampleSymlink, slice: oneSlice}}, expected: map[string]manifest.ReportEntry{ "/example-link": { Path: "/example-link", diff --git a/internal/testutil/nopcloser.go b/internal/testutil/nopcloser.go index 8e00eefa..7f72c653 100644 --- a/internal/testutil/nopcloser.go +++ b/internal/testutil/nopcloser.go @@ -5,13 +5,14 @@ import ( ) // readSeekNopCloser is an io.ReadSeeker that does nothing on Close. -// It is an extension of io.NopCloser that also implements io.Seeker. type readSeekNopCloser struct { io.ReadSeeker } func (readSeekNopCloser) Close() error { return nil } +// ReadSeekNopCloser is an extension of io.NopCloser that also implements +// io.Seeker. func ReadSeekNopCloser(r io.ReadSeeker) io.ReadSeekCloser { return readSeekNopCloser{r} } diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index f208311a..dbba8651 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -33,24 +33,23 @@ func TreeDump(dir string) map[string]string { if finfo.Mode()&fs.ModeSticky != 0 { fperm |= 01000 } - var resultEntry string + var entry string switch ftype { case fs.ModeDir: path = "/" + path + "/" - resultEntry = fmt.Sprintf("dir %#o", fperm) + entry = fmt.Sprintf("dir %#o", fperm) case fs.ModeSymlink: lpath, err := os.Readlink(fpath) if err != nil { return err } path = "/" + path - resultEntry = fmt.Sprintf("symlink %s", lpath) + entry = fmt.Sprintf("symlink %s", lpath) case 0: // Regular data, err := os.ReadFile(fpath) if err != nil { return fmt.Errorf("cannot read file: %w", err) } - var entry string if len(data) == 0 { entry = fmt.Sprintf("file %#o empty", fperm) } else { @@ -58,11 +57,10 @@ func TreeDump(dir string) map[string]string { entry = fmt.Sprintf("file %#o %.4x", fperm, sum) } path = "/" + path - resultEntry = entry default: return fmt.Errorf("unknown file type %d: %s", ftype, fpath) } - result[path] = resultEntry + result[path] = entry if ftype != fs.ModeDir { stat, ok := finfo.Sys().(*syscall.Stat_t) if !ok { @@ -82,8 +80,7 @@ func TreeDump(dir string) map[string]string { // Append identifiers to paths who share an inode e.g. hard links. for i := 0; i < len(inodes); i++ { - paths := pathsByInodes[inodes[i]] - for _, path := range paths { + for _, path := range pathsByInodes[inodes[i]] { result[path] = fmt.Sprintf("%s <%d>", result[path], i+1) } } From 16adea9d0a032f643fed5860d60b1df359225bf9 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 16 Dec 2024 17:32:22 +0100 Subject: [PATCH 03/11] chore: rename Hlk to Hrd --- internal/deb/extract_test.go | 14 +++++++------- internal/slicer/slicer_test.go | 22 +++++++++++----------- internal/testutil/pkgdata.go | 6 +++--- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 36f71b94..12ec020d 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -335,7 +335,7 @@ var extractTests = []extractTest{{ }, }, result: map[string]string{ - "/日本/": "dir 0766", + "/日本/": "dir 0766", "/日本/語": "file 0644 85738f8f", }, notCreated: []string{}, @@ -359,7 +359,7 @@ var extractTests = []extractTest{{ pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "text for file"), - testutil.Hlk(0644, "./hardlink", "./file"), + testutil.Hrd(0644, "./hardlink", "./file"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -378,7 +378,7 @@ var extractTests = []extractTest{{ pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "text for file"), - testutil.Hlk(0644, "./hardlink", "./file"), + testutil.Hrd(0644, "./hardlink", "./file"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -395,7 +395,7 @@ var extractTests = []extractTest{{ summary: "Dangling hard link", pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), - testutil.Hlk(0644, "./hardlink", "./non-existing-target"), + testutil.Hrd(0644, "./hardlink", "./non-existing-target"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -409,8 +409,8 @@ var extractTests = []extractTest{{ summary: "Multiple dangling hard links", pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), - testutil.Hlk(0644, "./hardlink1", "./non-existing-target"), - testutil.Hlk(0644, "./hardlink2", "./non-existing-target"), + testutil.Hrd(0644, "./hardlink1", "./non-existing-target"), + testutil.Hrd(0644, "./hardlink2", "./non-existing-target"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ @@ -427,7 +427,7 @@ var extractTests = []extractTest{{ pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Lnk(0644, "./symlink", "./file"), - testutil.Hlk(0644, "./hardlink", "./symlink"), + testutil.Hrd(0644, "./hardlink", "./symlink"), }), options: deb.ExtractOptions{ Extract: map[string][]deb.ExtractInfo{ diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 4e20ddd5..f2e98ce0 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1488,7 +1488,7 @@ var slicerTests = []slicerTest{{ Data: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "foo"), - testutil.Hlk(0644, "./hardlink", "./file"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1521,8 +1521,8 @@ var slicerTests = []slicerTest{{ Data: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "foo"), - testutil.Hlk(0644, "./hardlink1", "./file"), - testutil.Hlk(0644, "./hardlink2", "./file"), + testutil.Hrd(0644, "./hardlink1", "./file"), + testutil.Hrd(0644, "./hardlink2", "./file"), }), }}, release: map[string]string{ @@ -1552,8 +1552,8 @@ var slicerTests = []slicerTest{{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file1", "text for file1"), testutil.Reg(0644, "./file2", "text for file2"), - testutil.Hlk(0644, "./hardlink1", "./file1"), - testutil.Hlk(0644, "./hardlink2", "./file2"), + testutil.Hrd(0644, "./hardlink1", "./file1"), + testutil.Hrd(0644, "./hardlink2", "./file2"), }), }}, release: map[string]string{ @@ -1586,7 +1586,7 @@ var slicerTests = []slicerTest{{ Data: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "foo"), - testutil.Hlk(0644, "./hardlink", "./file"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1615,7 +1615,7 @@ var slicerTests = []slicerTest{{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "foo"), testutil.Lnk(0644, "./symlink", "./file"), - testutil.Hlk(0644, "./hardlink", "./symlink"), + testutil.Hrd(0644, "./hardlink", "./symlink"), }), }}, release: map[string]string{ @@ -1647,14 +1647,14 @@ var slicerTests = []slicerTest{{ Data: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file1", "foo"), - testutil.Hlk(0644, "./hardlink1", "./file1"), + testutil.Hrd(0644, "./hardlink1", "./file1"), }), }, { Name: "test-package2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file2", "foo"), - testutil.Hlk(0644, "./hardlink2", "./file2"), + testutil.Hrd(0644, "./hardlink2", "./file2"), }), }}, release: map[string]string{ @@ -1696,7 +1696,7 @@ var slicerTests = []slicerTest{{ Data: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "foo"), - testutil.Hlk(0644, "./hardlink", "./file"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1721,7 +1721,7 @@ var slicerTests = []slicerTest{{ Data: testutil.MustMakeDeb([]testutil.TarEntry{ testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "foo"), - testutil.Hlk(0644, "./hardlink", "./file"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ diff --git a/internal/testutil/pkgdata.go b/internal/testutil/pkgdata.go index 587d5dd9..5875d93c 100644 --- a/internal/testutil/pkgdata.go +++ b/internal/testutil/pkgdata.go @@ -198,9 +198,9 @@ func Lnk(mode int64, path, target string) TarEntry { } } -// Hlk is a shortcut for creating a hard link TarEntry structure (with -// tar.Typeflag set to tar.TypeLink). Hlk stands for "Hard LinK". -func Hlk(mode int64, path, target string) TarEntry { +// Hrd is a shortcut for creating a hard link TarEntry structure (with +// tar.Typeflag set to tar.TypeLink). Hrd stands for "HaRD link". +func Hrd(mode int64, path, target string) TarEntry { return TarEntry{ Header: tar.Header{ Typeflag: tar.TypeLink, From 321bacea7bc18935a1ef80ac6d21a01608ecac6c Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 16 Dec 2024 17:43:02 +0100 Subject: [PATCH 04/11] chore: sanitizeTarPath returns explicit ok --- internal/deb/extract.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 02cbfb02..6c43e1e6 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -134,8 +134,8 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { return err } - sourcePath := sanitizeTarPath(tarHeader.Name) - if sourcePath == "" { + sourcePath, ok := sanitizeTarPath(tarHeader.Name) + if !ok { continue } @@ -242,7 +242,10 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { // The hard link could not be created because the content // was not extracted previously. Add this hard link entry // to the pending list to extract later. - relLinkPath := sanitizeTarPath(tarHeader.Linkname) + relLinkPath, ok := sanitizeTarPath(tarHeader.Linkname) + if !ok { + return fmt.Errorf("invalid link target %s", tarHeader.Linkname) + } info := pendingHardLink{ path: targetPath, extractInfos: extractInfos, @@ -318,8 +321,8 @@ func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) err return err } - sourcePath := sanitizeTarPath(tarHeader.Name) - if sourcePath == "" { + sourcePath, ok := sanitizeTarPath(tarHeader.Name) + if !ok { continue } @@ -431,9 +434,9 @@ func parentDirs(path string) []string { // sanitizeTarPath removes the leading "./" from the source path in the tarball, // and verifies that the path is not empty. -func sanitizeTarPath(path string) string { +func sanitizeTarPath(path string) (string, bool) { if len(path) < 3 || path[0] != '.' || path[1] != '/' { - return "" + return "", false } - return path[1:] + return path[1:], true } From 801f3c5dcf187edd019211ed422a466176f09562 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 16 Dec 2024 17:43:31 +0100 Subject: [PATCH 05/11] simplify map lookup --- internal/deb/extract.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 6c43e1e6..688fea20 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -326,8 +326,8 @@ func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) err continue } - links, ok := opts.pendingLinks[sourcePath] - if !ok || len(links) == 0 { + links := opts.pendingLinks[sourcePath] + if len(links) == 0 { continue } From 44149b7ca2d88a2f486ade4de5d4ff69b945c54f Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 16 Dec 2024 17:49:08 +0100 Subject: [PATCH 06/11] remove unnecessary concurrency safety --- internal/manifest/report.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/manifest/report.go b/internal/manifest/report.go index 77ef0a0a..af24194b 100644 --- a/internal/manifest/report.go +++ b/internal/manifest/report.go @@ -5,7 +5,6 @@ import ( "io/fs" "path/filepath" "strings" - "sync/atomic" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/setup" @@ -32,7 +31,7 @@ type Report struct { Entries map[string]ReportEntry // lastHardLinkID is used internally to allocate unique HardLinkID for hard // links. - lastHardLinkID atomic.Uint64 + lastHardLinkID uint64 } // NewReport returns an empty report for content that will be based at the @@ -67,8 +66,8 @@ func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { return fmt.Errorf("cannot add hard link %s to report: target %s not previously added", relPath, relLinkPath) } if entry.HardLinkID == 0 { - entry.HardLinkID = r.lastHardLinkID.Add(1) - r.lastHardLinkID.Store(entry.HardLinkID) + r.lastHardLinkID += 1 + entry.HardLinkID = r.lastHardLinkID r.Entries[relLinkPath] = entry } hardLinkID = entry.HardLinkID From d6ea9947fd5e5128cf326a08b7de2919c074ec60 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 16 Dec 2024 17:51:19 +0100 Subject: [PATCH 07/11] move seeking outside --- internal/deb/extract.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 688fea20..d44082fd 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -263,6 +263,13 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { ExtractOptions: options, pendingLinks: pendingHardLinks, } + offset, err := pkgReader.Seek(0, io.SeekStart) + if err != nil { + return err + } + if offset != 0 { + return fmt.Errorf("internal error: cannot seek to the beginning of the package") + } err = extractHardLinks(pkgReader, extractHardLinkOptions) if err != nil { return err @@ -298,13 +305,6 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { - offset, err := pkgReader.Seek(0, io.SeekStart) - if err != nil { - return err - } - if offset != 0 { - return fmt.Errorf("internal error: cannot seek to the beginning of the package") - } dataReader, err := getDataReader(pkgReader) if err != nil { return err From 7616cb743bd38cb045dfdc3c1dbc6c1dc7cd9c29 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 16 Dec 2024 18:05:31 +0100 Subject: [PATCH 08/11] only report first error hardlink missing target --- internal/deb/extract.go | 5 +---- internal/deb/extract_test.go | 4 +--- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index d44082fd..689d8718 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -373,11 +373,8 @@ func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) err link.path, target)) } } - if len(errs) == 1 { - return fmt.Errorf("%s", errs[0]) - } sort.Strings(errs) - return fmt.Errorf("\n- %s", strings.Join(errs, "\n- ")) + return fmt.Errorf("%s", errs[0]) } return nil diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 12ec020d..d4d6b540 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -419,9 +419,7 @@ var extractTests = []extractTest{{ }}, }, }, - error: `cannot extract from package "test-package": ` + - `\n- cannot create hard link /hardlink1: no content at /non-existing-target` + - `\n- cannot create hard link /hardlink2: no content at /non-existing-target`, + error: `cannot extract from package "test-package": cannot create hard link /hardlink1: no content at /non-existing-target`, }, { summary: "Hard link does not follow the symlink", pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ From 31d2f014453cd62d597b2d8b2b00cb0f8ba54c62 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 16 Dec 2024 18:11:04 +0100 Subject: [PATCH 09/11] lint --- internal/deb/extract_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index d4d6b540..353552b1 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -335,7 +335,7 @@ var extractTests = []extractTest{{ }, }, result: map[string]string{ - "/日本/": "dir 0766", + "/日本/": "dir 0766", "/日本/語": "file 0644 85738f8f", }, notCreated: []string{}, From 78808c84e4abafc12ca7f2943a128c90dea07a54 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Sun, 17 Mar 2024 13:27:13 +0100 Subject: [PATCH 10/11] rename hard_link_id to inode --- internal/manifest/manifest.go | 8 ++-- internal/manifest/manifest_test.go | 52 ++++++++++----------- internal/manifest/report.go | 33 +++++++------- internal/manifest/report_test.go | 72 +++++++++++++++--------------- internal/slicer/slicer_test.go | 6 +-- 5 files changed, 86 insertions(+), 85 deletions(-) diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 4f3b35fd..d8a09d92 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -39,7 +39,7 @@ type Path struct { FinalSHA256 string `json:"final_sha256,omitempty"` Size uint64 `json:"size,omitempty"` Link string `json:"link,omitempty"` - HardLinkID uint64 `json:"hard_link_id,omitempty"` + Inode uint64 `json:"inode,omitempty"` } type Content struct { @@ -292,7 +292,7 @@ func manifestAddReport(dbw *jsonwall.DBWriter, report *Report) error { FinalSHA256: entry.FinalSHA256, Size: uint64(entry.Size), Link: entry.Link, - HardLinkID: entry.HardLinkID, + Inode: entry.Inode, }) if err != nil { return err @@ -345,10 +345,10 @@ func fastValidate(options *WriteOptions) (err error) { return fmt.Errorf("path %q refers to missing slice %s", entry.Path, slice.String()) } } - if entry.HardLinkID != 0 { + if entry.Inode != 0 { // TODO remove the following line after upgrading to Go 1.22 or higher. e := entry - hardLinkGroups[e.HardLinkID] = append(hardLinkGroups[e.HardLinkID], &e) + hardLinkGroups[e.Inode] = append(hardLinkGroups[e.Inode], &e) } } // Entries within a hard link group must have same content. diff --git a/internal/manifest/manifest_test.go b/internal/manifest/manifest_test.go index 53b28993..7c0a72b2 100644 --- a/internal/manifest/manifest_test.go +++ b/internal/manifest/manifest_test.go @@ -43,9 +43,9 @@ var readManifestTests = []struct { {"kind":"package","name":"pkg1","version":"v1","sha256":"hash1","arch":"arch1"} {"kind":"package","name":"pkg2","version":"v2","sha256":"hash2","arch":"arch2"} {"kind":"path","path":"/dir/file","mode":"0644","slices":["pkg1_myslice"],"sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","final_sha256":"8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6","size":21} - {"kind":"path","path":"/dir/file2","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"hard_link_id":1} + {"kind":"path","path":"/dir/file2","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"inode":1} {"kind":"path","path":"/dir/foo/bar/","mode":"01777","slices":["pkg2_myotherslice","pkg1_myslice"]} - {"kind":"path","path":"/dir/hardlink","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"hard_link_id":1} + {"kind":"path","path":"/dir/hardlink","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"inode":1} {"kind":"path","path":"/dir/link/file","mode":"0644","slices":["pkg1_myslice"],"link":"/dir/file"} {"kind":"path","path":"/manifest/manifest.wall","mode":"0644","slices":["pkg1_manifest"]} {"kind":"slice","name":"pkg1_manifest"} @@ -55,9 +55,9 @@ var readManifestTests = []struct { mfest: &manifestContents{ Paths: []*manifest.Path{ {Kind: "path", Path: "/dir/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", FinalSHA256: "8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6", Size: 0x15, Link: ""}, - {Kind: "path", Path: "/dir/file2", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", Size: 0x03, Link: "", HardLinkID: 0x01}, + {Kind: "path", Path: "/dir/file2", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", Size: 0x03, Link: "", Inode: 0x01}, {Kind: "path", Path: "/dir/foo/bar/", Mode: "01777", Slices: []string{"pkg2_myotherslice", "pkg1_myslice"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""}, - {Kind: "path", Path: "/dir/hardlink", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", Size: 0x03, Link: "", HardLinkID: 0x01}, + {Kind: "path", Path: "/dir/hardlink", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", Size: 0x03, Link: "", Inode: 0x01}, {Kind: "path", Path: "/dir/link/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: "/dir/file"}, {Kind: "path", Path: "/manifest/manifest.wall", Mode: "0644", Slices: []string{"pkg1_manifest"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""}, }, @@ -560,7 +560,7 @@ var generateManifestTests = []struct { Size: 1234, Slices: map[*setup.Slice]bool{slice1: true}, FinalSHA256: "final-hash", - HardLinkID: 1, + Inode: 1, }, "/hardlink": { Path: "/hardlink", @@ -569,7 +569,7 @@ var generateManifestTests = []struct { Size: 1234, Slices: map[*setup.Slice]bool{slice1: true}, FinalSHA256: "final-hash", - HardLinkID: 1, + Inode: 1, }, }, }, @@ -588,7 +588,7 @@ var generateManifestTests = []struct { Size: 1234, SHA256: "hash", FinalSHA256: "final-hash", - HardLinkID: 1, + Inode: 1, }, { Kind: "path", Path: "/hardlink", @@ -597,7 +597,7 @@ var generateManifestTests = []struct { Size: 1234, SHA256: "hash", FinalSHA256: "final-hash", - HardLinkID: 1, + Inode: 1, }}, Packages: []*manifest.Package{{ Kind: "package", @@ -626,9 +626,9 @@ var generateManifestTests = []struct { Root: "/", Entries: map[string]manifest.ReportEntry{ "/file": { - Path: "/file", - Slices: map[*setup.Slice]bool{slice1: true}, - HardLinkID: 2, + Path: "/file", + Slices: map[*setup.Slice]bool{slice1: true}, + Inode: 2, }, }, }, @@ -639,9 +639,9 @@ var generateManifestTests = []struct { Root: "/", Entries: map[string]manifest.ReportEntry{ "/file": { - Path: "/file", - Slices: map[*setup.Slice]bool{slice1: true}, - HardLinkID: 1, + Path: "/file", + Slices: map[*setup.Slice]bool{slice1: true}, + Inode: 1, }, }, }, @@ -652,20 +652,20 @@ var generateManifestTests = []struct { Root: "/", Entries: map[string]manifest.ReportEntry{ "/file": { - Path: "/file", - Mode: 0456, - SHA256: "hash", - Size: 1234, - Slices: map[*setup.Slice]bool{slice1: true}, - HardLinkID: 1, + Path: "/file", + Mode: 0456, + SHA256: "hash", + Size: 1234, + Slices: map[*setup.Slice]bool{slice1: true}, + Inode: 1, }, "/hardlink": { - Path: "/hardlink", - Mode: 0456, - SHA256: "different-hash", - Size: 1234, - Slices: map[*setup.Slice]bool{slice1: true}, - HardLinkID: 1, + Path: "/hardlink", + Mode: 0456, + SHA256: "different-hash", + Size: 1234, + Slices: map[*setup.Slice]bool{slice1: true}, + Inode: 1, }, }, }, diff --git a/internal/manifest/report.go b/internal/manifest/report.go index af24194b..a74d8064 100644 --- a/internal/manifest/report.go +++ b/internal/manifest/report.go @@ -18,8 +18,9 @@ type ReportEntry struct { Slices map[*setup.Slice]bool Link string FinalSHA256 string - // If HardLinkID is greater than 0, all entries with the same id represent hard links to the same inode. - HardLinkID uint64 + // If Inode is greater than 0, all entries represent hard links to the same + // inode. + Inode uint64 } // Report holds the information about files and directories created when slicing @@ -29,9 +30,9 @@ type Report struct { Root string // Entries holds all reported content, indexed by their path. Entries map[string]ReportEntry - // lastHardLinkID is used internally to allocate unique HardLinkID for hard + // lastInode is used internally to allocate unique Inode for hard // links. - lastHardLinkID uint64 + lastInode uint64 } // NewReport returns an empty report for content that will be based at the @@ -57,7 +58,7 @@ func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { return fmt.Errorf("cannot add path to report: %s", err) } - var hardLinkID uint64 + var inode uint64 fsEntryCpy := *fsEntry if fsEntry.HardLink { relLinkPath, _ := r.sanitizeAbsPath(fsEntry.Link, false) @@ -65,12 +66,12 @@ func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { if !ok { return fmt.Errorf("cannot add hard link %s to report: target %s not previously added", relPath, relLinkPath) } - if entry.HardLinkID == 0 { - r.lastHardLinkID += 1 - entry.HardLinkID = r.lastHardLinkID + if entry.Inode == 0 { + r.lastInode += 1 + entry.Inode = r.lastInode r.Entries[relLinkPath] = entry } - hardLinkID = entry.HardLinkID + inode = entry.Inode fsEntryCpy.SHA256 = entry.SHA256 fsEntryCpy.Size = entry.Size fsEntryCpy.Link = entry.Link @@ -90,13 +91,13 @@ func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { r.Entries[relPath] = entry } else { r.Entries[relPath] = ReportEntry{ - Path: relPath, - Mode: fsEntry.Mode, - SHA256: fsEntryCpy.SHA256, - Size: fsEntryCpy.Size, - Slices: map[*setup.Slice]bool{slice: true}, - Link: fsEntryCpy.Link, - HardLinkID: hardLinkID, + Path: relPath, + Mode: fsEntry.Mode, + SHA256: fsEntryCpy.SHA256, + Size: fsEntryCpy.Size, + Slices: map[*setup.Slice]bool{slice: true}, + Link: fsEntryCpy.Link, + Inode: inode, } } return nil diff --git a/internal/manifest/report_test.go b/internal/manifest/report_test.go index 9195d864..f6f4c04b 100644 --- a/internal/manifest/report_test.go +++ b/internal/manifest/report_test.go @@ -281,20 +281,20 @@ var reportTests = []struct { {entry: sampleHardLink, slice: oneSlice}}, expected: map[string]manifest.ReportEntry{ "/example-file": { - Path: "/example-file", - Mode: 0777, - SHA256: "example-file_hash", - Size: 5678, - Slices: map[*setup.Slice]bool{oneSlice: true}, - HardLinkID: 1, + Path: "/example-file", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + Inode: 1, }, "/example-hard-link": { - Path: "/example-hard-link", - Mode: 0777, - SHA256: "example-file_hash", - Size: 5678, - Slices: map[*setup.Slice]bool{oneSlice: true}, - HardLinkID: 1, + Path: "/example-hard-link", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + Inode: 1, }, }, }, { @@ -324,36 +324,36 @@ var reportTests = []struct { }}, expected: map[string]manifest.ReportEntry{ "/example-file": { - Path: "/example-file", - Mode: 0777, - SHA256: "example-file_hash", - Size: 5678, - Slices: map[*setup.Slice]bool{oneSlice: true}, - HardLinkID: 1, + Path: "/example-file", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + Inode: 1, }, "/example-hard-link": { - Path: "/example-hard-link", - Mode: 0777, - SHA256: "example-file_hash", - Size: 5678, - Slices: map[*setup.Slice]bool{oneSlice: true}, - HardLinkID: 1, + Path: "/example-hard-link", + Mode: 0777, + SHA256: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + Inode: 1, }, "/another-file": { - Path: "/another-file", - Mode: 0777, - SHA256: "another-file_hash", - Size: 5678, - Slices: map[*setup.Slice]bool{otherSlice: true}, - HardLinkID: 2, + Path: "/another-file", + Mode: 0777, + SHA256: "another-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{otherSlice: true}, + Inode: 2, }, "/another-hard-link": { - Path: "/another-hard-link", - Mode: 0777, - SHA256: "another-file_hash", - Size: 5678, - Slices: map[*setup.Slice]bool{otherSlice: true}, - HardLinkID: 2, + Path: "/another-hard-link", + Mode: 0777, + SHA256: "another-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{otherSlice: true}, + Inode: 2, }, }, }} diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index f2e98ce0..d9c62cff 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1910,9 +1910,9 @@ func treeDumpManifestPaths(mfest *manifest.Manifest) (map[string]string, error) } } - if path.HardLinkID != 0 { - // Append to the end of the path dump. - fsDump = fmt.Sprintf("%s <%d>", fsDump, path.HardLinkID) + if path.Inode != 0 { + // Append to the end of the path dump. + fsDump = fmt.Sprintf("%s <%d>", fsDump, path.Inode) } // append {slice1, ..., sliceN} to the end of the path dump. From e82685db9f1bc2940c49a15d5d80fe4455b70c51 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Sun, 17 Mar 2024 13:41:35 +0100 Subject: [PATCH 11/11] remove hardlink bool and use mode and link instead --- internal/fsutil/create.go | 35 +++++++++++++++++------------- internal/fsutil/create_test.go | 21 +++++++++++++++++- internal/manifest/manifest.go | 5 +---- internal/manifest/manifest_test.go | 14 ------------ internal/manifest/report.go | 3 ++- internal/manifest/report_test.go | 26 ++++++++++------------ internal/slicer/slicer.go | 2 +- 7 files changed, 56 insertions(+), 50 deletions(-) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 2d9a112f..92b1592f 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -27,12 +27,11 @@ type CreateOptions struct { } type Entry struct { - Path string - Mode fs.FileMode - SHA256 string - Size int - Link string - HardLink bool + Path string + Mode fs.FileMode + SHA256 string + Size int + Link string } // Create creates a filesystem entry according to the provided options and returns @@ -48,7 +47,6 @@ func Create(options *CreateOptions) (*Entry, error) { var err error var hash string - var hardLink bool if o.MakeParents { if err := os.MkdirAll(filepath.Dir(o.Path), 0755); err != nil { return nil, err @@ -59,7 +57,6 @@ func Create(options *CreateOptions) (*Entry, error) { case 0: if o.Link != "" { err = createHardLink(o) - hardLink = true } else { err = createFile(o) hash = hex.EncodeToString(rp.h.Sum(nil)) @@ -75,12 +72,21 @@ func Create(options *CreateOptions) (*Entry, error) { return nil, err } + // Entry should describe the created file, not the target the link points to. s, err := os.Lstat(o.Path) if err != nil { return nil, err } mode := s.Mode() - if o.OverrideMode && mode != o.Mode && o.Link == "" { + if o.Link != "" { + if options.Mode.IsRegular() { + // Hard link. + // In the case where the hard link points to a symlink the entry + // should identify the created file and not the symlink. A hard link + // is identified by the mode being regular and link not empty. + mode = mode &^ fs.ModeSymlink + } + } else if o.OverrideMode && mode != o.Mode { err := os.Chmod(o.Path, o.Mode) if err != nil { return nil, err @@ -89,12 +95,11 @@ func Create(options *CreateOptions) (*Entry, error) { } entry := &Entry{ - Path: o.Path, - Mode: mode, - SHA256: hash, - Size: rp.size, - Link: o.Link, - HardLink: hardLink, + Path: o.Path, + Mode: mode, + SHA256: hash, + Size: rp.size, + Link: o.Link, } return entry, nil } diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index a87b2b0d..1332191b 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -161,6 +161,23 @@ var createTests = []createTest{{ opts.Link = filepath.Join(dir, opts.Link) }, error: `link /[^ ]*/file /[^ ]*/hardlink: file exists`, +}, { + summary: "Hard link to symlink", + options: fsutil.CreateOptions{ + Path: "hardlink", + Link: "bar", + Mode: 0644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) { + err := os.Symlink("foo", filepath.Join(dir, "bar")) + c.Assert(err, IsNil) + opts.Link = filepath.Join(dir, opts.Link) + }, + result: map[string]string{ + "/bar": "symlink foo <1>", + "/hardlink": "symlink foo <1>", + }, }, { summary: "The mode of a dir can be overridden", options: fsutil.CreateOptions{ @@ -285,10 +302,12 @@ func (s *S) TestCreate(c *C) { c.Assert(err, IsNil) c.Assert(testutil.TreeDump(dir), DeepEquals, test.result) - if entry.HardLink { + if options.Mode.IsRegular() && options.Link != "" { // We should test hard link entries differently because // fsutil.Create does not return hash or size when it creates hard // links. + c.Assert(entry.Link, Equals, options.Link) + c.Assert(entry.Mode.IsRegular(), Equals, true) pathInfo, err := os.Lstat(entry.Path) c.Assert(err, IsNil) linkInfo, err := os.Lstat(entry.Link) diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index d8a09d92..aeb40df7 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -365,7 +365,7 @@ func fastValidate(options *WriteOptions) (err error) { }) e0 := entries[0] for _, e := range entries[1:] { - if e.Link != e0.Link || e.Mode != e0.Mode || e.SHA256 != e0.SHA256 || + if e.Link != e0.Link || unixPerm(e.Mode) != unixPerm(e0.Mode) || e.SHA256 != e0.SHA256 || e.Size != e0.Size || e.FinalSHA256 != e0.FinalSHA256 { return fmt.Errorf("hard linked paths %q and %q have diverging contents", e0.Path, e.Path) } @@ -385,9 +385,6 @@ func validateReportEntry(entry *ReportEntry) (err error) { switch entry.Mode & fs.ModeType { case 0: // Regular file. - if entry.Link != "" { - return fmt.Errorf("link set for regular file") - } case fs.ModeDir: if entry.Link != "" { return fmt.Errorf("link set for directory") diff --git a/internal/manifest/manifest_test.go b/internal/manifest/manifest_test.go index 7c0a72b2..de2a71c8 100644 --- a/internal/manifest/manifest_test.go +++ b/internal/manifest/manifest_test.go @@ -407,20 +407,6 @@ var generateManifestTests = []struct { }, packageInfo: []*archive.PackageInfo{}, error: `internal error: invalid manifest: slice package1_slice1 refers to missing package "package1"`, -}, { - summary: "Invalid path: link set for regular file", - report: &manifest.Report{ - Root: "/", - Entries: map[string]manifest.ReportEntry{ - "/file": { - Path: "/file", - Mode: 0456, - Slices: map[*setup.Slice]bool{slice1: true}, - Link: "something", - }, - }, - }, - error: `internal error: invalid manifest: path "/file" has invalid options: link set for regular file`, }, { summary: "Invalid path: slices is empty", report: &manifest.Report{ diff --git a/internal/manifest/report.go b/internal/manifest/report.go index a74d8064..4fe9915f 100644 --- a/internal/manifest/report.go +++ b/internal/manifest/report.go @@ -60,7 +60,8 @@ func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { var inode uint64 fsEntryCpy := *fsEntry - if fsEntry.HardLink { + if fsEntry.Mode.IsRegular() && fsEntry.Link != "" { + // Hard link. relLinkPath, _ := r.sanitizeAbsPath(fsEntry.Link, false) entry, ok := r.Entries[relLinkPath] if !ok { diff --git a/internal/manifest/report_test.go b/internal/manifest/report_test.go index f6f4c04b..34e7d5e4 100644 --- a/internal/manifest/report_test.go +++ b/internal/manifest/report_test.go @@ -49,10 +49,9 @@ var sampleSymlink = fsutil.Entry{ } var sampleHardLink = fsutil.Entry{ - Path: "/base/example-hard-link", - Mode: sampleFile.Mode, - Link: "/base/example-file", - HardLink: true, + Path: "/base/example-hard-link", + Mode: sampleFile.Mode, + Link: "/base/example-file", } var sampleFileMutated = fsutil.Entry{ @@ -199,16 +198,16 @@ var reportTests = []struct { }, { summary: "Error for same path distinct link", add: []sliceAndEntry{ - {entry: sampleFile, slice: oneSlice}, + {entry: sampleSymlink, slice: oneSlice}, {entry: fsutil.Entry{ - Path: sampleFile.Path, - Mode: sampleFile.Mode, - SHA256: sampleFile.SHA256, - Size: sampleFile.Size, + Path: sampleSymlink.Path, + Mode: sampleSymlink.Mode, + SHA256: sampleSymlink.SHA256, + Size: sampleSymlink.Size, Link: "distinct link", }, slice: oneSlice}, }, - err: `path /example-file reported twice with diverging link: "distinct link" != ""`, + err: `path /example-link reported twice with diverging link: "distinct link" != "/base/example-file"`, }, { summary: "Error for path outside root", add: []sliceAndEntry{ @@ -315,10 +314,9 @@ var reportTests = []struct { slice: otherSlice, }, { entry: fsutil.Entry{ - Path: "/base/another-hard-link", - Mode: 0777, - Link: "/base/another-file", - HardLink: true, + Path: "/base/another-hard-link", + Mode: 0777, + Link: "/base/another-file", }, slice: otherSlice, }}, diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 464de4f2..39f4fc4a 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -212,7 +212,7 @@ func Run(options *RunOptions) error { data := pathData{ mutable: mutable, until: until, - hardLink: entry.HardLink, + hardLink: entry.Mode.IsRegular() && entry.Link != "", } addKnownPath(knownPaths, relPath, data) }