Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

archive: Add Info() method #69

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions internal/archive/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,17 @@ import (
"github.com/canonical/chisel/internal/deb"
)

type PackageInfo interface {
Name() string
Version() string
Arch() string
SHA256() string
}

type Archive interface {
Options() *Options
Fetch(pkg string) (io.ReadCloser, error)
Info(pkg string) PackageInfo
Exists(pkg string) bool
}

Expand Down Expand Up @@ -86,6 +94,22 @@ func (a *ubuntuArchive) Exists(pkg string) bool {
return err == nil
}

type pkgInfo struct{ control.Section }

var _ PackageInfo = pkgInfo{}

func (info pkgInfo) Name() string { return info.Get("Package") }
func (info pkgInfo) Version() string { return info.Get("Version") }
func (info pkgInfo) Arch() string { return info.Get("Architecture") }
func (info pkgInfo) SHA256() string { return info.Get("SHA256") }

func (a *ubuntuArchive) Info(pkg string) PackageInfo {
if section, _, _ := a.selectPackage(pkg); section != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why drop errors on the floor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just felt that the Info() method (and selectPackage() too) resembles a map read and since it's guaranteed to not hold nil values and will not fail in any expected way (as selectPackage only returns error when it doesn't find an entry in the map), it can just return nil on missing entry.

I understand that this is an interface and so it should be designed for unknown future changes to the implementation, but I assumed that Archive instances are expected to be fully loaded with archive metadata after creation. If e.g. we introduced some lazy loading in future, then even Exists() would have to return error in case lazy loading fails.

Anyway, after #80 gets merged, I'm going to update to return err from selectPackage. Python's dictionaries also fail on KeyError 🤷

return &pkgInfo{section}
}
return nil
}

