Skip to content

refactor: PackageInfo interface #320

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.

Sign up for GitHub

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

Open
upils wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: PackageInfo interface #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions internal/archive/archive.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ type PackageInfo struct {
SHA256 string
}

func (p *PackageInfo) PkgName() string { return p.Name }
func (p *PackageInfo) PkgVersion() string { return p.Version }
func (p *PackageInfo) PkgRevision() int { return 0 }
func (p *PackageInfo) PkgArch() string { return p.Arch }
func (p *PackageInfo) PkgDigestKind() cache.DigestKind { return cache.SHA256 }
func (p *PackageInfo) PkgDigest() string { return p.SHA256 }

type Options struct {
Label string
Version string
Expand Down
52 changes: 36 additions & 16 deletions internal/manifestutil/manifestutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,26 @@ import (
"strings"

"github.com/canonical/chisel/internal/apacheutil"
"github.com/canonical/chisel/internal/archive"
"github.com/canonical/chisel/internal/cache"
"github.com/canonical/chisel/internal/setup"
"github.com/canonical/chisel/public/jsonwall"
"github.com/canonical/chisel/public/manifest"
)

// PackageInfo describes a package as obtained from its source, abstracting
// over archives, stores, and any other backend.
type PackageInfo interface {
PkgName() string
PkgVersion() string
// PkgRevision further identifies the package when the source versions are
// not unique on their own. It returns 0 when the source does not use
// revisions.
PkgRevision() int
PkgArch() string
PkgDigestKind() cache.DigestKind
PkgDigest() string
}

const DefaultFilename = "manifest.wall"

// FindPaths finds the paths marked with "generate:manifest" and
Expand All @@ -35,7 +49,7 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice {
}

type WriteOptions struct {
PackageInfo []*archive.PackageInfo
PackageInfo []PackageInfo
Selection []*setup.Slice
Report *Report
}
Expand Down Expand Up @@ -69,14 +83,14 @@ func Write(options *WriteOptions, writer io.Writer) error {
return err
}

func manifestAddPackages(dbw *jsonwall.DBWriter, infos []*archive.PackageInfo) error {
func manifestAddPackages(dbw *jsonwall.DBWriter, infos []PackageInfo) error {
for _, info := range infos {
err := dbw.Add(&manifest.Package{
Kind: "package",
Name: info.Name,
Version: info.Version,
Digest: info.SHA256,
Arch: info.Arch,
Name: info.PkgName(),
Version: info.PkgVersion(),
Digest: info.PkgDigest(),
Arch: info.PkgArch(),
})
if err != nil {
return err
Expand Down Expand Up @@ -155,7 +169,7 @@ func fastValidate(options *WriteOptions) (err error) {
if err != nil {
return err
}
pkgExist[pkg.Name] = true
pkgExist[pkg.PkgName()] = true
}
sliceExist := map[string]bool{}
for _, slice := range options.Selection {
Expand Down Expand Up @@ -250,18 +264,24 @@ func validateReportEntry(entry *ReportEntry) (err error) {
return nil
}

func validatePackage(pkg *archive.PackageInfo) (err error) {
if pkg.Name == "" {
func validatePackage(pkg PackageInfo) (err error) {
name := pkg.PkgName()
if name == "" {
return fmt.Errorf("package name not set")
}
if pkg.Arch == "" {
return fmt.Errorf("package %q missing arch", pkg.Name)
if pkg.PkgArch() == "" {
return fmt.Errorf("package %q missing arch", name)
}
if pkg.SHA256 == "" {
return fmt.Errorf("package %q missing sha256", pkg.Name)
// The manifest records the package digest as a SHA256 one. Fail rather
// than recording a digest of another kind under that name.
// TODO: record packages whose digest is not a SHA256 one, such as the
// ones coming from a store. This requires recording the digest kind in
// the manifest as well.
if pkg.PkgDigestKind() != cache.SHA256 || pkg.PkgDigest() == "" {
return fmt.Errorf("package %q missing sha256", name)
}
if pkg.Version == "" {
return fmt.Errorf("package %q missing version", pkg.Name)
if pkg.PkgVersion() == "" {
return fmt.Errorf("package %q missing version", name)
}
return nil
}
Expand Down
105 changes: 60 additions & 45 deletions internal/manifestutil/manifestutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ var slice2 = &setup.Slice{
var generateManifestTests = []struct {
summary string
report *manifestutil.Report
packageInfo []*archive.PackageInfo
packageInfo []manifestutil.PackageInfo
selection []*setup.Slice
expected *apachetestutil.ManifestContents
error string
Expand All @@ -148,17 +148,20 @@ var generateManifestTests = []struct {
},
},
},
packageInfo: []*archive.PackageInfo{{
Name: "package1",
Version: "v1",
Arch: "a1",
SHA256: "s1",
}, {
Name: "package2",
Version: "v2",
Arch: "a2",
SHA256: "s2",
}},
packageInfo: []manifestutil.PackageInfo{
&archive.PackageInfo{
Name: "package1",
Version: "v1",
Arch: "a1",
SHA256: "s1",
},
&archive.PackageInfo{
Name: "package2",
Version: "v2",
Arch: "a2",
SHA256: "s2",
},
},
expected: &apachetestutil.ManifestContents{
Paths: []*manifest.Path{{
Kind: "path",
Expand Down Expand Up @@ -241,7 +244,7 @@ var generateManifestTests = []struct {
},
},
},
packageInfo: []*archive.PackageInfo{},
packageInfo: []manifestutil.PackageInfo{},
error: `internal error: invalid manifest: slice package1_slice1 refers to missing package "package1"`,
}, {
summary: "Invalid path: slices is empty",
Expand Down Expand Up @@ -395,12 +398,14 @@ var generateManifestTests = []struct {
},
},
},
packageInfo: []*archive.PackageInfo{{
Name: "package1",
Version: "v1",
Arch: "a1",
SHA256: "s1",
}},
packageInfo: []manifestutil.PackageInfo{
&archive.PackageInfo{
Name: "package1",
Version: "v1",
Arch: "a1",
SHA256: "s1",
},
},
expected: &apachetestutil.ManifestContents{
Paths: []*manifest.Path{{
Kind: "path",
Expand Down Expand Up @@ -494,35 +499,43 @@ var generateManifestTests = []struct {
error: `internal error: invalid manifest: hard linked paths "/file" and "/hardlink" have diverging contents`,
}, {
summary: "Invalid package: missing name",
packageInfo: []*archive.PackageInfo{{
Version: "v1",
Arch: "a1",
SHA256: "s1",
}},
packageInfo: []manifestutil.PackageInfo{
&archive.PackageInfo{
Version: "v1",
Arch: "a1",
SHA256: "s1",
},
},
error: `internal error: invalid manifest: package name not set`,
}, {
summary: "Invalid package: missing version",
packageInfo: []*archive.PackageInfo{{
Name: "package-1",
Arch: "a1",
SHA256: "s1",
}},
packageInfo: []manifestutil.PackageInfo{
&archive.PackageInfo{
Name: "package-1",
Arch: "a1",
SHA256: "s1",
},
},
error: `internal error: invalid manifest: package "package-1" missing version`,
}, {
summary: "Invalid package: missing arch",
packageInfo: []*archive.PackageInfo{{
Name: "package-1",
Version: "v1",
SHA256: "s1",
}},
packageInfo: []manifestutil.PackageInfo{
&archive.PackageInfo{
Name: "package-1",
Version: "v1",
SHA256: "s1",
},
},
error: `internal error: invalid manifest: package "package-1" missing arch`,
}, {
summary: "Invalid package: missing sha256",
packageInfo: []*archive.PackageInfo{{
Name: "package-1",
Version: "v1",
Arch: "a1",
}},
packageInfo: []manifestutil.PackageInfo{
&archive.PackageInfo{
Name: "package-1",
Version: "v1",
Arch: "a1",
},
},
error: `internal error: invalid manifest: package "package-1" missing sha256`,
}}

Expand All @@ -533,12 +546,14 @@ func (s *S) TestGenerateManifests(c *C) {
test.selection = []*setup.Slice{slice1}
}
if test.packageInfo == nil {
test.packageInfo = []*archive.PackageInfo{{
Name: "package1",
Version: "v1",
Arch: "a1",
SHA256: "s1",
}}
test.packageInfo = []manifestutil.PackageInfo{
&archive.PackageInfo{
Name: "package1",
Version: "v1",
Arch: "a1",
SHA256: "s1",
},
}
}

options := &manifestutil.WriteOptions{
Expand Down
4 changes: 2 additions & 2 deletions internal/slicer/slicer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func Run(options *RunOptions) error {

// Fetch all packages, using the selection order.
packages := make(map[string]io.ReadSeekCloser)
var pkgInfos []*archive.PackageInfo
var pkgInfos []manifestutil.PackageInfo
for _, slice := range options.Selection.Slices {
if packages[slice.Package] != nil {
continue
Expand Down Expand Up @@ -352,7 +352,7 @@ func Run(options *RunOptions) error {
}

func generateManifests(targetDir string, selection *setup.Selection,
report *manifestutil.Report, pkgInfos []*archive.PackageInfo) error {
report *manifestutil.Report, pkgInfos []manifestutil.PackageInfo) error {
manifestSlices := manifestutil.FindPaths(selection.Slices)
if len(manifestSlices) == 0 {
// Nothing to do.
Expand Down
Loading