From 29b413eb053cd3b88034188f06257408b23c1ac4 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 12 Oct 2023 14:20:19 +0600 Subject: [PATCH 01/20] feat: add find command This commit adds the ``find`` command which can be used to query the chisel releases for available slices. --- Usage: chisel find [find-OPTIONS] [...] 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. [1] https://github.com/canonical/chisel-releases [find command options] --release= Chisel release branch or directory --- cmd/chisel/cmd_cut.go | 48 ++++++----- cmd/chisel/cmd_find.go | 119 +++++++++++++++++++++++++++ cmd/chisel/cmd_find_test.go | 160 ++++++++++++++++++++++++++++++++++++ cmd/chisel/cmd_help.go | 2 +- cmd/chisel/export_test.go | 2 + 5 files changed, 311 insertions(+), 20 deletions(-) create mode 100644 cmd/chisel/cmd_find.go create mode 100644 cmd/chisel/cmd_find_test.go diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index fd680b6f..fc42eaab 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -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 } @@ -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 +} diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go new file mode 100644 index 00000000..5c20cdf2 --- /dev/null +++ b/cmd/chisel/cmd_find.go @@ -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. + +[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:""` + + Positional struct { + Query []string `positional-arg-name:"" 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, " ")) + 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) + 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) + } + w.Flush() + + return nil +} + +const maxStrDist int64 = 3 + +// matchSlice returns true if a slice (partially) matches with a query. +func matchSlice(slice *setup.Slice, query string) bool { + // 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 +} + +// findSlices goes through the release searching for any slices that matches +// the query string. It returns a list of slices who matches the query. +func findSlices(release *setup.Release, query string) (slices []*setup.Slice, err error) { + if release == nil { + return nil, fmt.Errorf("cannot find slice: invalid release") + } + 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) +} diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go new file mode 100644 index 00000000..84348773 --- /dev/null +++ b/cmd/chisel/cmd_find_test.go @@ -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") +} diff --git a/cmd/chisel/cmd_help.go b/cmd/chisel/cmd_help.go index a6314f49..66de9150 100644 --- a/cmd/chisel/cmd_help.go +++ b/cmd/chisel/cmd_help.go @@ -159,7 +159,7 @@ var helpCategories = []helpCategory{{ }, { Label: "Action", Description: "make things happen", - Commands: []string{"cut"}, + Commands: []string{"cut", "find"}, }} var ( diff --git a/cmd/chisel/export_test.go b/cmd/chisel/export_test.go index dd852b44..a99a7831 100644 --- a/cmd/chisel/export_test.go +++ b/cmd/chisel/export_test.go @@ -17,3 +17,5 @@ func FakeIsStdinTTY(t bool) (restore func()) { isStdinTTY = oldIsStdinTTY } } + +var FindSlices = findSlices From 3ab898e2b092df8fe2bcc37c33464b4f6a09ac26 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Fri, 13 Oct 2023 15:36:29 +0600 Subject: [PATCH 02/20] fix(cmd/find): change query matching logic This commit changes the logic of matching a query against a slice. Following the new behaviour, a slice name will be listed as a match if the query is a sub-string of the slice name, in pkg_slice format. Or, it will be listed if the query is atmost 1 (One) Levenshtein distance [1] away from the slice name, in pkg_slice format. [1] https://en.wikipedia.org/wiki/Levenshtein_distance --- cmd/chisel/cmd_find.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 5c20cdf2..d3adcd6f 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -73,23 +73,19 @@ func (cmd *cmdFind) Execute(args []string) error { return nil } -const maxStrDist int64 = 3 +const maxStrDist int64 = 1 // matchSlice returns true if a slice (partially) matches with a query. func matchSlice(slice *setup.Slice, query string) bool { - // 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 { + // check if the query is a substring of the pkg_slice slice name + if strings.Contains(slice.String(), query) { 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 { + // check if the query string is atmost ``maxStrDist`` Levenshtein [1] + // distance away from the pkg_slice slice name. + // [1] https://en.wikipedia.org/wiki/Levenshtein_distance + dist := strdist.Distance(slice.String(), query, strdist.StandardCost, maxStrDist+1) + if dist <= maxStrDist { return true } return false From 0858f6af5b6e0edbead85c053e69e5ab9bd0c796 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Fri, 13 Oct 2023 15:49:42 +0600 Subject: [PATCH 03/20] test(cmd/find): fix tests for new matching logic 3ab898e introduced new changes in behaviour. Fix the tests accordingly. --- cmd/chisel/cmd_find_test.go | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 84348773..078bc3a6 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -105,16 +105,9 @@ var findTests = []findTest{{ sampleRelease.Packages["python3.10"].Slices["config"], }, }, { - summary: "Check partial matching with pkg_slice format query", + summary: "Check substring matching", 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", + query: "ython", expSlices: []*setup.Slice{ sampleRelease.Packages["python3.10"].Slices["bins"], sampleRelease.Packages["python3.10"].Slices["config"], @@ -123,16 +116,11 @@ var findTests = []findTest{{ sampleRelease.Packages["python3.10"].Slices["utils"], }, }, { - summary: "Check partial matching for slice names", + summary: "Check partial matching", release: sampleRelease, - query: "conf", + query: "python3.1x_bins", 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", From 828055c228e6e315d887e6501203b3946c76ee27 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Mon, 16 Oct 2023 13:26:23 +0600 Subject: [PATCH 04/20] fix(cmd/find): update grammar on find --help --- cmd/chisel/cmd_find.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index d3adcd6f..fbfe4785 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -17,7 +17,7 @@ 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 +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 From e84776d79e9ddb95ee8e7bf24ac9532edf0c6b2c Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Mon, 16 Oct 2023 13:26:56 +0600 Subject: [PATCH 05/20] test(cmd/find): add new test for no matching slices --- cmd/chisel/cmd_find_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 078bc3a6..7af033c1 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -122,6 +122,11 @@ var findTests = []findTest{{ 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", From 6d04d2d29fe6b1f7c3d1faa9c7bcb13a494877b8 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Fri, 20 Oct 2023 08:34:01 +0600 Subject: [PATCH 06/20] fix: address feedback from Ben --- cmd/chisel/cmd_find.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index fbfe4785..1f9101e1 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -73,33 +73,30 @@ func (cmd *cmdFind) Execute(args []string) error { return nil } -const maxStrDist int64 = 1 +// fuzzyMatchSlice reports whether a slice (partially) matches the query. +func fuzzyMatchSlice(slice *setup.Slice, query string) bool { + const maxStrDist = 1 -// matchSlice returns true if a slice (partially) matches with a query. -func matchSlice(slice *setup.Slice, query string) bool { // check if the query is a substring of the pkg_slice slice name if strings.Contains(slice.String(), query) { return true } - // check if the query string is atmost ``maxStrDist`` Levenshtein [1] + // 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 dist := strdist.Distance(slice.String(), query, strdist.StandardCost, maxStrDist+1) - if dist <= maxStrDist { - return true - } - return false + return dist <= maxStrDist } -// findSlices goes through the release searching for any slices that matches -// the query string. It returns a list of slices who matches the query. +// findSlices goes through the release searching for any slices that match +// the query string. It returns a list of slices that match the query. func findSlices(release *setup.Release, query string) (slices []*setup.Slice, err error) { if release == nil { return nil, fmt.Errorf("cannot find slice: invalid release") } for _, pkg := range release.Packages { for _, slice := range pkg.Slices { - if slice != nil && matchSlice(slice, query) { + if slice != nil && fuzzyMatchSlice(slice, query) { slices = append(slices, slice) } } From 03f077a80a80634fa510b3520f50d95d749c86f0 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Fri, 20 Oct 2023 15:16:36 +0600 Subject: [PATCH 07/20] fix(help): move find to the Basic category --- cmd/chisel/cmd_help.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/chisel/cmd_help.go b/cmd/chisel/cmd_help.go index 66de9150..a0ebd273 100644 --- a/cmd/chisel/cmd_help.go +++ b/cmd/chisel/cmd_help.go @@ -155,11 +155,11 @@ 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", - Commands: []string{"cut", "find"}, + Commands: []string{"cut"}, }} var ( From 67d2f7d73bcc833bba4e389992efe58011f668ce Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Mon, 4 Dec 2023 16:21:01 +0600 Subject: [PATCH 08/20] refactor: move ``GetRelease`` to setup package --- cmd/chisel/cmd_cut.go | 67 +---------------------------------------- cmd/chisel/cmd_find.go | 5 +-- internal/setup/setup.go | 60 ++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 70 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index fc42eaab..ea3db6f8 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -3,11 +3,6 @@ package main import ( "github.com/jessevdk/go-flags" - "fmt" - "os" - "regexp" - "strings" - "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/setup" @@ -54,7 +49,7 @@ func (cmd *cmdCut) Execute(args []string) error { sliceKeys[i] = sliceKey } - release, _, err := getRelease(cmd.Release) + release, _, err := setup.GetRelease(cmd.Release) if err != nil { return err } @@ -86,63 +81,3 @@ func (cmd *cmdCut) Execute(args []string) error { TargetDir: cmd.RootDir, }) } - -// TODO These need testing, and maybe moving into a common file. - -var releaseExp = regexp.MustCompile(`^([a-z](?:-?[a-z0-9]){2,})-([0-9]+(?:\.?[0-9])+)$`) - -func parseReleaseInfo(release string) (label, version string, err error) { - match := releaseExp.FindStringSubmatch(release) - if match == nil { - return "", "", fmt.Errorf("invalid release reference: %q", release) - } - return match[1], match[2], nil -} - -func readReleaseInfo() (label, version string, err error) { - data, err := os.ReadFile("/etc/lsb-release") - if err == nil { - const labelPrefix = "DISTRIB_ID=" - const versionPrefix = "DISTRIB_RELEASE=" - for _, line := range strings.Split(string(data), "\n") { - switch { - case strings.HasPrefix(line, labelPrefix): - label = strings.ToLower(line[len(labelPrefix):]) - case strings.HasPrefix(line, versionPrefix): - version = line[len(versionPrefix):] - } - if label != "" && version != "" { - return label, version, nil - } - } - } - 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 -} diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 1f9101e1..3e9f89f9 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -49,7 +49,7 @@ func (cmd *cmdFind) Execute(args []string) error { return fmt.Errorf("no search term specified") } - release, releaseLabel, err := getRelease(cmd.Release) + release, releaseLabel, err := setup.GetRelease(cmd.Release) if err != nil { return err } @@ -91,9 +91,6 @@ func fuzzyMatchSlice(slice *setup.Slice, query string) bool { // findSlices goes through the release searching for any slices that match // the query string. It returns a list of slices that match the query. func findSlices(release *setup.Release, query string) (slices []*setup.Slice, err error) { - if release == nil { - return nil, fmt.Errorf("cannot find slice: invalid release") - } for _, pkg := range release.Packages { for _, slice := range pkg.Slices { if slice != nil && fuzzyMatchSlice(slice, query) { diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 451bcc3c..e3034cac 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -623,3 +623,63 @@ func Select(release *Release, slices []SliceKey) (*Selection, error) { return selection, nil } + +// TODO These need testing + +var releaseExp = regexp.MustCompile(`^([a-z](?:-?[a-z0-9]){2,})-([0-9]+(?:\.?[0-9])+)$`) + +func parseReleaseInfo(release string) (label, version string, err error) { + match := releaseExp.FindStringSubmatch(release) + if match == nil { + return "", "", fmt.Errorf("invalid release reference: %q", release) + } + return match[1], match[2], nil +} + +func readReleaseInfo() (label, version string, err error) { + data, err := os.ReadFile("/etc/lsb-release") + if err == nil { + const labelPrefix = "DISTRIB_ID=" + const versionPrefix = "DISTRIB_RELEASE=" + for _, line := range strings.Split(string(data), "\n") { + switch { + case strings.HasPrefix(line, labelPrefix): + label = strings.ToLower(line[len(labelPrefix):]) + case strings.HasPrefix(line, versionPrefix): + version = line[len(versionPrefix):] + } + if label != "" && version != "" { + return label, version, nil + } + } + } + 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 *Release, releaseLabel string, err error) { + if strings.Contains(releaseStr, "/") { + release, err = 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 = FetchRelease(&FetchOptions{ + Label: label, + Version: version, + }) + releaseLabel = label + "-" + version + } + if err != nil { + return nil, "", err + } + return release, releaseLabel, nil +} From 26fa232ceb7c7ca0cdf60b8bbbc1d303df60fc16 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Mon, 4 Dec 2023 16:24:15 +0600 Subject: [PATCH 09/20] test: adjust tests for previous refactoring --- cmd/chisel/cmd_find_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 7af033c1..53418d25 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -127,10 +127,6 @@ var findTests = []findTest{{ release: sampleRelease, query: "foo_bar", expSlices: nil, -}, { - summary: "Ensure error for nil release", - query: "foo", - expError: ".*invalid release", }} func (s *ChiselSuite) TestFindSlices(c *C) { From cb77d8b40e5d8b8c459c727f414b17fd728d06a1 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Fri, 8 Dec 2023 16:47:31 +0600 Subject: [PATCH 10/20] refactor: rename ``setup.GetRelease`` to ``readOrFetchRelease`` The function is moved back to the `main` package and is declared in cmd/chisel/helpers.go file. --- cmd/chisel/cmd_cut.go | 2 +- cmd/chisel/cmd_find.go | 2 +- cmd/chisel/helpers.go | 73 +++++++++++++++++++++++++++++++++++++++++ internal/setup/setup.go | 60 --------------------------------- 4 files changed, 75 insertions(+), 62 deletions(-) create mode 100644 cmd/chisel/helpers.go diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index ea3db6f8..f8992270 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -49,7 +49,7 @@ func (cmd *cmdCut) Execute(args []string) error { sliceKeys[i] = sliceKey } - release, _, err := setup.GetRelease(cmd.Release) + release, _, err := readOrFetchRelease(cmd.Release) if err != nil { return err } diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 3e9f89f9..8260d43f 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -49,7 +49,7 @@ func (cmd *cmdFind) Execute(args []string) error { return fmt.Errorf("no search term specified") } - release, releaseLabel, err := setup.GetRelease(cmd.Release) + release, releaseLabel, err := readOrFetchRelease(cmd.Release) if err != nil { return err } diff --git a/cmd/chisel/helpers.go b/cmd/chisel/helpers.go new file mode 100644 index 00000000..14b9bd8d --- /dev/null +++ b/cmd/chisel/helpers.go @@ -0,0 +1,73 @@ +package main + +import ( + "fmt" + "os" + "regexp" + "strings" + + "github.com/canonical/chisel/internal/setup" +) + +// TODO These need testing + +var releaseExp = regexp.MustCompile(`^([a-z](?:-?[a-z0-9]){2,})-([0-9]+(?:\.?[0-9])+)$`) + +func parseReleaseInfo(release string) (label, version string, err error) { + match := releaseExp.FindStringSubmatch(release) + if match == nil { + return "", "", fmt.Errorf("invalid release reference: %q", release) + } + return match[1], match[2], nil +} + +func readReleaseInfo() (label, version string, err error) { + data, err := os.ReadFile("/etc/lsb-release") + if err == nil { + const labelPrefix = "DISTRIB_ID=" + const versionPrefix = "DISTRIB_RELEASE=" + for _, line := range strings.Split(string(data), "\n") { + switch { + case strings.HasPrefix(line, labelPrefix): + label = strings.ToLower(line[len(labelPrefix):]) + case strings.HasPrefix(line, versionPrefix): + version = line[len(versionPrefix):] + } + if label != "" && version != "" { + return label, version, nil + } + } + } + return "", "", fmt.Errorf("cannot infer release via /etc/lsb-release, see the --release option") +} + +// readOrFetchRelease takes a release branch name or a release directory path. +// It fetches or reads the chisel-release depending on the nature of input and +// returns the release and release label. +// If the input is empty, it tries to read the release label from the host +// system and fetch the chisel-release accordingly. +func readOrFetchRelease(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 +} diff --git a/internal/setup/setup.go b/internal/setup/setup.go index e3034cac..451bcc3c 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -623,63 +623,3 @@ func Select(release *Release, slices []SliceKey) (*Selection, error) { return selection, nil } - -// TODO These need testing - -var releaseExp = regexp.MustCompile(`^([a-z](?:-?[a-z0-9]){2,})-([0-9]+(?:\.?[0-9])+)$`) - -func parseReleaseInfo(release string) (label, version string, err error) { - match := releaseExp.FindStringSubmatch(release) - if match == nil { - return "", "", fmt.Errorf("invalid release reference: %q", release) - } - return match[1], match[2], nil -} - -func readReleaseInfo() (label, version string, err error) { - data, err := os.ReadFile("/etc/lsb-release") - if err == nil { - const labelPrefix = "DISTRIB_ID=" - const versionPrefix = "DISTRIB_RELEASE=" - for _, line := range strings.Split(string(data), "\n") { - switch { - case strings.HasPrefix(line, labelPrefix): - label = strings.ToLower(line[len(labelPrefix):]) - case strings.HasPrefix(line, versionPrefix): - version = line[len(versionPrefix):] - } - if label != "" && version != "" { - return label, version, nil - } - } - } - 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 *Release, releaseLabel string, err error) { - if strings.Contains(releaseStr, "/") { - release, err = 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 = FetchRelease(&FetchOptions{ - Label: label, - Version: version, - }) - releaseLabel = label + "-" + version - } - if err != nil { - return nil, "", err - } - return release, releaseLabel, nil -} From b8a8450357122a5fee486db4a8672858bc5a10c0 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Fri, 8 Mar 2024 07:35:05 +0600 Subject: [PATCH 11/20] refactor: reword comments --- cmd/chisel/cmd_find.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 8260d43f..926c2247 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -77,13 +77,12 @@ func (cmd *cmdFind) Execute(args []string) error { func fuzzyMatchSlice(slice *setup.Slice, query string) bool { const maxStrDist = 1 - // check if the query is a substring of the pkg_slice slice name + // Check if the query is a substring of the pkg_slice slice name. 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 + // Check if the query string is at most ``maxStrDist`` Levenshtein distance + // away from the pkg_slice slice name. dist := strdist.Distance(slice.String(), query, strdist.StandardCost, maxStrDist+1) return dist <= maxStrDist } From 809b008b1e52263f00dbb558ea5a1b205aa7f284 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Wed, 5 Jun 2024 10:09:00 +0200 Subject: [PATCH 12/20] address meeting comments --- cmd/chisel/cmd_cut.go | 5 +- cmd/chisel/cmd_find.go | 54 +++++++------- cmd/chisel/cmd_find_test.go | 139 ++++++++++++++---------------------- tests/find/task.yaml | 14 ++++ 4 files changed, 100 insertions(+), 112 deletions(-) create mode 100644 tests/find/task.yaml diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index f8992270..25d75997 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -13,10 +13,13 @@ var shortCutHelp = "Cut a tree with selected slices" var longCutHelp = ` The cut command uses the provided selection of package slices to create a new filesystem tree in the root location. + +By default it fetches the slices for the latest Ubuntu +version, unless the --release flag is used. ` var cutDescs = map[string]string{ - "release": "Chisel release directory", + "release": "Chisel release directory or Ubuntu version", "root": "Root for generated content", "arch": "Package architecture", } diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 926c2247..6497b98b 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -14,17 +14,14 @@ import ( var shortFindHelp = "Find existing slices" var longFindHelp = ` -The find command queries the chisel releases for available slices. +The find command queries the slice definitions for matching 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 +By default it fetches the slices for the latest Ubuntu +version, unless the --release flag is used. ` var findDescs = map[string]string{ - "release": "Chisel release branch or directory", + "release": "Chisel release directory or Ubuntu version", } type cmdFind struct { @@ -44,22 +41,17 @@ func (cmd *cmdFind) Execute(args []string) error { return ErrExtraArgs } - query := strings.TrimSpace(strings.Join(cmd.Positional.Query, " ")) - if query == "" { - return fmt.Errorf("no search term specified") - } - release, releaseLabel, err := readOrFetchRelease(cmd.Release) if err != nil { return err } - slices, err := findSlices(release, query) + slices, err := findSlices(release, cmd.Positional.Query) if err != nil { return err } - if slices == nil { - fmt.Fprintf(Stdout, "No matching slices for \"%s\"\n", query) + if len(slices) == 0 { + fmt.Fprintf(Stdout, "No matching slices for \"%s\"\n", strings.Join(cmd.Positional.Query, " ")) return nil } @@ -73,26 +65,34 @@ func (cmd *cmdFind) Execute(args []string) error { return nil } -// fuzzyMatchSlice reports whether a slice (partially) matches the query. -func fuzzyMatchSlice(slice *setup.Slice, query string) bool { +// match reports whether a slice (partially) matches the query. +func match(slice *setup.Slice, query string) bool { const maxStrDist = 1 - - // Check if the query is a substring of the pkg_slice slice name. - if strings.Contains(slice.String(), query) { - return true + fuzzyMatch := func(str, query string) bool { + return strdist.Distance(str, query, strdist.StandardCost, maxStrDist+1) <= maxStrDist } - // Check if the query string is at most ``maxStrDist`` Levenshtein distance - // away from the pkg_slice slice name. - dist := strdist.Distance(slice.String(), query, strdist.StandardCost, maxStrDist+1) - return dist <= maxStrDist + return strings.Contains(slice.String(), query) || + fuzzyMatch(slice.Name, query) || + fuzzyMatch(slice.Package, query) || + fuzzyMatch(slice.String(), query) } // findSlices goes through the release searching for any slices that match // the query string. It returns a list of slices that match the query. -func findSlices(release *setup.Release, query string) (slices []*setup.Slice, err error) { +func findSlices(release *setup.Release, queries []string) (slices []*setup.Slice, err error) { for _, pkg := range release.Packages { for _, slice := range pkg.Slices { - if slice != nil && fuzzyMatchSlice(slice, query) { + if slice == nil { + continue + } + allMatch := true + for _, query := range queries { + if !match(slice, query) { + allMatch = false + break + } + } + if allMatch { slices = append(slices, slice) } } diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 53418d25..9a6be021 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -4,16 +4,32 @@ import ( . "gopkg.in/check.v1" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/internal/testutil" chisel "github.com/canonical/chisel/cmd/chisel" ) type findTest struct { - summary string - release *setup.Release - query string - expSlices []*setup.Slice - expError string + summary string + release *setup.Release + query []string + result []*setup.Slice +} + +func makeSamplePackage(pkg string, slices []string) *setup.Package { + slicesMap := map[string]*setup.Slice{} + for _, slice := range slices { + slicesMap[slice] = &setup.Slice{ + Package: pkg, + Name: slice, + } + } + return &setup.Package{ + Name: pkg, + Path: "slices/" + pkg, + Archive: "ubuntu", + Slices: slicesMap, + } } var sampleRelease = &setup.Release{ @@ -28,68 +44,16 @@ var sampleRelease = &setup.Release{ }, }, 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", - }, - }, - }, + "openjdk-8-jdk": makeSamplePackage("openjdk-8-jdk", []string{"bins", "config", "core", "libs", "utils"}), + "python3.10": makeSamplePackage("python3.10", []string{"bins", "config", "core", "libs", "utils"}), }, } var findTests = []findTest{{ - summary: "Ensure search with package names", + summary: "Search by package name", release: sampleRelease, - query: "python3.10", - expSlices: []*setup.Slice{ + query: []string{"python3.10"}, + result: []*setup.Slice{ sampleRelease.Packages["python3.10"].Slices["bins"], sampleRelease.Packages["python3.10"].Slices["config"], sampleRelease.Packages["python3.10"].Slices["core"], @@ -97,18 +61,18 @@ var findTests = []findTest{{ sampleRelease.Packages["python3.10"].Slices["utils"], }, }, { - summary: "Ensure search with slice names", + summary: "Search by slice name", release: sampleRelease, - query: "config", - expSlices: []*setup.Slice{ + query: []string{"config"}, + result: []*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{ + query: []string{"ython"}, + result: []*setup.Slice{ sampleRelease.Packages["python3.10"].Slices["bins"], sampleRelease.Packages["python3.10"].Slices["config"], sampleRelease.Packages["python3.10"].Slices["core"], @@ -118,32 +82,39 @@ var findTests = []findTest{{ }, { summary: "Check partial matching", release: sampleRelease, - query: "python3.1x_bins", - expSlices: []*setup.Slice{ + query: []string{"python3.1x_bins"}, + result: []*setup.Slice{ + sampleRelease.Packages["python3.10"].Slices["bins"], + }, +}, { + summary: "Check no matching slice", + release: sampleRelease, + query: []string{"foo_bar"}, + result: nil, +}, { + summary: "Several terms all match", + release: sampleRelease, + query: []string{"python", "s"}, + result: []*setup.Slice{ sampleRelease.Packages["python3.10"].Slices["bins"], + sampleRelease.Packages["python3.10"].Slices["libs"], + sampleRelease.Packages["python3.10"].Slices["utils"], }, }, { - summary: "Check no matching slice", - release: sampleRelease, - query: "foo_bar", - expSlices: nil, + summary: "Several terms one does not match", + release: sampleRelease, + query: []string{"python", "slice"}, + result: nil, }} func (s *ChiselSuite) TestFindSlices(c *C) { for _, test := range findTests { - c.Logf("Summary: %s", test.summary) + for _, query := range testutil.Permutations(test.query) { + c.Logf("Summary: %s", test.summary) - slices, err := chisel.FindSlices(test.release, test.query) - if test.expError == "" { + slices, err := chisel.FindSlices(test.release, query) c.Assert(err, IsNil) - } else { - c.Assert(err, ErrorMatches, test.expError) + c.Assert(slices, DeepEquals, test.result) } - 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") -} diff --git a/tests/find/task.yaml b/tests/find/task.yaml new file mode 100644 index 00000000..b87fca1b --- /dev/null +++ b/tests/find/task.yaml @@ -0,0 +1,14 @@ +summary: Chisel can find slice by slice name, package name or a combination + +execute: | + find() { + fullname=$1 + shift + query=$@ + chisel find --release ${OS}-${RELEASE} $query | grep $fullname + } + + find "ca-certificates_data" "ca-certificates_data" + find "ca-certificates_data" "ca-certificates" "data" + find "ca-certificates_data" "data" "ca-certificates" + ! find "ca-certificates_data" "ca-certificates" "foo" \ No newline at end of file From 1186a98a1beb297572306a79845d697620a9092a Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 18 Jun 2024 12:04:18 +0200 Subject: [PATCH 13/20] change match to add wildcards instead of using substring --- cmd/chisel/cmd_find.go | 24 ++++++++++++------------ cmd/chisel/cmd_find_test.go | 11 +++++++++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 6497b98b..088e4e66 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -15,6 +15,7 @@ import ( var shortFindHelp = "Find existing slices" var longFindHelp = ` The find command queries the slice definitions for matching slices. +Globs are allowed in the query. By default it fetches the slices for the latest Ubuntu version, unless the --release flag is used. @@ -67,27 +68,26 @@ func (cmd *cmdFind) Execute(args []string) error { // match reports whether a slice (partially) matches the query. func match(slice *setup.Slice, query string) bool { - const maxStrDist = 1 - fuzzyMatch := func(str, query string) bool { - return strdist.Distance(str, query, strdist.StandardCost, maxStrDist+1) <= maxStrDist + if !strings.HasPrefix(query, "*") { + query = "*" + query } - return strings.Contains(slice.String(), query) || - fuzzyMatch(slice.Name, query) || - fuzzyMatch(slice.Package, query) || - fuzzyMatch(slice.String(), query) + if !strings.HasSuffix(query, "*") { + query += "*" + } + return strdist.GlobPath(slice.String(), query) } -// findSlices goes through the release searching for any slices that match -// the query string. It returns a list of slices that match the query. -func findSlices(release *setup.Release, queries []string) (slices []*setup.Slice, err error) { +// findSlices returns slices from the provided release that match all of the +// query strings (AND). +func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, err error) { for _, pkg := range release.Packages { for _, slice := range pkg.Slices { if slice == nil { continue } allMatch := true - for _, query := range queries { - if !match(slice, query) { + for _, term := range query { + if !match(slice, term) { allMatch = false break } diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 9a6be021..4c8e1c9c 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -80,9 +80,16 @@ var findTests = []findTest{{ sampleRelease.Packages["python3.10"].Slices["utils"], }, }, { - summary: "Check partial matching", + summary: "Check glob matching (*)", release: sampleRelease, - query: []string{"python3.1x_bins"}, + query: []string{"python3.*_bins"}, + result: []*setup.Slice{ + sampleRelease.Packages["python3.10"].Slices["bins"], + }, +}, { + summary: "Check glob matching (?)", + release: sampleRelease, + query: []string{"python3.1?_bins"}, result: []*setup.Slice{ sampleRelease.Packages["python3.10"].Slices["bins"], }, From 99b9496ab607b885349702781b2262bf5743aced Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 18 Jun 2024 12:13:19 +0200 Subject: [PATCH 14/20] change output and remove releaseLabel --- cmd/chisel/cmd_cut.go | 2 +- cmd/chisel/cmd_find.go | 6 +++--- cmd/chisel/helpers.go | 20 +++++++++----------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 5e6ac1d8..1c081ee3 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -52,7 +52,7 @@ func (cmd *cmdCut) Execute(args []string) error { sliceKeys[i] = sliceKey } - release, _, err := readOrFetchRelease(cmd.Release) + release, err := obtainRelease(cmd.Release) if err != nil { return err } diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 088e4e66..9f5c3aec 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -42,7 +42,7 @@ func (cmd *cmdFind) Execute(args []string) error { return ErrExtraArgs } - release, releaseLabel, err := readOrFetchRelease(cmd.Release) + release, err := obtainRelease(cmd.Release) if err != nil { return err } @@ -57,9 +57,9 @@ func (cmd *cmdFind) Execute(args []string) error { } w := tabWriter() - fmt.Fprintf(w, "Slice\tPackage\tRelease\n") + fmt.Fprintf(w, "Slice\tNotes\n") for _, s := range slices { - fmt.Fprintf(w, "%s\t%s\t%s\n", s, s.Package, releaseLabel) + fmt.Fprintf(w, "%s\t%s\n", s, "-") } w.Flush() diff --git a/cmd/chisel/helpers.go b/cmd/chisel/helpers.go index 14b9bd8d..7506888e 100644 --- a/cmd/chisel/helpers.go +++ b/cmd/chisel/helpers.go @@ -41,15 +41,14 @@ func readReleaseInfo() (label, version string, err error) { return "", "", fmt.Errorf("cannot infer release via /etc/lsb-release, see the --release option") } -// readOrFetchRelease takes a release branch name or a release directory path. -// It fetches or reads the chisel-release depending on the nature of input and -// returns the release and release label. -// If the input is empty, it tries to read the release label from the host -// system and fetch the chisel-release accordingly. -func readOrFetchRelease(releaseStr string) (release *setup.Release, releaseLabel string, err error) { +// obtainRelease returns the Chisel release information matching the provided string, +// fetching it if necessary. The provided string should be either: +// * "-", +// * the path to a directory containing a previously fetched release, +// * "" and Chisel will attempt to read the release label from the host. +func obtainRelease(releaseStr string) (release *setup.Release, err error) { if strings.Contains(releaseStr, "/") { release, err = setup.ReadRelease(releaseStr) - releaseLabel = releaseStr } else { var label, version string if releaseStr == "" { @@ -58,16 +57,15 @@ func readOrFetchRelease(releaseStr string) (release *setup.Release, releaseLabel label, version, err = parseReleaseInfo(releaseStr) } if err != nil { - return nil, "", err + return nil, err } release, err = setup.FetchRelease(&setup.FetchOptions{ Label: label, Version: version, }) - releaseLabel = label + "-" + version } if err != nil { - return nil, "", err + return nil, err } - return release, releaseLabel, nil + return release, nil } From d8353c8067c6ef8e104e8990b9018271eb7b8d90 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 18 Jun 2024 12:15:16 +0200 Subject: [PATCH 15/20] use stderr when no slices match --- cmd/chisel/cmd_find.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 9f5c3aec..ea09eccc 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -52,7 +52,7 @@ func (cmd *cmdFind) Execute(args []string) error { return err } if len(slices) == 0 { - fmt.Fprintf(Stdout, "No matching slices for \"%s\"\n", strings.Join(cmd.Positional.Query, " ")) + fmt.Fprintf(Stderr, "No matching slices for \"%s\"\n", strings.Join(cmd.Positional.Query, " ")) return nil } From a9d8910c5efe31d7459583c4ec16bdb69e491f9e Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Wed, 19 Jun 2024 09:45:08 +0200 Subject: [PATCH 16/20] meeting comments with Cris --- cmd/chisel/cmd_find.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index ea09eccc..e88b9eb4 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -15,7 +15,7 @@ import ( var shortFindHelp = "Find existing slices" var longFindHelp = ` The find command queries the slice definitions for matching slices. -Globs are allowed in the query. +Globs (* and ?) are allowed in the query. By default it fetches the slices for the latest Ubuntu version, unless the --release flag is used. @@ -57,7 +57,7 @@ func (cmd *cmdFind) Execute(args []string) error { } w := tabWriter() - fmt.Fprintf(w, "Slice\tNotes\n") + fmt.Fprintf(w, "Slice\tSummary\n") for _, s := range slices { fmt.Fprintf(w, "%s\t%s\n", s, "-") } From d0c78e8ac6312a56e24b6ab7d0436e0b5bc0d8ae Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 28 Jun 2024 12:19:56 +0200 Subject: [PATCH 17/20] change algorithm to use distance of 1 and underscores --- cmd/chisel/cmd_find.go | 18 +++++++---- cmd/chisel/cmd_find_test.go | 53 ++++++++++++++++++++------------ internal/strdist/strdist.go | 26 ++++++++++++++-- internal/strdist/strdist_test.go | 6 ++++ 4 files changed, 75 insertions(+), 28 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index e88b9eb4..ac8a5ddc 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -68,18 +68,24 @@ func (cmd *cmdFind) Execute(args []string) error { // match reports whether a slice (partially) matches the query. func match(slice *setup.Slice, query string) bool { - if !strings.HasPrefix(query, "*") { - query = "*" + query + var term string + switch { + case strings.HasPrefix(query, "_"): + query = strings.TrimPrefix(query, "_") + term = slice.Name + case strings.Contains(query, "_"): + term = slice.String() + default: + term = slice.Package } - if !strings.HasSuffix(query, "*") { - query += "*" - } - return strdist.GlobPath(slice.String(), query) + query = strings.ReplaceAll(query, "**", "⁑") + return strdist.Distance(term, query, strdist.StandardGlobCost, 0) <= 1 } // findSlices returns slices from the provided release that match all of the // query strings (AND). func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, err error) { + slices = []*setup.Slice{} for _, pkg := range release.Packages { for _, slice := range pkg.Slices { if slice == nil { diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 4c8e1c9c..7d7c0d7a 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -63,22 +63,21 @@ var findTests = []findTest{{ }, { summary: "Search by slice name", release: sampleRelease, - query: []string{"config"}, + query: []string{"_config"}, result: []*setup.Slice{ sampleRelease.Packages["openjdk-8-jdk"].Slices["config"], sampleRelease.Packages["python3.10"].Slices["config"], }, }, { - summary: "Check substring matching", + summary: "Slice search without leading underscore", release: sampleRelease, - query: []string{"ython"}, - result: []*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"], - }, + query: []string{"config"}, + result: []*setup.Slice{}, +}, { + summary: "Check distance greater than one", + release: sampleRelease, + query: []string{"python3."}, + result: []*setup.Slice{}, }, { summary: "Check glob matching (*)", release: sampleRelease, @@ -89,36 +88,50 @@ var findTests = []findTest{{ }, { summary: "Check glob matching (?)", release: sampleRelease, - query: []string{"python3.1?_bins"}, + query: []string{"python3.1?_co*"}, result: []*setup.Slice{ - sampleRelease.Packages["python3.10"].Slices["bins"], + sampleRelease.Packages["python3.10"].Slices["config"], + sampleRelease.Packages["python3.10"].Slices["core"], }, }, { summary: "Check no matching slice", release: sampleRelease, query: []string{"foo_bar"}, - result: nil, + result: []*setup.Slice{}, }, { summary: "Several terms all match", release: sampleRelease, - query: []string{"python", "s"}, + query: []string{"python*", "_co*"}, + result: []*setup.Slice{ + sampleRelease.Packages["python3.10"].Slices["config"], + sampleRelease.Packages["python3.10"].Slices["core"], + }, +}, { + summary: "Distance of one in each term", + release: sampleRelease, + query: []string{"python3.1", "_lib"}, result: []*setup.Slice{ - sampleRelease.Packages["python3.10"].Slices["bins"], sampleRelease.Packages["python3.10"].Slices["libs"], - sampleRelease.Packages["python3.10"].Slices["utils"], }, }, { - summary: "Several terms one does not match", + summary: "Query with underscore is matched against full name", + release: sampleRelease, + query: []string{"python3.1_libs"}, + result: []*setup.Slice{ + sampleRelease.Packages["python3.10"].Slices["libs"], + }, +}, { + summary: "Several terms, one does not match", release: sampleRelease, query: []string{"python", "slice"}, - result: nil, + result: []*setup.Slice{}, }} func (s *ChiselSuite) TestFindSlices(c *C) { for _, test := range findTests { - for _, query := range testutil.Permutations(test.query) { - c.Logf("Summary: %s", test.summary) + c.Logf("Summary: %s", test.summary) + for _, query := range testutil.Permutations(test.query) { slices, err := chisel.FindSlices(test.release, query) c.Assert(err, IsNil) c.Assert(slices, DeepEquals, test.result) diff --git a/internal/strdist/strdist.go b/internal/strdist/strdist.go index eb0c1dca..404102b1 100644 --- a/internal/strdist/strdist.go +++ b/internal/strdist/strdist.go @@ -28,6 +28,28 @@ func StandardCost(ar, br rune) Cost { return Cost{SwapAB: 1, DeleteA: 1, InsertB: 1} } +// StandardGlobCost encodes the standard Levenshtein distance with support for +// "*", "?" and "**". However, because it works on runes "**" has to be encoded +// as "⁑" in the strings. +// +// Supported wildcards: +// +// ? - Any one character +// * - Any zero or more characters +// ⁑ - Any zero or more characters +func StandardGlobCost(ar, br rune) Cost { + if ar == '⁑' || br == '⁑' { + return Cost{SwapAB: 0, DeleteA: 0, InsertB: 0} + } + if ar == '*' || br == '*' { + return Cost{SwapAB: 0, DeleteA: 0, InsertB: 0} + } + if ar == '?' || br == '?' { + return Cost{SwapAB: 0, DeleteA: 1, InsertB: 1} + } + return StandardCost(ar, br) +} + func Distance(a, b string, f CostFunc, cut int64) int64 { if a == b { return 0 @@ -102,8 +124,8 @@ func Distance(a, b string, f CostFunc, cut int64) int64 { // Supported wildcards: // // ? - Any one character, except for / -// * - Any zero or more characters, execept for / -// ** - Any zero or more characrers, including / +// * - Any zero or more characters, except for / +// ** - Any zero or more characters, including / func GlobPath(a, b string) bool { a = strings.ReplaceAll(a, "**", "⁑") b = strings.ReplaceAll(b, "**", "⁑") diff --git a/internal/strdist/strdist_test.go b/internal/strdist/strdist_test.go index 7f8975a0..41ebbe41 100644 --- a/internal/strdist/strdist_test.go +++ b/internal/strdist/strdist_test.go @@ -58,6 +58,12 @@ var distanceTests = []distanceTest{ {f: strdist.GlobCost, r: 1, a: "a**f/hij", b: "abc/def/hik"}, {f: strdist.GlobCost, r: 2, a: "a**fg", b: "abc/def/hik"}, {f: strdist.GlobCost, r: 0, a: "a**f/hij/klm", b: "abc/d**m"}, + {f: strdist.StandardGlobCost, r: 3, a: "abcdefg", b: "axcdfgh"}, + {f: strdist.StandardGlobCost, r: 2, cut: 2, a: "abcdef", b: "abc"}, + {f: strdist.StandardGlobCost, r: 2, cut: 3, a: "abcdef", b: "abcd"}, + {f: strdist.StandardGlobCost, r: 1, a: "a*", b: "foo"}, + {f: strdist.StandardGlobCost, r: 1, a: "af*", b: "foo"}, + {f: strdist.StandardGlobCost, r: 1, a: "f?", b: "foo"}, } func (s *S) TestDistance(c *C) { From fffa3cac908ccb61709acf1d54c1ce8756147297 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 28 Jun 2024 12:38:08 +0200 Subject: [PATCH 18/20] update spread tests --- tests/find/task.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/find/task.yaml b/tests/find/task.yaml index b87fca1b..283d7bc3 100644 --- a/tests/find/task.yaml +++ b/tests/find/task.yaml @@ -9,6 +9,6 @@ execute: | } find "ca-certificates_data" "ca-certificates_data" - find "ca-certificates_data" "ca-certificates" "data" - find "ca-certificates_data" "data" "ca-certificates" - ! find "ca-certificates_data" "ca-certificates" "foo" \ No newline at end of file + find "ca-certificates_data" "ca-certificates" "_data" + find "ca-certificates_data" "_data" "ca-certificates" + ! find "ca-certificates_data" "ca-certificates" "foo" From d124e2503566c90d7702f9db1f42ac7b938cb283 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 28 Jun 2024 17:34:07 +0200 Subject: [PATCH 19/20] help strings --- cmd/chisel/cmd_cut.go | 6 +++--- cmd/chisel/cmd_find.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 1c081ee3..06c43920 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -14,12 +14,12 @@ var longCutHelp = ` The cut command uses the provided selection of package slices to create a new filesystem tree in the root location. -By default it fetches the slices for the latest Ubuntu -version, unless the --release flag is used. +By default it fetches the slices for the same Ubuntu version as the +current host, unless the --release flag is used. ` var cutDescs = map[string]string{ - "release": "Chisel release directory or Ubuntu version", + "release": "Chisel release name or directory (e.g. ubuntu-22.04)", "root": "Root for generated content", "arch": "Package architecture", } diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index ac8a5ddc..812e83f1 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -17,12 +17,12 @@ var longFindHelp = ` The find command queries the slice definitions for matching slices. Globs (* and ?) are allowed in the query. -By default it fetches the slices for the latest Ubuntu -version, unless the --release flag is used. +By default it fetches the slices for the same Ubuntu version as the +current host, unless the --release flag is used. ` var findDescs = map[string]string{ - "release": "Chisel release directory or Ubuntu version", + "release": "Chisel release name or directory (e.g. ubuntu-22.04)", } type cmdFind struct { From 7dbe3744912f2d58805f689bc04cf90a9660bc2c Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 28 Jun 2024 18:03:31 +0200 Subject: [PATCH 20/20] move cost to private funciton --- cmd/chisel/cmd_find.go | 24 +++++++++++++++++++++++- internal/strdist/strdist.go | 22 ---------------------- internal/strdist/strdist_test.go | 6 ------ 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 812e83f1..91c3f7f7 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -79,7 +79,7 @@ func match(slice *setup.Slice, query string) bool { term = slice.Package } query = strings.ReplaceAll(query, "**", "⁑") - return strdist.Distance(term, query, strdist.StandardGlobCost, 0) <= 1 + return strdist.Distance(term, query, distWithGlobs, 0) <= 1 } // findSlices returns slices from the provided release that match all of the @@ -112,3 +112,25 @@ func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, func tabWriter() *tabwriter.Writer { return tabwriter.NewWriter(Stdout, 5, 3, 2, ' ', 0) } + +// distWithGlobs encodes the standard Levenshtein distance with support for +// "*", "?" and "**". However, because it works on runes "**" has to be encoded +// as "⁑" in the strings. +// +// Supported wildcards: +// +// ? - Any one character +// * - Any zero or more characters +// ⁑ - Any zero or more characters +func distWithGlobs(ar, br rune) strdist.Cost { + if ar == '⁑' || br == '⁑' { + return strdist.Cost{SwapAB: 0, DeleteA: 0, InsertB: 0} + } + if ar == '*' || br == '*' { + return strdist.Cost{SwapAB: 0, DeleteA: 0, InsertB: 0} + } + if ar == '?' || br == '?' { + return strdist.Cost{SwapAB: 0, DeleteA: 1, InsertB: 1} + } + return strdist.StandardCost(ar, br) +} diff --git a/internal/strdist/strdist.go b/internal/strdist/strdist.go index 404102b1..f205bfcf 100644 --- a/internal/strdist/strdist.go +++ b/internal/strdist/strdist.go @@ -28,28 +28,6 @@ func StandardCost(ar, br rune) Cost { return Cost{SwapAB: 1, DeleteA: 1, InsertB: 1} } -// StandardGlobCost encodes the standard Levenshtein distance with support for -// "*", "?" and "**". However, because it works on runes "**" has to be encoded -// as "⁑" in the strings. -// -// Supported wildcards: -// -// ? - Any one character -// * - Any zero or more characters -// ⁑ - Any zero or more characters -func StandardGlobCost(ar, br rune) Cost { - if ar == '⁑' || br == '⁑' { - return Cost{SwapAB: 0, DeleteA: 0, InsertB: 0} - } - if ar == '*' || br == '*' { - return Cost{SwapAB: 0, DeleteA: 0, InsertB: 0} - } - if ar == '?' || br == '?' { - return Cost{SwapAB: 0, DeleteA: 1, InsertB: 1} - } - return StandardCost(ar, br) -} - func Distance(a, b string, f CostFunc, cut int64) int64 { if a == b { return 0 diff --git a/internal/strdist/strdist_test.go b/internal/strdist/strdist_test.go index 41ebbe41..7f8975a0 100644 --- a/internal/strdist/strdist_test.go +++ b/internal/strdist/strdist_test.go @@ -58,12 +58,6 @@ var distanceTests = []distanceTest{ {f: strdist.GlobCost, r: 1, a: "a**f/hij", b: "abc/def/hik"}, {f: strdist.GlobCost, r: 2, a: "a**fg", b: "abc/def/hik"}, {f: strdist.GlobCost, r: 0, a: "a**f/hij/klm", b: "abc/d**m"}, - {f: strdist.StandardGlobCost, r: 3, a: "abcdefg", b: "axcdfgh"}, - {f: strdist.StandardGlobCost, r: 2, cut: 2, a: "abcdef", b: "abc"}, - {f: strdist.StandardGlobCost, r: 2, cut: 3, a: "abcdef", b: "abcd"}, - {f: strdist.StandardGlobCost, r: 1, a: "a*", b: "foo"}, - {f: strdist.StandardGlobCost, r: 1, a: "af*", b: "foo"}, - {f: strdist.StandardGlobCost, r: 1, a: "f?", b: "foo"}, } func (s *S) TestDistance(c *C) {