func (a *ubuntuArchive) selectPackage(pkg string) (control.Section, *ubuntuIndex, error) {
var selectedVersion string
var selectedSection control.Section
Expand Down
33 changes: 33 additions & 0 deletions internal/archive/archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,39 @@ func (s *httpSuite) TestArchiveLabels(c *C) {
c.Assert(err, ErrorMatches, `.*\bno Ubuntu section`)
}

func (s *httpSuite) TestPackageInfo(c *C) {
s.prepareArchive("jammy", "22.04", "amd64", []string{"main", "universe"})

options := archive.Options{
Label: "ubuntu",
Version: "22.04",
Arch: "amd64",
Suites: []string{"jammy"},
Components: []string{"main", "universe"},
CacheDir: c.MkDir(),
}

archive, err := archive.Open(&options)
c.Assert(err, IsNil)

info1 := archive.Info("mypkg1")
c.Assert(info1, NotNil)
c.Assert(info1.Name(), Equals, "mypkg1")
c.Assert(info1.Version(), Equals, "1.1")
c.Assert(info1.Arch(), Equals, "amd64")
c.Assert(info1.SHA256(), Equals, "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05")

info3 := archive.Info("mypkg3")
c.Assert(info3, NotNil)
c.Assert(info3.Name(), Equals, "mypkg3")
c.Assert(info3.Version(), Equals, "1.3")
c.Assert(info3.Arch(), Equals, "amd64")
c.Assert(info3.SHA256(), Equals, "fe377bf13ba1a5cb287cb4e037e6e7321281c929405ae39a72358ef0f5d179aa")

info99 := archive.Info("mypkg99")
c.Assert(info99, IsNil)
}

func read(r io.Reader) string {
data, err := io.ReadAll(r)
if err != nil {
Expand Down
141 changes: 133 additions & 8 deletions internal/slicer/slicer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,21 @@ import (
"github.com/canonical/chisel/internal/testutil"
)

var (
Reg = testutil.Reg
Dir = testutil.Dir
Lnk = testutil.Lnk
)

type testPackage struct {
info map[string]string
content []byte
}

type slicerTest struct {
summary string
arch string
pkgs map[string]map[string]testPackage
release map[string]string
slices []setup.SliceKey
hackopt func(c *C, opts *slicer.RunOptions)
Expand Down Expand Up @@ -515,6 +527,77 @@ var slicerTests = []slicerTest{{
"/usr/bin/": "dir 0755",
"/usr/bin/hello": "file 0775 eaf29575",
},
}, {
summary: "Custom archives with custom packages",
pkgs: map[string]map[string]testPackage{
"leptons": {
"electron": testPackage{
content: testutil.MustMakeDeb([]testutil.TarEntry{
Dir(0755, "./"),
Dir(0755, "./mass/"),
Reg(0644, "./mass/electron", "9.1093837015E−31 kg\n"),
Dir(0755, "./usr/"),
Dir(0755, "./usr/share/"),
Dir(0755, "./usr/share/doc/"),
Dir(0755, "./usr/share/doc/electron/"),
Reg(0644, "./usr/share/doc/electron/copyright", ""),
}),
},
},
"hadrons": {
"proton": testPackage{
content: testutil.MustMakeDeb([]testutil.TarEntry{
Dir(0755, "./"),
Dir(0755, "./mass/"),
Reg(0644, "./mass/proton", "1.67262192369E−27 kg\n"),
}),
},
},
},
release: map[string]string{
"chisel.yaml": `
format: chisel-v1
archives:
leptons:
version: 1
suites: [main]
components: [main, universe]
default: true
hadrons:
version: 1
suites: [main]
components: [main]
`,
"slices/mydir/electron.yaml": `
package: electron
slices:
mass:
contents:
/mass/electron:
`,
"slices/mydir/proton.yaml": `
package: proton
archive: hadrons
slices:
mass:
contents:
/mass/proton:
`,
},
slices: []setup.SliceKey{
{"electron", "mass"},
{"proton", "mass"},
},
result: map[string]string{
"/mass/": "dir 0755",
"/mass/electron": "file 0644 a1258e30",
"/mass/proton": "file 0644 a2390d10",
"/usr/": "dir 0755",
"/usr/share/": "dir 0755",
"/usr/share/doc/": "dir 0755",
"/usr/share/doc/electron/": "dir 0755",
"/usr/share/doc/electron/copyright": "file 0644 empty",
},
}}

const defaultChiselYaml = `
Expand All @@ -525,9 +608,25 @@ const defaultChiselYaml = `
components: [main, universe]
`

type testPackageInfo map[string]string

var _ archive.PackageInfo = (testPackageInfo)(nil)

func (info testPackageInfo) Name() string { return info["Package"] }
func (info testPackageInfo) Version() string { return info["Version"] }
func (info testPackageInfo) Arch() string { return info["Architecture"] }
func (info testPackageInfo) SHA256() string { return info["SHA256"] }

func (s testPackageInfo) Get(key string) (value string) {
if s != nil {
value = s[key]
}
return
}

type testArchive struct {
options archive.Options
pkgs map[string][]byte
pkgs map[string]testPackage
}

func (a *testArchive) Options() *archive.Options {
Expand All @@ -536,7 +635,7 @@ func (a *testArchive) Options() *archive.Options {

func (a *testArchive) Fetch(pkg string) (io.ReadCloser, error) {
if data, ok := a.pkgs[pkg]; ok {
return io.NopCloser(bytes.NewBuffer(data)), nil
return io.NopCloser(bytes.NewBuffer(data.content)), nil
}
return nil, fmt.Errorf("attempted to open %q package", pkg)
}
Expand All @@ -546,6 +645,18 @@ func (a *testArchive) Exists(pkg string) bool {
return ok
}

func (a *testArchive) Info(pkg string) archive.PackageInfo {
var info map[string]string
if pkgData, ok := a.pkgs[pkg]; ok {
if info = pkgData.info; info == nil {
info = map[string]string{
"Version": "1.0",
}
}
}
return testPackageInfo(info)
}

func (s *S) TestRun(c *C) {
for _, test := range slicerTests {
c.Logf("Summary: %s", test.summary)
Expand All @@ -569,16 +680,23 @@ func (s *S) TestRun(c *C) {
selection, err := setup.Select(release, test.slices)
c.Assert(err, IsNil)

pkgs := map[string][]byte{
"base-files": testutil.PackageData["base-files"],
pkgs := map[string]testPackage{
"base-files": testPackage{content: testutil.PackageData["base-files"]},
}
for name, entries := range packageEntries {
deb, err := testutil.MakeDeb(entries)
c.Assert(err, IsNil)
pkgs[name] = deb
pkgs[name] = testPackage{content: deb}
}
archives := map[string]archive.Archive{}
for name, setupArchive := range release.Archives {
var archivePkgs map[string]testPackage
if test.pkgs != nil {
archivePkgs = test.pkgs[name]
}
if archivePkgs == nil {
archivePkgs = pkgs
}
archive := &testArchive{
options: archive.Options{
Label: setupArchive.Name,
Expand All @@ -587,7 +705,7 @@ func (s *S) TestRun(c *C) {
Components: setupArchive.Components,
Arch: test.arch,
},
pkgs: pkgs,
pkgs: archivePkgs,
}
archives[name] = archive
}
Expand All @@ -611,8 +729,15 @@ func (s *S) TestRun(c *C) {

if test.result != nil {
result := make(map[string]string, len(copyrightEntries)+len(test.result))
for k, v := range copyrightEntries {
result[k] = v
if test.pkgs == nil {
// This was added in order to not specify copyright entries for each
// existing test. These tests use only the base-files embedded
// package. Custom packages may not include copyright entries
// though. So if a test defines any custom packages, it must include
// copyright entries explicitly in the results.
for k, v := range copyrightEntries {
result[k] = v
}
}
for k, v := range test.result {
result[k] = v
Expand Down
Loading