Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions cmd/chisel/cmd_cut.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"errors"
"fmt"
"slices"
"time"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/canonical/chisel/internal/cache"
"github.com/canonical/chisel/internal/setup"
"github.com/canonical/chisel/internal/slicer"
"github.com/canonical/chisel/internal/store"
)

var shortCutHelp = "Cut a tree with selected slices"
Expand Down Expand Up @@ -121,9 +123,29 @@ func (cmd *cmdCut) Execute(args []string) error {
}
}

stores := make(map[string]store.Store)
for storeName, storeInfo := range release.Stores {
openStore, err := store.Open(&store.Options{
Arch: cmd.Arch,
CacheDir: cache.DefaultDir("chisel"),
Kind: storeInfo.Kind,
Version: storeInfo.Version,
})
if err != nil {
var unknownStoreKindError *store.UnknownStoreKindError
if errors.As(err, &unknownStoreKindError) {
logf("Store %q ignored: %v", storeName, err)
continue
}
return err
}
stores[storeName] = openStore
}

err = slicer.Run(&slicer.RunOptions{
Selection: selection,
Archives: archives,
Stores: stores,
TargetDir: cmd.RootDir,
})
return err
Expand Down
2 changes: 2 additions & 0 deletions cmd/chisel/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/canonical/chisel/internal/deb"
"github.com/canonical/chisel/internal/setup"
"github.com/canonical/chisel/internal/slicer"
"github.com/canonical/chisel/internal/store"
//"github.com/canonical/chisel/internal/logger"
)

