From 81f351a0daeb8415126078478a23ac6662def2e7 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 28 Jun 2024 17:04:24 +0200 Subject: [PATCH] feat: report mutated files (#131) The paths with "mutable: true" property can be "mutated" with mutation scripts. Thus, the files matching those paths may have a different size and hash after the mutation scripts have been run. The report should reflect these changes by updating the entries of the mutated paths. Paths can also have the "until: mutate" property. This means that those files are available until the mutation scripts have been run and are not present in the final file system. Thus, they should not be part of the report either. This commit adds support for both -- it updates the changed properties of mutated files and makes sure not to add report entries for "until: mutate". --- internal/fsutil/create.go | 7 +- internal/fsutil/create_test.go | 48 +++++------ internal/scripts/scripts.go | 22 ++++- internal/scripts/scripts_test.go | 68 +++++++++++++++ internal/slicer/report.go | 73 ++++++++++++---- internal/slicer/report_test.go | 135 ++++++++++++++++++++++-------- internal/slicer/slicer.go | 39 +++++---- internal/slicer/slicer_test.go | 137 +++++++++++++++++-------------- internal/testutil/treedump.go | 24 ++++++ 9 files changed, 394 insertions(+), 159 deletions(-) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 90bcfb5e..48b12cb7 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -45,6 +45,7 @@ func Create(options *CreateOptions) (*Entry, error) { return nil, err } } + switch o.Mode & fs.ModeType { case 0: err = createFile(o) @@ -60,9 +61,13 @@ func Create(options *CreateOptions) (*Entry, error) { return nil, err } + s, err := os.Lstat(o.Path) + if err != nil { + return nil, err + } entry := &Entry{ Path: o.Path, - Mode: o.Mode, + Mode: s.Mode(), Hash: hash, Size: rp.size, Link: o.Link, diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 50c0c746..7288d878 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -2,7 +2,6 @@ package fsutil_test import ( "bytes" - "fmt" "io/fs" "os" "path/filepath" @@ -80,6 +79,20 @@ var createTests = []createTest{{ // mode is not updated. "/foo/": "dir 0765", }, +}, { + options: fsutil.CreateOptions{ + Path: "foo", + // Mode should be ignored for existing entry. + Mode: 0644, + Data: bytes.NewBufferString("changed"), + }, + hackdir: func(c *C, dir string) { + c.Assert(os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666), IsNil) + }, + result: map[string]string{ + // mode is not updated. + "/foo": "file 0666 d67e2e94", + }, }} func (s *S) TestCreate(c *C) { @@ -111,33 +124,12 @@ func (s *S) TestCreate(c *C) { 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. - c.Assert(dumpFSEntry(entry, dir)[test.options.Path], DeepEquals, test.result[test.options.Path]) - } -} - -// dumpFSEntry returns the file entry in the same format as [testutil.TreeDump]. -func dumpFSEntry(fsEntry *fsutil.Entry, root string) map[string]string { - result := make(map[string]string) - path := strings.TrimPrefix(fsEntry.Path, root) - fperm := fsEntry.Mode.Perm() - if fsEntry.Mode&fs.ModeSticky != 0 { - fperm |= 01000 - } - switch fsEntry.Mode.Type() { - case fs.ModeDir: - result[path+"/"] = fmt.Sprintf("dir %#o", fperm) - case fs.ModeSymlink: - result[path] = fmt.Sprintf("symlink %s", fsEntry.Link) - case 0: // Regular - var entry string - if fsEntry.Size == 0 { - entry = fmt.Sprintf("file %#o empty", fsEntry.Mode.Perm()) - } else { - entry = fmt.Sprintf("file %#o %s", fperm, fsEntry.Hash[:8]) + 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 + "/" } - result[path] = entry - default: - panic(fmt.Errorf("unknown file type %d: %s", fsEntry.Mode.Type(), path)) + c.Assert(testutil.TreeDumpEntry(entry), DeepEquals, test.result[slashPath]) } - return result } diff --git a/internal/scripts/scripts.go b/internal/scripts/scripts.go index f4c83352..eead1a81 100644 --- a/internal/scripts/scripts.go +++ b/internal/scripts/scripts.go @@ -1,13 +1,16 @@ package scripts import ( - "go.starlark.net/resolve" - "go.starlark.net/starlark" - + "bytes" "fmt" "os" "path/filepath" "strings" + + "go.starlark.net/resolve" + "go.starlark.net/starlark" + + "github.com/canonical/chisel/internal/fsutil" ) func init() { @@ -33,6 +36,9 @@ type ContentValue struct { RootDir string CheckRead func(path string) error CheckWrite func(path string) error + // OnWrite has to be called after a successful write with the entry resulting + // from the write. + OnWrite func(entry *fsutil.Entry) error } // Content starlark.Value interface @@ -171,10 +177,18 @@ func (c *ContentValue) Write(thread *starlark.Thread, fn *starlark.Builtin, args // No mode parameter for now as slices are supposed to list files // explicitly instead. - err = os.WriteFile(fpath, fdata, 0644) + entry, err := fsutil.Create(&fsutil.CreateOptions{ + Path: fpath, + Data: bytes.NewReader(fdata), + Mode: 0644, + }) if err != nil { return nil, c.polishError(path, err) } + err = c.OnWrite(entry) + if err != nil { + return nil, err + } return starlark.None, nil } diff --git a/internal/scripts/scripts_test.go b/internal/scripts/scripts_test.go index 211e5545..f86711a6 100644 --- a/internal/scripts/scripts_test.go +++ b/internal/scripts/scripts_test.go @@ -4,9 +4,11 @@ import ( "fmt" "os" "path/filepath" + "strings" . "gopkg.in/check.v1" + "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/testutil" ) @@ -17,6 +19,7 @@ type scriptsTest struct { hackdir func(c *C, dir string) script string result map[string]string + mutated map[string]string checkr func(path string) error checkw func(path string) error error string @@ -44,6 +47,10 @@ var scriptsTests = []scriptsTest{{ "/foo/file1.txt": "file 0644 5b41362b", "/foo/file2.txt": "file 0644 d98cf53e", }, + mutated: map[string]string{ + "/foo/file1.txt": "file 0644 5b41362b", + "/foo/file2.txt": "file 0644 d98cf53e", + }, }, { summary: "Read a file", content: map[string]string{ @@ -59,6 +66,9 @@ var scriptsTests = []scriptsTest{{ "/foo/file1.txt": "file 0644 5b41362b", "/foo/file2.txt": "file 0644 5b41362b", }, + mutated: map[string]string{ + "/foo/file2.txt": "file 0644 5b41362b", + }, }, { summary: "List a directory", content: map[string]string{ @@ -77,6 +87,53 @@ var scriptsTests = []scriptsTest{{ "/bar/": "dir 0755", "/bar/file3.txt": "file 0644 5b41362b", }, +}, { + summary: "OnWrite is called for modified files only", + content: map[string]string{ + "foo/file1.txt": `placeholder`, + "foo/file2.txt": `placeholder`, + // This file is not mutable, it cannot be written to. + "foo/file3.txt": `placeholder`, + }, + script: ` + content.write("/foo/file1.txt", "data1") + content.write("/foo/file2.txt", "data2") + `, + checkw: func(p string) error { + if p == "foo/file3.txt" { + return fmt.Errorf("no write: %s", p) + } + return nil + }, + result: map[string]string{ + "/foo/": "dir 0755", + "/foo/file1.txt": "file 0644 5b41362b", + "/foo/file2.txt": "file 0644 d98cf53e", + "/foo/file3.txt": "file 0644 40978892", + }, + mutated: map[string]string{ + "/foo/file1.txt": "file 0644 5b41362b", + "/foo/file2.txt": "file 0644 d98cf53e", + }, +}, { + summary: "Mode is not changed when writing to a file", + content: map[string]string{ + "foo/file1.txt": ``, + "foo/file2.txt": ``, + }, + hackdir: func(c *C, dir string) { + fpath1 := filepath.Join(dir, "foo/file1.txt") + _ = os.Chmod(fpath1, 0744) + }, + script: ` + content.write("/foo/file1.txt", "data1") + content.write("/foo/file2.txt", "data2") + `, + result: map[string]string{ + "/foo/": "dir 0755", + "/foo/file1.txt": "file 0744 5b41362b", + "/foo/file2.txt": "file 0644 d98cf53e", + }, }, { summary: "Forbid relative paths", content: map[string]string{ @@ -217,10 +274,17 @@ func (s *S) TestScripts(c *C) { test.hackdir(c, rootDir) } + mutatedFiles := map[string]string{} content := &scripts.ContentValue{ RootDir: rootDir, CheckRead: test.checkr, CheckWrite: test.checkw, + OnWrite: func(entry *fsutil.Entry) error { + // Set relative path. + entry.Path = strings.TrimPrefix(entry.Path, rootDir) + mutatedFiles[entry.Path] = testutil.TreeDumpEntry(entry) + return nil + }, } namespace := map[string]scripts.Value{ "content": content, @@ -237,6 +301,10 @@ func (s *S) TestScripts(c *C) { } c.Assert(testutil.TreeDump(rootDir), DeepEquals, test.result) + + if test.mutated != nil { + c.Assert(mutatedFiles, DeepEquals, test.mutated) + } } } diff --git a/internal/slicer/report.go b/internal/slicer/report.go index 22600399..8897f518 100644 --- a/internal/slicer/report.go +++ b/internal/slicer/report.go @@ -11,12 +11,13 @@ import ( ) type ReportEntry struct { - Path string - Mode fs.FileMode - Hash string - Size int - Slices map[*setup.Slice]bool - Link string + Path string + Mode fs.FileMode + Hash string + Size int + Slices map[*setup.Slice]bool + Link string + FinalHash string } // Report holds the information about files and directories created when slicing @@ -30,31 +31,32 @@ type Report struct { // NewReport returns an empty report for content that will be based at the // provided root path. -func NewReport(root string) *Report { - return &Report{ +func NewReport(root string) (*Report, error) { + if !filepath.IsAbs(root) { + return nil, fmt.Errorf("cannot use relative path for report root: %q", root) + } + report := &Report{ Root: filepath.Clean(root) + "/", Entries: make(map[string]ReportEntry), } + return report, nil } func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { - if !strings.HasPrefix(fsEntry.Path, r.Root) { - return fmt.Errorf("cannot add path %q outside of root %q", fsEntry.Path, r.Root) - } - relPath := filepath.Clean("/" + strings.TrimPrefix(fsEntry.Path, r.Root)) - if fsEntry.Mode.IsDir() { - relPath = relPath + "/" + relPath, err := r.sanitizeAbsPath(fsEntry.Path, fsEntry.Mode.IsDir()) + if err != nil { + return fmt.Errorf("cannot add path to report: %s", err) } if entry, ok := r.Entries[relPath]; ok { if fsEntry.Mode != entry.Mode { - return fmt.Errorf("path %q reported twice with diverging mode: %q != %q", relPath, 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 %q reported twice with diverging link: %q != %q", relPath, 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 %q reported twice with diverging size: %d != %d", relPath, fsEntry.Size, entry.Size) + return fmt.Errorf("path %s reported twice with diverging size: %d != %d", relPath, fsEntry.Size, entry.Size) } else if fsEntry.Hash != entry.Hash { - return fmt.Errorf("path %q reported twice with diverging hash: %q != %q", relPath, fsEntry.Hash, entry.Hash) + return fmt.Errorf("path %s reported twice with diverging hash: %q != %q", relPath, fsEntry.Hash, entry.Hash) } entry.Slices[slice] = true r.Entries[relPath] = entry @@ -70,3 +72,38 @@ func (r *Report) Add(slice *setup.Slice, fsEntry *fsutil.Entry) error { } return nil } + +// Mutate updates the FinalHash and Size of an existing path entry. +func (r *Report) Mutate(fsEntry *fsutil.Entry) error { + relPath, err := r.sanitizeAbsPath(fsEntry.Path, fsEntry.Mode.IsDir()) + if err != nil { + return fmt.Errorf("cannot mutate path in report: %s", err) + } + + entry, ok := r.Entries[relPath] + if !ok { + return fmt.Errorf("cannot mutate path in report: %s not previously added", relPath) + } + if entry.Mode.IsDir() { + return fmt.Errorf("cannot mutate path in report: %s is a directory", relPath) + } + if entry.Hash == fsEntry.Hash { + // Content has not changed, nothing to do. + return nil + } + entry.FinalHash = fsEntry.Hash + entry.Size = fsEntry.Size + r.Entries[relPath] = entry + return nil +} + +func (r *Report) sanitizeAbsPath(path string, isDir bool) (relPath string, err error) { + if !strings.HasPrefix(path, r.Root) { + return "", fmt.Errorf("%s outside of root %s", path, r.Root) + } + relPath = filepath.Clean("/" + strings.TrimPrefix(path, r.Root)) + if isDir { + relPath = relPath + "/" + } + return relPath, nil +} diff --git a/internal/slicer/report_test.go b/internal/slicer/report_test.go index 0b04f70e..762b35ad 100644 --- a/internal/slicer/report_test.go +++ b/internal/slicer/report_test.go @@ -27,25 +27,31 @@ var otherSlice = &setup.Slice{ } var sampleDir = fsutil.Entry{ - Path: "/base/exampleDir/", + Path: "/base/example-dir/", Mode: fs.ModeDir | 0654, Link: "", } var sampleFile = fsutil.Entry{ - Path: "/base/exampleFile", + Path: "/base/example-file", Mode: 0777, - Hash: "exampleFile_hash", + Hash: "example-file_hash", Size: 5678, Link: "", } var sampleLink = fsutil.Entry{ - Path: "/base/exampleLink", + Path: "/base/example-link", Mode: 0777, - Hash: "exampleFile_hash", + Hash: "example-file_hash", Size: 5678, - Link: "/base/exampleFile", + Link: "/base/example-file", +} + +var sampleFileMutated = fsutil.Entry{ + Path: sampleFile.Path, + Hash: sampleFile.Hash + "_changed", + Size: sampleFile.Size + 10, } type sliceAndEntry struct { @@ -56,6 +62,7 @@ type sliceAndEntry struct { var reportTests = []struct { summary string add []sliceAndEntry + mutate []*fsutil.Entry // indexed by path. expected map[string]slicer.ReportEntry // error after adding the last [sliceAndEntry]. @@ -64,8 +71,8 @@ var reportTests = []struct { summary: "Regular directory", add: []sliceAndEntry{{entry: sampleDir, slice: oneSlice}}, expected: map[string]slicer.ReportEntry{ - "/exampleDir/": { - Path: "/exampleDir/", + "/example-dir/": { + Path: "/example-dir/", Mode: fs.ModeDir | 0654, Slices: map[*setup.Slice]bool{oneSlice: true}, Link: "", @@ -77,8 +84,8 @@ var reportTests = []struct { {entry: sampleDir, slice: otherSlice}, }, expected: map[string]slicer.ReportEntry{ - "/exampleDir/": { - Path: "/exampleDir/", + "/example-dir/": { + Path: "/example-dir/", Mode: fs.ModeDir | 0654, Slices: map[*setup.Slice]bool{oneSlice: true, otherSlice: true}, Link: "", @@ -87,10 +94,10 @@ var reportTests = []struct { summary: "Regular file", add: []sliceAndEntry{{entry: sampleFile, slice: oneSlice}}, expected: map[string]slicer.ReportEntry{ - "/exampleFile": { - Path: "/exampleFile", + "/example-file": { + Path: "/example-file", Mode: 0777, - Hash: "exampleFile_hash", + Hash: "example-file_hash", Size: 5678, Slices: map[*setup.Slice]bool{oneSlice: true}, Link: "", @@ -99,13 +106,13 @@ var reportTests = []struct { summary: "Regular file link", add: []sliceAndEntry{{entry: sampleLink, slice: oneSlice}}, expected: map[string]slicer.ReportEntry{ - "/exampleLink": { - Path: "/exampleLink", + "/example-link": { + Path: "/example-link", Mode: 0777, - Hash: "exampleFile_hash", + Hash: "example-file_hash", Size: 5678, Slices: map[*setup.Slice]bool{oneSlice: true}, - Link: "/base/exampleFile", + Link: "/base/example-file", }}, }, { summary: "Several entries", @@ -114,16 +121,16 @@ var reportTests = []struct { {entry: sampleFile, slice: otherSlice}, }, expected: map[string]slicer.ReportEntry{ - "/exampleDir/": { - Path: "/exampleDir/", + "/example-dir/": { + Path: "/example-dir/", Mode: fs.ModeDir | 0654, Slices: map[*setup.Slice]bool{oneSlice: true}, Link: "", }, - "/exampleFile": { - Path: "/exampleFile", + "/example-file": { + Path: "/example-file", Mode: 0777, - Hash: "exampleFile_hash", + Hash: "example-file_hash", Size: 5678, Slices: map[*setup.Slice]bool{otherSlice: true}, Link: "", @@ -135,10 +142,10 @@ var reportTests = []struct { {entry: sampleFile, slice: oneSlice}, }, expected: map[string]slicer.ReportEntry{ - "/exampleFile": { - Path: "/exampleFile", + "/example-file": { + Path: "/example-file", Mode: 0777, - Hash: "exampleFile_hash", + Hash: "example-file_hash", Size: 5678, Slices: map[*setup.Slice]bool{oneSlice: true}, Link: "", @@ -155,7 +162,7 @@ var reportTests = []struct { Link: sampleFile.Link, }, slice: oneSlice}, }, - err: `path "/exampleFile" reported twice with diverging mode: "----------" != "-rwxrwxrwx"`, + err: `path /example-file reported twice with diverging mode: 0000 != 0777`, }, { summary: "Error for same path distinct hash", add: []sliceAndEntry{ @@ -168,7 +175,7 @@ var reportTests = []struct { Link: sampleFile.Link, }, slice: oneSlice}, }, - err: `path "/exampleFile" reported twice with diverging hash: "distinct hash" != "exampleFile_hash"`, + err: `path /example-file reported twice with diverging hash: "distinct hash" != "example-file_hash"`, }, { summary: "Error for same path distinct size", add: []sliceAndEntry{ @@ -181,7 +188,7 @@ var reportTests = []struct { Link: sampleFile.Link, }, slice: oneSlice}, }, - err: `path "/exampleFile" reported twice with diverging size: 0 != 5678`, + err: `path /example-file reported twice with diverging size: 0 != 5678`, }, { summary: "Error for same path distinct link", add: []sliceAndEntry{ @@ -194,28 +201,85 @@ var reportTests = []struct { Link: "distinct link", }, slice: oneSlice}, }, - err: `path "/exampleFile" reported twice with diverging link: "distinct link" != ""`, + err: `path /example-file reported twice with diverging link: "distinct link" != ""`, }, { summary: "Error for path outside root", add: []sliceAndEntry{ {entry: fsutil.Entry{Path: "/file"}, slice: oneSlice}, }, - err: `cannot add path "/file" outside of root "/base/"`, + err: `cannot add path to report: /file outside of root /base/`, +}, { + summary: "Error for mutated path outside root", + mutate: []*fsutil.Entry{{Path: "/file"}}, + err: `cannot mutate path in report: /file outside of root /base/`, }, { summary: "File name has root prefix but without the directory slash", add: []sliceAndEntry{ {entry: fsutil.Entry{Path: "/basefile"}, slice: oneSlice}, }, - err: `cannot add path "/basefile" outside of root "/base/"`, + err: `cannot add path to report: /basefile outside of root /base/`, +}, { + summary: "Add mutated regular file", + add: []sliceAndEntry{ + {entry: sampleFile, slice: oneSlice}, + {entry: sampleDir, slice: oneSlice}, + }, + mutate: []*fsutil.Entry{&sampleFileMutated}, + expected: map[string]slicer.ReportEntry{ + "/example-dir/": { + Path: "/example-dir/", + Mode: fs.ModeDir | 0654, + Slices: map[*setup.Slice]bool{oneSlice: true}, + Link: "", + }, + "/example-file": { + Path: "/example-file", + Mode: 0777, + Hash: "example-file_hash", + Size: 5688, + Slices: map[*setup.Slice]bool{oneSlice: true}, + Link: "", + FinalHash: "example-file_hash_changed", + }}, +}, { + summary: "Calling mutated with identical content to initial file", + add: []sliceAndEntry{ + {entry: sampleFile, slice: oneSlice}, + }, + mutate: []*fsutil.Entry{&sampleFile}, + expected: map[string]slicer.ReportEntry{ + "/example-file": { + Path: "/example-file", + Mode: 0777, + Hash: "example-file_hash", + Size: 5678, + Slices: map[*setup.Slice]bool{oneSlice: true}, + Link: "", + // FinalHash is not updated. + FinalHash: "", + }}, +}, { + summary: "Mutated paths must refer to previously added entries", + mutate: []*fsutil.Entry{&sampleFileMutated}, + err: `cannot mutate path in report: /example-file not previously added`, +}, { + summary: "Cannot mutate directory", + add: []sliceAndEntry{{entry: sampleDir, slice: oneSlice}}, + mutate: []*fsutil.Entry{&sampleDir}, + err: `cannot mutate path in report: /example-dir/ is a directory`, }} -func (s *S) TestReportAdd(c *C) { +func (s *S) TestReport(c *C) { for _, test := range reportTests { - report := slicer.NewReport("/base/") var err error + report, err := slicer.NewReport("/base/") + c.Assert(err, IsNil) for _, si := range test.add { err = report.Add(si.slice, &si.entry) } + for _, e := range test.mutate { + err = report.Mutate(e) + } if test.err != "" { c.Assert(err, ErrorMatches, test.err) continue @@ -224,3 +288,8 @@ func (s *S) TestReportAdd(c *C) { c.Assert(report.Entries, DeepEquals, test.expected, Commentf(test.summary)) } } + +func (s *S) TestRootRelativePath(c *C) { + _, err := slicer.NewReport("../base/") + c.Assert(err, ErrorMatches, `cannot use relative path for report root: "../base/"`) +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 13ab3978..1f59c6c8 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -66,21 +66,18 @@ func (cc *contentChecker) checkKnown(path string) error { } func Run(options *RunOptions) (*Report, error) { - report := NewReport(options.TargetDir) - oldUmask := syscall.Umask(0) defer func() { syscall.Umask(oldUmask) }() targetDir := filepath.Clean(options.TargetDir) - targetDirAbs := targetDir - if !filepath.IsAbs(targetDirAbs) { + if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() if err != nil { return nil, fmt.Errorf("cannot obtain current directory: %w", err) } - targetDirAbs = filepath.Join(dir, targetDir) + targetDir = filepath.Join(dir, targetDir) } // Build information to process the selection. @@ -165,7 +162,13 @@ func Run(options *RunOptions) (*Report, error) { knownPaths := map[string]pathData{} addKnownPath(knownPaths, "/", pathData{}) - // Creates the filesystem entry and adds it to the report. + report, err := NewReport(targetDir) + if err != nil { + return nil, fmt.Errorf("internal error: cannot create report: %w", err) + } + + // Creates the filesystem entry and adds it to the report. It also updates + // knownPaths with the files created. create := func(extractInfos []deb.ExtractInfo, o *fsutil.CreateOptions) error { entry, err := fsutil.Create(o) if err != nil { @@ -201,9 +204,12 @@ func Run(options *RunOptions) (*Report, error) { if pathInfo.Until == setup.UntilNone { until = setup.UntilNone } - err := report.Add(slice, entry) - if err != nil { - return err + // Do not add paths with "until: mutate". + if pathInfo.Until != setup.UntilMutate { + err := report.Add(slice, entry) + if err != nil { + return err + } } } @@ -255,9 +261,13 @@ func Run(options *RunOptions) (*Report, error) { if err != nil { return nil, err } - err = report.Add(slice, entry) - if err != nil { - return nil, err + + // Do not add paths with "until: mutate". + if pathInfo.Until != setup.UntilMutate { + err = report.Add(slice, entry) + if err != nil { + return nil, err + } } } } @@ -266,9 +276,10 @@ func Run(options *RunOptions) (*Report, error) { // dependencies must run before dependents. checker := contentChecker{knownPaths} content := &scripts.ContentValue{ - RootDir: targetDirAbs, + RootDir: targetDir, CheckWrite: checker.checkMutable, CheckRead: checker.checkKnown, + OnWrite: report.Mutate, } for _, slice := range options.Selection.Slices { opts := scripts.RunOptions{ @@ -284,7 +295,7 @@ func Run(options *RunOptions) (*Report, error) { } } - err := removeAfterMutate(targetDirAbs, knownPaths) + err = removeAfterMutate(targetDir, knownPaths) if err != nil { return nil, err } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 3be612f5..7c76f8c2 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -31,12 +31,8 @@ type slicerTest struct { slices []setup.SliceKey hackopt func(c *C, opts *slicer.RunOptions) filesystem map[string]string - // TODO: - // The results of the report do not conform to the planned implementation - // yet. Namely: - // * We do not track removed directories or changes done in Starlark. - report map[string]string - error string + report map[string]string + error string } var packageEntries = map[string][]testutil.TarEntry{ @@ -409,7 +405,7 @@ var slicerTests = []slicerTest{{ "/dir/text-file": "file 0644 d98cf53e", }, report: map[string]string{ - "/dir/text-file": "file 0644 5b41362b {test-package_myslice}", + "/dir/text-file": "file 0644 5b41362b d98cf53e {test-package_myslice}", }, }, { summary: "Script: read a file", @@ -435,7 +431,7 @@ var slicerTests = []slicerTest{{ }, report: map[string]string{ "/dir/text-file-1": "file 0644 5b41362b {test-package_myslice}", - "/foo/text-file-2": "file 0644 d98cf53e {test-package_myslice}", + "/foo/text-file-2": "file 0644 d98cf53e 5b41362b {test-package_myslice}", }, }, { summary: "Script: use 'until' to remove file after mutate", @@ -459,9 +455,7 @@ var slicerTests = []slicerTest{{ "/foo/text-file-2": "file 0644 5b41362b", }, report: map[string]string{ - // TODO this path needs to be removed from the report. - "/dir/text-file-1": "file 0644 5b41362b {test-package_myslice}", - "/foo/text-file-2": "file 0644 d98cf53e {test-package_myslice}", + "/foo/text-file-2": "file 0644 d98cf53e 5b41362b {test-package_myslice}", }, }, { summary: "Script: use 'until' to remove wildcard after mutate", @@ -480,14 +474,7 @@ var slicerTests = []slicerTest{{ "/dir/": "dir 0755", "/other-dir/": "dir 0755", }, - report: map[string]string{ - // TODO These first three entries should be removed from the report. - "/dir/nested/": "dir 0755 {test-package_myslice}", - "/dir/nested/file": "file 0644 84237a05 {test-package_myslice}", - "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice}", - - "/other-dir/text-file": "file 0644 5b41362b {test-package_myslice}", - }, + report: map[string]string{}, }, { summary: "Script: 'until' does not remove non-empty directories", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -507,9 +494,29 @@ var slicerTests = []slicerTest{{ "/dir/nested/file-copy": "file 0644 cc55e2ec", }, report: map[string]string{ - "/dir/nested/": "dir 0755 {test-package_myslice}", "/dir/nested/file-copy": "file 0644 cc55e2ec {test-package_myslice}", }, +}, { + summary: "Script: writing same contents to existing file does not set the final hash in report", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /dir/text-file: {text: data1, mutable: true} + mutate: | + content.write("/dir/text-file", "data1") + `, + }, + filesystem: map[string]string{ + "/dir/": "dir 0755", + "/dir/text-file": "file 0644 5b41362b", + }, + report: map[string]string{ + "/dir/text-file": "file 0644 5b41362b {test-package_myslice}", + }, }, { summary: "Script: cannot write non-mutable files", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -525,6 +532,35 @@ var slicerTests = []slicerTest{{ `, }, error: `slice test-package_myslice: cannot write file which is not mutable: /dir/text-file`, +}, { + summary: "Script: cannot write to unlisted file", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + mutate: | + content.write("/dir/text-file", "data") + `, + }, + error: `slice test-package_myslice: cannot write file which is not mutable: /dir/text-file`, +}, { + summary: "Script: cannot write to directory", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /dir/: {make: true} + mutate: | + content.write("/dir/", "data") + `, + }, + error: `slice test-package_myslice: cannot write file which is not mutable: /dir/`, }, { summary: "Script: cannot read unlisted content", slices: []setup.SliceKey{{"test-package", "myslice2"}}, @@ -565,9 +601,10 @@ var slicerTests = []slicerTest{{ slices: myslice: contents: - /dir/text-file: {text: data1} + /dir/text-file: {text: data1, mutable: true} mutate: | content.read("/dir/text-file") + content.write("/dir/text-file", "data2") `, }, hackopt: func(c *C, opts *slicer.RunOptions) { @@ -828,16 +865,16 @@ var slicerTests = []slicerTest{{ "/dir/several/levels/deep/": "dir 0755", }, report: map[string]string{ - "/dir/": "dir 0755 {test-package_myslice1,test-package_myslice2}", - "/dir/file": "file 0644 cc55e2ec {test-package_myslice1,test-package_myslice2}", - "/dir/nested/": "dir 0755 {test-package_myslice1,test-package_myslice2}", - "/dir/nested/file": "file 0644 84237a05 {test-package_myslice1,test-package_myslice2}", - "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice1,test-package_myslice2}", - "/dir/other-file": "file 0644 63d5dd49 {test-package_myslice1,test-package_myslice2}", - "/dir/several/": "dir 0755 {test-package_myslice1,test-package_myslice2}", - "/dir/several/levels/": "dir 0755 {test-package_myslice1,test-package_myslice2}", - "/dir/several/levels/deep/": "dir 0755 {test-package_myslice1,test-package_myslice2}", - "/dir/several/levels/deep/file": "file 0644 6bc26dff {test-package_myslice1,test-package_myslice2}", + "/dir/": "dir 0755 {test-package_myslice2}", + "/dir/file": "file 0644 cc55e2ec {test-package_myslice2}", + "/dir/nested/": "dir 0755 {test-package_myslice2}", + "/dir/nested/file": "file 0644 84237a05 {test-package_myslice2}", + "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice2}", + "/dir/other-file": "file 0644 63d5dd49 {test-package_myslice2}", + "/dir/several/": "dir 0755 {test-package_myslice2}", + "/dir/several/levels/": "dir 0755 {test-package_myslice2}", + "/dir/several/levels/deep/": "dir 0755 {test-package_myslice2}", + "/dir/several/levels/deep/file": "file 0644 6bc26dff {test-package_myslice2}", }, }, { summary: "Overlapping globs, until:mutate and reading from script", @@ -874,12 +911,11 @@ var slicerTests = []slicerTest{{ "/dir/several/levels/deep/file": "file 0644 6bc26dff", }, report: map[string]string{ - // TODO, myslice2 should not appear in the report, this is pending PR #131. "/dir/": "dir 0755 {test-package_myslice1}", "/dir/file": "file 0644 cc55e2ec {test-package_myslice1}", - "/dir/nested/": "dir 0755 {test-package_myslice1,test-package_myslice2}", - "/dir/nested/file": "file 0644 84237a05 {test-package_myslice1,test-package_myslice2}", - "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice1,test-package_myslice2}", + "/dir/nested/": "dir 0755 {test-package_myslice1}", + "/dir/nested/file": "file 0644 84237a05 {test-package_myslice1}", + "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice1}", "/dir/other-file": "file 0644 63d5dd49 {test-package_myslice1}", "/dir/several/": "dir 0755 {test-package_myslice1}", "/dir/several/levels/": "dir 0755 {test-package_myslice1}", @@ -921,9 +957,8 @@ var slicerTests = []slicerTest{{ "/dir/several/levels/deep/file": "file 0644 6bc26dff", }, report: map[string]string{ - // TODO, myslice2 should not appear in the report, this is pending PR #131. "/dir/": "dir 0755 {test-package_myslice1}", - "/dir/file": "file 0644 cc55e2ec {test-package_myslice1,test-package_myslice2}", + "/dir/file": "file 0644 cc55e2ec {test-package_myslice1}", "/dir/nested/": "dir 0755 {test-package_myslice1}", "/dir/nested/file": "file 0644 84237a05 {test-package_myslice1}", "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice1}", @@ -960,17 +995,7 @@ var slicerTests = []slicerTest{{ "/dir/file": "file 0644 cc55e2ec", }, report: map[string]string{ - // TODO, myslice1 should not appear in the report, this is pending PR #131. - "/dir/": "dir 0755 {test-package_myslice1}", - "/dir/file": "file 0644 cc55e2ec {test-package_myslice1,test-package_myslice2}", - "/dir/nested/": "dir 0755 {test-package_myslice1}", - "/dir/nested/file": "file 0644 84237a05 {test-package_myslice1}", - "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice1}", - "/dir/other-file": "file 0644 63d5dd49 {test-package_myslice1}", - "/dir/several/": "dir 0755 {test-package_myslice1}", - "/dir/several/levels/": "dir 0755 {test-package_myslice1}", - "/dir/several/levels/deep/": "dir 0755 {test-package_myslice1}", - "/dir/several/levels/deep/file": "file 0644 6bc26dff {test-package_myslice1}", + "/dir/file": "file 0644 cc55e2ec {test-package_myslice2}", }, }, { summary: "Overlapping glob and single entry, until:mutate on both and reading from script", @@ -995,19 +1020,7 @@ var slicerTests = []slicerTest{{ `, }, filesystem: map[string]string{}, - report: map[string]string{ - // TODO, this should be empty, this is pending PR #131. - "/dir/": "dir 0755 {test-package_myslice1}", - "/dir/file": "file 0644 cc55e2ec {test-package_myslice1,test-package_myslice2}", - "/dir/nested/": "dir 0755 {test-package_myslice1}", - "/dir/nested/file": "file 0644 84237a05 {test-package_myslice1}", - "/dir/nested/other-file": "file 0644 6b86b273 {test-package_myslice1}", - "/dir/other-file": "file 0644 63d5dd49 {test-package_myslice1}", - "/dir/several/": "dir 0755 {test-package_myslice1}", - "/dir/several/levels/": "dir 0755 {test-package_myslice1}", - "/dir/several/levels/deep/": "dir 0755 {test-package_myslice1}", - "/dir/several/levels/deep/file": "file 0644 6bc26dff {test-package_myslice1}", - }, + report: map[string]string{}, }} var defaultChiselYaml = ` @@ -1156,6 +1169,8 @@ func treeDumpReport(report *slicer.Report) map[string]string { case 0: // Regular if entry.Size == 0 { fsDump = fmt.Sprintf("file %#o empty", entry.Mode.Perm()) + } else if entry.FinalHash != "" { + fsDump = fmt.Sprintf("file %#o %s %s", fperm, entry.Hash[:8], entry.FinalHash[:8]) } else { fsDump = fmt.Sprintf("file %#o %s", fperm, entry.Hash[:8]) } diff --git a/internal/testutil/treedump.go b/internal/testutil/treedump.go index 72457887..26aa164d 100644 --- a/internal/testutil/treedump.go +++ b/internal/testutil/treedump.go @@ -6,6 +6,8 @@ import ( "io/fs" "os" "path/filepath" + + "github.com/canonical/chisel/internal/fsutil" ) func TreeDump(dir string) map[string]string { @@ -60,3 +62,25 @@ func TreeDump(dir string) map[string]string { } return result } + +// TreeDumpEntry the file information in the same format as [testutil.TreeDump]. +func TreeDumpEntry(entry *fsutil.Entry) string { + fperm := entry.Mode.Perm() + if entry.Mode&fs.ModeSticky != 0 { + fperm |= 01000 + } + switch entry.Mode.Type() { + case fs.ModeDir: + return fmt.Sprintf("dir %#o", fperm) + case fs.ModeSymlink: + return fmt.Sprintf("symlink %s", entry.Link) + case 0: // Regular + if entry.Size == 0 { + return fmt.Sprintf("file %#o empty", entry.Mode.Perm()) + } else { + return fmt.Sprintf("file %#o %s", fperm, entry.Hash[:8]) + } + default: + panic(fmt.Errorf("unknown file type %d: %s", entry.Mode.Type(), entry.Path)) + } +}