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 7 commits
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
}
112 changes: 112 additions & 0 deletions cmd/chisel/cmd_find.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
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 the chisel-releases repository[1] or a particular directory. If left
unspecified, it queries with the release info found in /etc/lsb-release.

[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
}

// fuzzyMatchSlice reports whether a slice (partially) matches the query.
func fuzzyMatchSlice(slice *setup.Slice, query string) bool {
const maxStrDist = 1

// check if the query is a substring of the pkg_slice slice name
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
if strings.Contains(slice.String(), query) {
return true
}
// check if the query string is at most ``maxStrDist`` Levenshtein [1]
// distance away from the pkg_slice slice name.
// [1] https://en.wikipedia.org/wiki/Levenshtein_distance
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
dist := strdist.Distance(slice.String(), query, strdist.StandardCost, maxStrDist+1)
rebornplusplus marked this conversation as resolved.
Show resolved Hide resolved
return dist <= maxStrDist
}

// findSlices goes through the release searching for any slices that match
// the query string. It returns a list of slices that match the query.
Copy link
Contributor

Choose a reason for hiding this comment

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

// findSlices returns slices from the provided release that contain all of the query strings (AND).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done in 1186a98.

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 && fuzzyMatchSlice(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)
}
153 changes: 153 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,153 @@
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 substring matching",
release: sampleRelease,
query: "ython",
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",
release: sampleRelease,
query: "python3.1x_bins",
expSlices: []*setup.Slice{
sampleRelease.Packages["python3.10"].Slices["bins"],
},
}, {
summary: "Check no matching slice",
release: sampleRelease,
query: "foo_bar",
expSlices: nil,
}, {
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 @@ -155,7 +155,7 @@ type helpCategory struct {
var helpCategories = []helpCategory{{
Label: "Basic",
Description: "general operations",
Commands: []string{"help", "version"},
Commands: []string{"find", "help", "version"},
}, {
Label: "Action",
Description: "make things happen",
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
Loading