From f91d4498df94314ae797908bee77b0cc8e437e8e Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 30 Jun 2026 09:15:13 +0200 Subject: [PATCH 01/21] feat: extract bins --- internal/archive/archive_test.go | 2 ++ internal/slicer/slicer.go | 2 ++ internal/tarball/extract.go | 13 ++++++++--- internal/tarball/extract_test.go | 17 +++++++++++++++ internal/tarball/xz.go | 15 +++++++++++++ internal/testutil/pkgdata.go | 37 ++++++++++++++++++++++++++++++++ 6 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 internal/tarball/xz.go diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index 90e3f0047..fcabc2a25 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -17,6 +17,7 @@ import ( "strings" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/archive/testarchive" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" @@ -986,6 +987,7 @@ func (s *S) testOpenArchiveArch(c *C, test realArchiveTest, arch string) { err = tarball.Extract(pkg, &tarball.ExtractOptions{ Package: test.pkg, TargetDir: extractDir, + OpenData: deb.DataReader, Extract: map[string][]tarball.ExtractInfo{ fmt.Sprintf("/usr/share/doc/%s/copyright", test.pkg): { {Path: "/copyright"}, diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 6f4783b88..00ed3ee37 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,6 +16,7 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" @@ -243,6 +244,7 @@ func Run(options *RunOptions) error { Package: slice.Package, Extract: extract[slice.Package], TargetDir: targetDir, + OpenData: deb.DataReader, Create: create, }) reader.Close() diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 40db2cbac..ce1fda5b2 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -12,7 +12,6 @@ import ( "strings" "syscall" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/strdist" ) @@ -21,6 +20,11 @@ type ExtractOptions struct { Package string TargetDir string Extract map[string][]ExtractInfo + // OpenData opens the uncompressed tar data stream of the package. It + // abstracts over the package format (e.g. a deb archive opener or a plain + // tarball opener), allowing Extract to operate on any package whose data + // payload is a tar stream. + OpenData func(io.ReadSeeker) (io.ReadCloser, error) // Create can optionally be set to control the creation of extracted entries. // extractInfos is set to the matching entries in Extract, and is nil in cases where // the created entry is implicit and unlisted (for example, parent directories). @@ -35,6 +39,9 @@ type ExtractInfo struct { } func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { + if options.OpenData == nil { + return nil, fmt.Errorf("internal error: ExtractOptions.OpenData is unset") + } for extractPath, extractInfos := range options.Extract { isGlob := strings.ContainsAny(extractPath, "*?") if isGlob { @@ -83,7 +90,7 @@ func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { } func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { - dataReader, err := deb.DataReader(pkgReader) + dataReader, err := options.OpenData(pkgReader) if err != nil { return err } @@ -300,7 +307,7 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { - dataReader, err := deb.DataReader(pkgReader) + dataReader, err := opts.OpenData(pkgReader) if err != nil { return err } diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index fd5eb6147..f855d9bc4 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -10,6 +10,7 @@ import ( . "gopkg.in/check.v1" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" @@ -496,6 +497,8 @@ func (s *S) TestExtract(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir + // The test fixtures are .deb archives, so use the deb data opener. + options.OpenData = deb.DataReader createdPaths := make(map[string]bool) options.Create = func(_ []tarball.ExtractInfo, o *fsutil.CreateOptions) error { relPath := filepath.Clean("/" + strings.TrimPrefix(o.Path, dir)) @@ -605,6 +608,8 @@ func (s *S) TestExtractCreateCallback(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir + // The test fixtures are .deb archives, so use the deb data opener. + options.OpenData = deb.DataReader createExtractInfos := map[string][]tarball.ExtractInfo{} options.Create = func(extractInfos []tarball.ExtractInfo, o *fsutil.CreateOptions) error { if extractInfos == nil { @@ -628,3 +633,15 @@ func (s *S) TestExtractCreateCallback(c *C) { c.Assert(createExtractInfos, DeepEquals, test.calls) } } + +func (s *S) TestExtractMissingOpenData(c *C) { + options := tarball.ExtractOptions{ + Package: "test-package", + TargetDir: c.MkDir(), + Extract: map[string][]tarball.ExtractInfo{ + "/dir/file": {{Path: "/dir/file"}}, + }, + } + err := tarball.Extract(bytes.NewReader(testutil.PackageData["test-package"]), &options) + c.Assert(err, ErrorMatches, `cannot extract from package "test-package": internal error: ExtractOptions.OpenData is unset`) +} diff --git a/internal/tarball/xz.go b/internal/tarball/xz.go new file mode 100644 index 000000000..700215a09 --- /dev/null +++ b/internal/tarball/xz.go @@ -0,0 +1,15 @@ +package tarball + +import ( + "io" + + "github.com/ulikunitz/xz" +) + +func XZDataReader(pkgReader io.ReadSeeker) (io.ReadCloser, error) { + xzReader, err := xz.NewReader(pkgReader) + if err != nil { + return nil, err + } + return io.NopCloser(xzReader), nil +} diff --git a/internal/testutil/pkgdata.go b/internal/testutil/pkgdata.go index f901873d3..8fb8fb8b2 100644 --- a/internal/testutil/pkgdata.go +++ b/internal/testutil/pkgdata.go @@ -8,6 +8,7 @@ import ( "github.com/blakesmith/ar" "github.com/klauspost/compress/zstd" + "github.com/ulikunitz/xz" ) var PackageData = map[string][]byte{} @@ -160,6 +161,42 @@ func MustMakeDeb(entries []TarEntry) []byte { return data } +// compressBytesXz compresses the input using XZ, the compression format used by +// store (bin) packages. +func compressBytesXz(input []byte) ([]byte, error) { + var buf bytes.Buffer + writer, err := xz.NewWriter(&buf) + if err != nil { + return nil, err + } + if _, err = writer.Write(input); err != nil { + return nil, err + } + if err = writer.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// MakeBin builds a store (bin) package from the given tar entries: an +// XZ-compressed plain tarball. Unlike MakeDeb, there is no ar container. +func MakeBin(entries []TarEntry) ([]byte, error) { + tarData, err := makeTar(entries) + if err != nil { + return nil, err + } + return compressBytesXz(tarData) +} + +// MustMakeBin is the panicking variant of MakeBin. +func MustMakeBin(entries []TarEntry) []byte { + data, err := MakeBin(entries) + if err != nil { + panic(err) + } + return data +} + // Reg is a shortcut for creating a regular file TarEntry structure (with // tar.Typeflag set tar.TypeReg). Reg stands for "REGular file". func Reg(mode int64, path, content string) TarEntry { From 166344644b84dbb1058028913eac09dbda4c45a4 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 29 Jul 2026 11:54:44 +0200 Subject: [PATCH 02/21] refactor: rework approach --- .../cmd_debug_check_release_archives.go | 2 +- internal/archive/archive_test.go | 5 +- internal/deb/extract.go | 7 ++- internal/slicer/slicer.go | 3 +- internal/tarball/extract.go | 58 ++++++++++++++----- internal/tarball/extract_test.go | 16 +++-- internal/tarball/xz.go | 15 ----- 7 files changed, 57 insertions(+), 49 deletions(-) delete mode 100644 internal/tarball/xz.go diff --git a/cmd/chisel/cmd_debug_check_release_archives.go b/cmd/chisel/cmd_debug_check_release_archives.go index c9c6270e4..29a8189a8 100644 --- a/cmd/chisel/cmd_debug_check_release_archives.go +++ b/cmd/chisel/cmd_debug_check_release_archives.go @@ -150,7 +150,7 @@ func computePathObservations(release *setup.Release, archives map[string]archive if err != nil { return nil, err } - dataReader, err := deb.DataReader(pkgReader) + dataReader, err := deb.OpenTar(pkgReader) if err != nil { return nil, err } diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index fcabc2a25..efde6658c 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -17,8 +17,8 @@ import ( "strings" "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/archive/testarchive" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" ) @@ -984,10 +984,9 @@ func (s *S) testOpenArchiveArch(c *C, test realArchiveTest, arch string) { c.Assert(info.Name, DeepEquals, test.pkg) c.Assert(info.Arch, DeepEquals, arch) - err = tarball.Extract(pkg, &tarball.ExtractOptions{ + err = tarball.Extract(pkg, deb.OpenTar, &tarball.ExtractOptions{ Package: test.pkg, TargetDir: extractDir, - OpenData: deb.DataReader, Extract: map[string][]tarball.ExtractInfo{ fmt.Sprintf("/usr/share/doc/%s/copyright", test.pkg): { {Path: "/copyright"}, diff --git a/internal/deb/extract.go b/internal/deb/extract.go index bf543c2d8..49c0ba13b 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -10,9 +10,10 @@ import ( "github.com/ulikunitz/xz" ) -// DataReader takes a Reader for the ar file belonging to a Debian package and -// returns a Reader to the inner tarball. -func DataReader(pkgReader io.ReadSeeker) (io.ReadCloser, error) { +// OpenTar takes a Reader for the ar file belonging to a Debian package and +// returns a Reader to the uncompressed inner tarball. It implements +// tarball.OpenTarFunc. +func OpenTar(pkgReader io.ReadSeeker) (io.ReadCloser, error) { arReader := ar.NewReader(pkgReader) var dataReader io.ReadCloser for dataReader == nil { diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 00ed3ee37..8fcc1c0e2 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -240,11 +240,10 @@ func Run(options *RunOptions) error { if reader == nil { continue } - err := tarball.Extract(reader, &tarball.ExtractOptions{ + err := tarball.Extract(reader, deb.OpenTar, &tarball.ExtractOptions{ Package: slice.Package, Extract: extract[slice.Package], TargetDir: targetDir, - OpenData: deb.DataReader, Create: create, }) reader.Close() diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index ce1fda5b2..bc78534ac 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -12,19 +12,41 @@ import ( "strings" "syscall" + "github.com/ulikunitz/xz" + "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/strdist" ) +// OpenTarFunc opens the uncompressed tar stream carried by a package, hiding +// the package format from Extract. An implementation may unwrap a container +// before decompressing (see deb.OpenTar) or decompress the package itself (see +// OpenXZ). +// +// Implementations must satisfy the following contract: +// +// - They must read pkgReader from its current offset, which is always the +// start of the package. +// - They must be stateless. Extract may call the same function more than once +// for the same package, rewinding pkgReader to the start beforehand, and +// each call must yield the complete tar stream again. +// - The caller closes the returned reader. +type OpenTarFunc func(pkgReader io.ReadSeeker) (io.ReadCloser, error) + +// OpenXZ opens a package which is a plain XZ-compressed tarball, such as a +// store (bin) package. It implements OpenTarFunc. +func OpenXZ(pkgReader io.ReadSeeker) (io.ReadCloser, error) { + xzReader, err := xz.NewReader(pkgReader) + if err != nil { + return nil, err + } + return io.NopCloser(xzReader), nil +} + type ExtractOptions struct { Package string TargetDir string Extract map[string][]ExtractInfo - // OpenData opens the uncompressed tar data stream of the package. It - // abstracts over the package format (e.g. a deb archive opener or a plain - // tarball opener), allowing Extract to operate on any package whose data - // payload is a tar stream. - OpenData func(io.ReadSeeker) (io.ReadCloser, error) // Create can optionally be set to control the creation of extracted entries. // extractInfos is set to the matching entries in Extract, and is nil in cases where // the created entry is implicit and unlisted (for example, parent directories). @@ -39,9 +61,6 @@ type ExtractInfo struct { } func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { - if options.OpenData == nil { - return nil, fmt.Errorf("internal error: ExtractOptions.OpenData is unset") - } for extractPath, extractInfos := range options.Extract { isGlob := strings.ContainsAny(extractPath, "*?") if isGlob { @@ -65,7 +84,10 @@ func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { return options, nil } -func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { +// Extract extracts from pkgReader the entries listed in options.Extract. +// openTar opens the tar stream carried by the package and must not be nil; see +// OpenTarFunc for the contract it must satisfy. +func Extract(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractOptions) (err error) { defer func() { if err != nil { err = fmt.Errorf("cannot extract from package %q: %w", options.Package, err) @@ -74,6 +96,10 @@ func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { logf("Extracting files from package %q...", options.Package) + if openTar == nil { + return fmt.Errorf("internal error: no tar opener provided") + } + validOpts, err := getValidOptions(options) if err != nil { return err @@ -86,11 +112,11 @@ func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { return err } - return extractData(pkgReader, validOpts) + return extractData(pkgReader, openTar, validOpts) } -func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { - dataReader, err := options.OpenData(pkgReader) +func extractData(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractOptions) error { + dataReader, err := openTar(pkgReader) if err != nil { return err } @@ -172,7 +198,7 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { } var contentCache []byte - var contentIsCached = len(targetPaths) > 1 && !sourceIsDir + contentIsCached := len(targetPaths) > 1 && !sourceIsDir if contentIsCached { // Read and cache the content so it may be reused. // As an alternative, to avoid having an entire file in @@ -272,7 +298,7 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { if err != nil { return err } - err = extractHardLinks(pkgReader, extractHardLinkOptions) + err = extractHardLinks(pkgReader, openTar, extractHardLinkOptions) if err != nil { return err } @@ -306,8 +332,8 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. -func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { - dataReader, err := opts.OpenData(pkgReader) +func extractHardLinks(pkgReader io.ReadSeeker, openTar OpenTarFunc, opts *extractHardLinkOptions) error { + dataReader, err := openTar(pkgReader) if err != nil { return err } diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index f855d9bc4..a2246bbec 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -497,8 +497,6 @@ func (s *S) TestExtract(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir - // The test fixtures are .deb archives, so use the deb data opener. - options.OpenData = deb.DataReader createdPaths := make(map[string]bool) options.Create = func(_ []tarball.ExtractInfo, o *fsutil.CreateOptions) error { relPath := filepath.Clean("/" + strings.TrimPrefix(o.Path, dir)) @@ -514,7 +512,8 @@ func (s *S) TestExtract(c *C) { test.hackopt(c, &options) } - err := tarball.Extract(bytes.NewReader(test.pkgdata), &options) + // The test fixtures are .deb archives, so use the deb tar opener. + err := tarball.Extract(bytes.NewReader(test.pkgdata), deb.OpenTar, &options) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue @@ -608,8 +607,6 @@ func (s *S) TestExtractCreateCallback(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir - // The test fixtures are .deb archives, so use the deb data opener. - options.OpenData = deb.DataReader createExtractInfos := map[string][]tarball.ExtractInfo{} options.Create = func(extractInfos []tarball.ExtractInfo, o *fsutil.CreateOptions) error { if extractInfos == nil { @@ -627,14 +624,15 @@ func (s *S) TestExtractCreateCallback(c *C) { return nil } - err := tarball.Extract(bytes.NewReader(test.pkgdata), &options) + // The test fixtures are .deb archives, so use the deb tar opener. + err := tarball.Extract(bytes.NewReader(test.pkgdata), deb.OpenTar, &options) c.Assert(err, IsNil) c.Assert(createExtractInfos, DeepEquals, test.calls) } } -func (s *S) TestExtractMissingOpenData(c *C) { +func (s *S) TestExtractMissingOpenTar(c *C) { options := tarball.ExtractOptions{ Package: "test-package", TargetDir: c.MkDir(), @@ -642,6 +640,6 @@ func (s *S) TestExtractMissingOpenData(c *C) { "/dir/file": {{Path: "/dir/file"}}, }, } - err := tarball.Extract(bytes.NewReader(testutil.PackageData["test-package"]), &options) - c.Assert(err, ErrorMatches, `cannot extract from package "test-package": internal error: ExtractOptions.OpenData is unset`) + err := tarball.Extract(bytes.NewReader(testutil.PackageData["test-package"]), nil, &options) + c.Assert(err, ErrorMatches, `cannot extract from package "test-package": internal error: no tar opener provided`) } diff --git a/internal/tarball/xz.go b/internal/tarball/xz.go deleted file mode 100644 index 700215a09..000000000 --- a/internal/tarball/xz.go +++ /dev/null @@ -1,15 +0,0 @@ -package tarball - -import ( - "io" - - "github.com/ulikunitz/xz" -) - -func XZDataReader(pkgReader io.ReadSeeker) (io.ReadCloser, error) { - xzReader, err := xz.NewReader(pkgReader) - if err != nil { - return nil, err - } - return io.NopCloser(xzReader), nil -} From 3435cee29c65c8eda6477bc253dd4f2a1845d625 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 29 Jul 2026 13:23:15 +0200 Subject: [PATCH 03/21] refactor: refining --- internal/deb/extract.go | 5 ++--- internal/tarball/extract.go | 20 ++++++------------- internal/testutil/pkgdata.go | 37 ------------------------------------ 3 files changed, 8 insertions(+), 54 deletions(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 49c0ba13b..d25f1f883 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -11,9 +11,8 @@ import ( ) // OpenTar takes a Reader for the ar file belonging to a Debian package and -// returns a Reader to the uncompressed inner tarball. It implements -// tarball.OpenTarFunc. -func OpenTar(pkgReader io.ReadSeeker) (io.ReadCloser, error) { +// returns a Reader to the uncompressed inner tarball. +func OpenTar(pkgReader io.Reader) (io.ReadCloser, error) { arReader := ar.NewReader(pkgReader) var dataReader io.ReadCloser for dataReader == nil { diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index bc78534ac..26c8ec6a9 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -23,19 +23,14 @@ import ( // before decompressing (see deb.OpenTar) or decompress the package itself (see // OpenXZ). // -// Implementations must satisfy the following contract: -// -// - They must read pkgReader from its current offset, which is always the -// start of the package. -// - They must be stateless. Extract may call the same function more than once -// for the same package, rewinding pkgReader to the start beforehand, and -// each call must yield the complete tar stream again. -// - The caller closes the returned reader. -type OpenTarFunc func(pkgReader io.ReadSeeker) (io.ReadCloser, error) +// Extract reads pkgReader from the start of the package, closes the returned +// reader, and may open the same package more than once, rewinding pkgReader +// beforehand. +type OpenTarFunc func(pkgReader io.Reader) (io.ReadCloser, error) // OpenXZ opens a package which is a plain XZ-compressed tarball, such as a -// store (bin) package. It implements OpenTarFunc. -func OpenXZ(pkgReader io.ReadSeeker) (io.ReadCloser, error) { +// store (bin) package. +func OpenXZ(pkgReader io.Reader) (io.ReadCloser, error) { xzReader, err := xz.NewReader(pkgReader) if err != nil { return nil, err @@ -84,9 +79,6 @@ func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { return options, nil } -// Extract extracts from pkgReader the entries listed in options.Extract. -// openTar opens the tar stream carried by the package and must not be nil; see -// OpenTarFunc for the contract it must satisfy. func Extract(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractOptions) (err error) { defer func() { if err != nil { diff --git a/internal/testutil/pkgdata.go b/internal/testutil/pkgdata.go index 8fb8fb8b2..f901873d3 100644 --- a/internal/testutil/pkgdata.go +++ b/internal/testutil/pkgdata.go @@ -8,7 +8,6 @@ import ( "github.com/blakesmith/ar" "github.com/klauspost/compress/zstd" - "github.com/ulikunitz/xz" ) var PackageData = map[string][]byte{} @@ -161,42 +160,6 @@ func MustMakeDeb(entries []TarEntry) []byte { return data } -// compressBytesXz compresses the input using XZ, the compression format used by -// store (bin) packages. -func compressBytesXz(input []byte) ([]byte, error) { - var buf bytes.Buffer - writer, err := xz.NewWriter(&buf) - if err != nil { - return nil, err - } - if _, err = writer.Write(input); err != nil { - return nil, err - } - if err = writer.Close(); err != nil { - return nil, err - } - return buf.Bytes(), nil -} - -// MakeBin builds a store (bin) package from the given tar entries: an -// XZ-compressed plain tarball. Unlike MakeDeb, there is no ar container. -func MakeBin(entries []TarEntry) ([]byte, error) { - tarData, err := makeTar(entries) - if err != nil { - return nil, err - } - return compressBytesXz(tarData) -} - -// MustMakeBin is the panicking variant of MakeBin. -func MustMakeBin(entries []TarEntry) []byte { - data, err := MakeBin(entries) - if err != nil { - panic(err) - } - return data -} - // Reg is a shortcut for creating a regular file TarEntry structure (with // tar.Typeflag set tar.TypeReg). Reg stands for "REGular file". func Reg(mode int64, path, content string) TarEntry { From 09a01bd80b8ef36a30af041c46a2a62dc64c212e Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 29 Jul 2026 13:30:16 +0200 Subject: [PATCH 04/21] refactor: refining --- internal/tarball/extract.go | 6 +----- internal/tarball/extract_test.go | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 26c8ec6a9..28d05be9e 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -22,10 +22,6 @@ import ( // the package format from Extract. An implementation may unwrap a container // before decompressing (see deb.OpenTar) or decompress the package itself (see // OpenXZ). -// -// Extract reads pkgReader from the start of the package, closes the returned -// reader, and may open the same package more than once, rewinding pkgReader -// beforehand. type OpenTarFunc func(pkgReader io.Reader) (io.ReadCloser, error) // OpenXZ opens a package which is a plain XZ-compressed tarball, such as a @@ -190,7 +186,7 @@ func extractData(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractO } var contentCache []byte - contentIsCached := len(targetPaths) > 1 && !sourceIsDir + var contentIsCached = len(targetPaths) > 1 && !sourceIsDir if contentIsCached { // Read and cache the content so it may be reused. // As an alternative, to avoid having an entire file in diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index a2246bbec..9c5fe3523 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -19,6 +19,8 @@ import ( type extractTest struct { summary string pkgdata []byte + // openTar must match the format of pkgdata. It defaults to deb.OpenTar. + openTar tarball.OpenTarFunc options tarball.ExtractOptions hackopt func(c *C, o *tarball.ExtractOptions) result map[string]string @@ -512,8 +514,11 @@ func (s *S) TestExtract(c *C) { test.hackopt(c, &options) } - // The test fixtures are .deb archives, so use the deb tar opener. - err := tarball.Extract(bytes.NewReader(test.pkgdata), deb.OpenTar, &options) + openTar := test.openTar + if openTar == nil { + openTar = deb.OpenTar + } + err := tarball.Extract(bytes.NewReader(test.pkgdata), openTar, &options) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue @@ -541,6 +546,8 @@ func (s *S) TestExtract(c *C) { var extractCreateCallbackTests = []struct { summary string pkgdata []byte + // openTar must match the format of pkgdata. It defaults to deb.OpenTar. + openTar tarball.OpenTarFunc options tarball.ExtractOptions calls map[string][]tarball.ExtractInfo }{{ @@ -624,8 +631,11 @@ func (s *S) TestExtractCreateCallback(c *C) { return nil } - // The test fixtures are .deb archives, so use the deb tar opener. - err := tarball.Extract(bytes.NewReader(test.pkgdata), deb.OpenTar, &options) + openTar := test.openTar + if openTar == nil { + openTar = deb.OpenTar + } + err := tarball.Extract(bytes.NewReader(test.pkgdata), openTar, &options) c.Assert(err, IsNil) c.Assert(createExtractInfos, DeepEquals, test.calls) From 8d87fae3b1ea212fe44dedc2188d1e452c52646f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 19 Aug 2026 11:02:15 +0200 Subject: [PATCH 05/21] docs: clean comments --- internal/tarball/extract.go | 2 +- internal/tarball/extract_test.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 28d05be9e..de014be7e 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -25,7 +25,7 @@ import ( type OpenTarFunc func(pkgReader io.Reader) (io.ReadCloser, error) // OpenXZ opens a package which is a plain XZ-compressed tarball, such as a -// store (bin) package. +// bin package. func OpenXZ(pkgReader io.Reader) (io.ReadCloser, error) { xzReader, err := xz.NewReader(pkgReader) if err != nil { diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index 9c5fe3523..324936095 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -19,7 +19,6 @@ import ( type extractTest struct { summary string pkgdata []byte - // openTar must match the format of pkgdata. It defaults to deb.OpenTar. openTar tarball.OpenTarFunc options tarball.ExtractOptions hackopt func(c *C, o *tarball.ExtractOptions) @@ -546,7 +545,6 @@ func (s *S) TestExtract(c *C) { var extractCreateCallbackTests = []struct { summary string pkgdata []byte - // openTar must match the format of pkgdata. It defaults to deb.OpenTar. openTar tarball.OpenTarFunc options tarball.ExtractOptions calls map[string][]tarball.ExtractInfo From 1a38ccdf8f5930bb2da03a8479165827314b76be Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 1 Sep 2026 09:33:48 +0200 Subject: [PATCH 06/21] docs: remove mentions of packages in tarball pkg --- internal/tarball/extract.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index de014be7e..d98ba8bb8 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -18,14 +18,13 @@ import ( "github.com/canonical/chisel/internal/strdist" ) -// OpenTarFunc opens the uncompressed tar stream carried by a package, hiding -// the package format from Extract. An implementation may unwrap a container -// before decompressing (see deb.OpenTar) or decompress the package itself (see -// OpenXZ). +// OpenTarFunc returns a reader over the uncompressed tar stream contained in +// its input, hiding the container and compression details from Extract. An +// implementation may unwrap a container before decompressing (see deb.OpenTar) +// or decompress the input directly (see OpenXZ). type OpenTarFunc func(pkgReader io.Reader) (io.ReadCloser, error) -// OpenXZ opens a package which is a plain XZ-compressed tarball, such as a -// bin package. +// OpenXZ opens a plain XZ-compressed tarball. func OpenXZ(pkgReader io.Reader) (io.ReadCloser, error) { xzReader, err := xz.NewReader(pkgReader) if err != nil { @@ -138,8 +137,8 @@ func extractData(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractO // create them with the permissions defined in the tarball. // // The assumption is that the tar entries of the parent directories appear - // before the entry for the file itself. This is the case for .deb files but - // not for all tarballs. + // before the entry for the file itself. This is the case for the tarballs + // produced by common packaging tools but not for all tarballs. tarDirMode := make(map[string]fs.FileMode) tarReader := tar.NewReader(dataReader) for { @@ -382,7 +381,7 @@ func extractHardLinks(pkgReader io.ReadSeeker, openTar OpenTarFunc, opts *extrac } // If there are pending links, that means the link targets do not come from - // this package. + // this tarball. if len(opts.pendingLinks) > 0 { var targets []string for target := range opts.pendingLinks { From ee8732aad0f842ad513f45ea9a6db29e3dcdfef7 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 3 Sep 2026 09:30:48 +0200 Subject: [PATCH 07/21] style: refine naming and docs --- internal/tarball/extract.go | 32 +++++++++++++++----------------- internal/tarball/extract_test.go | 4 ++-- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index d98ba8bb8..597e8ab96 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -18,14 +18,12 @@ import ( "github.com/canonical/chisel/internal/strdist" ) -// OpenTarFunc returns a reader over the uncompressed tar stream contained in -// its input, hiding the container and compression details from Extract. An -// implementation may unwrap a container before decompressing (see deb.OpenTar) -// or decompress the input directly (see OpenXZ). -type OpenTarFunc func(pkgReader io.Reader) (io.ReadCloser, error) - -// OpenXZ opens a plain XZ-compressed tarball. -func OpenXZ(pkgReader io.Reader) (io.ReadCloser, error) { +// TarOpener returns a reader over the uncompressed tar stream contained in +// its input, hiding the container and compression details from Extract. +type TarOpener func(pkgReader io.Reader) (io.ReadCloser, error) + +// OpenXZTar returns a reader over the decompressed XZ stream. +func OpenXZTar(pkgReader io.Reader) (io.ReadCloser, error) { xzReader, err := xz.NewReader(pkgReader) if err != nil { return nil, err @@ -74,7 +72,7 @@ func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { return options, nil } -func Extract(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractOptions) (err error) { +func Extract(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) (err error) { defer func() { if err != nil { err = fmt.Errorf("cannot extract from package %q: %w", options.Package, err) @@ -83,7 +81,7 @@ func Extract(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractOptio logf("Extracting files from package %q...", options.Package) - if openTar == nil { + if opener == nil { return fmt.Errorf("internal error: no tar opener provided") } @@ -99,11 +97,11 @@ func Extract(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractOptio return err } - return extractData(pkgReader, openTar, validOpts) + return extractData(pkgReader, opener, validOpts) } -func extractData(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractOptions) error { - dataReader, err := openTar(pkgReader) +func extractData(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) error { + dataReader, err := opener(pkgReader) if err != nil { return err } @@ -285,7 +283,7 @@ func extractData(pkgReader io.ReadSeeker, openTar OpenTarFunc, options *ExtractO if err != nil { return err } - err = extractHardLinks(pkgReader, openTar, extractHardLinkOptions) + err = extractHardLinks(pkgReader, opener, extractHardLinkOptions) if err != nil { return err } @@ -319,8 +317,8 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. -func extractHardLinks(pkgReader io.ReadSeeker, openTar OpenTarFunc, opts *extractHardLinkOptions) error { - dataReader, err := openTar(pkgReader) +func extractHardLinks(pkgReader io.ReadSeeker, opener TarOpener, opts *extractHardLinkOptions) error { + dataReader, err := opener(pkgReader) if err != nil { return err } @@ -381,7 +379,7 @@ func extractHardLinks(pkgReader io.ReadSeeker, openTar OpenTarFunc, opts *extrac } // If there are pending links, that means the link targets do not come from - // this tarball. + // this package. if len(opts.pendingLinks) > 0 { var targets []string for target := range opts.pendingLinks { diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index 324936095..436bea074 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -19,7 +19,7 @@ import ( type extractTest struct { summary string pkgdata []byte - openTar tarball.OpenTarFunc + openTar tarball.TarOpener options tarball.ExtractOptions hackopt func(c *C, o *tarball.ExtractOptions) result map[string]string @@ -545,7 +545,7 @@ func (s *S) TestExtract(c *C) { var extractCreateCallbackTests = []struct { summary string pkgdata []byte - openTar tarball.OpenTarFunc + openTar tarball.TarOpener options tarball.ExtractOptions calls map[string][]tarball.ExtractInfo }{{ From ca72fd65d108d3e9d1a10fbadf78228e921b832b Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 8 Sep 2026 15:24:49 +0200 Subject: [PATCH 08/21] refactor: rework approach --- .../cmd_debug_check_release_archives.go | 4 +- internal/archive/archive_test.go | 3 +- internal/deb/extract.go | 49 ---------- internal/slicer/slicer.go | 4 +- internal/tarball/extract.go | 92 +++++++++++++++---- internal/tarball/extract_test.go | 35 ++----- 6 files changed, 88 insertions(+), 99 deletions(-) delete mode 100644 internal/deb/extract.go diff --git a/cmd/chisel/cmd_debug_check_release_archives.go b/cmd/chisel/cmd_debug_check_release_archives.go index 29a8189a8..0225e29f0 100644 --- a/cmd/chisel/cmd_debug_check_release_archives.go +++ b/cmd/chisel/cmd_debug_check_release_archives.go @@ -13,8 +13,8 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/internal/tarball" ) var shortCheckReleaseArchivesHelp = "Check the release's archives" @@ -150,7 +150,7 @@ func computePathObservations(release *setup.Release, archives map[string]archive if err != nil { return nil, err } - dataReader, err := deb.OpenTar(pkgReader) + dataReader, err := tarball.DataReader(pkgReader, tarball.DebFormat) if err != nil { return nil, err } diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index 8c40eeced..bc5687213 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -1236,8 +1236,9 @@ func (s *S) testOpenArchiveArch(c *C, test realArchiveTest, arch string) { c.Assert(info.Name, DeepEquals, test.pkg) c.Assert(info.Arch, DeepEquals, arch) - err = tarball.Extract(pkg, deb.OpenTar, &tarball.ExtractOptions{ + err = tarball.Extract(pkg, &tarball.ExtractOptions{ Package: test.pkg, + Format: tarball.DebFormat, TargetDir: extractDir, Extract: map[string][]tarball.ExtractInfo{ fmt.Sprintf("/usr/share/doc/%s/copyright", test.pkg): { diff --git a/internal/deb/extract.go b/internal/deb/extract.go deleted file mode 100644 index d25f1f883..000000000 --- a/internal/deb/extract.go +++ /dev/null @@ -1,49 +0,0 @@ -package deb - -import ( - "compress/gzip" - "fmt" - "io" - - "github.com/blakesmith/ar" - "github.com/klauspost/compress/zstd" - "github.com/ulikunitz/xz" -) - -// OpenTar takes a Reader for the ar file belonging to a Debian package and -// returns a Reader to the uncompressed inner tarball. -func OpenTar(pkgReader io.Reader) (io.ReadCloser, error) { - arReader := ar.NewReader(pkgReader) - var dataReader io.ReadCloser - for dataReader == nil { - arHeader, err := arReader.Next() - if err == io.EOF { - return nil, fmt.Errorf("no data payload") - } - if err != nil { - return nil, err - } - switch arHeader.Name { - case "data.tar.gz": - gzipReader, err := gzip.NewReader(arReader) - if err != nil { - return nil, err - } - dataReader = gzipReader - case "data.tar.xz": - xzReader, err := xz.NewReader(arReader) - if err != nil { - return nil, err - } - dataReader = io.NopCloser(xzReader) - case "data.tar.zst": - zstdReader, err := zstd.NewReader(arReader) - if err != nil { - return nil, err - } - dataReader = zstdReader.IOReadCloser() - } - } - - return dataReader, nil -} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 994f65ba2..6c528c651 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,7 +16,6 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" @@ -240,8 +239,9 @@ func Run(options *RunOptions) error { if reader == nil { continue } - err := tarball.Extract(reader, deb.OpenTar, &tarball.ExtractOptions{ + err := tarball.Extract(reader, &tarball.ExtractOptions{ Package: slice.Package, + Format: tarball.DebFormat, Extract: extract[slice.Package], TargetDir: targetDir, Create: create, diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 597e8ab96..c8b79c859 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -3,6 +3,7 @@ package tarball import ( "archive/tar" "bytes" + "compress/gzip" "fmt" "io" "io/fs" @@ -12,27 +13,82 @@ import ( "strings" "syscall" + "github.com/blakesmith/ar" + "github.com/klauspost/compress/zstd" "github.com/ulikunitz/xz" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/strdist" ) -// TarOpener returns a reader over the uncompressed tar stream contained in -// its input, hiding the container and compression details from Extract. -type TarOpener func(pkgReader io.Reader) (io.ReadCloser, error) +// Format identifies the format of a package. +type Format string -// OpenXZTar returns a reader over the decompressed XZ stream. -func OpenXZTar(pkgReader io.Reader) (io.ReadCloser, error) { - xzReader, err := xz.NewReader(pkgReader) - if err != nil { - return nil, err +const ( + // DebFormat is the Debian package format: an ar archive holding a + // compressed data tarball. + DebFormat Format = "deb" + // BinFormat is the bin package format: a plain XZ-compressed tarball. + BinFormat Format = "bin" +) + +// DataReader returns a reader over the uncompressed tar stream contained in +// the given package, based on its format. +func DataReader(pkgReader io.Reader, format Format) (io.ReadCloser, error) { + switch format { + case DebFormat: + return debDataReader(pkgReader) + case BinFormat: + xzReader, err := xz.NewReader(pkgReader) + if err != nil { + return nil, err + } + return io.NopCloser(xzReader), nil + } + return nil, fmt.Errorf("internal error: unsupported package format: %q", format) +} + +// debDataReader returns a reader over the data tarball contained in the ar +// file of a Debian package. +func debDataReader(pkgReader io.Reader) (io.ReadCloser, error) { + arReader := ar.NewReader(pkgReader) + var dataReader io.ReadCloser + for dataReader == nil { + arHeader, err := arReader.Next() + if err == io.EOF { + return nil, fmt.Errorf("no data payload") + } + if err != nil { + return nil, err + } + switch arHeader.Name { + case "data.tar.gz": + gzipReader, err := gzip.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = gzipReader + case "data.tar.xz": + xzReader, err := xz.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = io.NopCloser(xzReader) + case "data.tar.zst": + zstdReader, err := zstd.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = zstdReader.IOReadCloser() + } } - return io.NopCloser(xzReader), nil + + return dataReader, nil } type ExtractOptions struct { Package string + Format Format TargetDir string Extract map[string][]ExtractInfo // Create can optionally be set to control the creation of extracted entries. @@ -72,7 +128,7 @@ func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { return options, nil } -func Extract(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) (err error) { +func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { defer func() { if err != nil { err = fmt.Errorf("cannot extract from package %q: %w", options.Package, err) @@ -81,10 +137,6 @@ func Extract(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) logf("Extracting files from package %q...", options.Package) - if opener == nil { - return fmt.Errorf("internal error: no tar opener provided") - } - validOpts, err := getValidOptions(options) if err != nil { return err @@ -97,11 +149,11 @@ func Extract(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) return err } - return extractData(pkgReader, opener, validOpts) + return extractData(pkgReader, validOpts) } -func extractData(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) error { - dataReader, err := opener(pkgReader) +func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { + dataReader, err := DataReader(pkgReader, options.Format) if err != nil { return err } @@ -283,7 +335,7 @@ func extractData(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOpti if err != nil { return err } - err = extractHardLinks(pkgReader, opener, extractHardLinkOptions) + err = extractHardLinks(pkgReader, extractHardLinkOptions) if err != nil { return err } @@ -317,8 +369,8 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. -func extractHardLinks(pkgReader io.ReadSeeker, opener TarOpener, opts *extractHardLinkOptions) error { - dataReader, err := opener(pkgReader) +func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { + dataReader, err := DataReader(pkgReader, opts.Format) if err != nil { return err } diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index 436bea074..0b3b670cb 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -10,7 +10,6 @@ import ( . "gopkg.in/check.v1" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" @@ -19,7 +18,7 @@ import ( type extractTest struct { summary string pkgdata []byte - openTar tarball.TarOpener + format tarball.Format options tarball.ExtractOptions hackopt func(c *C, o *tarball.ExtractOptions) result map[string]string @@ -498,6 +497,9 @@ func (s *S) TestExtract(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir + if options.Format == "" { + options.Format = tarball.DebFormat + } createdPaths := make(map[string]bool) options.Create = func(_ []tarball.ExtractInfo, o *fsutil.CreateOptions) error { relPath := filepath.Clean("/" + strings.TrimPrefix(o.Path, dir)) @@ -513,11 +515,7 @@ func (s *S) TestExtract(c *C) { test.hackopt(c, &options) } - openTar := test.openTar - if openTar == nil { - openTar = deb.OpenTar - } - err := tarball.Extract(bytes.NewReader(test.pkgdata), openTar, &options) + err := tarball.Extract(bytes.NewReader(test.pkgdata), &options) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue @@ -545,7 +543,7 @@ func (s *S) TestExtract(c *C) { var extractCreateCallbackTests = []struct { summary string pkgdata []byte - openTar tarball.TarOpener + format tarball.Format options tarball.ExtractOptions calls map[string][]tarball.ExtractInfo }{{ @@ -612,6 +610,9 @@ func (s *S) TestExtractCreateCallback(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir + if options.Format == "" { + options.Format = tarball.DebFormat + } createExtractInfos := map[string][]tarball.ExtractInfo{} options.Create = func(extractInfos []tarball.ExtractInfo, o *fsutil.CreateOptions) error { if extractInfos == nil { @@ -629,25 +630,9 @@ func (s *S) TestExtractCreateCallback(c *C) { return nil } - openTar := test.openTar - if openTar == nil { - openTar = deb.OpenTar - } - err := tarball.Extract(bytes.NewReader(test.pkgdata), openTar, &options) + err := tarball.Extract(bytes.NewReader(test.pkgdata), &options) c.Assert(err, IsNil) c.Assert(createExtractInfos, DeepEquals, test.calls) } } - -func (s *S) TestExtractMissingOpenTar(c *C) { - options := tarball.ExtractOptions{ - Package: "test-package", - TargetDir: c.MkDir(), - Extract: map[string][]tarball.ExtractInfo{ - "/dir/file": {{Path: "/dir/file"}}, - }, - } - err := tarball.Extract(bytes.NewReader(testutil.PackageData["test-package"]), nil, &options) - c.Assert(err, ErrorMatches, `cannot extract from package "test-package": internal error: no tar opener provided`) -} From dc8e94bd6caedca91cb578482c7fbeef33c6266c Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 8 Sep 2026 15:40:05 +0200 Subject: [PATCH 09/21] fix: clean superfluous import --- internal/archive/archive_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index bc5687213..3a83d73d5 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -20,7 +20,6 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/archive/testarchive" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" ) From f882d4711b3bf0c15f56f9ed419a4409deccbf7a Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 8 Sep 2026 15:50:27 +0200 Subject: [PATCH 10/21] test: remove unused field for now --- internal/tarball/extract_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index 0b3b670cb..5f233a92a 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -18,7 +18,6 @@ import ( type extractTest struct { summary string pkgdata []byte - format tarball.Format options tarball.ExtractOptions hackopt func(c *C, o *tarball.ExtractOptions) result map[string]string From 116752e32f3f83aac412e8a512be3dfe1ea74063 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 9 Sep 2026 08:32:37 +0200 Subject: [PATCH 11/21] refactor: put deb DataReader back in deb package --- internal/deb/extract.go | 49 +++++++++++++++++++++++++++++++++++++ internal/tarball/extract.go | 44 ++------------------------------- 2 files changed, 51 insertions(+), 42 deletions(-) create mode 100644 internal/deb/extract.go diff --git a/internal/deb/extract.go b/internal/deb/extract.go new file mode 100644 index 000000000..2bce509f4 --- /dev/null +++ b/internal/deb/extract.go @@ -0,0 +1,49 @@ +package deb + +import ( + "compress/gzip" + "fmt" + "io" + + "github.com/blakesmith/ar" + "github.com/klauspost/compress/zstd" + "github.com/ulikunitz/xz" +) + +// DataReader takes a Reader for the ar file belonging to a Debian package and +// returns a Reader to the inner tarball. +func DataReader(pkgReader io.Reader) (io.ReadCloser, error) { + arReader := ar.NewReader(pkgReader) + var dataReader io.ReadCloser + for dataReader == nil { + arHeader, err := arReader.Next() + if err == io.EOF { + return nil, fmt.Errorf("no data payload") + } + if err != nil { + return nil, err + } + switch arHeader.Name { + case "data.tar.gz": + gzipReader, err := gzip.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = gzipReader + case "data.tar.xz": + xzReader, err := xz.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = io.NopCloser(xzReader) + case "data.tar.zst": + zstdReader, err := zstd.NewReader(arReader) + if err != nil { + return nil, err + } + dataReader = zstdReader.IOReadCloser() + } + } + + return dataReader, nil +} \ No newline at end of file diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index c8b79c859..3807d1544 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -3,7 +3,6 @@ package tarball import ( "archive/tar" "bytes" - "compress/gzip" "fmt" "io" "io/fs" @@ -13,10 +12,9 @@ import ( "strings" "syscall" - "github.com/blakesmith/ar" - "github.com/klauspost/compress/zstd" "github.com/ulikunitz/xz" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/strdist" ) @@ -37,7 +35,7 @@ const ( func DataReader(pkgReader io.Reader, format Format) (io.ReadCloser, error) { switch format { case DebFormat: - return debDataReader(pkgReader) + return deb.DataReader(pkgReader) case BinFormat: xzReader, err := xz.NewReader(pkgReader) if err != nil { @@ -48,44 +46,6 @@ func DataReader(pkgReader io.Reader, format Format) (io.ReadCloser, error) { return nil, fmt.Errorf("internal error: unsupported package format: %q", format) } -// debDataReader returns a reader over the data tarball contained in the ar -// file of a Debian package. -func debDataReader(pkgReader io.Reader) (io.ReadCloser, error) { - arReader := ar.NewReader(pkgReader) - var dataReader io.ReadCloser - for dataReader == nil { - arHeader, err := arReader.Next() - if err == io.EOF { - return nil, fmt.Errorf("no data payload") - } - if err != nil { - return nil, err - } - switch arHeader.Name { - case "data.tar.gz": - gzipReader, err := gzip.NewReader(arReader) - if err != nil { - return nil, err - } - dataReader = gzipReader - case "data.tar.xz": - xzReader, err := xz.NewReader(arReader) - if err != nil { - return nil, err - } - dataReader = io.NopCloser(xzReader) - case "data.tar.zst": - zstdReader, err := zstd.NewReader(arReader) - if err != nil { - return nil, err - } - dataReader = zstdReader.IOReadCloser() - } - } - - return dataReader, nil -} - type ExtractOptions struct { Package string Format Format From e9c4d881ab915066a7a8807bb78a506418d44341 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 9 Sep 2026 08:34:03 +0200 Subject: [PATCH 12/21] style: add missing trailing newline --- internal/deb/extract.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 2bce509f4..88fc4ad56 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -46,4 +46,4 @@ func DataReader(pkgReader io.Reader) (io.ReadCloser, error) { } return dataReader, nil -} \ No newline at end of file +} From b1ec3903276a6de50ddeec6703775323858ca56d Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 9 Sep 2026 13:40:51 +0200 Subject: [PATCH 13/21] style: refine naming --- cmd/chisel/cmd_debug_check_release_archives.go | 2 +- internal/tarball/extract.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/chisel/cmd_debug_check_release_archives.go b/cmd/chisel/cmd_debug_check_release_archives.go index 0225e29f0..c7931d9a6 100644 --- a/cmd/chisel/cmd_debug_check_release_archives.go +++ b/cmd/chisel/cmd_debug_check_release_archives.go @@ -150,7 +150,7 @@ func computePathObservations(release *setup.Release, archives map[string]archive if err != nil { return nil, err } - dataReader, err := tarball.DataReader(pkgReader, tarball.DebFormat) + dataReader, err := tarball.DebFormat.TarStream(pkgReader) if err != nil { return nil, err } diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 3807d1544..0760a1fef 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -30,9 +30,9 @@ const ( BinFormat Format = "bin" ) -// DataReader returns a reader over the uncompressed tar stream contained in -// the given package, based on its format. -func DataReader(pkgReader io.Reader, format Format) (io.ReadCloser, error) { +// TarStream returns a reader over the uncompressed tar stream contained +// in the given package. +func (format Format) TarStream(pkgReader io.Reader) (io.ReadCloser, error) { switch format { case DebFormat: return deb.DataReader(pkgReader) @@ -113,7 +113,7 @@ func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { } func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { - dataReader, err := DataReader(pkgReader, options.Format) + dataReader, err := options.Format.TarStream(pkgReader) if err != nil { return err } @@ -330,7 +330,7 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { - dataReader, err := DataReader(pkgReader, opts.Format) + dataReader, err := opts.Format.TarStream(pkgReader) if err != nil { return err } From a8ad8137611ca866f6f3d33f2be2d44fa6ad3082 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 9 Sep 2026 13:46:18 +0200 Subject: [PATCH 14/21] style: improve naming consistency --- internal/tarball/extract.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 0760a1fef..f61ed21e1 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -113,11 +113,11 @@ func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { } func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { - dataReader, err := options.Format.TarStream(pkgReader) + tarStream, err := options.Format.TarStream(pkgReader) if err != nil { return err } - defer dataReader.Close() + defer tarStream.Close() oldUmask := syscall.Umask(0) defer func() { @@ -150,7 +150,7 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { // before the entry for the file itself. This is the case for the tarballs // produced by common packaging tools but not for all tarballs. tarDirMode := make(map[string]fs.FileMode) - tarReader := tar.NewReader(dataReader) + tarReader := tar.NewReader(tarStream) for { tarHeader, err := tarReader.Next() if err == io.EOF { @@ -330,13 +330,13 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { - dataReader, err := opts.Format.TarStream(pkgReader) + tarStream, err := opts.Format.TarStream(pkgReader) if err != nil { return err } - defer dataReader.Close() + defer tarStream.Close() - tarReader := tar.NewReader(dataReader) + tarReader := tar.NewReader(tarStream) for { tarHeader, err := tarReader.Next() if err == io.EOF { From 6ad029e50f078420f157ddcfa850e11f070c53d7 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 9 Sep 2026 13:54:03 +0200 Subject: [PATCH 15/21] style: avoid ambiguity --- internal/tarball/extract.go | 16 ++++++++-------- internal/tarball/extract_test.go | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index f61ed21e1..a0c7af11a 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -19,20 +19,20 @@ import ( "github.com/canonical/chisel/internal/strdist" ) -// Format identifies the format of a package. -type Format string +// PkgFormat identifies the format of a package. +type PkgFormat string const ( // DebFormat is the Debian package format: an ar archive holding a // compressed data tarball. - DebFormat Format = "deb" + DebFormat PkgFormat = "deb" // BinFormat is the bin package format: a plain XZ-compressed tarball. - BinFormat Format = "bin" + BinFormat PkgFormat = "bin" ) -// TarStream returns a reader over the uncompressed tar stream contained -// in the given package. -func (format Format) TarStream(pkgReader io.Reader) (io.ReadCloser, error) { +// TarStream returns a reader over the raw, uncompressed tar stream contained +// in the given package. The stream is not parsed. +func (format PkgFormat) TarStream(pkgReader io.Reader) (io.ReadCloser, error) { switch format { case DebFormat: return deb.DataReader(pkgReader) @@ -48,7 +48,7 @@ func (format Format) TarStream(pkgReader io.Reader) (io.ReadCloser, error) { type ExtractOptions struct { Package string - Format Format + Format PkgFormat TargetDir string Extract map[string][]ExtractInfo // Create can optionally be set to control the creation of extracted entries. diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index 5f233a92a..ed43414ef 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -542,7 +542,7 @@ func (s *S) TestExtract(c *C) { var extractCreateCallbackTests = []struct { summary string pkgdata []byte - format tarball.Format + format tarball.PkgFormat options tarball.ExtractOptions calls map[string][]tarball.ExtractInfo }{{ From 150e904987aa4b6f1b6bd5da23cae0e0e37056ff Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 10 Sep 2026 11:21:40 +0200 Subject: [PATCH 16/21] refactor: rework approach with interface --- .../cmd_debug_check_release_archives.go | 5 +- internal/archive/archive_test.go | 4 +- internal/bin/extract.go | 36 +++++++++++++ internal/bin/extract_test.go | 44 +++++++++++++++ internal/bin/log.go | 53 +++++++++++++++++++ internal/bin/suite_test.go | 25 +++++++++ internal/deb/extract.go | 26 +++++++-- internal/deb/extract_test.go | 33 ++++++++++++ internal/slicer/slicer.go | 4 +- internal/tarball/extract.go | 51 +++++------------- internal/tarball/extract_test.go | 12 ++--- internal/testutil/pkgdata.go | 34 ++++++++++++ 12 files changed, 271 insertions(+), 56 deletions(-) create mode 100644 internal/bin/extract.go create mode 100644 internal/bin/extract_test.go create mode 100644 internal/bin/log.go create mode 100644 internal/bin/suite_test.go create mode 100644 internal/deb/extract_test.go diff --git a/cmd/chisel/cmd_debug_check_release_archives.go b/cmd/chisel/cmd_debug_check_release_archives.go index c7931d9a6..833a73a89 100644 --- a/cmd/chisel/cmd_debug_check_release_archives.go +++ b/cmd/chisel/cmd_debug_check_release_archives.go @@ -13,8 +13,8 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/setup" - "github.com/canonical/chisel/internal/tarball" ) var shortCheckReleaseArchivesHelp = "Check the release's archives" @@ -150,7 +150,8 @@ func computePathObservations(release *setup.Release, archives map[string]archive if err != nil { return nil, err } - dataReader, err := tarball.DebFormat.TarStream(pkgReader) + debPkg := deb.OpenPkg(pkgReader) + dataReader, err := debPkg.TarStream() if err != nil { return nil, err } diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index 3a83d73d5..011072705 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -20,6 +20,7 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/archive/testarchive" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" ) @@ -1235,9 +1236,8 @@ func (s *S) testOpenArchiveArch(c *C, test realArchiveTest, arch string) { c.Assert(info.Name, DeepEquals, test.pkg) c.Assert(info.Arch, DeepEquals, arch) - err = tarball.Extract(pkg, &tarball.ExtractOptions{ + err = tarball.Extract(deb.OpenPkg(pkg), &tarball.ExtractOptions{ Package: test.pkg, - Format: tarball.DebFormat, TargetDir: extractDir, Extract: map[string][]tarball.ExtractInfo{ fmt.Sprintf("/usr/share/doc/%s/copyright", test.pkg): { diff --git a/internal/bin/extract.go b/internal/bin/extract.go new file mode 100644 index 000000000..159313c68 --- /dev/null +++ b/internal/bin/extract.go @@ -0,0 +1,36 @@ +package bin + +import ( + "io" + + "github.com/ulikunitz/xz" +) + +// Pkg reads the tar stream of a bin package held in a seekable reader. +// A bin package is a plain XZ-compressed tarball. +type Pkg struct { + reader io.ReadSeekCloser +} + +// OpenPkg wraps a seekable reader over bin package data. +func OpenPkg(reader io.ReadSeekCloser) *Pkg { + return &Pkg{reader: reader} +} + +// TarStream returns a reader over the tar stream of the bin package, +// reading from the current position. +func (p *Pkg) TarStream() (io.ReadCloser, error) { + xzReader, err := xz.NewReader(p.reader) + if err != nil { + return nil, err + } + return io.NopCloser(xzReader), nil +} + +func (p *Pkg) Seek(offset int64, whence int) (int64, error) { + return p.reader.Seek(offset, whence) +} + +func (p *Pkg) Close() error { + return p.reader.Close() +} diff --git a/internal/bin/extract_test.go b/internal/bin/extract_test.go new file mode 100644 index 000000000..03ef41a30 --- /dev/null +++ b/internal/bin/extract_test.go @@ -0,0 +1,44 @@ +package bin_test + +import ( + "bytes" + "io" + + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/bin" + "github.com/canonical/chisel/internal/tarball" + "github.com/canonical/chisel/internal/testutil" +) + +// Compile-time check that Pkg implements tarball.PkgReader. +var _ tarball.PkgReader = (*bin.Pkg)(nil) + +func (s *S) TestPkgTarStream(c *C) { + pkg := bin.OpenPkg(testutil.ReadSeekNopCloser( + bytes.NewReader(testutil.MustMakeBin([]testutil.TarEntry{ + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "content"), + })))) + + tarStream, err := pkg.TarStream() + c.Assert(err, IsNil) + err = tarStream.Close() + c.Assert(err, IsNil) + + // A second stream can be obtained after rewinding. + _, err = pkg.Seek(0, io.SeekStart) + c.Assert(err, IsNil) + tarStream, err = pkg.TarStream() + c.Assert(err, IsNil) + err = tarStream.Close() + c.Assert(err, IsNil) +} + +func (s *S) TestPkgTarStreamInvalid(c *C) { + pkg := bin.OpenPkg(testutil.ReadSeekNopCloser( + bytes.NewReader([]byte("not an xz stream")))) + + _, err := pkg.TarStream() + c.Assert(err, ErrorMatches, "xz.*") +} diff --git a/internal/bin/log.go b/internal/bin/log.go new file mode 100644 index 000000000..e0f72a8df --- /dev/null +++ b/internal/bin/log.go @@ -0,0 +1,53 @@ +package bin + +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...)) + } +} diff --git a/internal/bin/suite_test.go b/internal/bin/suite_test.go new file mode 100644 index 000000000..0a450d8ec --- /dev/null +++ b/internal/bin/suite_test.go @@ -0,0 +1,25 @@ +package bin_test + +import ( + "testing" + + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/bin" +) + +func Test(t *testing.T) { TestingT(t) } + +type S struct{} + +var _ = Suite(&S{}) + +func (s *S) SetUpTest(c *C) { + bin.SetDebug(true) + bin.SetLogger(c) +} + +func (s *S) TearDownTest(c *C) { + bin.SetDebug(false) + bin.SetLogger(nil) +} diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 88fc4ad56..4dc926de4 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -10,10 +10,20 @@ import ( "github.com/ulikunitz/xz" ) -// DataReader takes a Reader for the ar file belonging to a Debian package and -// returns a Reader to the inner tarball. -func DataReader(pkgReader io.Reader) (io.ReadCloser, error) { - arReader := ar.NewReader(pkgReader) +// Pkg reads the tar stream of a Debian package held in a seekable reader. +type Pkg struct { + reader io.ReadSeekCloser +} + +// OpenPkg wraps a seekable reader over Debian package data. +func OpenPkg(reader io.ReadSeekCloser) *Pkg { + return &Pkg{reader: reader} +} + +// TarStream returns a reader over the data tarball of the Debian +// package, reading from the current position. +func (p *Pkg) TarStream() (io.ReadCloser, error) { + arReader := ar.NewReader(p.reader) var dataReader io.ReadCloser for dataReader == nil { arHeader, err := arReader.Next() @@ -47,3 +57,11 @@ func DataReader(pkgReader io.Reader) (io.ReadCloser, error) { return dataReader, nil } + +func (p *Pkg) Seek(offset int64, whence int) (int64, error) { + return p.reader.Seek(offset, whence) +} + +func (p *Pkg) Close() error { + return p.reader.Close() +} diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go new file mode 100644 index 000000000..7342a4990 --- /dev/null +++ b/internal/deb/extract_test.go @@ -0,0 +1,33 @@ +package deb_test + +import ( + "bytes" + "io" + + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/deb" + "github.com/canonical/chisel/internal/tarball" + "github.com/canonical/chisel/internal/testutil" +) + +// Compile-time check that Pkg implements tarball.PkgReader. +var _ tarball.PkgReader = (*deb.Pkg)(nil) + +func (s *S) TestPkgTarStream(c *C) { + pkg := deb.OpenPkg(testutil.ReadSeekNopCloser( + bytes.NewReader(testutil.PackageData["test-package"]))) + + tarStream, err := pkg.TarStream() + c.Assert(err, IsNil) + err = tarStream.Close() + c.Assert(err, IsNil) + + // A second stream can be obtained after rewinding. + _, err = pkg.Seek(0, io.SeekStart) + c.Assert(err, IsNil) + tarStream, err = pkg.TarStream() + c.Assert(err, IsNil) + err = tarStream.Close() + c.Assert(err, IsNil) +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 6c528c651..ebd0eb400 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,6 +16,7 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" @@ -239,9 +240,8 @@ func Run(options *RunOptions) error { if reader == nil { continue } - err := tarball.Extract(reader, &tarball.ExtractOptions{ + err := tarball.Extract(deb.OpenPkg(reader), &tarball.ExtractOptions{ Package: slice.Package, - Format: tarball.DebFormat, Extract: extract[slice.Package], TargetDir: targetDir, Create: create, diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index a0c7af11a..a9d584e47 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -12,43 +12,20 @@ import ( "strings" "syscall" - "github.com/ulikunitz/xz" - - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/strdist" ) -// PkgFormat identifies the format of a package. -type PkgFormat string - -const ( - // DebFormat is the Debian package format: an ar archive holding a - // compressed data tarball. - DebFormat PkgFormat = "deb" - // BinFormat is the bin package format: a plain XZ-compressed tarball. - BinFormat PkgFormat = "bin" -) - -// TarStream returns a reader over the raw, uncompressed tar stream contained -// in the given package. The stream is not parsed. -func (format PkgFormat) TarStream(pkgReader io.Reader) (io.ReadCloser, error) { - switch format { - case DebFormat: - return deb.DataReader(pkgReader) - case BinFormat: - xzReader, err := xz.NewReader(pkgReader) - if err != nil { - return nil, err - } - return io.NopCloser(xzReader), nil - } - return nil, fmt.Errorf("internal error: unsupported package format: %q", format) +// PkgReader provides the tar stream of a package. TarStream reads from +// the current position, which the caller controls through Seek. +type PkgReader interface { + // TarStream returns a reader over the raw, unparsed tar stream. + TarStream() (io.ReadCloser, error) + io.Seeker } type ExtractOptions struct { Package string - Format PkgFormat TargetDir string Extract map[string][]ExtractInfo // Create can optionally be set to control the creation of extracted entries. @@ -88,7 +65,7 @@ func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { return options, nil } -func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { +func Extract(pkg PkgReader, options *ExtractOptions) (err error) { defer func() { if err != nil { err = fmt.Errorf("cannot extract from package %q: %w", options.Package, err) @@ -109,11 +86,11 @@ func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) { return err } - return extractData(pkgReader, validOpts) + return extractData(pkg, validOpts) } -func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { - tarStream, err := options.Format.TarStream(pkgReader) +func extractData(pkg PkgReader, options *ExtractOptions) error { + tarStream, err := pkg.TarStream() if err != nil { return err } @@ -291,11 +268,11 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { ExtractOptions: options, pendingLinks: pendingHardLinks, } - _, err := pkgReader.Seek(0, io.SeekStart) + _, err := pkg.Seek(0, io.SeekStart) if err != nil { return err } - err = extractHardLinks(pkgReader, extractHardLinkOptions) + err = extractHardLinks(pkg, extractHardLinkOptions) if err != nil { return err } @@ -329,8 +306,8 @@ type extractHardLinkOptions struct { // extractHardLinks iterates through the tarball a second time to extract the // hard links that were not extracted in the first pass. -func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) error { - tarStream, err := opts.Format.TarStream(pkgReader) +func extractHardLinks(pkg PkgReader, opts *extractHardLinkOptions) error { + tarStream, err := pkg.TarStream() if err != nil { return err } diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index ed43414ef..4c51808b5 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -10,6 +10,7 @@ import ( . "gopkg.in/check.v1" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" @@ -496,9 +497,6 @@ func (s *S) TestExtract(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir - if options.Format == "" { - options.Format = tarball.DebFormat - } createdPaths := make(map[string]bool) options.Create = func(_ []tarball.ExtractInfo, o *fsutil.CreateOptions) error { relPath := filepath.Clean("/" + strings.TrimPrefix(o.Path, dir)) @@ -514,7 +512,7 @@ func (s *S) TestExtract(c *C) { test.hackopt(c, &options) } - err := tarball.Extract(bytes.NewReader(test.pkgdata), &options) + err := tarball.Extract(deb.OpenPkg(testutil.ReadSeekNopCloser(bytes.NewReader(test.pkgdata))), &options) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue @@ -542,7 +540,6 @@ func (s *S) TestExtract(c *C) { var extractCreateCallbackTests = []struct { summary string pkgdata []byte - format tarball.PkgFormat options tarball.ExtractOptions calls map[string][]tarball.ExtractInfo }{{ @@ -609,9 +606,6 @@ func (s *S) TestExtractCreateCallback(c *C) { options := test.options options.Package = "test-package" options.TargetDir = dir - if options.Format == "" { - options.Format = tarball.DebFormat - } createExtractInfos := map[string][]tarball.ExtractInfo{} options.Create = func(extractInfos []tarball.ExtractInfo, o *fsutil.CreateOptions) error { if extractInfos == nil { @@ -629,7 +623,7 @@ func (s *S) TestExtractCreateCallback(c *C) { return nil } - err := tarball.Extract(bytes.NewReader(test.pkgdata), &options) + err := tarball.Extract(deb.OpenPkg(testutil.ReadSeekNopCloser(bytes.NewReader(test.pkgdata))), &options) c.Assert(err, IsNil) c.Assert(createExtractInfos, DeepEquals, test.calls) diff --git a/internal/testutil/pkgdata.go b/internal/testutil/pkgdata.go index f901873d3..84ff79cec 100644 --- a/internal/testutil/pkgdata.go +++ b/internal/testutil/pkgdata.go @@ -8,6 +8,7 @@ import ( "github.com/blakesmith/ar" "github.com/klauspost/compress/zstd" + "github.com/ulikunitz/xz" ) var PackageData = map[string][]byte{} @@ -160,6 +161,39 @@ func MustMakeDeb(entries []TarEntry) []byte { return data } +func compressBytesXZ(input []byte) ([]byte, error) { + var buf bytes.Buffer + writer, err := xz.NewWriter(&buf) + if err != nil { + return nil, err + } + if _, err = writer.Write(input); err != nil { + return nil, err + } + if err = writer.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// MakeBin returns the bytes of a bin package holding the given tar +// entries: a plain XZ-compressed tarball. +func MakeBin(entries []TarEntry) ([]byte, error) { + tarData, err := makeTar(entries) + if err != nil { + return nil, err + } + return compressBytesXZ(tarData) +} + +func MustMakeBin(entries []TarEntry) []byte { + data, err := MakeBin(entries) + if err != nil { + panic(err) + } + return data +} + // Reg is a shortcut for creating a regular file TarEntry structure (with // tar.Typeflag set tar.TypeReg). Reg stands for "REGular file". func Reg(mode int64, path, content string) TarEntry { From 791224ba434a15626669c061b20202a2353bc873 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 11 Sep 2026 17:34:58 +0200 Subject: [PATCH 17/21] refactor: refine interface --- .../cmd_debug_check_release_archives.go | 4 +- internal/archive/archive.go | 7 +- internal/archive/archive_test.go | 59 +++++++++---- internal/archive/testarchive/testarchive.go | 5 +- internal/bin/extract.go | 8 +- internal/bin/extract_test.go | 24 +++--- internal/deb/extract.go | 8 +- internal/deb/extract_test.go | 45 +++++++--- internal/slicer/slicer.go | 5 +- internal/tarball/extract.go | 10 +-- internal/tarball/extract_test.go | 84 +++++++++---------- internal/testutil/archive.go | 7 +- internal/testutil/pkgreader.go | 30 +++++++ 13 files changed, 183 insertions(+), 113 deletions(-) create mode 100644 internal/testutil/pkgreader.go diff --git a/cmd/chisel/cmd_debug_check_release_archives.go b/cmd/chisel/cmd_debug_check_release_archives.go index 833a73a89..9db689805 100644 --- a/cmd/chisel/cmd_debug_check_release_archives.go +++ b/cmd/chisel/cmd_debug_check_release_archives.go @@ -13,7 +13,6 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/setup" ) @@ -150,8 +149,7 @@ func computePathObservations(release *setup.Release, archives map[string]archive if err != nil { return nil, err } - debPkg := deb.OpenPkg(pkgReader) - dataReader, err := debPkg.TarStream() + dataReader, err := pkgReader.TarStream() if err != nil { return nil, err } diff --git a/internal/archive/archive.go b/internal/archive/archive.go index d301494b5..a1976c5c0 100644 --- a/internal/archive/archive.go +++ b/internal/archive/archive.go @@ -16,11 +16,12 @@ import ( "github.com/canonical/chisel/internal/control" "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/pgputil" + "github.com/canonical/chisel/internal/tarball" ) type Archive interface { Options() *Options - Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, error) + Fetch(pkg string) (tarball.PkgReader, *PackageInfo, error) Exists(pkg string) bool Info(pkg string) (*PackageInfo, error) } @@ -140,7 +141,7 @@ func (a *ubuntuArchive) selectPackage(pkg string) (control.Section, *ubuntuIndex return selectedSection, selectedIndex, nil } -func (a *ubuntuArchive) Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, error) { +func (a *ubuntuArchive) Fetch(pkg string) (tarball.PkgReader, *PackageInfo, error) { section, index, err := a.selectPackage(pkg) if err != nil { return nil, nil, err @@ -153,7 +154,7 @@ func (a *ubuntuArchive) Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, erro return nil, nil, err } info := sectionPackageInfo(section) - return reader, info, nil + return deb.OpenPkg(reader), info, nil } func (a *ubuntuArchive) Info(pkg string) (*PackageInfo, error) { diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index 011072705..b5bd9a4df 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -1,9 +1,7 @@ package archive_test import ( - "golang.org/x/crypto/openpgp/packet" - . "gopkg.in/check.v1" - + "archive/tar" "crypto/sha256" "crypto/sha512" "debug/elf" @@ -18,9 +16,11 @@ import ( "path/filepath" "strings" + "golang.org/x/crypto/openpgp/packet" + . "gopkg.in/check.v1" + "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/archive/testarchive" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" ) @@ -256,7 +256,7 @@ func (s *httpSuite) TestFetchPackage(c *C) { Name: "mypkg1", Version: "1.1", Arch: "amd64", - SHA256: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", + SHA256: "ff175644a17301e047ac757681f6e16b1c410228d9fe50441bba8438a7c45fa2", }) c.Assert(read(pkg), Equals, "mypkg1 1.1 data") @@ -267,7 +267,7 @@ func (s *httpSuite) TestFetchPackage(c *C) { Name: "mypkg4", Version: "1.4", Arch: "amd64", - SHA256: "54af70097b30b33cfcbb6911ad3d0df86c2d458928169e348fa7873e4fc678e4", + SHA256: "fe0b0023af4cd5786a2563faadf6ec31e48079a9b176356b008261c1590b6df9", }) c.Assert(read(pkg), Equals, "mypkg4 1.4 data") } @@ -323,13 +323,16 @@ func (s *httpSuite) TestFetchBothDigests(c *C) { Name: "mypkg1", Version: "1.1", Arch: "amd64", - SHA256: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", + SHA256: "ff175644a17301e047ac757681f6e16b1c410228d9fe50441bba8438a7c45fa2", }) c.Assert(read(pkg), Equals, "mypkg1 1.1 data") // Pin the cache key: with both digests advertised, the package is cached // under its strongest digest. - sha512Digest := fmt.Sprintf("%x", sha512.Sum512([]byte("mypkg1 1.1 data"))) + sha512Digest := fmt.Sprintf("%x", sha512.Sum512(testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./data", "mypkg1 1.1 data"), + }))) _, err = os.Stat(filepath.Join(options.CacheDir, "sha512", sha512Digest)) c.Assert(err, IsNil) } @@ -360,7 +363,7 @@ func (s *httpSuite) TestFetchPortsPackage(c *C) { Name: "mypkg1", Version: "1.1", Arch: "arm64", - SHA256: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", + SHA256: "ff175644a17301e047ac757681f6e16b1c410228d9fe50441bba8438a7c45fa2", }) c.Assert(read(pkg), Equals, "mypkg1 1.1 data") @@ -371,7 +374,7 @@ func (s *httpSuite) TestFetchPortsPackage(c *C) { Name: "mypkg4", Version: "1.4", Arch: "arm64", - SHA256: "54af70097b30b33cfcbb6911ad3d0df86c2d458928169e348fa7873e4fc678e4", + SHA256: "fe0b0023af4cd5786a2563faadf6ec31e48079a9b176356b008261c1590b6df9", }) c.Assert(read(pkg), Equals, "mypkg4 1.4 data") } @@ -383,7 +386,10 @@ func (s *httpSuite) TestFetchSecurityPackage(c *C) { err := release.Walk(func(item testarchive.Item) error { if p, ok := item.(*testarchive.Package); ok && p.Name == "mypkg1" { p.Version = fmt.Sprintf("%s.%d", p.Version, i) - p.Data = []byte("package from " + suite) + p.Data = testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./data", "package from "+suite), + }) } return nil }) @@ -411,7 +417,7 @@ func (s *httpSuite) TestFetchSecurityPackage(c *C) { Name: "mypkg1", Version: "1.1.2.2", Arch: "amd64", - SHA256: "5448585bdd916e5023eff2bc1bc3b30bcc6ee9db9c03e531375a6a11ddf0913c", + SHA256: "e3732bc52b8a11c8e749266c1eee5548ab02cbaaf84c826277e9ae245d1099b7", }) c.Assert(read(pkg), Equals, "package from jammy-security") @@ -421,7 +427,7 @@ func (s *httpSuite) TestFetchSecurityPackage(c *C) { Name: "mypkg2", Version: "1.2", Arch: "amd64", - SHA256: "a4b4f3f3a8fa09b69e3ba23c60a41a1f8144691fd371a2455812572fd02e6f79", + SHA256: "0d229011ec711ef268779580130dc034409e42124d3c25b01e0b71eac90284ad", }) c.Assert(read(pkg), Equals, "mypkg2 1.2 data") } @@ -666,7 +672,7 @@ var packageInfoTests = []struct { Name: "mypkg1", Version: "1.1", Arch: "amd64", - SHA256: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", + SHA256: "ff175644a17301e047ac757681f6e16b1c410228d9fe50441bba8438a7c45fa2", }, }, { summary: "Package not found in archive", @@ -701,12 +707,29 @@ func (s *httpSuite) TestPackageInfo(c *C) { } } -func read(r io.Reader) string { - data, err := io.ReadAll(r) +func read(pkg tarball.PkgReader) string { + tarStream, err := pkg.TarStream() if err != nil { panic(err) } - return string(data) + defer tarStream.Close() + tarReader := tar.NewReader(tarStream) + for { + tarHeader, err := tarReader.Next() + if err == io.EOF { + panic("no data file in package") + } + if err != nil { + panic(err) + } + if tarHeader.Name == "./data" { + data, err := io.ReadAll(tarReader) + if err != nil { + panic(err) + } + return string(data) + } + } } // fetchRequestStatus checks whether a request was made whose URL path @@ -1236,7 +1259,7 @@ func (s *S) testOpenArchiveArch(c *C, test realArchiveTest, arch string) { c.Assert(info.Name, DeepEquals, test.pkg) c.Assert(info.Arch, DeepEquals, arch) - err = tarball.Extract(deb.OpenPkg(pkg), &tarball.ExtractOptions{ + err = tarball.Extract(pkg, &tarball.ExtractOptions{ Package: test.pkg, TargetDir: extractDir, Extract: map[string][]tarball.ExtractInfo{ diff --git a/internal/archive/testarchive/testarchive.go b/internal/archive/testarchive/testarchive.go index c4ef7822b..af4e1351a 100644 --- a/internal/archive/testarchive/testarchive.go +++ b/internal/archive/testarchive/testarchive.go @@ -106,7 +106,10 @@ func (p *Package) Section() []byte { func (p *Package) Content() []byte { if len(p.Data) == 0 { - return []byte(p.Name + " " + p.Version + " data") + return testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./data", p.Name+" "+p.Version+" data"), + }) } return p.Data } diff --git a/internal/bin/extract.go b/internal/bin/extract.go index 159313c68..33fa29e1b 100644 --- a/internal/bin/extract.go +++ b/internal/bin/extract.go @@ -20,6 +20,10 @@ func OpenPkg(reader io.ReadSeekCloser) *Pkg { // TarStream returns a reader over the tar stream of the bin package, // reading from the current position. func (p *Pkg) TarStream() (io.ReadCloser, error) { + _, err := p.reader.Seek(0, io.SeekStart) + if err != nil { + return nil, err + } xzReader, err := xz.NewReader(p.reader) if err != nil { return nil, err @@ -27,10 +31,6 @@ func (p *Pkg) TarStream() (io.ReadCloser, error) { return io.NopCloser(xzReader), nil } -func (p *Pkg) Seek(offset int64, whence int) (int64, error) { - return p.reader.Seek(offset, whence) -} - func (p *Pkg) Close() error { return p.reader.Close() } diff --git a/internal/bin/extract_test.go b/internal/bin/extract_test.go index 03ef41a30..5af77ffa8 100644 --- a/internal/bin/extract_test.go +++ b/internal/bin/extract_test.go @@ -1,8 +1,8 @@ package bin_test import ( + "archive/tar" "bytes" - "io" . "gopkg.in/check.v1" @@ -21,18 +21,16 @@ func (s *S) TestPkgTarStream(c *C) { testutil.Reg(0o644, "./file", "content"), })))) - tarStream, err := pkg.TarStream() - c.Assert(err, IsNil) - err = tarStream.Close() - c.Assert(err, IsNil) - - // A second stream can be obtained after rewinding. - _, err = pkg.Seek(0, io.SeekStart) - c.Assert(err, IsNil) - tarStream, err = pkg.TarStream() - c.Assert(err, IsNil) - err = tarStream.Close() - c.Assert(err, IsNil) + // Each call returns a fresh stream over the same content. + for i := 0; i < 2; i++ { + tarStream, err := pkg.TarStream() + c.Assert(err, IsNil) + tarReader := tar.NewReader(tarStream) + _, err = tarReader.Next() + c.Assert(err, IsNil) + err = tarStream.Close() + c.Assert(err, IsNil) + } } func (s *S) TestPkgTarStreamInvalid(c *C) { diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 4dc926de4..5cf6dc6fc 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -23,6 +23,10 @@ func OpenPkg(reader io.ReadSeekCloser) *Pkg { // TarStream returns a reader over the data tarball of the Debian // package, reading from the current position. func (p *Pkg) TarStream() (io.ReadCloser, error) { + _, err := p.reader.Seek(0, io.SeekStart) + if err != nil { + return nil, err + } arReader := ar.NewReader(p.reader) var dataReader io.ReadCloser for dataReader == nil { @@ -58,10 +62,6 @@ func (p *Pkg) TarStream() (io.ReadCloser, error) { return dataReader, nil } -func (p *Pkg) Seek(offset int64, whence int) (int64, error) { - return p.reader.Seek(offset, whence) -} - func (p *Pkg) Close() error { return p.reader.Close() } diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 7342a4990..1b90474ca 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -1,8 +1,8 @@ package deb_test import ( + "archive/tar" "bytes" - "io" . "gopkg.in/check.v1" @@ -18,16 +18,39 @@ func (s *S) TestPkgTarStream(c *C) { pkg := deb.OpenPkg(testutil.ReadSeekNopCloser( bytes.NewReader(testutil.PackageData["test-package"]))) - tarStream, err := pkg.TarStream() - c.Assert(err, IsNil) - err = tarStream.Close() - c.Assert(err, IsNil) + // Each call returns a fresh stream over the same content. + for i := 0; i < 2; i++ { + tarStream, err := pkg.TarStream() + c.Assert(err, IsNil) + tarReader := tar.NewReader(tarStream) + _, err = tarReader.Next() + c.Assert(err, IsNil) + err = tarStream.Close() + c.Assert(err, IsNil) + } +} - // A second stream can be obtained after rewinding. - _, err = pkg.Seek(0, io.SeekStart) - c.Assert(err, IsNil) - tarStream, err = pkg.TarStream() - c.Assert(err, IsNil) - err = tarStream.Close() +func (s *S) TestPkgExtract(c *C) { + pkg := deb.OpenPkg(testutil.ReadSeekNopCloser( + bytes.NewReader(testutil.PackageData["test-package"]))) + + dir := c.MkDir() + err := tarball.Extract(pkg, &tarball.ExtractOptions{ + Package: "test-package", + TargetDir: dir, + Extract: map[string][]tarball.ExtractInfo{ + "/dir/file": {{Path: "/dir/file"}}, + "/dir/nested/": {{ + Path: "/dir/nested/", + }}, + }, + }) c.Assert(err, IsNil) + + result := testutil.TreeDump(dir) + c.Assert(result, DeepEquals, map[string]string{ + "/dir/": "dir 0755", + "/dir/file": "file 0644 cc55e2ec", + "/dir/nested/": "dir 0755", + }) } diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index ebd0eb400..3253093af 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,7 +16,6 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" @@ -147,7 +146,7 @@ func Run(options *RunOptions) error { } // Fetch all packages, using the selection order. - packages := make(map[string]io.ReadSeekCloser) + packages := make(map[string]tarball.PkgReader) var pkgInfos []manifestutil.PackageInfo for _, slice := range options.Selection.Slices { if packages[slice.Package] != nil { @@ -240,7 +239,7 @@ func Run(options *RunOptions) error { if reader == nil { continue } - err := tarball.Extract(deb.OpenPkg(reader), &tarball.ExtractOptions{ + err := tarball.Extract(reader, &tarball.ExtractOptions{ Package: slice.Package, Extract: extract[slice.Package], TargetDir: targetDir, diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index a9d584e47..ca36eaf90 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -16,12 +16,12 @@ import ( "github.com/canonical/chisel/internal/strdist" ) -// PkgReader provides the tar stream of a package. TarStream reads from -// the current position, which the caller controls through Seek. +// PkgReader provides the tar stream of a package. TarStream must +// always provide a fresh reader, from the start. type PkgReader interface { // TarStream returns a reader over the raw, unparsed tar stream. TarStream() (io.ReadCloser, error) - io.Seeker + io.Closer } type ExtractOptions struct { @@ -268,10 +268,6 @@ func extractData(pkg PkgReader, options *ExtractOptions) error { ExtractOptions: options, pendingLinks: pendingHardLinks, } - _, err := pkg.Seek(0, io.SeekStart) - if err != nil { - return err - } err = extractHardLinks(pkg, extractHardLinkOptions) if err != nil { return err diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index 4c51808b5..93b5ff15c 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -1,7 +1,6 @@ package tarball_test import ( - "bytes" "os" "path" "path/filepath" @@ -10,7 +9,6 @@ import ( . "gopkg.in/check.v1" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" @@ -18,7 +16,7 @@ import ( type extractTest struct { summary string - pkgdata []byte + pkg *testutil.TestPkg options tarball.ExtractOptions hackopt func(c *C, o *tarball.ExtractOptions) result map[string]string @@ -29,14 +27,14 @@ type extractTest struct { var extractTests = []extractTest{{ summary: "Extract nothing", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: nil, }, result: map[string]string{}, }, { summary: "Extract a few entries", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/file": []tarball.ExtractInfo{{ @@ -70,7 +68,7 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Extract a few entries, nil Create closure", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/file": []tarball.ExtractInfo{{ @@ -106,7 +104,7 @@ var extractTests = []extractTest{{ }, }, { summary: "Copy a couple of entries elsewhere", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/file": []tarball.ExtractInfo{{ @@ -128,7 +126,7 @@ var extractTests = []extractTest{{ notCreated: []string{"/foo/", "/foo/bar/"}, }, { summary: "Copy same file twice", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/file": []tarball.ExtractInfo{{ @@ -148,7 +146,7 @@ var extractTests = []extractTest{{ notCreated: []string{"/dir/bar/", "/dir/foo/"}, }, { summary: "Globbing a single dir level", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/s*/": []tarball.ExtractInfo{{ @@ -163,7 +161,7 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Globbing for files with multiple levels at once", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/s**": []tarball.ExtractInfo{{ @@ -181,7 +179,7 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Globbing multiple paths", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/s**": []tarball.ExtractInfo{{ @@ -203,7 +201,7 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Globbing must have matching source and target", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/foo/b**": []tarball.ExtractInfo{{ @@ -214,7 +212,7 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": when using wildcards source and target paths must match: /foo/b\*\*`, }, { summary: "Globbing must also have a single target", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/foo/b**": []tarball.ExtractInfo{{ @@ -227,7 +225,7 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": when using wildcards source and target paths must match: /foo/b\*\*`, }, { summary: "Globbing cannot change modes", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/n**": []tarball.ExtractInfo{{ @@ -239,7 +237,7 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": when using wildcards source and target paths must match: /dir/n\*\*`, }, { summary: "Missing file", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/missing-file": []tarball.ExtractInfo{{ @@ -250,7 +248,7 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": no content at /missing-file`, }, { summary: "Missing directory", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/missing-dir/": []tarball.ExtractInfo{{ @@ -261,7 +259,7 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": no content at /missing-dir/`, }, { summary: "Missing glob", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/missing-dir/**": []tarball.ExtractInfo{{ @@ -272,7 +270,7 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": no content at /missing-dir/\*\*`, }, { summary: "Missing multiple entries", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/missing-file": []tarball.ExtractInfo{{ @@ -286,7 +284,7 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": no content at:\n- /missing-dir/\n- /missing-file`, }, { summary: "Optional entries may be missing", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/": []tarball.ExtractInfo{{ @@ -308,7 +306,7 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Optional entries mixed in cannot be missing", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/missing-file": []tarball.ExtractInfo{{ @@ -323,11 +321,11 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": no content at /dir/missing-file`, }, { summary: "Extract non-ASCII path and preserve parent directories permissions", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Dir(0766, "./日本/"), testutil.Reg(0644, "./日本/語", "whatever"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/日本/語": []tarball.ExtractInfo{{ @@ -342,7 +340,7 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Entries for same destination must have the same mode", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/": []tarball.ExtractInfo{{ @@ -357,11 +355,11 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`, }, { summary: "Single hard link entry can be extracted with the content entry", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "text for file"), testutil.Hrd(0644, "./hardlink", "./file"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/**": []tarball.ExtractInfo{{ @@ -376,11 +374,11 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Single hard link entry can be extracted without the content entry", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Reg(0644, "./file", "text for file"), testutil.Hrd(0644, "./hardlink", "./file"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/hardlink": []tarball.ExtractInfo{{ @@ -394,10 +392,10 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Dangling hard link", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Hrd(0644, "./hardlink", "./non-existing-target"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/hardlink": []tarball.ExtractInfo{{ @@ -408,11 +406,11 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": cannot create hard link /hardlink: no content at /non-existing-target`, }, { summary: "Multiple dangling hard links", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Hrd(0644, "./hardlink1", "./non-existing-target"), testutil.Hrd(0644, "./hardlink2", "./non-existing-target"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/**": []tarball.ExtractInfo{{ @@ -423,11 +421,11 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": cannot create hard link /hardlink1: no content at /non-existing-target`, }, { summary: "Hard link does not follow the symlink", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Lnk(0644, "./symlink", "./file"), testutil.Hrd(0644, "./hardlink", "./symlink"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/**": []tarball.ExtractInfo{{ @@ -442,7 +440,7 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Explicit extraction overrides existing file", - pkgdata: testutil.PackageData["test-package"], + pkg: testutil.NewTestPkg(testutil.TestPackageEntries...), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/": []tarball.ExtractInfo{{ @@ -461,10 +459,10 @@ var extractTests = []extractTest{{ notCreated: []string{}, }, { summary: "Hardlink cannot escape target directory", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Hrd(0644, "./hardlink", "/etc/group"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/**": []tarball.ExtractInfo{{ @@ -475,10 +473,10 @@ var extractTests = []extractTest{{ error: `cannot extract from package "test-package": invalid link target /etc/group`, }, { summary: "Cannot extract outside of target directory", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Reg(0644, "./../file", "hijacking system file"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/**": []tarball.ExtractInfo{{ @@ -512,7 +510,7 @@ func (s *S) TestExtract(c *C) { test.hackopt(c, &options) } - err := tarball.Extract(deb.OpenPkg(testutil.ReadSeekNopCloser(bytes.NewReader(test.pkgdata))), &options) + err := tarball.Extract(test.pkg, &options) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue @@ -539,16 +537,16 @@ func (s *S) TestExtract(c *C) { var extractCreateCallbackTests = []struct { summary string - pkgdata []byte + pkg *testutil.TestPkg options tarball.ExtractOptions calls map[string][]tarball.ExtractInfo }{{ summary: "Create is called with the set of ExtractInfo(s) that match the file", - pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{ + pkg: testutil.NewTestPkg( testutil.Dir(0755, "./"), testutil.Dir(0766, "./dir/"), testutil.Reg(0644, "./dir/file", "whatever"), - }), + ), options: tarball.ExtractOptions{ Extract: map[string][]tarball.ExtractInfo{ "/dir/": []tarball.ExtractInfo{{ @@ -623,7 +621,7 @@ func (s *S) TestExtractCreateCallback(c *C) { return nil } - err := tarball.Extract(deb.OpenPkg(testutil.ReadSeekNopCloser(bytes.NewReader(test.pkgdata))), &options) + err := tarball.Extract(test.pkg, &options) c.Assert(err, IsNil) c.Assert(createExtractInfos, DeepEquals, test.calls) diff --git a/internal/testutil/archive.go b/internal/testutil/archive.go index d06fd1b0c..260dd608e 100644 --- a/internal/testutil/archive.go +++ b/internal/testutil/archive.go @@ -3,9 +3,10 @@ package testutil import ( "bytes" "fmt" - "io" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/deb" + "github.com/canonical/chisel/internal/tarball" ) type TestArchive struct { @@ -26,7 +27,7 @@ func (a *TestArchive) Options() *archive.Options { return &a.Opts } -func (a *TestArchive) Fetch(pkgName string) (io.ReadSeekCloser, *archive.PackageInfo, error) { +func (a *TestArchive) Fetch(pkgName string) (tarball.PkgReader, *archive.PackageInfo, error) { pkg, ok := a.Packages[pkgName] if !ok { return nil, nil, fmt.Errorf("cannot find package %q in archive", pkgName) @@ -37,7 +38,7 @@ func (a *TestArchive) Fetch(pkgName string) (io.ReadSeekCloser, *archive.Package SHA256: pkg.Hash, Arch: pkg.Arch, } - return ReadSeekNopCloser(bytes.NewReader(pkg.Data)), info, nil + return deb.OpenPkg(ReadSeekNopCloser(bytes.NewReader(pkg.Data))), info, nil } func (a *TestArchive) Exists(pkg string) bool { diff --git a/internal/testutil/pkgreader.go b/internal/testutil/pkgreader.go new file mode 100644 index 000000000..be159ae38 --- /dev/null +++ b/internal/testutil/pkgreader.go @@ -0,0 +1,30 @@ +package testutil + +import ( + "bytes" + "io" +) + +// TestPkg is a tarball.PkgReader over an in-memory tar stream built from +// the given entries. +type TestPkg struct { + tarData []byte +} + +// NewTestPkg returns a TestPkg holding a tar stream with the given entries. +func NewTestPkg(entries ...TarEntry) *TestPkg { + data, err := makeTar(entries) + if err != nil { + panic(err) + } + return &TestPkg{tarData: data} +} + +// TarStream returns a fresh reader over the tar stream. +func (p *TestPkg) TarStream() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(p.tarData)), nil +} + +func (p *TestPkg) Close() error { + return nil +} From 50b03d1bc47c63270ff5df80ee9c47b473647dc2 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 14 Sep 2026 08:57:56 +0200 Subject: [PATCH 18/21] tests: polish tests --- internal/bin/extract_test.go | 15 ++++++--------- internal/deb/extract_test.go | 7 ++----- internal/tarball/extract.go | 2 +- internal/testutil/pkgreader.go | 13 +++++++++++++ 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/internal/bin/extract_test.go b/internal/bin/extract_test.go index 5af77ffa8..dc191d040 100644 --- a/internal/bin/extract_test.go +++ b/internal/bin/extract_test.go @@ -2,7 +2,6 @@ package bin_test import ( "archive/tar" - "bytes" . "gopkg.in/check.v1" @@ -15,14 +14,13 @@ import ( var _ tarball.PkgReader = (*bin.Pkg)(nil) func (s *S) TestPkgTarStream(c *C) { - pkg := bin.OpenPkg(testutil.ReadSeekNopCloser( - bytes.NewReader(testutil.MustMakeBin([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "content"), - })))) + pkg := testutil.NewBinPkg(testutil.MustMakeBin([]testutil.TarEntry{ + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "content"), + })) // Each call returns a fresh stream over the same content. - for i := 0; i < 2; i++ { + for range 2 { tarStream, err := pkg.TarStream() c.Assert(err, IsNil) tarReader := tar.NewReader(tarStream) @@ -34,8 +32,7 @@ func (s *S) TestPkgTarStream(c *C) { } func (s *S) TestPkgTarStreamInvalid(c *C) { - pkg := bin.OpenPkg(testutil.ReadSeekNopCloser( - bytes.NewReader([]byte("not an xz stream")))) + pkg := testutil.NewBinPkg([]byte("not an xz stream")) _, err := pkg.TarStream() c.Assert(err, ErrorMatches, "xz.*") diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index 1b90474ca..d81fb9c33 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -2,7 +2,6 @@ package deb_test import ( "archive/tar" - "bytes" . "gopkg.in/check.v1" @@ -15,8 +14,7 @@ import ( var _ tarball.PkgReader = (*deb.Pkg)(nil) func (s *S) TestPkgTarStream(c *C) { - pkg := deb.OpenPkg(testutil.ReadSeekNopCloser( - bytes.NewReader(testutil.PackageData["test-package"]))) + pkg := testutil.NewDebPkg(testutil.PackageData["test-package"]) // Each call returns a fresh stream over the same content. for i := 0; i < 2; i++ { @@ -31,8 +29,7 @@ func (s *S) TestPkgTarStream(c *C) { } func (s *S) TestPkgExtract(c *C) { - pkg := deb.OpenPkg(testutil.ReadSeekNopCloser( - bytes.NewReader(testutil.PackageData["test-package"]))) + pkg := testutil.NewDebPkg(testutil.PackageData["test-package"]) dir := c.MkDir() err := tarball.Extract(pkg, &tarball.ExtractOptions{ diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index ca36eaf90..6d243e87c 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -16,7 +16,7 @@ import ( "github.com/canonical/chisel/internal/strdist" ) -// PkgReader provides the tar stream of a package. TarStream must +// PkgReader provides the tar stream of a package. TarStream must // always provide a fresh reader, from the start. type PkgReader interface { // TarStream returns a reader over the raw, unparsed tar stream. diff --git a/internal/testutil/pkgreader.go b/internal/testutil/pkgreader.go index be159ae38..499d1befe 100644 --- a/internal/testutil/pkgreader.go +++ b/internal/testutil/pkgreader.go @@ -3,6 +3,9 @@ package testutil import ( "bytes" "io" + + "github.com/canonical/chisel/internal/bin" + "github.com/canonical/chisel/internal/deb" ) // TestPkg is a tarball.PkgReader over an in-memory tar stream built from @@ -28,3 +31,13 @@ func (p *TestPkg) TarStream() (io.ReadCloser, error) { func (p *TestPkg) Close() error { return nil } + +// NewDebPkg returns a deb.Pkg over the given Debian package data. +func NewDebPkg(data []byte) *deb.Pkg { + return deb.OpenPkg(ReadSeekNopCloser(bytes.NewReader(data))) +} + +// NewBinPkg returns a bin.Pkg over the given bin package data. +func NewBinPkg(data []byte) *bin.Pkg { + return bin.OpenPkg(ReadSeekNopCloser(bytes.NewReader(data))) +} From 798ce59c28364f722e5d9316a203a630eca956e9 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 14 Sep 2026 08:58:33 +0200 Subject: [PATCH 19/21] docs: fix misleading docs --- internal/bin/extract.go | 2 +- internal/deb/extract.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/bin/extract.go b/internal/bin/extract.go index 33fa29e1b..052c85080 100644 --- a/internal/bin/extract.go +++ b/internal/bin/extract.go @@ -18,7 +18,7 @@ func OpenPkg(reader io.ReadSeekCloser) *Pkg { } // TarStream returns a reader over the tar stream of the bin package, -// reading from the current position. +// from the start of the package. func (p *Pkg) TarStream() (io.ReadCloser, error) { _, err := p.reader.Seek(0, io.SeekStart) if err != nil { diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 5cf6dc6fc..187b69333 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -21,7 +21,7 @@ func OpenPkg(reader io.ReadSeekCloser) *Pkg { } // TarStream returns a reader over the data tarball of the Debian -// package, reading from the current position. +// package, from the start of the package. func (p *Pkg) TarStream() (io.ReadCloser, error) { _, err := p.reader.Seek(0, io.SeekStart) if err != nil { From ec2c67c6ec3286fde992e9c7382965c78dfce32f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 14 Sep 2026 09:29:49 +0200 Subject: [PATCH 20/21] refactor: rename to fit new abstraction --- internal/tarball/extract.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 6d243e87c..d1f11ea28 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -86,10 +86,10 @@ func Extract(pkg PkgReader, options *ExtractOptions) (err error) { return err } - return extractData(pkg, validOpts) + return extractEntries(pkg, validOpts) } -func extractData(pkg PkgReader, options *ExtractOptions) error { +func extractEntries(pkg PkgReader, options *ExtractOptions) error { tarStream, err := pkg.TarStream() if err != nil { return err From 6f2fe07757c36d05c744b1122c9f9bf3eb404440 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 16 Sep 2026 08:21:07 +0200 Subject: [PATCH 21/21] docs: refine docs --- internal/bin/extract.go | 5 ----- internal/bin/extract_test.go | 1 - internal/deb/extract.go | 6 ++---- internal/deb/extract_test.go | 1 - internal/tarball/extract.go | 3 +-- internal/testutil/pkgdata.go | 2 -- internal/testutil/pkgreader.go | 4 +--- 7 files changed, 4 insertions(+), 18 deletions(-) diff --git a/internal/bin/extract.go b/internal/bin/extract.go index 052c85080..fc5508275 100644 --- a/internal/bin/extract.go +++ b/internal/bin/extract.go @@ -6,19 +6,14 @@ import ( "github.com/ulikunitz/xz" ) -// Pkg reads the tar stream of a bin package held in a seekable reader. -// A bin package is a plain XZ-compressed tarball. type Pkg struct { reader io.ReadSeekCloser } -// OpenPkg wraps a seekable reader over bin package data. func OpenPkg(reader io.ReadSeekCloser) *Pkg { return &Pkg{reader: reader} } -// TarStream returns a reader over the tar stream of the bin package, -// from the start of the package. func (p *Pkg) TarStream() (io.ReadCloser, error) { _, err := p.reader.Seek(0, io.SeekStart) if err != nil { diff --git a/internal/bin/extract_test.go b/internal/bin/extract_test.go index dc191d040..4d60358d0 100644 --- a/internal/bin/extract_test.go +++ b/internal/bin/extract_test.go @@ -10,7 +10,6 @@ import ( "github.com/canonical/chisel/internal/testutil" ) -// Compile-time check that Pkg implements tarball.PkgReader. var _ tarball.PkgReader = (*bin.Pkg)(nil) func (s *S) TestPkgTarStream(c *C) { diff --git a/internal/deb/extract.go b/internal/deb/extract.go index 187b69333..4b56e12e5 100644 --- a/internal/deb/extract.go +++ b/internal/deb/extract.go @@ -10,18 +10,16 @@ import ( "github.com/ulikunitz/xz" ) -// Pkg reads the tar stream of a Debian package held in a seekable reader. type Pkg struct { reader io.ReadSeekCloser } -// OpenPkg wraps a seekable reader over Debian package data. func OpenPkg(reader io.ReadSeekCloser) *Pkg { return &Pkg{reader: reader} } -// TarStream returns a reader over the data tarball of the Debian -// package, from the start of the package. +// TarStream returns a ReadCloser to the inner tarball of +// a Debian package. func (p *Pkg) TarStream() (io.ReadCloser, error) { _, err := p.reader.Seek(0, io.SeekStart) if err != nil { diff --git a/internal/deb/extract_test.go b/internal/deb/extract_test.go index d81fb9c33..73f8d245f 100644 --- a/internal/deb/extract_test.go +++ b/internal/deb/extract_test.go @@ -10,7 +10,6 @@ import ( "github.com/canonical/chisel/internal/testutil" ) -// Compile-time check that Pkg implements tarball.PkgReader. var _ tarball.PkgReader = (*deb.Pkg)(nil) func (s *S) TestPkgTarStream(c *C) { diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index d1f11ea28..4dec52afd 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -16,10 +16,9 @@ import ( "github.com/canonical/chisel/internal/strdist" ) -// PkgReader provides the tar stream of a package. TarStream must -// always provide a fresh reader, from the start. type PkgReader interface { // TarStream returns a reader over the raw, unparsed tar stream. + // Each call returns a fresh stream, from its start. TarStream() (io.ReadCloser, error) io.Closer } diff --git a/internal/testutil/pkgdata.go b/internal/testutil/pkgdata.go index 84ff79cec..5eca2e8bb 100644 --- a/internal/testutil/pkgdata.go +++ b/internal/testutil/pkgdata.go @@ -176,8 +176,6 @@ func compressBytesXZ(input []byte) ([]byte, error) { return buf.Bytes(), nil } -// MakeBin returns the bytes of a bin package holding the given tar -// entries: a plain XZ-compressed tarball. func MakeBin(entries []TarEntry) ([]byte, error) { tarData, err := makeTar(entries) if err != nil { diff --git a/internal/testutil/pkgreader.go b/internal/testutil/pkgreader.go index 499d1befe..3ecb4ac2a 100644 --- a/internal/testutil/pkgreader.go +++ b/internal/testutil/pkgreader.go @@ -8,8 +8,7 @@ import ( "github.com/canonical/chisel/internal/deb" ) -// TestPkg is a tarball.PkgReader over an in-memory tar stream built from -// the given entries. +// TestPkg is a PkgReader over an in-memory tar stream. type TestPkg struct { tarData []byte } @@ -23,7 +22,6 @@ func NewTestPkg(entries ...TarEntry) *TestPkg { return &TestPkg{tarData: data} } -// TarStream returns a fresh reader over the tar stream. func (p *TestPkg) TarStream() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(p.tarData)), nil }