Expand Down Expand Up @@ -327,6 +328,7 @@ func run() error {
deb.SetLogger(log.Default())
setup.SetLogger(log.Default())
slicer.SetLogger(log.Default())
store.SetLogger(log.Default())
SetLogger(log.Default())

parser := Parser()
Expand Down
72 changes: 38 additions & 34 deletions internal/slicer/slicer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ import (
"github.com/canonical/chisel/internal/manifestutil"
"github.com/canonical/chisel/internal/scripts"
"github.com/canonical/chisel/internal/setup"
"github.com/canonical/chisel/internal/store"
)

const manifestMode fs.FileMode = 0644

type RunOptions struct {
Selection *setup.Selection
Archives map[string]archive.Archive
Stores map[string]store.Store

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Per the comments in the other PR, this distinction makes sense and makes the code simpler in a way. Normally in Go you don't define generics to abstract over a type (i.e. a store) but to abstract over an operation, in this case there are clearly two:

  • How to fetch a package.
  • Record information from the package in manifest.

As we discussed in the previous PR, this was contemplated but it turned out to be much more complex AFAIK. I also see the difficulty in Go where the type system is not powerful so it is hard to model fetching where archive.fetch(name) takes only one args vs three in store.fetch(name, track, risk) for example.

@upils upils Jun 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.

We agree. I could have reworked the Archive interface to have something more generic, used for archives and stores, but in reality they are different enough that 2 different interfaces made more sense.

As I go through the complete implementation I may discover that this new interface must be tweaked.

TargetDir string
}

Expand All @@ -50,8 +52,8 @@ type pkgSourceInfo struct {
arch string
kind sourceKind
archive archive.Archive
// TODO: add store handle when store support is implemented.
pkg *setup.Package
store store.Store
pkg *setup.Package
}

type contentChecker struct {
Expand Down Expand Up @@ -107,7 +109,7 @@ func Run(options *RunOptions) error {
targetDir = filepath.Join(dir, targetDir)
}

pkgSources, err := resolvePkgSources(options.Archives, options.Selection)
pkgSources, err := resolvePkgSources(options.Archives, options.Stores, options.Selection)
if err != nil {
return err
}
Expand Down Expand Up @@ -170,19 +172,27 @@ func Run(options *RunOptions) error {
continue
}
src := pkgSources[slice.Package]
// Store packages are distributed as "ar" archives, whose extraction is
// not yet implemented. Fail until store handling and the "ar" format
// support are in place.
var reader io.ReadSeekCloser
if src.kind == sourceStore {
return fmt.Errorf("cannot fetch package %q from store: store packages are not yet supported", src.pkg.Name)
}
reader, info, err := src.archive.Fetch(src.pkg.RealName)
if err != nil {
return err
// The store channel track is "<default-track>-<store version>",
// e.g. "3.1-26.10". The version pins the release series.
// Risk is left unspecified for now; the store applies its default.
// In the future the risk will optionnaly come from the CLI.
track := src.pkg.DefaultTrack + "-" + src.store.Options().Version
reader, _, err = src.store.Fetch(src.pkg.RealName, track, "")
if err != nil {
return err
}
} else {
var info *archive.PackageInfo
reader, info, err = src.archive.Fetch(src.pkg.RealName)
if err != nil {
return err
}
pkgInfos = append(pkgInfos, info)
}
defer reader.Close()
packages[slice.Package] = reader
pkgInfos = append(pkgInfos, info)
}

// When creating content, record if a path is known and whether they are
Expand Down Expand Up @@ -262,6 +272,12 @@ func Run(options *RunOptions) error {
if reader == nil {
continue
}
src := pkgSources[slice.Package]
// Store packages are distributed as plain tarballs, whose extraction
// is not yet implemented. Fail until the format support is in place.
if src.kind != sourceArchive {
return fmt.Errorf("cannot extract package %q from store: store packages are not yet supported", src.pkg.RealName)
}
err := deb.Extract(reader, &deb.ExtractOptions{
Package: slice.Package,
Extract: extract[slice.Package],
Expand Down Expand Up @@ -516,9 +532,9 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent
// resolvePkgSources determines the source for each package in the selection.
// For archive packages it selects the highest priority archive containing the
// package unless a particular archive is pinned within the slice definition
// file. For store packages it records the store reference. It returns a map
// file. For store packages it records the opened store handle. It returns a map
// of pkgSourceInfo indexed by package names.
func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Selection) (map[string]*pkgSourceInfo, error) {
func resolvePkgSources(archives map[string]archive.Archive, stores map[string]store.Store, selection *setup.Selection) (map[string]*pkgSourceInfo, error) {
sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives))
for _, archive := range selection.Release.Archives {
if archive.Priority < 0 {
Expand All @@ -539,10 +555,15 @@ func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Sel
}
pkg := selection.Release.Packages[s.Package]
if pkg.Store != "" {
storeHandle := stores[pkg.Store]
if storeHandle == nil {
return nil, fmt.Errorf("internal error: no store handle for store %q", pkg.Store)
}
pkgSources[pkg.Name] = &pkgSourceInfo{
// TODO: Fill with the live store handle when store support is implemented.
kind: sourceStore,
pkg: pkg,
arch: storeHandle.Options().Arch,

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 was hesitant in #15 because with only that diff it looked weird. However, it looks okay here. You can postpone the refactor for arch.

I am not sure the other PR will not get comments about this so it might be helpful to copy the idea of this diff in a comment in canonical/chisel just in case.

@upils upils Jun 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.

Good point. I added https://github.com/upils/chisel/pull/15/files#r3479609705 (and did the same on the PR submitted to the main repo)

kind: sourceStore,
store: storeHandle,
pkg: pkg,
}
Comment thread
upils marked this conversation as resolved.
continue
}
Expand Down Expand Up @@ -575,22 +596,5 @@ func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Sel
}
}

// Until a store is implemented as a package source there is no proper way to
// determine the architecture for store packages.
// So relying on the fact that all packages in a selection share the same architecture,
// we can borrow it from any archive package that was already resolved.
var arch string
for _, src := range pkgSources {
if src.kind == sourceArchive {
arch = src.arch
break
}
}
for _, src := range pkgSources {
if src.kind == sourceStore {
src.arch = arch
}
}

return pkgSources, nil
}
33 changes: 31 additions & 2 deletions internal/slicer/slicer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/canonical/chisel/internal/manifestutil"
"github.com/canonical/chisel/internal/setup"
"github.com/canonical/chisel/internal/slicer"
"github.com/canonical/chisel/internal/store"
"github.com/canonical/chisel/internal/testutil"
"github.com/canonical/chisel/public/manifest"
)
Expand Down Expand Up @@ -1982,6 +1983,14 @@ var slicerTests = []slicerTest{{
summary: "Store package fails as it is not yet supported",
slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}},
arch: "amd64",
pkgs: []*testutil.TestPackage{{
Name: "test-package",
Data: testutil.PackageData["test-package"],
}, {
Name: "store-pkg",
Store: "bin",
Data: testutil.PackageData["test-package"],
}},
release: map[string]string{
"chisel.yaml": testutil.DefaultChiselYamlWithStores,
"slices/mydir/test-package.yaml": `
Expand All @@ -1994,14 +2003,14 @@ var slicerTests = []slicerTest{{
"slices/mydir/store-pkg.yaml": `
package: store-pkg
store: bin
default-track: stable
default-track: 3.1
slices:
myslice:
contents:
/dir/store-file:
`,
},
error: `cannot fetch package "bin-store-pkg" from store: store packages are not yet supported`,
error: `cannot extract package "store-pkg" from store: store packages are not yet supported`,
}}

func (s *S) TestRun(c *C) {
Expand Down Expand Up @@ -2109,6 +2118,9 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) {
for name, setupArchive := range release.Archives {
pkgs := make(map[string]*testutil.TestPackage)
for _, pkg := range test.pkgs {
if pkg.Store != "" {
continue
}
if len(pkg.Archives) == 0 || slices.Contains(pkg.Archives, name) {
pkgs[pkg.Name] = pkg
}
Expand All @@ -2127,9 +2139,26 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) {
archives[name] = archive
}

stores := map[string]store.Store{}
for name, relStore := range release.Stores {
pkgs := make(map[string]*testutil.TestPackage)
for _, pkg := range test.pkgs {
if pkg.Store == name {
pkgs[pkg.Name] = pkg
}
}
stores[name] = &testutil.TestStore{
Packages: pkgs,
Opts: store.Options{
Version: relStore.Version,
},
}
}

options := slicer.RunOptions{
Selection: selection,
Archives: archives,
Stores: stores,
TargetDir: c.MkDir(),
}
if test.hackopt != nil {
Expand Down
19 changes: 19 additions & 0 deletions internal/store/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package store

import "net/http"

var (
ValidateDownloadURL = validateDownloadURL
BinStagingEnvVar = binStagingEnvVar
)

func FakeDo(do func(req *http.Request) (*http.Response, error)) (restore func()) {
_httpDo := httpDo
_bulkDo := bulkDo
httpDo = do
bulkDo = do
return func() {
httpDo = _httpDo
bulkDo = _bulkDo
}
}
53 changes: 53 additions & 0 deletions internal/store/log.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package store

import (
"fmt"
"sync"
)

// Avoid importing the log type information unnecessarily. There's a small cost
// associated with using an interface rather than the type. Depending on how
// often the logger is plugged in, it would be worth using the type instead.
type log_Logger interface {
Output(calldepth int, s string) error
}

var globalLoggerLock sync.Mutex
var globalLogger log_Logger
var globalDebug bool

// Specify the *log.Logger object where log messages should be sent to.
func SetLogger(logger log_Logger) {
globalLoggerLock.Lock()
globalLogger = logger
globalLoggerLock.Unlock()
}

// Enable the delivery of debug messages to the logger. Only meaningful
// if a logger is also set.
func SetDebug(debug bool) {
globalLoggerLock.Lock()
globalDebug = debug
globalLoggerLock.Unlock()
}

// logf sends to the logger registered via SetLogger the string resulting
// from running format and args through Sprintf.
func logf(format string, args ...any) {
globalLoggerLock.Lock()
defer globalLoggerLock.Unlock()
if globalLogger != nil {
globalLogger.Output(2, fmt.Sprintf(format, args...))
}
}

// debugf sends to the logger registered via SetLogger the string resulting
// from running format and args through Sprintf, but only if debugging was
// enabled via SetDebug.
func debugf(format string, args ...any) {
globalLoggerLock.Lock()
defer globalLoggerLock.Unlock()
if globalDebug && globalLogger != nil {
globalLogger.Output(2, fmt.Sprintf(format, args...))
}
}
Loading
Loading