Skip to content
Closed
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
60320ba
feat: search, load and validates manifests
upils Nov 26, 2025
c67c661
fix: revert unintentional change
upils Nov 26, 2025
22ae256
refactor: move things around
upils Nov 26, 2025
6cb437f
refactor: move logic to manifestutil
upils Nov 26, 2025
00825de
feat: validate rootfs with manifest
upils Nov 27, 2025
442bcc6
fix: debugging
upils Nov 27, 2025
e19f6f1
fix: improve hardlinks check
upils Nov 27, 2025
b6ecc27
feat: trying alternative approach
upils Nov 28, 2025
1406b18
feat: refine first approach
upils Dec 1, 2025
8aba035
refactor: dedicated verify.go
upils Dec 1, 2025
70b2c5e
refactor: rework verify* funcs
upils Dec 2, 2025
c8b7a39
fix: typos and inconsistencies
upils Dec 2, 2025
1181871
refactor: improve readability
upils Dec 2, 2025
31f1d3d
refactor: improve consistency
upils Dec 3, 2025
df49dfc
style: respect internal best practices
upils Dec 3, 2025
f4169b9
style: minor consistency fixes
upils Dec 4, 2025
aade454
fix: ignore size check on manifest
upils Dec 5, 2025
e0bd4a4
style: apply suggestions
upils Dec 8, 2025
2e572fb
style: apply suggestions
upils Dec 8, 2025
3c5de1c
style: apply suggestions
upils Dec 8, 2025
7eff973
style: apply suggestions
upils Dec 8, 2025
b7c7c66
style: apply suggestions
upils Dec 8, 2025
c7196e9
fix: apply suggestions
upils Dec 16, 2025
7539db7
refactor: simplify
upils Dec 19, 2025
cd22c1f
feat: intermediary representation
upils Jan 5, 2026
27d8645
refactor: simplify existing dir checking
upils Jan 5, 2026
373f3c1
refactor: regroup hardlink-related code
upils Jan 5, 2026
1414990
refactor: refine manifest checking API
upils Jan 5, 2026
2ce5b21
refactor: add obtainManifest
upils Jan 6, 2026
17162f4
refactor: simplify by removing SliceKeys function
upils Jan 6, 2026
c21ef34
style: remove redondant comment
upils Jan 13, 2026
77f5771
refactor: rework pathInfos comparison
upils Jan 14, 2026
40ef8e8
refactor: simplify FindPathsInRelease
upils Jan 14, 2026
0a25844
refactor: refine API
upils Jan 14, 2026
71c4496
refactor: simplify based on PR suggestions
upils Jan 15, 2026
4696dcc
style: tweak error messages
upils Jan 15, 2026
089109d
fix: fail if dir inconsistent with manifest
upils Jan 16, 2026
09faa6a
fix: use loaded manifest hash as reference
upils Jan 16, 2026
bd7a707
fix: raise error when path should not be hardlinked
upils Jan 19, 2026
3ea70ce
fix: collect and record reference manifest size
upils Jan 19, 2026
3efb7d2
feat: log root directory processing
upils Jan 19, 2026
a421818
refactor: more experiment around API design
upils Jan 21, 2026
e88de2d
refactor: experiment to improve API
upils Jan 21, 2026
acac421
refactor: more refining
upils Jan 22, 2026
0262cb2
refactor: CheckDir in slicer
upils Jan 22, 2026
7445fcd
refactor: add check in slicer
upils Jan 22, 2026
5fca0c5
refactor: simplify and reduce slicer API
upils Jan 22, 2026
fb6500b
ci: rerun
upils Jan 22, 2026
d7fa155
wip
upils Jan 23, 2026
43e8a71
refactor: cleaning
upils Jan 23, 2026
46d2119
fix: check manifest consistency at selection
upils Jan 23, 2026
164c68b
refactor: return a manifest from inspection
upils Jan 26, 2026
15550ad
refactor: apply PR suggestions
upils Jan 26, 2026
4ecb555
refactor: avoid iterating twice on slices
upils Jan 26, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions cmd/chisel/cmd_cut.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package main

import (
"fmt"
"os"
"path/filepath"
"slices"
"time"

"github.com/jessevdk/go-flags"

"github.com/canonical/chisel/internal/archive"
"github.com/canonical/chisel/internal/cache"
"github.com/canonical/chisel/internal/manifestutil"
"github.com/canonical/chisel/internal/setup"
"github.com/canonical/chisel/internal/slicer"
)
Expand Down Expand Up @@ -73,6 +76,44 @@ func (cmd *cmdCut) Execute(args []string) error {
}
}

