feat: support extraction of multiple package types by upils · Pull Request #318 · canonical/chisel

upils · 2026-07-31T07:43:21Z

Have you signed the CLA?

Prepare the extraction path for upcoming bin packages (plain XZ-compressed
tarballs), which today cannot be handled because the tar walker is hard-wired
to the .deb container format.

tarball now consumes a PkgReader interface — a package that can provide
its tar stream, freshly and from the start, on every call. This keeps the
two-pass hard-link rewind inside tarball, shared by all formats, while
each format owns its container knowledge: deb.DataReader became
deb.Pkg.TarStream, and bin.Pkg is added under internal/bin.
Archive.Fetch now returns a PkgReader directly, so the package type is
transparent to the slicer.

The walker's tests use a minimal in-memory testutil.TestPkg double, so
tarball no longer depends on deb even in tests; the real implementations
keep their own extraction coverage, and the fake test archive now publishes
structurally real debs to match.

niemeyer · 2026-08-31T21:58:22Z

 )

+// OpenTarFunc opens the uncompressed tar stream carried by a package, hiding
+// the package format from Extract. An implementation may unwrap a container


This is documenting a "package" inside internal/tarball.

It's a bit late here so that doesn't help, but this PR seems to suffer a bit from lack of clarity and precision about the encapsulations it's introducing and changing. In theory, if you want to "extract bins", we shouldn't have to touch the interface of either deb or tarball.

Happy to be lessoned on what you had in mind, though.

The main goal of this PR is exactly to not have to touch the interface of either deb or tarball anymore in the future. But I think it is worth adapting it a bit right now when preparing for multiple tarball containers, rather than later.

So far the tarball extraction logic assumed a deb was extracted. The TarOpener abstracts how the container looks like (ar, etc.) and gives a io.ReadCloser to the tarball logic. deb.DataReader was also needlessly receiving a io.ReadSeeker when it only needs a io.Reader, so I refined it while defining the TarOpener interface.

As a result of these changes, tarball does not depend on deb anymore and the extraction logic does not assume it works on a deb either.

I have rework the documentation to avoid mixing independent concepts.

niemeyer

On last week's review and discussion on Tuesday I already provided some hints that this is not looking great. It seems to be the same as last week still, so still not looking great.

niemeyer · 2026-09-07T20:49:47Z

 				return nil, err
 			}
-			dataReader, err := deb.DataReader(pkgReader)
+			dataReader, err := deb.OpenTar(pkgReader)


This is not opening "a tar"... this is opening the deb's "data" payload, per implementation.

niemeyer · 2026-09-07T20:54:07Z

 }

