From e9edc10c9511034ee853347f0ce29adfcf74b444 Mon Sep 17 00:00:00 2001 From: Phillip Nguyen Date: Wed, 10 Jul 2024 13:34:22 +0000 Subject: [PATCH] Allow using semantic names for priority levels. This maps certain strings like "rollback" or "canary" to a standard integer priority so that priorities in repo configs may be specified with these strings. --- client/client.go | 7 ++++--- googet.go | 35 +++++++++++++++++------------------ googet_test.go | 31 ++++++++++++++++++------------- googet_update.go | 3 ++- goolib/goospec.go | 3 ++- goolib/goospec_test.go | 29 +++++++++++++++-------------- priority/priority.go | 38 ++++++++++++++++++++++++++++++++++++++ priority/priority_test.go | 31 +++++++++++++++++++++++++++++++ 8 files changed, 127 insertions(+), 50 deletions(-) create mode 100644 priority/priority.go create mode 100644 priority/priority_test.go diff --git a/client/client.go b/client/client.go index 339d884..039eeae 100644 --- a/client/client.go +++ b/client/client.go @@ -33,6 +33,7 @@ import ( "cloud.google.com/go/storage" "github.com/google/googet/v2/goolib" "github.com/google/googet/v2/oswrap" + "github.com/google/googet/v2/priority" "github.com/google/logger" "golang.org/x/oauth2/google" "google.golang.org/api/googleapi" @@ -94,7 +95,7 @@ func (ps *PackageState) Match(pi goolib.PackageInfo) bool { // Repo represents a single downloaded repo. type Repo struct { - Priority int + Priority priority.Value Packages []goolib.RepoSpec } @@ -102,7 +103,7 @@ type Repo struct { type RepoMap map[string]Repo // AvailableVersions builds a RepoMap from a list of sources. -func AvailableVersions(ctx context.Context, srcs map[string]int, cacheDir string, cacheLife time.Duration, proxyServer string) RepoMap { +func AvailableVersions(ctx context.Context, srcs map[string]priority.Value, cacheDir string, cacheLife time.Duration, proxyServer string) RepoMap { rm := make(RepoMap) for r, pri := range srcs { rf, err := unmarshalRepoPackages(ctx, r, cacheDir, cacheLife, proxyServer) @@ -329,7 +330,7 @@ func FindRepoSpec(pi goolib.PackageInfo, repo Repo) (goolib.RepoSpec, error) { // package specs in psm. func latest(psm map[string][]*goolib.PkgSpec, rm RepoMap) (string, string) { var ver, repo string - var pri int + var pri priority.Value for r, pl := range psm { for _, pkg := range pl { q := rm[r].Priority diff --git a/googet.go b/googet.go index a5a30c5..26be5d7 100644 --- a/googet.go +++ b/googet.go @@ -25,13 +25,13 @@ import ( "os" "path" "path/filepath" - "strconv" "strings" "time" "github.com/go-yaml/yaml" "github.com/google/googet/v2/client" "github.com/google/googet/v2/goolib" + "github.com/google/googet/v2/priority" "github.com/google/googet/v2/system" "github.com/google/logger" "github.com/google/subcommands" @@ -39,14 +39,13 @@ import ( ) const ( - stateFile = "googet.state" - confFile = "googet.conf" - logFile = "googet.log" - cacheDir = "cache" - repoDir = "repos" - envVar = "GooGetRoot" - logSize = 10 * 1024 * 1024 - defaultPriority = 500 + stateFile = "googet.state" + confFile = "googet.conf" + logFile = "googet.log" + cacheDir = "cache" + repoDir = "repos" + envVar = "GooGetRoot" + logSize = 10 * 1024 * 1024 ) var ( @@ -84,11 +83,11 @@ type repoEntry struct { Name string URL string UseOAuth bool - Priority int + Priority priority.Value } // UnmarshalYAML provides custom unmarshalling for repoEntry objects. -func (r *repoEntry) UnmarshalYAML(unmarshal func(interface{}) error) error { +func (r *repoEntry) UnmarshalYAML(unmarshal func(any) error) error { var u map[string]string if err := unmarshal(&u); err != nil { return err @@ -103,7 +102,7 @@ func (r *repoEntry) UnmarshalYAML(unmarshal func(interface{}) error) error { r.UseOAuth = strings.ToLower(v) == "true" case "priority": var err error - r.Priority, err = strconv.Atoi(v) + r.Priority, err = priority.FromString(v) if err != nil { return fmt.Errorf("invalid priority: %v", v) } @@ -195,12 +194,12 @@ func validateRepoURL(u string) bool { // The repos are mapped to priority values. If a repo config does not specify a priority, the repo // is assigned the default priority value. If the same repo appears multiple times with different // priority values, it is mapped to the highest seen priority value. -func repoList(dir string) (map[string]int, error) { +func repoList(dir string) (map[string]priority.Value, error) { rfs, err := repos(dir) if err != nil { return nil, err } - result := make(map[string]int) + result := make(map[string]priority.Value) for _, rf := range rfs { for _, re := range rf.repoEntries { u := re.URL @@ -212,7 +211,7 @@ func repoList(dir string) (map[string]int, error) { } p := re.Priority if p <= 0 { - p = defaultPriority + p = priority.Default } if q, ok := result[u]; !ok || p > q { result[u] = p @@ -292,13 +291,13 @@ func readStateFromPath(sf string) (*client.GooGetState, error) { return client.UnmarshalState(b) } -func buildSources(s string) (map[string]int, error) { +func buildSources(s string) (map[string]priority.Value, error) { if s == "" { return repoList(filepath.Join(rootDir, repoDir)) } - m := make(map[string]int) + m := make(map[string]priority.Value) for _, src := range strings.Split(s, ",") { - m[src] = defaultPriority + m[src] = priority.Default } return m, nil } diff --git a/googet_test.go b/googet_test.go index ca9a6c9..cf2ebad 100644 --- a/googet_test.go +++ b/googet_test.go @@ -25,6 +25,7 @@ import ( "github.com/google/googet/v2/client" "github.com/google/googet/v2/goolib" "github.com/google/googet/v2/oswrap" + "github.com/google/googet/v2/priority" ) func TestRepoList(t *testing.T) { @@ -41,30 +42,34 @@ func TestRepoList(t *testing.T) { repoTests := []struct { content []byte - want map[string]int + want map[string]priority.Value allowUnsafeURL bool }{ {[]byte("\n"), nil, false}, {[]byte("# This is just a comment"), nil, false}, - {[]byte("url: " + testRepo), map[string]int{testRepo: 500}, false}, - {[]byte("\n # Comment\nurl: " + testRepo), map[string]int{testRepo: 500}, false}, - {[]byte("- url: " + testRepo), map[string]int{testRepo: 500}, false}, + {[]byte("url: " + testRepo), map[string]priority.Value{testRepo: priority.Default}, false}, + {[]byte("\n # Comment\nurl: " + testRepo), map[string]priority.Value{testRepo: priority.Default}, false}, + {[]byte("- url: " + testRepo), map[string]priority.Value{testRepo: priority.Default}, false}, // The HTTP repo should be dropped. {[]byte("- url: " + testHTTPRepo), nil, false}, // The HTTP repo should not be dropped. - {[]byte("- url: " + testHTTPRepo), map[string]int{testHTTPRepo: 500}, true}, - {[]byte("- URL: " + testRepo), map[string]int{testRepo: 500}, false}, + {[]byte("- url: " + testHTTPRepo), map[string]priority.Value{testHTTPRepo: priority.Default}, true}, + {[]byte("- URL: " + testRepo), map[string]priority.Value{testRepo: priority.Default}, false}, // The HTTP repo should be dropped. - {[]byte("- url: " + testRepo + "\n\n- URL: " + testHTTPRepo), map[string]int{testRepo: 500}, false}, + {[]byte("- url: " + testRepo + "\n\n- URL: " + testHTTPRepo), map[string]priority.Value{testRepo: priority.Default}, false}, // The HTTP repo should not be dropped. - {[]byte("- url: " + testRepo + "\n\n- URL: " + testHTTPRepo), map[string]int{testRepo: 500, testHTTPRepo: 500}, true}, - {[]byte("- url: " + testRepo + "\n\n- URL: " + testRepo), map[string]int{testRepo: 500}, false}, - {[]byte("- url: " + testRepo + "\n\n- url: " + testRepo), map[string]int{testRepo: 500}, false}, + {[]byte("- url: " + testRepo + "\n\n- URL: " + testHTTPRepo), map[string]priority.Value{testRepo: priority.Default, testHTTPRepo: 500}, true}, + {[]byte("- url: " + testRepo + "\n\n- URL: " + testRepo), map[string]priority.Value{testRepo: priority.Default}, false}, + {[]byte("- url: " + testRepo + "\n\n- url: " + testRepo), map[string]priority.Value{testRepo: priority.Default}, false}, // Should contain oauth- prefix - {[]byte("- url: " + testRepo + "\n useoauth: true"), map[string]int{"oauth-" + testRepo: 500}, false}, + {[]byte("- url: " + testRepo + "\n useoauth: true"), map[string]priority.Value{"oauth-" + testRepo: priority.Default}, false}, // Should not contain oauth- prefix - {[]byte("- url: " + testRepo + "\n useoauth: false"), map[string]int{testRepo: 500}, false}, - {[]byte("- url: " + testRepo + "\n priority: 1200"), map[string]int{testRepo: 1200}, false}, + {[]byte("- url: " + testRepo + "\n useoauth: false"), map[string]priority.Value{testRepo: priority.Default}, false}, + {[]byte("- url: " + testRepo + "\n priority: 1200"), map[string]priority.Value{testRepo: priority.Value(1200)}, false}, + {[]byte("- url: " + testRepo + "\n priority: default"), map[string]priority.Value{testRepo: priority.Default}, false}, + {[]byte("- url: " + testRepo + "\n priority: canary"), map[string]priority.Value{testRepo: priority.Canary}, false}, + {[]byte("- url: " + testRepo + "\n priority: pin"), map[string]priority.Value{testRepo: priority.Pin}, false}, + {[]byte("- url: " + testRepo + "\n priority: rollback"), map[string]priority.Value{testRepo: priority.Rollback}, false}, } for i, tt := range repoTests { diff --git a/googet_update.go b/googet_update.go index f70b66f..57841f9 100644 --- a/googet_update.go +++ b/googet_update.go @@ -25,6 +25,7 @@ import ( "github.com/google/googet/v2/client" "github.com/google/googet/v2/goolib" "github.com/google/googet/v2/install" + "github.com/google/googet/v2/priority" "github.com/google/logger" "github.com/google/subcommands" ) @@ -112,7 +113,7 @@ func updates(pm packageMap, rm client.RepoMap) []goolib.PackageInfo { logger.Info(err) continue } - c, err := goolib.ComparePriorityVersion(rm[r].Priority, v, defaultPriority, ver) + c, err := goolib.ComparePriorityVersion(rm[r].Priority, v, priority.Default, ver) if err != nil { logger.Error(err) continue diff --git a/goolib/goospec.go b/goolib/goospec.go index 33c5079..cf6813c 100644 --- a/goolib/goospec.go +++ b/goolib/goospec.go @@ -31,6 +31,7 @@ import ( "time" "github.com/blang/semver" + "github.com/google/googet/v2/priority" ) type build struct { @@ -170,7 +171,7 @@ func fixVer(ver string) string { } // ComparePriorityVersion compares (p1, v1) to (p2, v2) as priority-version tuples. -func ComparePriorityVersion(p1 int, v1 string, p2 int, v2 string) (int, error) { +func ComparePriorityVersion(p1 priority.Value, v1 string, p2 priority.Value, v2 string) (int, error) { if p1 < p2 { return -1, nil } diff --git a/goolib/goospec_test.go b/goolib/goospec_test.go index 532dcba..5586f5e 100644 --- a/goolib/goospec_test.go +++ b/goolib/goospec_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/blang/semver" + "github.com/google/googet/v2/priority" ) func mkVer(sem string, rel int64) Version { @@ -208,24 +209,24 @@ func TestBadCompare(t *testing.T) { func TestComparePriorityVersion(t *testing.T) { for _, tc := range []struct { desc string - p1 int + p1 priority.Value v1 string - p2 int + p2 priority.Value v2 string want int }{ - {"same priority, same version, lesser release", 500, "1.2.3@1", 500, "1.2.3@2", -1}, - {"same priority lesser version", 500, "1.2.3@1", 500, "1.2.4@2", -1}, - {"same priority, greater version", 500, "1.2.4@1", 500, "1.2.3@2", 1}, - {"same priority, same version", 500, "1.2.3", 500, "1.2.3", 0}, - {"lesser priority, same version, lesser release", 500, "1.2.3@1", 1000, "1.2.3@2", -1}, - {"lesser priority, lesser version", 500, "1.2.3@1", 1000, "1.2.4@2", -1}, - {"lesser priority, greater version", 500, "1.2.4@1", 1000, "1.2.3@2", -1}, - {"lesser priority, same version", 500, "1.2.3", 1000, "1.2.3", -1}, - {"greater priority, same version, lesser release", 1000, "1.2.3@1", 500, "1.2.3@2", 1}, - {"greater priority, lesser version", 1000, "1.2.3@1", 500, "1.2.4@2", 1}, - {"greater priority, greater version", 1000, "1.2.4@1", 500, "1.2.3@2", 1}, - {"greater priority, same version", 1000, "1.2.3", 500, "1.2.3", 1}, + {"same priority, same version, lesser release", priority.Value(500), "1.2.3@1", priority.Value(500), "1.2.3@2", -1}, + {"same priority lesser version", priority.Value(500), "1.2.3@1", priority.Value(500), "1.2.4@2", -1}, + {"same priority, greater version", priority.Value(500), "1.2.4@1", priority.Value(500), "1.2.3@2", 1}, + {"same priority, same version", priority.Value(500), "1.2.3", priority.Value(500), "1.2.3", 0}, + {"lesser priority, same version, lesser release", priority.Value(500), "1.2.3@1", priority.Value(1000), "1.2.3@2", -1}, + {"lesser priority, lesser version", priority.Value(500), "1.2.3@1", priority.Value(1000), "1.2.4@2", -1}, + {"lesser priority, greater version", priority.Value(500), "1.2.4@1", priority.Value(1000), "1.2.3@2", -1}, + {"lesser priority, same version", priority.Value(500), "1.2.3", priority.Value(1000), "1.2.3", -1}, + {"greater priority, same version, lesser release", priority.Value(1000), "1.2.3@1", priority.Value(500), "1.2.3@2", 1}, + {"greater priority, lesser version", priority.Value(1000), "1.2.3@1", priority.Value(500), "1.2.4@2", 1}, + {"greater priority, greater version", priority.Value(1000), "1.2.4@1", priority.Value(500), "1.2.3@2", 1}, + {"greater priority, same version", priority.Value(1000), "1.2.3", priority.Value(500), "1.2.3", 1}, } { t.Run(tc.desc, func(t *testing.T) { if got, err := ComparePriorityVersion(tc.p1, tc.v1, tc.p2, tc.v2); err != nil { diff --git a/priority/priority.go b/priority/priority.go new file mode 100644 index 0000000..1cf9c7a --- /dev/null +++ b/priority/priority.go @@ -0,0 +1,38 @@ +// Package priority provides standard priority levels. +package priority + +import "strconv" + +// Value is an integer priority associated with a repo. +type Value int + +const ( + // None is an unspecified priority level. + None Value = 0 + // Default is the default priority level for everything. + Default Value = 500 + // Canary is the priority level for canary repos. + Canary Value = 1300 + // Pin is the priority level for user-pinned repos. + Pin Value = 1400 + // Rollback is the priority level for rollback repos. + Rollback Value = 1500 +) + +// priorityNameToValue maps semantic priority names to their integer values. +var priorityNameToValue = map[string]Value{ + "default": Default, + "canary": Canary, + "pin": Pin, + "rollback": Rollback, +} + +// FromString converts the string s into a priority Value, where s represents +// either a semantic priority name or an integer. +func FromString(s string) (Value, error) { + if v, ok := priorityNameToValue[s]; ok { + return v, nil + } + i, err := strconv.Atoi(s) + return Value(i), err +} diff --git a/priority/priority_test.go b/priority/priority_test.go new file mode 100644 index 0000000..1e4ff2d --- /dev/null +++ b/priority/priority_test.go @@ -0,0 +1,31 @@ +package priority + +import ( + "testing" +) + +func TestFromString(t *testing.T) { + for _, tc := range []struct { + s string + want Value + wantErr bool + }{ + {s: "default", want: Default}, + {s: "canary", want: Canary}, + {s: "pin", want: Pin}, + {s: "rollback", want: Rollback}, + {s: "100", want: Value(100)}, + {s: "bad", wantErr: true}, + } { + t.Run(tc.s, func(t *testing.T) { + got, err := FromString(tc.s) + if err != nil && !tc.wantErr { + t.Errorf("FromString(%s) failed: %v", tc.s, err) + } else if err == nil && tc.wantErr { + t.Errorf("FromString(%s) got nil error, wanted non-nil", tc.s) + } else if got != tc.want { + t.Errorf("FromString(%s) got: %v, want: %v", tc.s, got, tc.want) + } + }) + } +}