// Get targetDir path
// Note: This is already done in `slicer.Run`. Extract it and avoid doing it twice?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this feature should be somewhere in the internal modules, not here necessarily. Maybe the functions can be internal and can be called and coordinated from here but the least amount of logic we have in cmd_cut.go the better.

Regarding the comment, I think it is good to extract it as long as inside cut we check that the path is absolute and return an error if it isn't. This is to maintain boundaries between components and not make them tightly coupled, but instead to document assumptions across boundaries as errors.

targetDir := filepath.Clean(cmd.RootDir)
if !filepath.IsAbs(targetDir) {
dir, err := os.Getwd()
if err != nil {
return fmt.Errorf("cannot obtain current directory: %w", err)
}
targetDir = filepath.Join(dir, targetDir)
}

targetDirEmpty, err := dirIsEmpty(targetDir)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to break compatibility will all previous versions of Chisel. I also think it is not ideal as many of our users expect to do:

<something>
./chisel ...
<something>

and the target directory might not be empty.

What about checking if the manifest exists directly? If it exists then we are recutting, if it doesn't then we cut normally overriding everything as usual.

if err != nil {
return err
}
Comment thread
upils marked this conversation as resolved.
if !targetDirEmpty {
manifest, err := manifestutil.FromDir(release, targetDir)
if err != nil {
// TODO: When enabling the feature, error out.
logf("Warning: %v", err)
Comment thread
upils marked this conversation as resolved.
Outdated
} else {
err = manifestutil.VerifyDir(manifest, targetDir)
if err != nil {
// TODO: When enabling the feature, error out.
logf("Warning: %v", err)
}
// Merge the slice keys used to build the existing rootfs with the ones
// explicitly requested slices.
// Previous slice keys contain both explicitly requested slices and slices
// resolved following essentials.
for _, s := range manifestutil.SliceKeys(manifest) {
Comment thread
upils marked this conversation as resolved.
Outdated
if !slices.Contains(sliceKeys, s) {
sliceKeys = append(sliceKeys, s)
}
}
}
}

selection, err := setup.Select(release, sliceKeys, cmd.Arch)
if err != nil {
return err
Expand Down Expand Up @@ -128,3 +169,12 @@ func (cmd *cmdCut) Execute(args []string) error {
})
return err
}

// dirIsEmpty checks whether the given directory is empty.
func dirIsEmpty(targetDir string) (bool, error) {
entries, err := os.ReadDir(targetDir)
if err != nil {
return false, fmt.Errorf("cannot read root directory %q: %v", targetDir, err)
}
return len(entries) == 0, nil
}
144 changes: 144 additions & 0 deletions internal/manifestutil/manifestutil.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package manifestutil