-func Extract(pkgReader io.ReadSeeker, options *ExtractOptions) (err error) {
+func Extract(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) (err error) {


Why would this take both an "opener" and a "reader"? This extracting pkgReader, right?

Your analogy in the other comment clarified the issue with the approach.

niemeyer · 2026-09-07T20:57:01Z

-func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
-	dataReader, err := deb.DataReader(pkgReader)
+func extractData(pkgReader io.ReadSeeker, opener TarOpener, options *ExtractOptions) error {
+	dataReader, err := opener(pkgReader)


This is not a good encapsulation. Imagine I hand you my phone and a USB-C charging cable, and ask you "Paul, can you please connect my charging cable on my phone for me, I want to charge it.", how would that feel? That's what is going on here.

That analogy was helpful, thanks. I have drastically redesigned the approach in this PR, so concerns are better separated. With the new PkgReader interface, the tarball pkg clearly defines what it needs to operate and the various package types (deb, bin) are handling package-specific knowledge.

upils · 2026-09-14T07:32:31Z


-func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
-	dataReader, err := deb.DataReader(pkgReader)
+func extractEntries(pkg PkgReader, options *ExtractOptions) error {


[Note to reviewer]: As you pointed out in another comment, I realized extractData was named because it extracted the content of the "data" container from a deb. With the current more generic approach, the existing name made little sense.
I also considered doExtract for the name but extractEntries is more precise.

upils · 2026-09-14T09:43:25Z

@@ -0,0 +1,36 @@
+package bin


[Note to reviewer]: The introduction of this package can be done in a follow up PR if we want this one to focus on the reworked interface. I added it as this is rather small and it illustrates the work in this PR is not artificial.

upils · 2026-09-14T10:02:19Z

 	"github.com/canonical/chisel/internal/strdist"
 )

+// PkgReader provides the tar stream of a package. TarStream must


[Note to reviewer]: We may need to come up with a better name for this tarball package, as it does not deal with general tarballs. I suggest "pkgcontent". This is a compound but both pkg and content names had good pros but still cons if used alone.

If we proceed, we should do that in a follow-up to keep this PR focused on the introduced interface.

upils · 2026-09-14T10:08:40Z

 		Version: "1.1",
 		Arch:    "amd64",
-		SHA256:  "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05",
+		SHA256:  "ff175644a17301e047ac757681f6e16b1c410228d9fe50441bba8438a7c45fa2",


[Note to reviewer]: These hashes changed because the shape of the input in the test changed. A simple []byte cannot be used anymore as the testarchive.Package now expects a valid package, providing extractable content. However the actual content of the input package did not change, so this is a benign change.

upils added 4 commits July 27, 2026 15:29

feat: extract bins

f91d449

refactor: rework approach

1663446

refactor: refining

3435cee

refactor: refining

09a01bd

upils added the Simple Nice for a quick look on a minute or two label Jul 31, 2026

upils mentioned this pull request Jul 31, 2026

feat: extract bins upils/chisel#18

Open

1 task

docs: clean comments

8d87fae

niemeyer requested changes Aug 31, 2026

View reviewed changes

upils added 3 commits September 1, 2026 09:33

docs: remove mentions of packages in tarball pkg

1a38ccd

Merge branch 'main' into support-bin-extract

ca8d6e8

style: refine naming and docs

ee8732a

upils changed the title ~~feat: extract bins~~ refactor: make tarball.Extract package-format agnostic Sep 3, 2026

niemeyer requested changes Sep 7, 2026

View reviewed changes

refactor: rework approach

ca72fd6

upils changed the title ~~refactor: make tarball.Extract package-format agnostic~~ feat: add package format support to tarball extraction Sep 8, 2026

upils added 9 commits September 8, 2026 15:40

fix: clean superfluous import

dc8e94b

test: remove unused field for now

f882d47

refactor: put deb DataReader back in deb package

116752e

style: add missing trailing newline

e9c4d88

style: refine naming

b1ec390

style: improve naming consistency

a8ad813

style: avoid ambiguity

6ad029e

refactor: rework approach with interface

150e904

refactor: refine interface

791224b

upils changed the title ~~feat: add package format support to tarball extraction~~ feat: support extraction of multiple package types Sep 14, 2026

upils added 3 commits September 14, 2026 08:57

tests: polish tests

50b03d1

docs: fix misleading docs

798ce59

refactor: rename to fit new abstraction

ec2c67c

upils commented Sep 14, 2026

View reviewed changes

upils removed the Simple Nice for a quick look on a minute or two label Sep 14, 2026

docs: refine docs

6f2fe07

Navigation Menu

feat: support extraction of multiple package types - #318

feat: support extraction of multiple package types#318
upils wants to merge 22 commits into
canonical:mainfrom
upils:support-bin-extract

upils commented Jul 31, 2026 •

edited

Loading

Uh oh!

niemeyer Aug 31, 2026

Uh oh!

upils Sep 3, 2026 •

edited

Loading

Uh oh!

niemeyer left a comment

Uh oh!

niemeyer Sep 7, 2026

Uh oh!

niemeyer Sep 7, 2026

Uh oh!

upils Sep 14, 2026

Uh oh!

niemeyer Sep 7, 2026

Uh oh!

upils Sep 14, 2026

Uh oh!

upils Sep 14, 2026

Uh oh!

upils Sep 14, 2026 •

edited

Loading

Uh oh!

upils Sep 14, 2026

Uh oh!

upils Sep 14, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

upils commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

upils Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niemeyer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

upils Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

upils Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

upils commented Jul 31, 2026 •

edited

Loading

upils Sep 3, 2026 •

edited

Loading

upils Sep 14, 2026 •

edited

Loading

upils Sep 14, 2026 •

edited

Loading