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

feat: add find command #99

Merged
merged 21 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
48 changes: 29 additions & 19 deletions cmd/chisel/cmd_cut.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,7 @@ func (cmd *cmdCut) Execute(args []string) error {
sliceKeys[i] = sliceKey
}

var release *setup.Release
var err error
if strings.Contains(cmd.Release, "/") {
release, err = setup.ReadRelease(cmd.Release)
} else {
var label, version string
if cmd.Release == "" {
label, version, err = readReleaseInfo()
} else {
label, version, err = parseReleaseInfo(cmd.Release)
}
if err != nil {
return err
}
release, err = setup.FetchRelease(&setup.FetchOptions{
Label: label,
Version: version,
})
}
release, _, err := getRelease(cmd.Release)
if err != nil {
return err
}
Expand Down Expand Up @@ -136,3 +118,31 @@ func readReleaseInfo() (label, version string, err error) {
}
return "", "", fmt.Errorf("cannot infer release via /etc/lsb-release, see the --release option")
}

// getRelease returns the release and release label (e.g. ubuntu-22.04 or
// /path/to/release/dir/ if a directory was passed as input).
func getRelease(releaseStr string) (release *setup.Release, releaseLabel string, err error) {
if strings.Contains(releaseStr, "/") {
release, err = setup.ReadRelease(releaseStr)
releaseLabel = releaseStr
} else {
var label, version string
if releaseStr == "" {
label, version, err = readReleaseInfo()
} else {
label, version, err = parseReleaseInfo(releaseStr)
}
if err != nil {
return nil, "", err
}
release, err = setup.FetchRelease(&setup.FetchOptions{
Label: label,
Version: version,
})
releaseLabel = label + "-" + version
}
if err != nil {
return nil, "", err
}
return release, releaseLabel, nil
}
119 changes: 119 additions & 0 deletions cmd/chisel/cmd_find.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package main

