From e82685db9f1bc2940c49a15d5d80fe4455b70c51 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Sun, 17 Mar 2024 13:41:35 +0100 Subject: [PATCH] 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) }