From 491cd0c6df5dd77826fc320c3eb855947f1cd056 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 26 Nov 2025 11:01:08 +0100 Subject: [PATCH 01/22] feat: search, load and validates manifests Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 16 +++ internal/manifestutil/manifestutil.go | 45 ++++++- internal/slicer/check.go | 184 ++++++++++++++++++++++++++ internal/slicer/slicer.go | 92 +++++++++++++ public/manifest/manifest.go | 4 + 5 files changed, 334 insertions(+), 7 deletions(-) create mode 100644 internal/slicer/check.go diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79..cca9055f 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -11,6 +11,7 @@ import ( "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/public/manifest" ) var shortCutHelp = "Cut a tree with selected slices" @@ -73,6 +74,21 @@ func (cmd *cmdCut) Execute(args []string) error { } } + mfest, err := slicer.Inspect(cmd.RootDir, release) + if err != nil { + return err + } + if mfest != nil { + mfest.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := setup.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) + } + selection, err := setup.Select(release, sliceKeys, cmd.Arch) if err != nil { return err diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 16b05402..f6bb7e60 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -18,22 +18,44 @@ import ( const DefaultFilename = "manifest.wall" +func collectManifests(slice *setup.Slice, collector func(path string, slice *setup.Slice)) { + for path, info := range slice.Contents { + if info.Generate == setup.GenerateManifest { + dir := strings.TrimSuffix(path, "**") + path = filepath.Join(dir, DefaultFilename) + collector(path, slice) + } + } +} + // FindPaths finds the paths marked with "generate:manifest" and // returns a map from the manifest path to all the slices that declare it. func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { manifestSlices := make(map[string][]*setup.Slice) + collector := func(path string, slice *setup.Slice) { + manifestSlices[path] = append(manifestSlices[path], slice) + } for _, slice := range slices { - for path, info := range slice.Contents { - if info.Generate == setup.GenerateManifest { - dir := strings.TrimSuffix(path, "**") - path = filepath.Join(dir, DefaultFilename) - manifestSlices[path] = append(manifestSlices[path], slice) - } - } + collectManifests(slice, collector) } return manifestSlices } +// FindPathsInRelease finds all the paths marked with "generate:manifest" +// for the given release. +func FindPathsInRelease(r *setup.Release) []string { + manifestPaths := make([]string,0) + collector := func(path string, slice *setup.Slice) { + manifestPaths = append(manifestPaths, path) + } + for _, pkg := range r.Packages { + for _, slice := range pkg.Slices { + collectManifests(slice, collector) + } + } + return manifestPaths +} + type WriteOptions struct { PackageInfo []*archive.PackageInfo Selection []*setup.Slice @@ -340,3 +362,12 @@ func Validate(mfest *manifest.Manifest) (err error) { } return nil } + +// CompareSchemas compares two manifest schema strings. +func CompareSchemas(va, vb string) int { + if va == manifest.Schema && va == vb { + return 0 + } + return -1 +} + diff --git a/internal/slicer/check.go b/internal/slicer/check.go new file mode 100644 index 00000000..ccf97605 --- /dev/null +++ b/internal/slicer/check.go @@ -0,0 +1,184 @@ +package slicer + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "syscall" + + "github.com/klauspost/compress/zstd" + + "github.com/canonical/chisel/internal/manifestutil" + "github.com/canonical/chisel/public/manifest" +) + +type pathInfo struct { + mode string + size int64 + link string + hash string +} + +func unixPerm(mode fs.FileMode) (perm uint32) { + perm = uint32(mode.Perm()) + if mode&fs.ModeSticky != 0 { + perm |= 0o1000 + } + return perm +} + +// checkRootDir checks the content of the target directory matches with +// the manifest. Files not managed by chisel are ignored. +// This function works under the assumption the manifest is valid. +func checkRootDir(mfest *manifest.Manifest, rootDir string) error { + singlePathsByFSInode := make(map[uint64]string) + fsInodeByManifestInode := make(map[uint64]uint64) + manifestInfos := make(map[string]*pathInfo) + err := mfest.IteratePaths("", func(path *manifest.Path) error { + pathHash := path.FinalSHA256 + if pathHash == "" { + pathHash = path.SHA256 + } + recordedPathInfo := &pathInfo{ + mode: path.Mode, + size: int64(path.Size), + link: path.Link, + hash: pathHash, + } + + fsInfo := &pathInfo{} + fullPath := filepath.Join(rootDir, path.Path) + info, err := os.Lstat(fullPath) + if err != nil { + return err + } + mode := info.Mode() + fsInfo.mode = fmt.Sprintf("0%o", unixPerm(mode)) + ftype := mode & fs.ModeType + switch ftype { + case fs.ModeDir: + // Nothing to do. + case fs.ModeSymlink: + fsInfo.link, err = os.Readlink(fullPath) + if err != nil { + return fmt.Errorf("cannot read symlink %q: %w", fullPath, err) + } + case 0: // Regular file. + h, err := contentHash(fullPath) + if err != nil { + return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) + } + fsInfo.hash = hex.EncodeToString(h) + fsInfo.size = info.Size() + default: + return fmt.Errorf("cannot check %q: unrecognized type %s", fullPath, mode.String()) + } + + // Collect manifests for tailored checking later. Adjust observed hash and + // size to still compare in a generic way. + if filepath.Base(path.Path) == manifestutil.DefaultFilename && recordedPathInfo.size == 0 && recordedPathInfo.hash == "" { + mfestInfo := *fsInfo + manifestInfos[path.Path] = &mfestInfo + fsInfo.size = 0 + fsInfo.hash = "" + } + + if recordedPathInfo.mode != fsInfo.mode { + return fmt.Errorf("inconsistent mode at %q: recorded %v, observed %v", path.Path, recordedPathInfo.mode, fsInfo.mode) + } + if recordedPathInfo.size != fsInfo.size { + return fmt.Errorf("inconsistent size at %q: recorded %v, observed %v", path.Path, recordedPathInfo.size, fsInfo.size) + } + if recordedPathInfo.link != fsInfo.link { + return fmt.Errorf("inconsistent link at %q: recorded %v, observed %v", path.Path, recordedPathInfo.link, fsInfo.link) + } + if recordedPathInfo.hash != fsInfo.hash { + return fmt.Errorf("inconsistent hash at %q: recorded %v, observed %v", path.Path, recordedPathInfo.hash, fsInfo.hash) + } + // Check hardlink. + if ftype != fs.ModeDir { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("cannot get syscall stat info for %q", info.Name()) + } + inode := stat.Ino + + if path.Inode == 0 { + // This path must not be linked to any other. + singlePath, ok := singlePathsByFSInode[inode] + if ok { + return fmt.Errorf("inconsistent content at %q: recorded no hardlink, observed hardlinked to %q", path.Path, singlePath) + } + singlePathsByFSInode[inode] = path.Path + } else { + recordedInode, ok := fsInodeByManifestInode[path.Inode] + if !ok { + fsInodeByManifestInode[path.Inode] = inode + } else if recordedInode != inode { + return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) + } + } + } + return nil + }) + if err != nil { + return err + } + + // Check manifests. + // They must all be valid manifests and be consistent per schema version. + schemaManifestInfos := make(map[string]*pathInfo) + for path, info := range manifestInfos { + fullPath := filepath.Join(rootDir, path) + f, err := os.Open(fullPath) + if err != nil { + return err + } + defer f.Close() + r, err := zstd.NewReader(f) + if err != nil { + return err + } + defer r.Close() + mfest, err = manifest.Read(r) + if err != nil { + return err + } + err = manifestutil.Validate(mfest) + if err != nil { + return err + } + schema := mfest.Schema() + refInfo, ok := schemaManifestInfos[schema] + if !ok { + schemaManifestInfos[schema] = info + continue + } + + if refInfo.size != info.size { + return fmt.Errorf("inconsistent manifest size for version %s at %q: recorded %v, observed %v", schema, path, refInfo.size, info.size) + } + if refInfo.hash != info.hash { + return fmt.Errorf("inconsistent manifest hash for version %s at %q: recorded %v, observed %v", schema, path, refInfo.hash, info.hash) + } + } + return nil +} + +func contentHash(path string) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + return h.Sum(nil), nil +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 9d3447fb..48eb0e11 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -3,10 +3,12 @@ package slicer import ( "archive/tar" "bytes" + "encoding/hex" "fmt" "io" "io/fs" "os" + "path" "path/filepath" "slices" "sort" @@ -21,6 +23,7 @@ import ( "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/public/manifest" ) const manifestMode fs.FileMode = 0644 @@ -537,3 +540,92 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel } return pkgArchive, nil } + +// Inspect examines and validates the targetDir. It returns, if found and valid +// the manifest representing the content in the targetDir. +func Inspect(targetDir string, release *setup.Release) (*manifest.Manifest, error) { + var mfest *manifest.Manifest + manifestPaths := manifestutil.FindPathsInRelease(release) + if len(manifestPaths) > 0 { + logf("Inspecting root directory...") + var err error + mfest, err = selectValidManifest(targetDir, manifestPaths) + if err != nil { + return nil, err + } + if mfest != nil { + err = checkRootDir(mfest, targetDir) + if err != nil { + return nil, err + } + } + } + return mfest, nil +} + +// selectValidManifest returns, if found, a valid manifest with the latest +// schema. Consistency with all other manifests with the same schema is verified +// so the selection is deterministic. +func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { + targetDir = filepath.Clean(targetDir) + if !filepath.IsAbs(targetDir) { + dir, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("cannot obtain current directory: %w", err) + } + targetDir = filepath.Join(dir, targetDir) + } + + type manifestHash struct { + path string + hash string + } + var selected *manifest.Manifest + schemaManifest := make(map[string]manifestHash) + for _, mfestPath := range manifestPaths { + err := func() error { + mfestFullPath := path.Join(targetDir, mfestPath) + f, err := os.Open(mfestFullPath) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + defer f.Close() + r, err := zstd.NewReader(f) + if err != nil { + return err + } + defer r.Close() + mfest, err := manifest.Read(r) + if err != nil { + return nil + } + err = manifestutil.Validate(mfest) + if err != nil { + return nil + } + + if selected == nil || manifestutil.CompareSchemas(mfest.Schema(), selected.Schema()) > 0 { + h, err := contentHash(mfestFullPath) + if err != nil { + return fmt.Errorf("cannot compute hash for %q: %w", mfestFullPath, err) + } + mfestHash := hex.EncodeToString(h) + refMfest, ok := schemaManifest[mfest.Schema()] + if !ok { + schemaManifest[mfest.Schema()] = manifestHash{mfestPath, mfestHash} + } else if refMfest.hash != mfestHash { + return fmt.Errorf("inconsistent manifests: %q and %q", refMfest.path, mfestPath) + } + selected = mfest + } + return nil + }() + if err != nil { + return nil, err + } + } + return selected, nil +} diff --git a/public/manifest/manifest.go b/public/manifest/manifest.go index 1e4809b8..65b362d5 100644 --- a/public/manifest/manifest.go +++ b/public/manifest/manifest.go @@ -68,6 +68,10 @@ func Read(reader io.Reader) (manifest *Manifest, err error) { return manifest, nil } +func (manifest *Manifest) Schema() string { + return manifest.db.Schema() +} + func (manifest *Manifest) IteratePaths(pathPrefix string, onMatch func(*Path) error) (err error) { return iteratePrefix(manifest, &Path{Kind: "path", Path: pathPrefix}, onMatch) } From a6e57f7290104bec2c17bfefc508d091badc971d Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 29 Jan 2026 17:19:12 +0100 Subject: [PATCH 02/22] feat: adapt to revised strategy --- cmd/chisel/cmd_cut.go | 3 +- internal/fsutil/create.go | 16 +- internal/manifestutil/manifestutil.go | 3 +- internal/manifestutil/manifestutil_test.go | 1 - internal/slicer/check.go | 184 --------------------- internal/slicer/slicer.go | 123 +++++++++----- 6 files changed, 100 insertions(+), 230 deletions(-) delete mode 100644 internal/slicer/check.go diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index cca9055f..940ec121 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -74,7 +74,7 @@ func (cmd *cmdCut) Execute(args []string) error { } } - mfest, err := slicer.Inspect(cmd.RootDir, release) + mfest, err := slicer.SelectValidManifest(cmd.RootDir, release) if err != nil { return err } @@ -141,6 +141,7 @@ func (cmd *cmdCut) Execute(args []string) error { Selection: selection, Archives: archives, TargetDir: cmd.RootDir, + Manifest: mfest, }) return err } diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 0503c96f..2f87f670 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -166,11 +166,19 @@ func createDir(o *CreateOptions) error { if err != nil { return err } - err = os.Mkdir(path, o.Mode) - if os.IsExist(err) { - return nil + fileinfo, err := os.Lstat(path) + if err == nil { + if fileinfo.IsDir() { + return nil + } + err = os.Remove(path) + if err != nil { + return err + } + } else if !os.IsNotExist(err) { + return err } - return err + return os.Mkdir(path, o.Mode) } func createFile(o *CreateOptions) error { diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index f6bb7e60..b2946d57 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -44,7 +44,7 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { // FindPathsInRelease finds all the paths marked with "generate:manifest" // for the given release. func FindPathsInRelease(r *setup.Release) []string { - manifestPaths := make([]string,0) + manifestPaths := make([]string, 0) collector := func(path string, slice *setup.Slice) { manifestPaths = append(manifestPaths, path) } @@ -370,4 +370,3 @@ func CompareSchemas(va, vb string) int { } return -1 } - diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index 2bab0a68..0c6c0c06 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -2,7 +2,6 @@ package manifestutil_test import ( "bytes" - "io" "io/fs" "os" "path" diff --git a/internal/slicer/check.go b/internal/slicer/check.go deleted file mode 100644 index ccf97605..00000000 --- a/internal/slicer/check.go +++ /dev/null @@ -1,184 +0,0 @@ -package slicer - -import ( - "crypto/sha256" - "encoding/hex" - "fmt" - "io" - "io/fs" - "os" - "path/filepath" - "syscall" - - "github.com/klauspost/compress/zstd" - - "github.com/canonical/chisel/internal/manifestutil" - "github.com/canonical/chisel/public/manifest" -) - -type pathInfo struct { - mode string - size int64 - link string - hash string -} - -func unixPerm(mode fs.FileMode) (perm uint32) { - perm = uint32(mode.Perm()) - if mode&fs.ModeSticky != 0 { - perm |= 0o1000 - } - return perm -} - -// checkRootDir checks the content of the target directory matches with -// the manifest. Files not managed by chisel are ignored. -// This function works under the assumption the manifest is valid. -func checkRootDir(mfest *manifest.Manifest, rootDir string) error { - singlePathsByFSInode := make(map[uint64]string) - fsInodeByManifestInode := make(map[uint64]uint64) - manifestInfos := make(map[string]*pathInfo) - err := mfest.IteratePaths("", func(path *manifest.Path) error { - pathHash := path.FinalSHA256 - if pathHash == "" { - pathHash = path.SHA256 - } - recordedPathInfo := &pathInfo{ - mode: path.Mode, - size: int64(path.Size), - link: path.Link, - hash: pathHash, - } - - fsInfo := &pathInfo{} - fullPath := filepath.Join(rootDir, path.Path) - info, err := os.Lstat(fullPath) - if err != nil { - return err - } - mode := info.Mode() - fsInfo.mode = fmt.Sprintf("0%o", unixPerm(mode)) - ftype := mode & fs.ModeType - switch ftype { - case fs.ModeDir: - // Nothing to do. - case fs.ModeSymlink: - fsInfo.link, err = os.Readlink(fullPath) - if err != nil { - return fmt.Errorf("cannot read symlink %q: %w", fullPath, err) - } - case 0: // Regular file. - h, err := contentHash(fullPath) - if err != nil { - return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) - } - fsInfo.hash = hex.EncodeToString(h) - fsInfo.size = info.Size() - default: - return fmt.Errorf("cannot check %q: unrecognized type %s", fullPath, mode.String()) - } - - // Collect manifests for tailored checking later. Adjust observed hash and - // size to still compare in a generic way. - if filepath.Base(path.Path) == manifestutil.DefaultFilename && recordedPathInfo.size == 0 && recordedPathInfo.hash == "" { - mfestInfo := *fsInfo - manifestInfos[path.Path] = &mfestInfo - fsInfo.size = 0 - fsInfo.hash = "" - } - - if recordedPathInfo.mode != fsInfo.mode { - return fmt.Errorf("inconsistent mode at %q: recorded %v, observed %v", path.Path, recordedPathInfo.mode, fsInfo.mode) - } - if recordedPathInfo.size != fsInfo.size { - return fmt.Errorf("inconsistent size at %q: recorded %v, observed %v", path.Path, recordedPathInfo.size, fsInfo.size) - } - if recordedPathInfo.link != fsInfo.link { - return fmt.Errorf("inconsistent link at %q: recorded %v, observed %v", path.Path, recordedPathInfo.link, fsInfo.link) - } - if recordedPathInfo.hash != fsInfo.hash { - return fmt.Errorf("inconsistent hash at %q: recorded %v, observed %v", path.Path, recordedPathInfo.hash, fsInfo.hash) - } - // Check hardlink. - if ftype != fs.ModeDir { - stat, ok := info.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("cannot get syscall stat info for %q", info.Name()) - } - inode := stat.Ino - - if path.Inode == 0 { - // This path must not be linked to any other. - singlePath, ok := singlePathsByFSInode[inode] - if ok { - return fmt.Errorf("inconsistent content at %q: recorded no hardlink, observed hardlinked to %q", path.Path, singlePath) - } - singlePathsByFSInode[inode] = path.Path - } else { - recordedInode, ok := fsInodeByManifestInode[path.Inode] - if !ok { - fsInodeByManifestInode[path.Inode] = inode - } else if recordedInode != inode { - return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) - } - } - } - return nil - }) - if err != nil { - return err - } - - // Check manifests. - // They must all be valid manifests and be consistent per schema version. - schemaManifestInfos := make(map[string]*pathInfo) - for path, info := range manifestInfos { - fullPath := filepath.Join(rootDir, path) - f, err := os.Open(fullPath) - if err != nil { - return err - } - defer f.Close() - r, err := zstd.NewReader(f) - if err != nil { - return err - } - defer r.Close() - mfest, err = manifest.Read(r) - if err != nil { - return err - } - err = manifestutil.Validate(mfest) - if err != nil { - return err - } - schema := mfest.Schema() - refInfo, ok := schemaManifestInfos[schema] - if !ok { - schemaManifestInfos[schema] = info - continue - } - - if refInfo.size != info.size { - return fmt.Errorf("inconsistent manifest size for version %s at %q: recorded %v, observed %v", schema, path, refInfo.size, info.size) - } - if refInfo.hash != info.hash { - return fmt.Errorf("inconsistent manifest hash for version %s at %q: recorded %v, observed %v", schema, path, refInfo.hash, info.hash) - } - } - return nil -} - -func contentHash(path string) ([]byte, error) { - f, err := os.Open(path) - if err != nil { - return nil, err - } - defer f.Close() - - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return nil, err - } - return h.Sum(nil), nil -} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 48eb0e11..1dcfd019 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -3,10 +3,12 @@ package slicer import ( "archive/tar" "bytes" + "crypto/sha256" "encoding/hex" "fmt" "io" "io/fs" + "maps" "os" "path" "path/filepath" @@ -32,6 +34,7 @@ type RunOptions struct { Selection *setup.Selection Archives map[string]archive.Archive TargetDir string + Manifest *manifest.Manifest } type pathData struct { @@ -93,6 +96,19 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } + var originalTargetDir string + if options.Manifest != nil { + tmpWorkDir, err := os.MkdirTemp(targetDir, "chisel-*") + if err != nil { + return fmt.Errorf("cannot create temporary working directory: %w", err) + } + originalTargetDir = targetDir + targetDir = tmpWorkDir + defer func() { + os.RemoveAll(tmpWorkDir) + }() + } + pkgArchive, err := selectPkgArchives(options.Archives, options.Selection) if err != nil { return err @@ -350,7 +366,41 @@ func Run(options *RunOptions) error { return err } - return generateManifests(targetDir, options.Selection, report, pkgInfos) + err = generateManifests(targetDir, options.Selection, report, pkgInfos) + if err != nil { + return err + } + + if options.Manifest != nil { + return upgrade(originalTargetDir, targetDir, report, options.Manifest) + } + return nil +} + +// upgrade upgrades content in targetDir using content in tempDir. +// Work on sorted list of content in tempDir +func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, oldManifest *manifest.Manifest) error { + paths := slices.Sorted(maps.Keys(newReport.Entries)) + for _, path := range paths { + entry := newReport.Entries[path] + var err error + switch entry.Mode & fs.ModeType { + case 0: + // rename file if hash different than same path in old manifest + case fs.ModeDir: + // create dir with proper mode + // or chmod existing dir + case fs.ModeSymlink: + // move symlink to dest + default: + err = fmt.Errorf("unsupported file type: %s", path) + } + if err != nil { + return err + } + } + + return nil } func generateManifests(targetDir string, selection *setup.Selection, @@ -541,32 +591,10 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel return pkgArchive, nil } -// Inspect examines and validates the targetDir. It returns, if found and valid -// the manifest representing the content in the targetDir. -func Inspect(targetDir string, release *setup.Release) (*manifest.Manifest, error) { - var mfest *manifest.Manifest - manifestPaths := manifestutil.FindPathsInRelease(release) - if len(manifestPaths) > 0 { - logf("Inspecting root directory...") - var err error - mfest, err = selectValidManifest(targetDir, manifestPaths) - if err != nil { - return nil, err - } - if mfest != nil { - err = checkRootDir(mfest, targetDir) - if err != nil { - return nil, err - } - } - } - return mfest, nil -} - -// selectValidManifest returns, if found, a valid manifest with the latest +// SelectValidManifest returns, if found, a valid manifest with the latest // schema. Consistency with all other manifests with the same schema is verified // so the selection is deterministic. -func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { +func SelectValidManifest(targetDir string, release *setup.Release) (*manifest.Manifest, error) { targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() @@ -575,6 +603,10 @@ func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Ma } targetDir = filepath.Join(dir, targetDir) } + manifestPaths := manifestutil.FindPathsInRelease(release) + if len(manifestPaths) == 0 { + return nil, nil + } type manifestHash struct { path string @@ -600,25 +632,26 @@ func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Ma defer r.Close() mfest, err := manifest.Read(r) if err != nil { - return nil + return err } err = manifestutil.Validate(mfest) if err != nil { - return nil + return err + } + // Verify consistency with other manifests with the same schema. + h, err := contentHash(mfestFullPath) + if err != nil { + return fmt.Errorf("cannot compute hash for %q: %w", mfestFullPath, err) + } + mfestHash := hex.EncodeToString(h) + refMfest, ok := schemaManifest[mfest.Schema()] + if !ok { + schemaManifest[mfest.Schema()] = manifestHash{mfestPath, mfestHash} + } else if refMfest.hash != mfestHash { + return fmt.Errorf("inconsistent manifests: %q and %q", refMfest.path, mfestPath) } if selected == nil || manifestutil.CompareSchemas(mfest.Schema(), selected.Schema()) > 0 { - h, err := contentHash(mfestFullPath) - if err != nil { - return fmt.Errorf("cannot compute hash for %q: %w", mfestFullPath, err) - } - mfestHash := hex.EncodeToString(h) - refMfest, ok := schemaManifest[mfest.Schema()] - if !ok { - schemaManifest[mfest.Schema()] = manifestHash{mfestPath, mfestHash} - } else if refMfest.hash != mfestHash { - return fmt.Errorf("inconsistent manifests: %q and %q", refMfest.path, mfestPath) - } selected = mfest } return nil @@ -629,3 +662,17 @@ func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Ma } return selected, nil } + +func contentHash(path string) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + return h.Sum(nil), nil +} From 5da04501bd541faf4a898c2ca8a0b8c564210ac8 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 30 Jan 2026 14:26:15 +0100 Subject: [PATCH 03/22] feat: implement upgrade --- internal/fsutil/create.go | 3 ++ internal/slicer/slicer.go | 99 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 2f87f670..60bfc2b0 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -169,6 +169,9 @@ func createDir(o *CreateOptions) error { fileinfo, err := os.Lstat(path) if err == nil { if fileinfo.IsDir() { + if fileinfo.Mode() != o.Mode && o.OverrideMode { + return os.Chmod(path, o.Mode) + } return nil } err = os.Remove(path) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 1dcfd019..67949d10 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -377,21 +377,56 @@ func Run(options *RunOptions) error { return nil } +// absPath requires root to be a clean path that ends in "/". +func absPath(root, relPath string) (string, error) { + path := filepath.Clean(filepath.Join(root, relPath)) + if !strings.HasPrefix(path, root) { + return "", fmt.Errorf("cannot create path %s outside of root %s", path, root) + } + return path, nil +} + // upgrade upgrades content in targetDir using content in tempDir. -// Work on sorted list of content in tempDir func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, oldManifest *manifest.Manifest) error { + logf("Upgrading existing content...") + filesToDelete := make([]*manifest.Path, 0) + oldPaths := make(map[string]*manifest.Path, 0) + err := oldManifest.IteratePaths("", func(path *manifest.Path) error { + _, ok := newReport.Entries[path.Path] + if !ok && strings.HasSuffix(path.Path, "/") { + // Keep directories. + filesToDelete = append(filesToDelete, path) + return nil + } + oldPaths[path.Path] = path + return nil + }) + if err != nil { + return err + } + paths := slices.Sorted(maps.Keys(newReport.Entries)) for _, path := range paths { + srcPath, err := absPath(tempDir, path) + if err != nil { + return err + } + dstPath, err := absPath(targetDir, path) + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(dstPath), 0o755); err != nil { + return err + } + entry := newReport.Entries[path] - var err error switch entry.Mode & fs.ModeType { case 0: - // rename file if hash different than same path in old manifest + err = upgradeFile(srcPath, dstPath, &entry) case fs.ModeDir: - // create dir with proper mode - // or chmod existing dir + err = upgradeDir(dstPath, &entry) case fs.ModeSymlink: - // move symlink to dest + err = os.Rename(srcPath, dstPath) default: err = fmt.Errorf("unsupported file type: %s", path) } @@ -400,9 +435,61 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o } } + // Delete old files + for _, pathToDelete := range filesToDelete { + path, err := absPath(tempDir, pathToDelete.Path) + if err != nil { + return err + } + err = os.Remove(path) + if err != nil { + return err + } + } return nil } +func upgradeFile(srcPath string, dstPath string, entry *manifestutil.ReportEntry) error { + fileinfo, err := os.Lstat(dstPath) + if err == nil { + h, err := contentHash(dstPath) + if err != nil { + return fmt.Errorf("cannot compute hash for %q: %w", dstPath, err) + } + oldHash := hex.EncodeToString(h) + newHash := entry.SHA256 + if newHash == "" { + newHash = entry.FinalSHA256 + } + if oldHash == newHash && entry.Mode == fileinfo.Mode() { + // Same file, do nothing. + return nil + } + } else if !os.IsNotExist(err) { + return err + } + return os.Rename(srcPath, dstPath) +} + +func upgradeDir(path string, entry *manifestutil.ReportEntry) error { + fileinfo, err := os.Lstat(path) + if err == nil { + if fileinfo.IsDir() { + if fileinfo.Mode() != entry.Mode { + return os.Chmod(path, entry.Mode) + } + return nil + } + err = os.Remove(path) + if err != nil { + return err + } + } else if !os.IsNotExist(err) { + return err + } + return os.Mkdir(path, entry.Mode) +} + func generateManifests(targetDir string, selection *setup.Selection, report *manifestutil.Report, pkgInfos []*archive.PackageInfo) error { manifestSlices := manifestutil.FindPaths(selection.Slices) From 5703751eff9926771d086ee7d43480e916848522 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 30 Jan 2026 14:43:30 +0100 Subject: [PATCH 04/22] fix: deletion --- cmd/chisel/cmd_cut.go | 2 +- internal/slicer/slicer.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 940ec121..5873c3a3 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -141,7 +141,7 @@ func (cmd *cmdCut) Execute(args []string) error { Selection: selection, Archives: archives, TargetDir: cmd.RootDir, - Manifest: mfest, + Manifest: mfest, }) return err } diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 67949d10..aed6f668 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -393,7 +393,7 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o oldPaths := make(map[string]*manifest.Path, 0) err := oldManifest.IteratePaths("", func(path *manifest.Path) error { _, ok := newReport.Entries[path.Path] - if !ok && strings.HasSuffix(path.Path, "/") { + if !ok && !strings.HasSuffix(path.Path, "/") { // Keep directories. filesToDelete = append(filesToDelete, path) return nil @@ -437,7 +437,7 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o // Delete old files for _, pathToDelete := range filesToDelete { - path, err := absPath(tempDir, pathToDelete.Path) + path, err := absPath(targetDir, pathToDelete.Path) if err != nil { return err } From 26bb2b3361c8fbdb078438eeeb9e144815e6da67 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 30 Jan 2026 14:49:19 +0100 Subject: [PATCH 05/22] fix: revert inadvertent change --- internal/manifestutil/manifestutil_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index 0c6c0c06..2bab0a68 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -2,6 +2,7 @@ package manifestutil_test import ( "bytes" + "io" "io/fs" "os" "path" From 4a14230ed08272abf1a7e4badeca243f6bcae7cc Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 30 Jan 2026 15:08:17 +0100 Subject: [PATCH 06/22] fix: check slice collecting error --- cmd/chisel/cmd_cut.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 5873c3a3..b0dd1eec 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -79,7 +79,7 @@ func (cmd *cmdCut) Execute(args []string) error { return err } if mfest != nil { - mfest.IterateSlices("", func(slice *manifest.Slice) error { + err = mfest.IterateSlices("", func(slice *manifest.Slice) error { sk, err := setup.ParseSliceKey(slice.Name) if err != nil { return err @@ -87,6 +87,9 @@ func (cmd *cmdCut) Execute(args []string) error { sliceKeys = append(sliceKeys, sk) return nil }) + if err != nil { + return err + } } selection, err := setup.Select(release, sliceKeys, cmd.Arch) From 865c1b8ae17a03e70df70b2ca7505e5de1053b15 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 30 Jan 2026 17:01:00 +0100 Subject: [PATCH 07/22] fix: simplify upgrade and improve deletion --- internal/slicer/slicer.go | 55 ++++++++++++++------------------------- 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index aed6f668..5de5813d 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -389,13 +389,12 @@ func absPath(root, relPath string) (string, error) { // upgrade upgrades content in targetDir using content in tempDir. func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, oldManifest *manifest.Manifest) error { logf("Upgrading existing content...") - filesToDelete := make([]*manifest.Path, 0) + pathsToDelete := make([]string, 0) oldPaths := make(map[string]*manifest.Path, 0) err := oldManifest.IteratePaths("", func(path *manifest.Path) error { _, ok := newReport.Entries[path.Path] - if !ok && !strings.HasSuffix(path.Path, "/") { - // Keep directories. - filesToDelete = append(filesToDelete, path) + if !ok { + pathsToDelete = append(pathsToDelete, path.Path) return nil } oldPaths[path.Path] = path @@ -422,11 +421,10 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o entry := newReport.Entries[path] switch entry.Mode & fs.ModeType { case 0: - err = upgradeFile(srcPath, dstPath, &entry) - case fs.ModeDir: - err = upgradeDir(dstPath, &entry) case fs.ModeSymlink: err = os.Rename(srcPath, dstPath) + case fs.ModeDir: + err = upgradeDir(dstPath, &entry) default: err = fmt.Errorf("unsupported file type: %s", path) } @@ -435,42 +433,29 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o } } - // Delete old files - for _, pathToDelete := range filesToDelete { - path, err := absPath(targetDir, pathToDelete.Path) + // Delete old paths + slices.Sort(pathsToDelete) + slices.Reverse(pathsToDelete) + for _, pathToDelete := range pathsToDelete { + path, err := absPath(targetDir, pathToDelete) if err != nil { return err } - err = os.Remove(path) - if err != nil { - return err + if strings.HasSuffix(path, "/") { + err = syscall.Rmdir(path) + if err != nil && err != syscall.ENOTEMPTY { + return err + } + } else { + err = os.Remove(path) + if err != nil { + return err + } } } return nil } -func upgradeFile(srcPath string, dstPath string, entry *manifestutil.ReportEntry) error { - fileinfo, err := os.Lstat(dstPath) - if err == nil { - h, err := contentHash(dstPath) - if err != nil { - return fmt.Errorf("cannot compute hash for %q: %w", dstPath, err) - } - oldHash := hex.EncodeToString(h) - newHash := entry.SHA256 - if newHash == "" { - newHash = entry.FinalSHA256 - } - if oldHash == newHash && entry.Mode == fileinfo.Mode() { - // Same file, do nothing. - return nil - } - } else if !os.IsNotExist(err) { - return err - } - return os.Rename(srcPath, dstPath) -} - func upgradeDir(path string, entry *manifestutil.ReportEntry) error { fileinfo, err := os.Lstat(path) if err == nil { From 0343ddcfe0db4d58613e7223ea6adb726ee2e559 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 30 Jan 2026 17:12:33 +0100 Subject: [PATCH 08/22] refactor: cleaning --- internal/slicer/slicer.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 5de5813d..7923d6cc 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -389,15 +389,12 @@ func absPath(root, relPath string) (string, error) { // upgrade upgrades content in targetDir using content in tempDir. func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, oldManifest *manifest.Manifest) error { logf("Upgrading existing content...") - pathsToDelete := make([]string, 0) - oldPaths := make(map[string]*manifest.Path, 0) + missingPaths := make([]string, 0) err := oldManifest.IteratePaths("", func(path *manifest.Path) error { _, ok := newReport.Entries[path.Path] if !ok { - pathsToDelete = append(pathsToDelete, path.Path) - return nil + missingPaths = append(missingPaths, path.Path) } - oldPaths[path.Path] = path return nil }) if err != nil { @@ -433,11 +430,11 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o } } - // Delete old paths - slices.Sort(pathsToDelete) - slices.Reverse(pathsToDelete) - for _, pathToDelete := range pathsToDelete { - path, err := absPath(targetDir, pathToDelete) + // Remove missing paths + slices.Sort(missingPaths) + slices.Reverse(missingPaths) + for _, relPath := range missingPaths { + path, err := absPath(targetDir, relPath) if err != nil { return err } From e11642485b9ffea7ee3bc008bfbaa43e40c4d0d1 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 2 Feb 2026 11:59:10 +0100 Subject: [PATCH 09/22] refactor: refine upgrade --- internal/slicer/slicer.go | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 7923d6cc..2e263fd5 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -386,22 +386,10 @@ func absPath(root, relPath string) (string, error) { return path, nil } -// upgrade upgrades content in targetDir using content in tempDir. -func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, oldManifest *manifest.Manifest) error { - logf("Upgrading existing content...") - missingPaths := make([]string, 0) - err := oldManifest.IteratePaths("", func(path *manifest.Path) error { - _, ok := newReport.Entries[path.Path] - if !ok { - missingPaths = append(missingPaths, path.Path) - } - return nil - }) - if err != nil { - return err - } - - paths := slices.Sorted(maps.Keys(newReport.Entries)) +// upgrade upgrades content in targetDir with content in tempDir. +func upgrade(targetDir string, tempDir string, report *manifestutil.Report, mfest *manifest.Manifest) error { + logf("Upgrading content...") + paths := slices.Sorted(maps.Keys(report.Entries)) for _, path := range paths { srcPath, err := absPath(tempDir, path) if err != nil { @@ -415,7 +403,7 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o return err } - entry := newReport.Entries[path] + entry := report.Entries[path] switch entry.Mode & fs.ModeType { case 0: case fs.ModeSymlink: @@ -431,8 +419,18 @@ func upgrade(targetDir string, tempDir string, newReport *manifestutil.Report, o } // Remove missing paths - slices.Sort(missingPaths) - slices.Reverse(missingPaths) + missingPaths := make([]string, 0) + err := mfest.IteratePaths("", func(path *manifest.Path) error { + _, ok := report.Entries[path.Path] + if !ok { + missingPaths = append(missingPaths, path.Path) + } + return nil + }) + if err != nil { + return err + } + sort.Sort(sort.Reverse(sort.StringSlice(missingPaths))) for _, relPath := range missingPaths { path, err := absPath(targetDir, relPath) if err != nil { From 5b9d408ba4f1c75719b48900919c2d79e508ebfe Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 3 Feb 2026 09:31:08 +0100 Subject: [PATCH 10/22] refactor: fsutil handles moving and removing --- internal/fsutil/move.go | 86 +++++++++++++++++++++++++++++++++++++++ internal/fsutil/remove.go | 50 +++++++++++++++++++++++ internal/slicer/slicer.go | 73 ++++++--------------------------- 3 files changed, 148 insertions(+), 61 deletions(-) create mode 100644 internal/fsutil/move.go create mode 100644 internal/fsutil/remove.go diff --git a/internal/fsutil/move.go b/internal/fsutil/move.go new file mode 100644 index 00000000..4276d8a6 --- /dev/null +++ b/internal/fsutil/move.go @@ -0,0 +1,86 @@ +package fsutil + +import ( + "fmt" + "io/fs" + "os" + "path/filepath" +) + +type MoveOptions struct { + SrcRoot string + DstRoot string + // Path is relative to Root. + Path string + Mode fs.FileMode + // If MakeParents is true, missing parent directories of Path are + // created with permissions 0755. + MakeParents bool + // If OverrideMode is true and entry already exists, update the mode. Does + // not affect symlinks. + OverrideMode bool +} + +// Move moves a filesystem entry according to the provided options. +// +// Move can return errors from the os package. +func Move(options *MoveOptions) error { + o, err := getValidMoveOptions(options) + if err != nil { + return err + } + + srcPath, err := absPath(options.SrcRoot, o.Path) + if err != nil { + return err + } + dstPath, err := absPath(options.DstRoot, o.Path) + if err != nil { + return err + } + + if o.MakeParents { + if err := os.MkdirAll(filepath.Dir(dstPath), 0o755); err != nil { + return err + } + } + + switch o.Mode & fs.ModeType { + case 0: + case fs.ModeSymlink: + err = os.Rename(srcPath, dstPath) + case fs.ModeDir: + createOptions := &CreateOptions{ + Root: o.DstRoot, + Path: o.Path, + Mode: o.Mode, + OverrideMode: o.OverrideMode, + } + err = createDir(createOptions) + default: + err = fmt.Errorf("unsupported file type: %s", o.Path) + } + if err != nil { + return err + } + + return nil +} + +func getValidMoveOptions(options *MoveOptions) (*MoveOptions, error) { + optsCopy := *options + o := &optsCopy + if o.SrcRoot == "" { + return nil, fmt.Errorf("internal error: MoveOptions.SrcRoot is unset") + } + if o.DstRoot == "" { + return nil, fmt.Errorf("internal error: MoveOptions.DstRoot is unset") + } + if o.SrcRoot != "/" { + o.SrcRoot = filepath.Clean(o.SrcRoot) + "/" + } + if o.DstRoot != "/" { + o.DstRoot = filepath.Clean(o.DstRoot) + "/" + } + return o, nil +} diff --git a/internal/fsutil/remove.go b/internal/fsutil/remove.go new file mode 100644 index 00000000..6a2c731c --- /dev/null +++ b/internal/fsutil/remove.go @@ -0,0 +1,50 @@ +package fsutil + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "syscall" +) + +type RemoveOptions struct { + Root string + // Path is relative to Root. + Path string +} + +func Remove(options *RemoveOptions) error { + options, err := getValidRemoveOptions(options) + if err != nil { + return err + } + path, err := absPath(options.Root, options.Path) + if err != nil { + return err + } + if strings.HasSuffix(path, "/") { + err = syscall.Rmdir(path) + if err != nil && err != syscall.ENOTEMPTY { + return err + } + } else { + err = os.Remove(path) + if err != nil { + return err + } + } + return nil +} + +func getValidRemoveOptions(options *RemoveOptions) (*RemoveOptions, error) { + optsCopy := *options + o := &optsCopy + if o.Root == "" { + return nil, fmt.Errorf("internal error: RemoveOptions.Root is unset") + } + if o.Root != "/" { + o.Root = filepath.Clean(o.Root) + "/" + } + return o, nil +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 2e263fd5..2ccab61b 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -377,42 +377,20 @@ func Run(options *RunOptions) error { return nil } -// absPath requires root to be a clean path that ends in "/". -func absPath(root, relPath string) (string, error) { - path := filepath.Clean(filepath.Join(root, relPath)) - if !strings.HasPrefix(path, root) { - return "", fmt.Errorf("cannot create path %s outside of root %s", path, root) - } - return path, nil -} - // upgrade upgrades content in targetDir with content in tempDir. func upgrade(targetDir string, tempDir string, report *manifestutil.Report, mfest *manifest.Manifest) error { logf("Upgrading content...") paths := slices.Sorted(maps.Keys(report.Entries)) for _, path := range paths { - srcPath, err := absPath(tempDir, path) - if err != nil { - return err - } - dstPath, err := absPath(targetDir, path) - if err != nil { - return err - } - if err := os.MkdirAll(filepath.Dir(dstPath), 0o755); err != nil { - return err - } - entry := report.Entries[path] - switch entry.Mode & fs.ModeType { - case 0: - case fs.ModeSymlink: - err = os.Rename(srcPath, dstPath) - case fs.ModeDir: - err = upgradeDir(dstPath, &entry) - default: - err = fmt.Errorf("unsupported file type: %s", path) - } + err := fsutil.Move(&fsutil.MoveOptions{ + SrcRoot: tempDir, + DstRoot: targetDir, + Path: path, + Mode: entry.Mode, + MakeParents: true, + OverrideMode: true, + }) if err != nil { return err } @@ -432,44 +410,17 @@ func upgrade(targetDir string, tempDir string, report *manifestutil.Report, mfes } sort.Sort(sort.Reverse(sort.StringSlice(missingPaths))) for _, relPath := range missingPaths { - path, err := absPath(targetDir, relPath) + err := fsutil.Remove(&fsutil.RemoveOptions{ + Root: targetDir, + Path: relPath, + }) if err != nil { return err } - if strings.HasSuffix(path, "/") { - err = syscall.Rmdir(path) - if err != nil && err != syscall.ENOTEMPTY { - return err - } - } else { - err = os.Remove(path) - if err != nil { - return err - } - } } return nil } -func upgradeDir(path string, entry *manifestutil.ReportEntry) error { - fileinfo, err := os.Lstat(path) - if err == nil { - if fileinfo.IsDir() { - if fileinfo.Mode() != entry.Mode { - return os.Chmod(path, entry.Mode) - } - return nil - } - err = os.Remove(path) - if err != nil { - return err - } - } else if !os.IsNotExist(err) { - return err - } - return os.Mkdir(path, entry.Mode) -} - func generateManifests(targetDir string, selection *setup.Selection, report *manifestutil.Report, pkgInfos []*archive.PackageInfo) error { manifestSlices := manifestutil.FindPaths(selection.Slices) From 7861c8dfbfa958310fd883b16de2a2c66d508522 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 3 Feb 2026 09:40:38 +0100 Subject: [PATCH 11/22] refactor: improve consistency --- internal/fsutil/create.go | 6 +++--- internal/fsutil/move.go | 6 +++--- internal/fsutil/remove.go | 4 ++++ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 60bfc2b0..e4987e64 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -42,7 +42,7 @@ type Entry struct { // // Create can return errors from the os package. func Create(options *CreateOptions) (*Entry, error) { - o, err := getValidOptions(options) + o, err := getValidCreateOptions(options) if err != nil { return nil, err } @@ -124,7 +124,7 @@ func Create(options *CreateOptions) (*Entry, error) { // information recorded in Entry. The Hash and Size attributes are set on // calling Close() on the Writer. func CreateWriter(options *CreateOptions) (io.WriteCloser, *Entry, error) { - o, err := getValidOptions(options) + o, err := getValidCreateOptions(options) if err != nil { return nil, nil, err } @@ -252,7 +252,7 @@ func createHardLink(o *CreateOptions) error { return err } -func getValidOptions(options *CreateOptions) (*CreateOptions, error) { +func getValidCreateOptions(options *CreateOptions) (*CreateOptions, error) { optsCopy := *options o := &optsCopy if o.Root == "" { diff --git a/internal/fsutil/move.go b/internal/fsutil/move.go index 4276d8a6..a69a8b0a 100644 --- a/internal/fsutil/move.go +++ b/internal/fsutil/move.go @@ -21,7 +21,7 @@ type MoveOptions struct { OverrideMode bool } -// Move moves a filesystem entry according to the provided options. +// Move moves or create a filesystem entry according to the provided options. // // Move can return errors from the os package. func Move(options *MoveOptions) error { @@ -50,13 +50,13 @@ func Move(options *MoveOptions) error { case fs.ModeSymlink: err = os.Rename(srcPath, dstPath) case fs.ModeDir: - createOptions := &CreateOptions{ + err = createDir(&CreateOptions{ Root: o.DstRoot, Path: o.Path, Mode: o.Mode, OverrideMode: o.OverrideMode, } - err = createDir(createOptions) +) default: err = fmt.Errorf("unsupported file type: %s", o.Path) } diff --git a/internal/fsutil/remove.go b/internal/fsutil/remove.go index 6a2c731c..c7884404 100644 --- a/internal/fsutil/remove.go +++ b/internal/fsutil/remove.go @@ -14,6 +14,10 @@ type RemoveOptions struct { Path string } +// Remove removes a filesystem entry according to the provided options. +// Non-empty directories are not removed. +// +// Remove can return errors from the os and syscall packages. func Remove(options *RemoveOptions) error { options, err := getValidRemoveOptions(options) if err != nil { From 27a0d229444031cd70c752afb12950d32f5db8ba Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 3 Feb 2026 09:42:38 +0100 Subject: [PATCH 12/22] style: fix lint error --- internal/fsutil/move.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/fsutil/move.go b/internal/fsutil/move.go index a69a8b0a..1a426633 100644 --- a/internal/fsutil/move.go +++ b/internal/fsutil/move.go @@ -55,8 +55,7 @@ func Move(options *MoveOptions) error { Path: o.Path, Mode: o.Mode, OverrideMode: o.OverrideMode, - } -) + }) default: err = fmt.Errorf("unsupported file type: %s", o.Path) } From 5f377b0a06d6cdce476c603332e19428ac1d41e0 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 3 Feb 2026 17:01:38 +0100 Subject: [PATCH 13/22] tests: Move and Remove --- internal/deb/extract_test.go | 2 +- internal/fsutil/create.go | 2 +- internal/fsutil/create_test.go | 4 +- internal/fsutil/move.go | 3 +- internal/fsutil/move_test.go | 258 +++++++++++++++++++++++++++++++++ internal/fsutil/remove.go | 9 +- internal/fsutil/remove_test.go | 145 ++++++++++++++++++ internal/slicer/slicer_test.go | 2 +- 8 files changed, 415 insertions(+), 10 deletions(-) create mode 100644 internal/fsutil/move_test.go create mode 100644 internal/fsutil/remove_test.go diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 1ec0b8a5..30f77816 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -485,7 +485,7 @@ var extractTests = []extractTest{{ }}, }, }, - error: `cannot extract from package "test-package": cannot create path /[a-z0-9\-\/]*/file outside of root /[a-z0-9\-\/]*`, + error: `cannot extract from package "test-package": cannot handle path /[a-z0-9\-\/]*/file outside of root /[a-z0-9\-\/]*`, }} func (s *S) TestExtract(c *C) { diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index e4987e64..3cd64708 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -268,7 +268,7 @@ func getValidCreateOptions(options *CreateOptions) (*CreateOptions, error) { func absPath(root, relPath string) (string, error) { path := filepath.Clean(filepath.Join(root, relPath)) if !strings.HasPrefix(path, root) { - return "", fmt.Errorf("cannot create path %s outside of root %s", path, root) + return "", fmt.Errorf("cannot handle path %s outside of root %s", path, root) } return path, nil } diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 5e908fcc..370568c0 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -279,7 +279,7 @@ var createTests = []createTest{{ Mode: 0666, Data: bytes.NewBufferString("hijacking system file"), }, - error: `cannot create path /file outside of root /root/`, + error: `cannot handle path /file outside of root /root/`, }, { summary: "Hardlink cannot escape Root", options: fsutil.CreateOptions{ @@ -410,7 +410,7 @@ var createWriterTests = []createWriterTest{{ Mode: 0644, MakeParents: true, }, - error: `cannot create path /file outside of root /root/`, + error: `cannot handle path /file outside of root /root/`, }} func (s *S) TestCreateWriter(c *C) { diff --git a/internal/fsutil/move.go b/internal/fsutil/move.go index 1a426633..e47f135d 100644 --- a/internal/fsutil/move.go +++ b/internal/fsutil/move.go @@ -46,8 +46,7 @@ func Move(options *MoveOptions) error { } switch o.Mode & fs.ModeType { - case 0: - case fs.ModeSymlink: + case 0, fs.ModeSymlink: err = os.Rename(srcPath, dstPath) case fs.ModeDir: err = createDir(&CreateOptions{ diff --git a/internal/fsutil/move_test.go b/internal/fsutil/move_test.go new file mode 100644 index 00000000..6b68ce09 --- /dev/null +++ b/internal/fsutil/move_test.go @@ -0,0 +1,258 @@ +package fsutil_test + +import ( + "io/fs" + "os" + "path/filepath" + "syscall" + + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/fsutil" + "github.com/canonical/chisel/internal/testutil" +) + +type moveTest struct { + summary string + options fsutil.MoveOptions + hackopt func(c *C, dir string, opts *fsutil.MoveOptions) + result map[string]string + error string +} + +var moveTests = []moveTest{{ + summary: "Move a file and create its parent directory", + options: fsutil.MoveOptions{ + Path: "bar", + Mode: 0o644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/bar"), []byte("data"), 0o644), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/bar": "file 0644 3a6eb079", + }, +}, { + summary: "Move a symlink", + options: fsutil.MoveOptions{ + Path: "foo", + Mode: fs.ModeSymlink, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/baz"), []byte("data"), 0o644), IsNil) + c.Assert(os.Symlink("baz", filepath.Join(dir, "src/foo")), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/src/baz": "file 0644 3a6eb079", + "/dst/": "dir 0755", + "/dst/foo": "symlink baz", + }, +}, { + summary: "Move (create) a directory", + options: fsutil.MoveOptions{ + Path: "foo/", + Mode: fs.ModeDir | 0o765, + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/foo/": "dir 0765", + }, +}, { + summary: "Move (create) a directory with sticky bit", + options: fsutil.MoveOptions{ + Path: "foo", + Mode: fs.ModeDir | fs.ModeSticky | 0o775, + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/foo/": "dir 01775", + }, +}, { + summary: "Cannot move (create) a parent directory without MakeParents set", + options: fsutil.MoveOptions{ + Path: "foo/bar", + Mode: fs.ModeDir | 0o775, + }, + error: `mkdir /[^ ]*/foo/bar: no such file or directory`, +}, { + summary: "Moving to an existing directory keeps the original mode", + options: fsutil.MoveOptions{ + Path: "foo", + Mode: fs.ModeDir | 0o775, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.Mkdir(filepath.Join(dir, "dst/foo/"), fs.ModeDir|0o765), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + // mode is not updated. + "/dst/foo/": "dir 0765", + }, +}, { + summary: "Moving to an existing directory overrides the mode is requested", + options: fsutil.MoveOptions{ + Path: "foo", + Mode: fs.ModeDir | 0o775, + OverrideMode: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.Mkdir(filepath.Join(dir, "dst/foo/"), fs.ModeDir|0o765), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + // mode is not updated. + "/dst/foo/": "dir 0775", + }, +}, { + summary: "Moving to an existing file overrides the original mode", + options: fsutil.MoveOptions{ + Path: "foo", + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/foo"), []byte("data"), 0o644), IsNil) + c.Assert(os.WriteFile(filepath.Join(dir, "dst/foo"), []byte("data"), 0o666), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/foo": "file 0644 3a6eb079", + }, +}, { + summary: "Move a hard link", + options: fsutil.MoveOptions{ + Path: "hardlink", + Mode: 0o644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/file"), []byte("data"), 0o644), IsNil) + c.Assert(os.Link(filepath.Join(dir, "src/file"), filepath.Join(dir, "src/hardlink")), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/src/file": "file 0644 3a6eb079 <1>", + "/dst/hardlink": "file 0644 3a6eb079 <1>", + }, +}, { + summary: "No error if hard link already exists", + options: fsutil.MoveOptions{ + Path: "hardlink", + Mode: 0o644, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/file"), []byte("data"), 0o644), IsNil) + c.Assert(os.WriteFile(filepath.Join(dir, "dst/foo"), []byte("data"), 0o644), IsNil) + c.Assert(os.Link(filepath.Join(dir, "src/file"), filepath.Join(dir, "src/hardlink")), IsNil) + c.Assert(os.Link(filepath.Join(dir, "dst/foo"), filepath.Join(dir, "dst/hardlink")), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/src/file": "file 0644 3a6eb079 <1>", + "/dst/foo": "file 0644 3a6eb079", + "/dst/hardlink": "file 0644 3a6eb079 <1>", + }, +}, { + summary: "Override a symlink", + options: fsutil.MoveOptions{ + Path: "foo", + Mode: 0o666 | fs.ModeSymlink, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/baz"), []byte("data"), 0o666), IsNil) + c.Assert(os.Symlink("baz", filepath.Join(dir, "src/foo")), IsNil) + c.Assert(os.WriteFile(filepath.Join(dir, "dst/bar"), []byte("data"), 0o644), IsNil) + c.Assert(os.Symlink("bar", filepath.Join(dir, "dst/foo")), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/src/baz": "file 0666 3a6eb079", + "/dst/": "dir 0755", + "/dst/bar": "file 0644 3a6eb079", + "/dst/foo": "symlink baz", + }, +}, { + summary: "Cannot move file from outside of Root", + options: fsutil.MoveOptions{ + SrcRoot: "/rootsrc", + DstRoot: "/rootdst", + Path: "../file", + Mode: 0o666, + }, + error: `cannot handle path /file outside of root /rootsrc`, +}, { + summary: "Cannot move inexistent file", + options: fsutil.MoveOptions{ + Path: "file", + Mode: 0o666, + }, + error: `rename .*/src/file .*/dst/file: no such file or directory`, +}, { + summary: "Cannot move file to outside of Root", + options: fsutil.MoveOptions{ + SrcRoot: "/rootsrc", + DstRoot: "/rootdst", + Path: "../rootsrc/file", + Mode: 0o666, + }, + error: `cannot handle path /rootsrc/file outside of root /rootdst`, +}} + +func (s *S) TestMove(c *C) { + oldUmask := syscall.Umask(0) + defer func() { + syscall.Umask(oldUmask) + }() + + for _, test := range moveTests { + c.Logf("Test: %s", test.summary) + if test.result == nil { + // Empty map for no files moved. + test.result = make(map[string]string) + } + c.Logf("Options: %v", test.options) + dir := c.MkDir() + options := test.options + if options.SrcRoot == "" { + srcRoot := filepath.Join(dir, "src") + c.Assert(os.Mkdir(srcRoot, 0o755), IsNil) + options.SrcRoot = srcRoot + } + if options.DstRoot == "" { + dstRoot := filepath.Join(dir, "dst") + c.Assert(os.Mkdir(dstRoot, 0o755), IsNil) + options.DstRoot = dstRoot + } + if test.hackopt != nil { + test.hackopt(c, dir, &options) + } + err := fsutil.Move(&options) + + if test.error != "" { + c.Assert(err, ErrorMatches, test.error) + continue + } + + c.Assert(err, IsNil) + c.Assert(testutil.TreeDump(dir), DeepEquals, test.result) + } +} + +func (s *S) TestMoveEmptyRoot(c *C) { + options := &fsutil.MoveOptions{SrcRoot: "", DstRoot: "foo/"} + err := fsutil.Move(options) + c.Assert(err, ErrorMatches, "internal error: MoveOptions.SrcRoot is unset") + options = &fsutil.MoveOptions{SrcRoot: "foo/", DstRoot: ""} + err = fsutil.Move(options) + c.Assert(err, ErrorMatches, "internal error: MoveOptions.DstRoot is unset") +} diff --git a/internal/fsutil/remove.go b/internal/fsutil/remove.go index c7884404..f53a34ae 100644 --- a/internal/fsutil/remove.go +++ b/internal/fsutil/remove.go @@ -27,14 +27,14 @@ func Remove(options *RemoveOptions) error { if err != nil { return err } - if strings.HasSuffix(path, "/") { + if strings.HasSuffix(options.Path, "/") { err = syscall.Rmdir(path) - if err != nil && err != syscall.ENOTEMPTY { + if err != nil && err != syscall.ENOTEMPTY && err != syscall.ENOENT { return err } } else { err = os.Remove(path) - if err != nil { + if err != nil && !os.IsNotExist(err) { return err } } @@ -47,6 +47,9 @@ func getValidRemoveOptions(options *RemoveOptions) (*RemoveOptions, error) { if o.Root == "" { return nil, fmt.Errorf("internal error: RemoveOptions.Root is unset") } + if o.Path == "" { + return nil, fmt.Errorf("internal error: RemoveOptions.Path is unset") + } if o.Root != "/" { o.Root = filepath.Clean(o.Root) + "/" } diff --git a/internal/fsutil/remove_test.go b/internal/fsutil/remove_test.go new file mode 100644 index 00000000..4b744167 --- /dev/null +++ b/internal/fsutil/remove_test.go @@ -0,0 +1,145 @@ +package fsutil_test + +import ( + "os" + "path/filepath" + "syscall" + + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/fsutil" + "github.com/canonical/chisel/internal/testutil" +) + +type removeTest struct { + summary string + options fsutil.RemoveOptions + hackopt func(c *C, dir string, opts *fsutil.RemoveOptions) + result map[string]string + error string +} + +var removeTests = []removeTest{{ + summary: "Remove a file", + options: fsutil.RemoveOptions{ + Path: "file", + }, + hackopt: func(c *C, dir string, opts *fsutil.RemoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0o644), IsNil) + }, + result: map[string]string{}, +}, { + summary: "Remove a non-existent file", + options: fsutil.RemoveOptions{ + Path: "file", + }, + result: map[string]string{}, +}, { + summary: "Remove a non-existent directory", + options: fsutil.RemoveOptions{ + Path: "foo/", + }, + result: map[string]string{}, +}, { + summary: "Remove an empty directory", + options: fsutil.RemoveOptions{ + Path: "foo/bar", + }, + hackopt: func(c *C, dir string, opts *fsutil.RemoveOptions) { + c.Assert(os.MkdirAll(filepath.Join(dir, "foo/bar"), 0o755), IsNil) + }, + result: map[string]string{ + "/foo/": "dir 0755", + }, +}, { + summary: "Do not remove non-empty directory", + options: fsutil.RemoveOptions{ + Path: "foo/", + }, + hackopt: func(c *C, dir string, opts *fsutil.RemoveOptions) { + c.Assert(os.MkdirAll(filepath.Join(dir, "foo"), 0o755), IsNil) + c.Assert(os.WriteFile(filepath.Join(dir, "foo/file"), []byte("data"), 0o644), IsNil) + }, + result: map[string]string{ + "/foo/": "dir 0755", + "/foo/file": "file 0644 3a6eb079", + }, +}, { + summary: "Remove a symlink and not the target", + options: fsutil.RemoveOptions{ + Path: "bar", + }, + hackopt: func(c *C, dir string, opts *fsutil.RemoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0o644), IsNil) + c.Assert(os.Symlink("foo", filepath.Join(dir, "bar")), IsNil) + }, + result: map[string]string{ + "/foo": "file 0644 3a6eb079", + }, +}, { + summary: "Remove a hard link", + options: fsutil.RemoveOptions{ + Path: "hardlink1", + }, + hackopt: func(c *C, dir string, opts *fsutil.RemoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "file"), []byte("data"), 0o644), IsNil) + c.Assert(os.Link(filepath.Join(dir, "file"), filepath.Join(dir, "hardlink1")), IsNil) + c.Assert(os.Link(filepath.Join(dir, "file"), filepath.Join(dir, "hardlink2")), IsNil) + }, + result: map[string]string{ + "/file": "file 0644 3a6eb079 <1>", + "/hardlink2": "file 0644 3a6eb079 <1>", + }, +}, { + summary: "Cannot remove file outside of Root", + options: fsutil.RemoveOptions{ + Root: "/root", + Path: "../file", + }, + error: `cannot handle path /file outside of root /root/`, +}} + +func (s *S) TestRemove(c *C) { + oldUmask := syscall.Umask(0) + defer func() { + syscall.Umask(oldUmask) + }() + + for _, test := range removeTests { + c.Logf("Test: %s", test.summary) + if test.result == nil { + // Empty map for no files left. + test.result = make(map[string]string) + } + c.Logf("Options: %v", test.options) + dir := c.MkDir() + options := test.options + if options.Root == "" { + options.Root = dir + } + if test.hackopt != nil { + test.hackopt(c, dir, &options) + } + err := fsutil.Remove(&options) + + if test.error != "" { + c.Assert(err, ErrorMatches, test.error) + continue + } + + c.Assert(err, IsNil) + c.Assert(testutil.TreeDump(dir), DeepEquals, test.result) + } +} + +func (s *S) TestRemoveEmptyRoot(c *C) { + options := &fsutil.RemoveOptions{Root: ""} + err := fsutil.Remove(options) + c.Assert(err, ErrorMatches, "internal error: RemoveOptions.Root is unset") +} + +func (s *S) TestRemoveEmptyPath(c *C) { + options := &fsutil.RemoveOptions{Root: "foo/"} + err := fsutil.Remove(options) + c.Assert(err, ErrorMatches, "internal error: RemoveOptions.Path is unset") +} diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index fe408e6d..d031393a 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1806,7 +1806,7 @@ var slicerTests = []slicerTest{{ /**: `, }, - error: `cannot extract from package "test-package": cannot create path /[a-z0-9\-\/]*/file outside of root /[a-z0-9\-\/]*`, + error: `cannot extract from package "test-package": cannot handle path /[a-z0-9\-\/]*/file outside of root /[a-z0-9\-\/]*`, }, { summary: "Extract conflicting paths with prefer from proper package", slices: []setup.SliceKey{ From f74265ca42e02eb453e0e8181661f599f67309db Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 3 Feb 2026 17:22:45 +0100 Subject: [PATCH 14/22] tests: FindPathsInRelease --- internal/manifestutil/manifestutil_test.go | 112 +++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index 2bab0a68..c0f35886 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -110,10 +110,122 @@ func (s *S) TestFindPaths(c *C) { } } +var findPathsInReleaseTests = []struct { + summary string + release *setup.Release + expected []string +}{{ + summary: "Single package with single slice", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{ + "/folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + }, + }, + }, + }, + expected: []string{"/folder/manifest.wall"}, +}, { + summary: "No slices with generate:manifest", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{}, + }, + }, + }, + }, + }, + expected: []string{}, +}, { + summary: "Multiple packages with multiple slices", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{ + "/folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + "slice2": { + Name: "slice2", + Contents: map[string]setup.PathInfo{ + "/folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + }, + }, + "package2": { + Name: "package2", + Slices: map[string]*setup.Slice{ + "slice3": { + Name: "slice3", + Contents: map[string]setup.PathInfo{}, + }, + "slice4": { + Name: "slice4", + Contents: map[string]setup.PathInfo{ + "/other-folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + }, + }, + }, + }, + // Note: /folder/manifest.wall appears twice because both slice1 and slice2 declare it + expected: []string{"/folder/manifest.wall", "/folder/manifest.wall", "/other-folder/manifest.wall"}, +}, { + summary: "Empty release", + release: &setup.Release{ + Packages: map[string]*setup.Package{}, + }, + expected: []string{}, +}} + +func (s *S) TestFindPathsInRelease(c *C) { + for _, test := range findPathsInReleaseTests { + c.Logf("Summary: %s", test.summary) + + manifestPaths := manifestutil.FindPathsInRelease(test.release) + + c.Assert(manifestPaths, HasLen, len(test.expected)) + slices.Sort(manifestPaths) + slices.Sort(test.expected) + c.Assert(manifestPaths, DeepEquals, test.expected) + } +} + var slice1 = &setup.Slice{ Package: "package1", Name: "slice1", } + var slice2 = &setup.Slice{ Package: "package2", Name: "slice2", From 799373c14c8599214389b853d9b20de664555238 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 3 Feb 2026 17:38:49 +0100 Subject: [PATCH 15/22] tests: more test cases for Move --- internal/fsutil/move_test.go | 82 ++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/internal/fsutil/move_test.go b/internal/fsutil/move_test.go index 6b68ce09..4b5c23b5 100644 --- a/internal/fsutil/move_test.go +++ b/internal/fsutil/move_test.go @@ -35,6 +35,35 @@ var moveTests = []moveTest{{ "/dst/": "dir 0755", "/dst/bar": "file 0644 3a6eb079", }, +}, { + summary: "Move a file when parent directory exists", + options: fsutil.MoveOptions{ + Path: "bar", + Mode: 0o644, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/bar"), []byte("data"), 0o644), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/bar": "file 0644 3a6eb079", + }, +}, { + summary: "Move an empty file", + options: fsutil.MoveOptions{ + Path: "empty", + Mode: 0o644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/empty"), []byte(""), 0o644), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/empty": "file 0644 empty", + }, }, { summary: "Move a symlink", options: fsutil.MoveOptions{ @@ -190,6 +219,59 @@ var moveTests = []moveTest{{ Mode: 0o666, }, error: `cannot handle path /file outside of root /rootsrc`, +}, { + summary: "Path with ./ component", + options: fsutil.MoveOptions{ + Path: "./file", + Mode: 0o644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/file"), []byte("data"), 0o644), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/file": "file 0644 3a6eb079", + }, +}, { + summary: "Path with ../ component normalizes correctly", + options: fsutil.MoveOptions{ + Path: "foo/../bar", + Mode: 0o644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/bar"), []byte("data"), 0o644), IsNil) + }, + result: map[string]string{ + "/src/": "dir 0755", + "/dst/": "dir 0755", + "/dst/bar": "file 0644 3a6eb079", + }, +}, { + summary: "Cannot move to a path where parent is a file", + options: fsutil.MoveOptions{ + Path: "file/subpath", + Mode: 0o644, + MakeParents: true, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/file"), []byte("data"), 0o644), IsNil) + c.Assert(os.WriteFile(filepath.Join(dir, "dst/file"), []byte("data"), 0o644), IsNil) + }, + error: `mkdir .*/dst/file: not a directory`, +}, { + summary: "Cannot move a file to overwrite a directory", + options: fsutil.MoveOptions{ + Path: "target", + Mode: 0o644, + }, + hackopt: func(c *C, dir string, opts *fsutil.MoveOptions) { + c.Assert(os.WriteFile(filepath.Join(dir, "src/target"), []byte("data"), 0o644), IsNil) + c.Assert(os.Mkdir(filepath.Join(dir, "dst/target"), 0o755), IsNil) + }, + error: `rename .*/src/target .*/dst/target: file exists`, }, { summary: "Cannot move inexistent file", options: fsutil.MoveOptions{ From dbe3f309f3198ff4163b36639dcc81be37fd7904 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 3 Feb 2026 17:52:41 +0100 Subject: [PATCH 16/22] tests: add spread test --- tests/recut/task.yaml | 57 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 tests/recut/task.yaml diff --git a/tests/recut/task.yaml b/tests/recut/task.yaml new file mode 100644 index 00000000..0a1a09d6 --- /dev/null +++ b/tests/recut/task.yaml @@ -0,0 +1,57 @@ +summary: Recut reuses manifest to restore previous slices + +execute: | + rootfs_folder=rootfs_${RELEASE} + mkdir -p $rootfs_folder + + chisel_release="./release_${RELEASE}" + mkdir -p ${chisel_release}/slices + + ref_chisel_release="ref-chisel-release_${RELEASE}" + git clone --depth=1 -b ${OS}-${RELEASE} \ + https://github.com/canonical/chisel-releases $ref_chisel_release + + cp ${ref_chisel_release}/chisel.yaml ${chisel_release}/chisel.yaml + + cat >${chisel_release}/slices/base-files.yaml <<'EOF' + package: base-files + + slices: + slice-a: + contents: + /etc/hostname: + slice-b: + contents: + /etc/issue: + manifest: + contents: + /chisel/**: {generate: manifest} + EOF + + EXTRA_OPTIONS="" + if [ "$RELEASE" = "23.10" -o "$RELEASE" = "20.04" ]; then + EXTRA_OPTIONS="--ignore=unmaintained " + fi + + # First cut generates manifest and installs slice-a + chisel cut --release $chisel_release \ + --root $rootfs_folder \ + $EXTRA_OPTIONS \ + base-files_slice-a base-files_manifest + + test -s ${rootfs_folder}/etc/hostname + test -f ${rootfs_folder}/chisel/manifest.wall + + # Remove a file from the first slice + rm -f ${rootfs_folder}/etc/hostname + test ! -e ${rootfs_folder}/etc/hostname + + # Second cut only requests slice-b, slice-a should be restored via manifest + chisel cut --release $chisel_release \ + --root $rootfs_folder \ + $EXTRA_OPTIONS \ + base-files_slice-b + + test -s ${rootfs_folder}/etc/hostname + test -s ${rootfs_folder}/etc/issue + test -f ${rootfs_folder}/chisel/manifest.wall From becfe42d306a3a9f37f1b7c22eb7d769ad0765ac Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 4 Feb 2026 08:35:55 +0100 Subject: [PATCH 17/22] tests: fix recut spread test --- tests/recut/task.yaml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/recut/task.yaml b/tests/recut/task.yaml index 0a1a09d6..a01edcb9 100644 --- a/tests/recut/task.yaml +++ b/tests/recut/task.yaml @@ -1,4 +1,4 @@ -summary: Recut reuses manifest to restore previous slices +summary: Recut relies on manifest to update existing content execute: | rootfs_folder=rootfs_${RELEASE} @@ -19,7 +19,7 @@ execute: | slices: slice-a: contents: - /etc/hostname: + /etc/debian_version: slice-b: contents: /etc/issue: @@ -39,12 +39,11 @@ execute: | $EXTRA_OPTIONS \ base-files_slice-a base-files_manifest - test -s ${rootfs_folder}/etc/hostname + test -s ${rootfs_folder}/etc/debian_version test -f ${rootfs_folder}/chisel/manifest.wall # Remove a file from the first slice - rm -f ${rootfs_folder}/etc/hostname - test ! -e ${rootfs_folder}/etc/hostname + rm -f ${rootfs_folder}/etc/debian_version # Second cut only requests slice-b, slice-a should be restored via manifest chisel cut --release $chisel_release \ @@ -52,6 +51,6 @@ execute: | $EXTRA_OPTIONS \ base-files_slice-b - test -s ${rootfs_folder}/etc/hostname + test -s ${rootfs_folder}/etc/debian_version test -s ${rootfs_folder}/etc/issue test -f ${rootfs_folder}/chisel/manifest.wall From 3c994b856ad7af5b9ba35f3b1dd8500bee82a631 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 4 Feb 2026 09:44:11 +0100 Subject: [PATCH 18/22] tests: SelectValidManifest --- internal/slicer/slicer_test.go | 206 +++++++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index d031393a..3d921e9e 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2017,6 +2017,118 @@ func (s *S) TestRun(c *C) { runSlicerTests(s, c, v2FormatTests) } +type selectValidManifestTest struct { + summary string + build func() *setup.Release + setup func(c *C, targetDir string, release *setup.Release) + noMatch bool + error string +} + +var selectValidManifestTests = []selectValidManifestTest{{ + summary: "No manifest paths in release", + build: func() *setup.Release { + return &setup.Release{Packages: map[string]*setup.Package{}} + }, + noMatch: true, +}, { + summary: "Manifest path missing in target", + build: func() *setup.Release { + return buildReleaseWithManifestDirs("/chisel/**") + }, + noMatch: true, +}, { + summary: "Valid manifest selected", + build: func() *setup.Release { + return buildReleaseWithManifestDirs("/chisel/**") + }, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := manifestPathForDir("/chisel/**") + writeManifest(c, targetDir, manifestPath, releaseManifestSlice(release), "hash1") + }, +}, { + summary: "Two consistent manifests are accepted", + build: func() *setup.Release { + return buildReleaseWithManifestDirs("/chisel-a/**", "/chisel-b/**") + }, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPathA := manifestPathForDir("/chisel-a/**") + manifestPathB := manifestPathForDir("/chisel-b/**") + slice := releaseManifestSlice(release) + writeManifest(c, targetDir, manifestPathA, slice, "hash1") + writeManifest(c, targetDir, manifestPathB, slice, "hash1") + }, +}, { + summary: "Inconsistent manifests with same schema are rejected", + build: func() *setup.Release { + return buildReleaseWithManifestDirs("/chisel-a/**", "/chisel-b/**") + }, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPathA := manifestPathForDir("/chisel-a/**") + manifestPathB := manifestPathForDir("/chisel-b/**") + slice := releaseManifestSlice(release) + writeManifest(c, targetDir, manifestPathA, slice, "hash1") + writeManifest(c, targetDir, manifestPathB, slice, "hash2") + }, + error: `inconsistent manifests: ".*" and ".*"`, +}, { + summary: "Invalid manifest data returns error", + build: func() *setup.Release { + return buildReleaseWithManifestDirs("/chisel/**") + }, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := filepath.Join(targetDir, manifestPathForDir("/chisel/**")) + err := os.MkdirAll(filepath.Dir(manifestPath), 0o755) + c.Assert(err, IsNil) + err = os.WriteFile(manifestPath, []byte("not-a-zstd-manifest"), 0o644) + c.Assert(err, IsNil) + }, + error: "cannot read manifest: invalid input: .*", +}, { + summary: "Manifest validation error is returned", + build: func() *setup.Release { + return buildReleaseWithManifestDirs("/chisel/**") + }, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := manifestPathForDir("/chisel/**") + writeInvalidManifest(c, targetDir, manifestPath) + }, + error: `invalid manifest: path /file has no matching entry in contents`, +}, { + summary: "Manifest read fails on invalid schema", + build: func() *setup.Release { + return buildReleaseWithManifestDirs("/chisel/**") + }, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := manifestPathForDir("/chisel/**") + writeInvalidSchemaManifest(c, targetDir, manifestPath) + }, + error: `cannot read manifest: unknown schema version "9.9"`, +}} + +func (s *S) TestSelectValidManifest(c *C) { + for _, test := range selectValidManifestTests { + c.Logf("Summary: %s", test.summary) + release := test.build() + targetDir := c.MkDir() + if test.setup != nil { + test.setup(c, targetDir, release) + } + mfest, err := slicer.SelectValidManifest(targetDir, release) + if test.error != "" { + c.Assert(err, ErrorMatches, test.error) + continue + } + c.Assert(err, IsNil) + if test.noMatch { + c.Assert(mfest, IsNil) + continue + } + c.Assert(mfest, NotNil) + c.Assert(mfest.Schema(), Equals, manifest.Schema) + } +} + func runSlicerTests(s *S, c *C, tests []slicerTest) { for _, test := range tests { for _, testSlices := range testutil.Permutations(test.slices) { @@ -2165,6 +2277,36 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { } } +func buildReleaseWithManifestDirs(dirs ...string) *setup.Release { + contents := map[string]setup.PathInfo{} + for _, dir := range dirs { + contents[dir] = setup.PathInfo{Kind: "generate", Generate: "manifest"} + } + return &setup.Release{ + Packages: map[string]*setup.Package{ + "test-package": { + Name: "test-package", + Slices: map[string]*setup.Slice{ + "manifest": { + Package: "test-package", + Name: "manifest", + Contents: contents, + }, + }, + }, + }, + } +} + +func releaseManifestSlice(release *setup.Release) *setup.Slice { + return release.Packages["test-package"].Slices["manifest"] +} + +func manifestPathForDir(dir string) string { + base := strings.TrimSuffix(dir, "**") + return path.Join(base, manifestutil.DefaultFilename) +} + func treeDumpManifestPaths(mfest *manifest.Manifest) (map[string]string, error) { result := make(map[string]string) err := mfest.IteratePaths("", func(path *manifest.Path) error { @@ -2241,3 +2383,67 @@ func readManifest(c *C, targetDir, manifestPath string) *manifest.Manifest { return mfest } + +func writeManifest(c *C, targetDir, manifestPath string, slice *setup.Slice, hash string) { + mfestPath := filepath.Join(targetDir, manifestPath) + err := os.MkdirAll(filepath.Dir(mfestPath), 0o755) + c.Assert(err, IsNil) + f, err := os.Create(mfestPath) + c.Assert(err, IsNil) + zw, err := zstd.NewWriter(f) + c.Assert(err, IsNil) + options := &manifestutil.WriteOptions{ + PackageInfo: []*archive.PackageInfo{{ + Name: slice.Package, + Version: "1.0", + Arch: "amd64", + SHA256: "pkg-hash", + }}, + Selection: []*setup.Slice{slice}, + Report: &manifestutil.Report{Root: "/", Entries: map[string]manifestutil.ReportEntry{ + "/file": { + Path: "/file", + Mode: 0o644, + SHA256: hash, + Size: 3, + Slices: map[*setup.Slice]bool{slice: true}, + }, + }}, + } + err = manifestutil.Write(options, zw) + c.Assert(err, IsNil) + c.Assert(zw.Close(), IsNil) + c.Assert(f.Close(), IsNil) +} + +func writeInvalidManifest(c *C, targetDir, manifestPath string) { + mfestPath := filepath.Join(targetDir, manifestPath) + err := os.MkdirAll(filepath.Dir(mfestPath), 0o755) + c.Assert(err, IsNil) + f, err := os.Create(mfestPath) + c.Assert(err, IsNil) + zw, err := zstd.NewWriter(f) + c.Assert(err, IsNil) + dbw := jsonwall.NewDBWriter(&jsonwall.DBWriterOptions{Schema: manifest.Schema}) + err = dbw.Add(&manifest.Path{Kind: "path", Path: "/file", Mode: "0644"}) + c.Assert(err, IsNil) + _, err = dbw.WriteTo(zw) + c.Assert(err, IsNil) + c.Assert(zw.Close(), IsNil) + c.Assert(f.Close(), IsNil) +} + +func writeInvalidSchemaManifest(c *C, targetDir, manifestPath string) { + mfestPath := filepath.Join(targetDir, manifestPath) + err := os.MkdirAll(filepath.Dir(mfestPath), 0o755) + c.Assert(err, IsNil) + f, err := os.Create(mfestPath) + c.Assert(err, IsNil) + zw, err := zstd.NewWriter(f) + c.Assert(err, IsNil) + dbw := jsonwall.NewDBWriter(&jsonwall.DBWriterOptions{Schema: "9.9"}) + _, err = dbw.WriteTo(zw) + c.Assert(err, IsNil) + c.Assert(zw.Close(), IsNil) + c.Assert(f.Close(), IsNil) +} From 6ebb893978f269a4f8d906a7f242dc5deb2295df Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 4 Feb 2026 09:47:25 +0100 Subject: [PATCH 19/22] fix: add missing deps --- internal/slicer/slicer_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 3d921e9e..c8f7c885 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -19,6 +19,7 @@ import ( "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" "github.com/canonical/chisel/internal/testutil" + "github.com/canonical/chisel/public/jsonwall" "github.com/canonical/chisel/public/manifest" ) From ceac217b9da0559af0d3656bc45ca8982af49ceb Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 4 Feb 2026 11:37:59 +0100 Subject: [PATCH 20/22] tests: recut feature --- internal/slicer/slicer_test.go | 180 +++++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index c8f7c885..17240857 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -15,6 +15,7 @@ import ( . "gopkg.in/check.v1" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" @@ -34,6 +35,7 @@ type slicerTest struct { pkgs []*testutil.TestPackage slices []setup.SliceKey hackopt func(c *C, opts *slicer.RunOptions) + prefill func(c *C, opts *slicer.RunOptions, release *setup.Release, manifestPath string) filesystem map[string]string manifestPaths map[string]string manifestPkgs map[string]string @@ -1979,6 +1981,147 @@ var slicerTests = []slicerTest{{ manifestPaths: map[string]string{ "/dir/file": "file 0644 cc55e2ec {test-package_third}", }, +}, { + summary: "Recut removes obsolete paths when selection shrinks", + slices: []setup.SliceKey{{"test-package", "slice2"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + slice1: + contents: + /foo: {text: data1} + slice2: + contents: + /bar: {text: data2} + `, + }, + prefill: func(c *C, opts *slicer.RunOptions, release *setup.Release, manifestPath string) { + pkg := release.Packages["test-package"] + slice1 := pkg.Slices["slice1"] + slice2 := pkg.Slices["slice2"] + manifestSlice := pkg.Slices["manifest"] + writeFile(c, opts.TargetDir, "/foo", []byte("data1"), 0o644) + writeFile(c, opts.TargetDir, "/bar", []byte("data2"), 0o644) + report, err := manifestutil.NewReport(opts.TargetDir) + c.Assert(err, IsNil) + err = report.Add(slice1, &fsutil.Entry{ + Path: filepath.Join(report.Root, "/foo"), + Mode: 0o644, + SHA256: "5b41362b", + Size: 5, + }) + c.Assert(err, IsNil) + err = report.Add(slice2, &fsutil.Entry{ + Path: filepath.Join(report.Root, "/bar"), + Mode: 0o644, + SHA256: "d98cf53e", + Size: 5, + }) + c.Assert(err, IsNil) + err = report.Add(manifestSlice, &fsutil.Entry{ + Path: filepath.Join(report.Root, manifestPath), + Mode: 0o644, + }) + c.Assert(err, IsNil) + writeManifestReport(c, opts.TargetDir, manifestPath, pkg.Name, []*setup.Slice{slice1, slice2, manifestSlice}, report) + opts.Manifest = readManifest(c, opts.TargetDir, manifestPath) + }, + filesystem: map[string]string{ + "/bar": "file 0644 d98cf53e", + }, + manifestPaths: map[string]string{ + "/bar": "file 0644 d98cf53e {test-package_slice2}", + }, +}, { + summary: "Recut restores modified content and mode", + slices: []setup.SliceKey{{"test-package", "slice1"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + slice1: + contents: + /file: {text: data1} + `, + }, + prefill: func(c *C, opts *slicer.RunOptions, release *setup.Release, manifestPath string) { + pkg := release.Packages["test-package"] + slice1 := pkg.Slices["slice1"] + manifestSlice := pkg.Slices["manifest"] + writeFile(c, opts.TargetDir, "/file", []byte("data1"), 0o644) + report, err := manifestutil.NewReport(opts.TargetDir) + c.Assert(err, IsNil) + err = report.Add(slice1, &fsutil.Entry{ + Path: filepath.Join(report.Root, "/file"), + Mode: 0o644, + SHA256: "5b41362b", + Size: 5, + }) + c.Assert(err, IsNil) + err = report.Add(manifestSlice, &fsutil.Entry{ + Path: filepath.Join(report.Root, manifestPath), + Mode: 0o644, + }) + c.Assert(err, IsNil) + writeManifestReport(c, opts.TargetDir, manifestPath, pkg.Name, []*setup.Slice{slice1, manifestSlice}, report) + modifiedPath := filepath.Join(opts.TargetDir, "file") + err = os.WriteFile(modifiedPath, []byte("data2"), 0o700) + c.Assert(err, IsNil) + opts.Manifest = readManifest(c, opts.TargetDir, manifestPath) + }, + filesystem: map[string]string{ + "/file": "file 0644 5b41362b", + }, + manifestPaths: map[string]string{ + "/file": "file 0644 5b41362b {test-package_slice1}", + }, +}, { + summary: "Recut keeps untracked files", + slices: []setup.SliceKey{{"test-package", "slice1"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + slice1: + contents: + /file: {text: data1} + `, + }, + prefill: func(c *C, opts *slicer.RunOptions, release *setup.Release, manifestPath string) { + pkg := release.Packages["test-package"] + slice1 := pkg.Slices["slice1"] + manifestSlice := pkg.Slices["manifest"] + writeFile(c, opts.TargetDir, "/file", []byte("data1"), 0o644) + report, err := manifestutil.NewReport(opts.TargetDir) + c.Assert(err, IsNil) + err = report.Add(slice1, &fsutil.Entry{ + Path: filepath.Join(report.Root, "/file"), + Mode: 0o644, + SHA256: "5b41362b", + Size: 5, + }) + c.Assert(err, IsNil) + err = report.Add(manifestSlice, &fsutil.Entry{ + Path: filepath.Join(report.Root, manifestPath), + Mode: 0o644, + }) + c.Assert(err, IsNil) + writeManifestReport(c, opts.TargetDir, manifestPath, pkg.Name, []*setup.Slice{slice1, manifestSlice}, report) + err = os.MkdirAll(filepath.Join(opts.TargetDir, "extra"), 0o755) + c.Assert(err, IsNil) + err = os.WriteFile(filepath.Join(opts.TargetDir, "extra", "untracked"), []byte("data"), 0o644) + c.Assert(err, IsNil) + opts.Manifest = readManifest(c, opts.TargetDir, manifestPath) + }, + filesystem: map[string]string{ + "/extra/": "dir 0755", + "/extra/untracked": "file 0644 3a6eb079", + "/file": "file 0644 5b41362b", + }, + manifestPaths: map[string]string{ + "/file": "file 0644 5b41362b {test-package_slice1}", + }, }} func (s *S) TestRun(c *C) { @@ -2224,6 +2367,9 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { if test.hackopt != nil { test.hackopt(c, &options) } + if test.prefill != nil { + test.prefill(c, &options, release, manifestPath) + } err = slicer.Run(&options) if test.error != "" { c.Assert(err, ErrorMatches, test.error) @@ -2415,6 +2561,32 @@ func writeManifest(c *C, targetDir, manifestPath string, slice *setup.Slice, has c.Assert(err, IsNil) c.Assert(zw.Close(), IsNil) c.Assert(f.Close(), IsNil) + c.Assert(os.Chmod(mfestPath, 0o644), IsNil) +} + +func writeManifestReport(c *C, targetDir, manifestPath, pkgName string, selection []*setup.Slice, report *manifestutil.Report) { + mfestPath := filepath.Join(targetDir, manifestPath) + err := os.MkdirAll(filepath.Dir(mfestPath), 0o755) + c.Assert(err, IsNil) + f, err := os.OpenFile(mfestPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o644) + c.Assert(err, IsNil) + zw, err := zstd.NewWriter(f) + c.Assert(err, IsNil) + options := &manifestutil.WriteOptions{ + PackageInfo: []*archive.PackageInfo{{ + Name: pkgName, + Version: "1.0", + Arch: "amd64", + SHA256: "pkg-hash", + }}, + Selection: selection, + Report: report, + } + err = manifestutil.Write(options, zw) + c.Assert(err, IsNil) + c.Assert(zw.Close(), IsNil) + c.Assert(f.Close(), IsNil) + c.Assert(os.Chmod(mfestPath, 0o644), IsNil) } func writeInvalidManifest(c *C, targetDir, manifestPath string) { @@ -2448,3 +2620,11 @@ func writeInvalidSchemaManifest(c *C, targetDir, manifestPath string) { c.Assert(zw.Close(), IsNil) c.Assert(f.Close(), IsNil) } + +func writeFile(c *C, targetDir, relPath string, data []byte, mode fs.FileMode) { + path := filepath.Join(targetDir, relPath) + err := os.MkdirAll(filepath.Dir(path), 0o755) + c.Assert(err, IsNil) + err = os.WriteFile(path, data, mode) + c.Assert(err, IsNil) +} From 7e34e3201199517dab0b74fbc189bb97032e249e Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 4 Feb 2026 13:35:46 +0100 Subject: [PATCH 21/22] fix: avoid duplicates in FindPathsInRelease --- internal/manifestutil/manifestutil.go | 7 ++++--- internal/manifestutil/manifestutil_test.go | 4 +--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index b2946d57..4ace8880 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "io/fs" + "maps" "path/filepath" "slices" "sort" @@ -44,16 +45,16 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { // FindPathsInRelease finds all the paths marked with "generate:manifest" // for the given release. func FindPathsInRelease(r *setup.Release) []string { - manifestPaths := make([]string, 0) + manifestPaths := make(map[string]struct{}) collector := func(path string, slice *setup.Slice) { - manifestPaths = append(manifestPaths, path) + manifestPaths[path] = struct{}{} } for _, pkg := range r.Packages { for _, slice := range pkg.Slices { collectManifests(slice, collector) } } - return manifestPaths + return slices.Sorted(maps.Keys(manifestPaths)) } type WriteOptions struct { diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index c0f35886..016c1c5a 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -150,7 +150,6 @@ var findPathsInReleaseTests = []struct { }, }, }, - expected: []string{}, }, { summary: "Multiple packages with multiple slices", release: &setup.Release{ @@ -199,13 +198,12 @@ var findPathsInReleaseTests = []struct { }, }, // Note: /folder/manifest.wall appears twice because both slice1 and slice2 declare it - expected: []string{"/folder/manifest.wall", "/folder/manifest.wall", "/other-folder/manifest.wall"}, + expected: []string{"/folder/manifest.wall", "/other-folder/manifest.wall"}, }, { summary: "Empty release", release: &setup.Release{ Packages: map[string]*setup.Package{}, }, - expected: []string{}, }} func (s *S) TestFindPathsInRelease(c *C) { From e1d4e17b54a9bbf0d6cbcf23cc73b98f937abca7 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 4 Feb 2026 15:34:17 +0100 Subject: [PATCH 22/22] ci: rerun