import (
"crypto/sha256"
"fmt"
"io"
"io/fs"
"os"
"path"
"path/filepath"
"slices"
"sort"
Expand All @@ -14,6 +17,7 @@ import (
"github.com/canonical/chisel/internal/setup"
"github.com/canonical/chisel/public/jsonwall"
"github.com/canonical/chisel/public/manifest"
"github.com/klauspost/compress/zstd"
)

const DefaultFilename = "manifest.wall"
Expand All @@ -34,6 +38,26 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice {
return manifestSlices
}

// FindPathsInRelease finds all the paths marked with "generate:manifest"
// for the given release.
func FindPathsInRelease(r *setup.Release) []string {
allSlices := []*setup.Slice{}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the Selection which should include all the slices we want to install?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate? Do you mean use Selection as the argument to FindPathsInRelease? Why would we want to do that? The intention was not to look into the subset of slices selected so I do not see a relation with the selection.

for _, pkg := range r.Packages {
for _, slice := range pkg.Slices {
allSlices = append(allSlices, slice)
}
}

manifestMap := FindPaths(allSlices)
Comment thread
upils marked this conversation as resolved.
Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit unfortunate that we have to iterate over all slices to collect them just to iterate over them again. Because the real business logic takes a path + info and returns the manifest path by validating then appending a filename, what about extracting only that bit into an auxiliary function instead? Then have both functions call the auxiliary one. Thoughts?

@upils upils Jan 26, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After implementing a naive version of FindPathsInRelease I thought the same. I did the extraction and ended up with something longer and harder to understand. So I opted for simplicity at this time but now I think I may have underestimated the problem when in the future the number of slices is more than 50x what we have today. I will rework that.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworked in 4ecb555


paths := make([]string, 0, len(manifestMap))
Comment thread
upils marked this conversation as resolved.
Outdated
for path := range manifestMap {
paths = append(paths, path)
}
Comment thread
upils marked this conversation as resolved.
Outdated

Comment thread
upils marked this conversation as resolved.
Outdated
return paths
}

type WriteOptions struct {
PackageInfo []*archive.PackageInfo
Selection []*setup.Slice
Expand Down Expand Up @@ -340,3 +364,123 @@ func Validate(mfest *manifest.Manifest) (err error) {
}
return nil
}

// FromDir extracts, validates and returns the manifest from a targetDir
Comment thread
upils marked this conversation as resolved.
Outdated
func FromDir(release *setup.Release, targetDir string) (*manifest.Manifest, error) {
manifestPaths := FindPathsInRelease(release)
if len(manifestPaths) == 0 {
// No manifest in the release means it cannot produce a rootfs that can
// be recut. Treat this case as cutting a new rootfs.
return nil, fmt.Errorf("no manifest generated for this release")
Comment thread
upils marked this conversation as resolved.
Outdated
}

// Select the first manifest of the list as the reference one for now.
manifestPath := manifestPaths[0]
manifest, err := load(path.Join(targetDir, manifestPath))
if err != nil {
return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err)
}

err = checkIdentical(targetDir, manifestPaths)
Comment thread
upils marked this conversation as resolved.
Outdated
if err != nil {
return nil, err
}

return manifest, nil
}

// load reads, validates and returns a manifest.
func load(manifestPath string) (*manifest.Manifest, error) {
Comment thread
upils marked this conversation as resolved.
Outdated
f, err := os.Open(manifestPath)
if err != nil {
return nil, err
}
defer f.Close()

r, err := zstd.NewReader(f)
if err != nil {
return nil, err
}
defer r.Close()

mfest, err := manifest.Read(r)
if err != nil {
return nil, err
}

err = Validate(mfest)
if err != nil {
return nil, err
}

return mfest, nil
}

// checkIdentical checks all manifests in the passed list are identical
// to the reference one.
func checkIdentical(targetDir string, manifestPaths []string) error {
if len(manifestPaths) == 0 {
return nil
}
refRelPath := manifestPaths[0]

ref := path.Join(targetDir, refRelPath)
Comment thread
upils marked this conversation as resolved.
Outdated
refHash, err := contentHash(ref)
if err != nil {
return fmt.Errorf("internal error: cannot compute hash for %q: %w", refRelPath, err)
Comment thread
upils marked this conversation as resolved.
Outdated
}

refInfo, err := os.Stat(ref)
if err != nil {
return fmt.Errorf("internal error: cannot get file info for %q: %w", refRelPath, err)
}
refMode := refInfo.Mode()
for _, manifestRelPath := range manifestPaths {
manifestPath := path.Join(targetDir, manifestRelPath)
manifestInfo, err := os.Stat(manifestPath)
if err != nil {
return fmt.Errorf("internal error: cannot get file info for %q: %w", manifestRelPath, err)
}
manifestMode := manifestInfo.Mode()
if manifestMode != refMode {
return fmt.Errorf("invalid manifest: permissions on %s (%s) different from %s (%s)", manifestRelPath, manifestMode, refRelPath, refMode)
}

manifestHash, err := contentHash(manifestPath)
if err != nil {
return fmt.Errorf("internal error: cannot compute hash for %q: %w", manifestRelPath, err)
}
if !slices.Equal(manifestHash, refHash) {
return fmt.Errorf("invalid manifest: %s is inconsistent with %s", manifestRelPath, refRelPath)
}
}
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
}

// SliceKeys returns setup.SliceKeys from a manifest.
func SliceKeys(mfest *manifest.Manifest) []setup.SliceKey {
Comment thread
upils marked this conversation as resolved.
Outdated
sliceKeys := []setup.SliceKey{}
mfest.IterateSlices("", func(slice *manifest.Slice) error {
sk, err := setup.ParseSliceKey(slice.Name)
if err != nil {
return err
}
sliceKeys = append(sliceKeys, sk)
return nil
})
return sliceKeys
}
Loading