import (
"fmt"
"sort"
"strings"
"text/tabwriter"

"github.com/jessevdk/go-flags"

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

var shortFindHelp = "Find existing slices"
var longFindHelp = `
The find command queries the chisel releases for available slices.

With the --release flag, it queries for slices in a particular branch
of chisel-releases repository[1] or a particular directory. If left
unspecified, it queries with the release info found in /etc/lsb-release.
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved

[1] https://github.com/canonical/chisel-releases
`

var findDescs = map[string]string{
"release": "Chisel release branch or directory",
}

type cmdFind struct {
Release string `long:"release" value-name:"<branch|dir>"`
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved

Positional struct {
Query []string `positional-arg-name:"<query>" required:"yes"`
} `positional-args:"yes"`
}

func init() {
addCommand("find", shortFindHelp, longFindHelp, func() flags.Commander { return &cmdFind{} }, findDescs, nil)
}

func (cmd *cmdFind) Execute(args []string) error {
if len(args) > 0 {
return ErrExtraArgs
}

query := strings.TrimSpace(strings.Join(cmd.Positional.Query, " "))
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
if query == "" {
return fmt.Errorf("no search term specified")
}

release, releaseLabel, err := getRelease(cmd.Release)
if err != nil {
return err
}

slices, err := findSlices(release, query)
if err != nil {
return err
}
if slices == nil {
fmt.Fprintf(Stdout, "No matching slices for \"%s\"\n", query)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you use fmt.Printf for simplicity?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am a bit sceptical about doing that since the existing codes make use of the Stdout variable for testing purposes and there are no occurrences of directly using fmt.Print*:

$ grep -rni "fmt.Print" ./
./internal/jsonwall/jsonwall.go:28://	        fmt.Println(app.Name, "version:", app.Version)
./internal/jsonwall/jsonwall.go:37://	                        fmt.Println(app.Name, "version:", app.Version)
$ grep -rni "fmt.FPrint.*Stdout" ./
./cmd/chisel/cmd_version.go:31:	fmt.Fprintf(Stdout, "%s\n", cmd.Version)
./cmd/chisel/cmd_help.go:177:	fmt.Fprintln(Stdout, longChiselDescription)
./cmd/chisel/cmd_help.go:178:	fmt.Fprintln(Stdout)
./cmd/chisel/cmd_help.go:179:	fmt.Fprintln(Stdout, chiselUsage)
./cmd/chisel/cmd_help.go:180:	fmt.Fprintln(Stdout)
./cmd/chisel/cmd_help.go:181:	fmt.Fprintln(Stdout, chiselHelpCategoriesIntro)
./cmd/chisel/cmd_help.go:185:	fmt.Fprintln(Stdout)
./cmd/chisel/cmd_help.go:186:	fmt.Fprintln(Stdout, chiselHelpAllFooter)
./cmd/chisel/cmd_help.go:191:	fmt.Fprintln(Stdout, chiselHelpFooter)
./cmd/chisel/cmd_help.go:197:	fmt.Fprintln(Stdout)
./cmd/chisel/cmd_help.go:205:		fmt.Fprintf(Stdout, "%*s: %s\n", maxLen+2, categ.Label, strings.Join(categ.Commands, ", "))
./cmd/chisel/cmd_help.go:230:		fmt.Fprintln(Stdout)
./cmd/chisel/cmd_help.go:231:		fmt.Fprintf(Stdout, "  %s (%s):\n", categ.Label, categ.Description)
./cmd/chisel/cmd_help.go:237:				fmt.Fprintf(Stdout, "    %*s  %s\n", -maxLen, name, cmd.ShortDescription)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then let's keep it like this if it is the convention. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Turns out stdout is used in the tests like we discussed weeks ago. This comment can be resolved.

return nil
}

w := tabWriter()
fmt.Fprintf(w, "Slice\tPackage\tRelease\n")
for _, s := range slices {
fmt.Fprintf(w, "%s\t%s\t%s\n", s, s.Package, releaseLabel)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use the typical package_slice syntax I think?

Also, why are we presenting Release in every line when this was an input parameter and will be the same on every line?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is conforming to the output format defined in the spec. Example taken from the doc:

Slice           Package     Release
apache2_bins    apache2     ubuntu-22.04
apache2_libs    apache2     ubuntu-22.04
....

Let's discuss it and potentially change it.

}
w.Flush()

return nil
}

const maxStrDist int64 = 3

// matchSlice returns true if a slice (partially) matches with a query.
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
func matchSlice(slice *setup.Slice, query string) bool {
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
// check if the query (partially) matches with "pkg_slice" format slice name.
dist := strdist.Distance(slice.String(), query, strdist.StandardCost, maxStrDist+1)
if dist <= maxStrDist {
return true
}
// check if the query (partially) matches with the package name.
distPkg := strdist.Distance(slice.Package, query, strdist.StandardCost, maxStrDist+1)
if distPkg <= maxStrDist {
return true
}
// check if the query (partially) matches with the slice name.
distSlice := strdist.Distance(slice.Name, query, strdist.StandardCost, maxStrDist+1)
if distSlice <= maxStrDist {
return true
}
return false
}
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved

// findSlices goes through the release searching for any slices that matches
// the query string. It returns a list of slices who matches the query.
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
func findSlices(release *setup.Release, query string) (slices []*setup.Slice, err error) {
if release == nil {
return nil, fmt.Errorf("cannot find slice: invalid release")
}
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
for _, pkg := range release.Packages {
for _, slice := range pkg.Slices {
if slice != nil && matchSlice(slice, query) {
slices = append(slices, slice)
}
}
}
sort.Slice(slices, func(i, j int) bool {
return slices[i].String() < slices[j].String()
})
return slices, nil
}

func tabWriter() *tabwriter.Writer {
return tabwriter.NewWriter(Stdout, 5, 3, 2, ' ', 0)
}
160 changes: 160 additions & 0 deletions cmd/chisel/cmd_find_test.go
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package main_test

import (
. "gopkg.in/check.v1"

"github.com/canonical/chisel/internal/setup"

chisel "github.com/canonical/chisel/cmd/chisel"
)

type findTest struct {
summary string
release *setup.Release
query string
expSlices []*setup.Slice
expError string
}

var sampleRelease = &setup.Release{
DefaultArchive: "ubuntu",

Archives: map[string]*setup.Archive{
"ubuntu": {
Name: "ubuntu",
Version: "22.04",
Suites: []string{"jammy", "jammy-security"},
Components: []string{"main", "other"},
},
},
Packages: map[string]*setup.Package{
"openjdk-8-jdk": {
Archive: "ubuntu",
Name: "openjdk-8-jdk",
Path: "slices/openjdk-8-jdk.yaml",
Slices: map[string]*setup.Slice{
"bins": {
Package: "openjdk-8-jdk",
Name: "bins",
},
"config": {
Package: "openjdk-8-jdk",
Name: "config",
},
"core": {
Package: "openjdk-8-jdk",
Name: "core",
},
"libs": {
Package: "openjdk-8-jdk",
Name: "libs",
},
"utils": {
Package: "openjdk-8-jdk",
Name: "utils",
},
},
},
"python3.10": {
Archive: "ubuntu",
Name: "python3.10",
Path: "slices/python3.10.yaml",
Slices: map[string]*setup.Slice{
"bins": {
Package: "python3.10",
Name: "bins",
},
"config": {
Package: "python3.10",
Name: "config",
},
"core": {
Package: "python3.10",
Name: "core",
},
"libs": {
Package: "python3.10",
Name: "libs",
},
"utils": {
Package: "python3.10",
Name: "utils",
},
},
},
},
}

var findTests = []findTest{{
summary: "Ensure search with package names",
release: sampleRelease,
query: "python3.10",
expSlices: []*setup.Slice{
sampleRelease.Packages["python3.10"].Slices["bins"],
sampleRelease.Packages["python3.10"].Slices["config"],
sampleRelease.Packages["python3.10"].Slices["core"],
sampleRelease.Packages["python3.10"].Slices["libs"],
sampleRelease.Packages["python3.10"].Slices["utils"],
},
}, {
summary: "Ensure search with slice names",
release: sampleRelease,
query: "config",
expSlices: []*setup.Slice{
sampleRelease.Packages["openjdk-8-jdk"].Slices["config"],
sampleRelease.Packages["python3.10"].Slices["config"],
},
}, {
summary: "Check partial matching with pkg_slice format query",
release: sampleRelease,
query: "python3_bins",
expSlices: []*setup.Slice{
sampleRelease.Packages["python3.10"].Slices["bins"],
},
}, {
summary: "Check partial matching for package names",
release: sampleRelease,
query: "python3",
expSlices: []*setup.Slice{
sampleRelease.Packages["python3.10"].Slices["bins"],
sampleRelease.Packages["python3.10"].Slices["config"],
sampleRelease.Packages["python3.10"].Slices["core"],
sampleRelease.Packages["python3.10"].Slices["libs"],
sampleRelease.Packages["python3.10"].Slices["utils"],
},
}, {
summary: "Check partial matching for slice names",
release: sampleRelease,
query: "conf",
expSlices: []*setup.Slice{
sampleRelease.Packages["openjdk-8-jdk"].Slices["bins"],
sampleRelease.Packages["openjdk-8-jdk"].Slices["config"],
sampleRelease.Packages["openjdk-8-jdk"].Slices["core"],
sampleRelease.Packages["python3.10"].Slices["bins"],
sampleRelease.Packages["python3.10"].Slices["config"],
sampleRelease.Packages["python3.10"].Slices["core"],
},
}, {
summary: "Ensure error for nil release",
query: "foo",
expError: ".*invalid release",
}}

func (s *ChiselSuite) TestFindSlices(c *C) {
for _, test := range findTests {
c.Logf("Summary: %s", test.summary)

slices, err := chisel.FindSlices(test.release, test.query)
if test.expError == "" {
c.Assert(err, IsNil)
} else {
c.Assert(err, ErrorMatches, test.expError)
}
c.Assert(slices, DeepEquals, test.expSlices)
}
}

func (s *ChiselSuite) TestFindCommandEmptyQuery(c *C) {
_, err := chisel.Parser().ParseArgs([]string{"find", ""})
c.Assert(err, ErrorMatches, ".*no search term specified")
}
2 changes: 1 addition & 1 deletion cmd/chisel/cmd_help.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ var helpCategories = []helpCategory{{
}, {
Label: "Action",
Description: "make things happen",
Commands: []string{"cut"},
Commands: []string{"cut", "find"},
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
}}

var (
Expand Down
2 changes: 2 additions & 0 deletions cmd/chisel/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ func FakeIsStdinTTY(t bool) (restore func()) {
isStdinTTY = oldIsStdinTTY
}
}

var FindSlices = findSlices