diff --git a/client/client.go b/client/client.go index a8af407..339d884 100644 --- a/client/client.go +++ b/client/client.go @@ -92,19 +92,28 @@ func (ps *PackageState) Match(pi goolib.PackageInfo) bool { return ps.PackageSpec.Name == pi.Name && (ps.PackageSpec.Arch == pi.Arch || pi.Arch == "") && (ps.PackageSpec.Version == pi.Ver || pi.Ver == "") } +// Repo represents a single downloaded repo. +type Repo struct { + Priority int + Packages []goolib.RepoSpec +} + // RepoMap describes each repo's packages as seen from a client. -type RepoMap map[string][]goolib.RepoSpec +type RepoMap map[string]Repo // AvailableVersions builds a RepoMap from a list of sources. -func AvailableVersions(ctx context.Context, srcs []string, cacheDir string, cacheLife time.Duration, proxyServer string) RepoMap { +func AvailableVersions(ctx context.Context, srcs map[string]int, cacheDir string, cacheLife time.Duration, proxyServer string) RepoMap { rm := make(RepoMap) - for _, r := range srcs { + for r, pri := range srcs { rf, err := unmarshalRepoPackages(ctx, r, cacheDir, cacheLife, proxyServer) if err != nil { logger.Errorf("error reading repo %q: %v", r, err) continue } - rm[r] = rf + rm[r] = Repo{ + Priority: pri, + Packages: rf, + } } return rm } @@ -157,7 +166,7 @@ func decode(index io.ReadCloser, ct, url, cf string) ([]goolib.RepoSpec, error) // unmarshalRepoPackages gets and unmarshals a repository URL or uses the cached contents // if mtime is less than cacheLife. -// Sucessfully unmarshalled contents will be written to a cache. +// Successfully unmarshalled contents will be written to a cache. func unmarshalRepoPackages(ctx context.Context, p, cacheDir string, cacheLife time.Duration, proxyServer string) ([]goolib.RepoSpec, error) { pName := strings.TrimPrefix(p, "oauth-") @@ -305,9 +314,9 @@ func unmarshalRepoPackagesGCS(ctx context.Context, bucket, object, url, cf strin return decode(r, "application/json", url, cf) } -// FindRepoSpec returns the element of pl whose PackageSpec matches pi. -func FindRepoSpec(pi goolib.PackageInfo, pl []goolib.RepoSpec) (goolib.RepoSpec, error) { - for _, p := range pl { +// FindRepoSpec returns the RepoSpec in repo whose PackageSpec matches pi. +func FindRepoSpec(pi goolib.PackageInfo, repo Repo) (goolib.RepoSpec, error) { + for _, p := range repo.Packages { ps := p.PackageSpec if ps.Name == pi.Name && ps.Arch == pi.Arch && ps.Version == pi.Ver { return p, nil @@ -316,66 +325,63 @@ func FindRepoSpec(pi goolib.PackageInfo, pl []goolib.RepoSpec) (goolib.RepoSpec, return goolib.RepoSpec{}, fmt.Errorf("no match found for package %s.%s.%s in repo", pi.Name, pi.Arch, pi.Ver) } -func latest(psm map[string][]*goolib.PkgSpec) (ver, repo string) { +// latest returns the version and repo having the greatest (priority, version) from the set of +// package specs in psm. +func latest(psm map[string][]*goolib.PkgSpec, rm RepoMap) (string, string) { + var ver, repo string + var pri int for r, pl := range psm { - for _, p := range pl { - if ver == "" { - repo = r - ver = p.Version - continue - } - c, err := goolib.Compare(p.Version, ver) - if err != nil { - logger.Errorf("compare of %s to %s failed with error: %v", p.Version, ver, err) + for _, pkg := range pl { + q := rm[r].Priority + c := 1 + if ver != "" { + var err error + if c, err = goolib.ComparePriorityVersion(q, pkg.Version, pri, ver); err != nil { + logger.Errorf("compare of %s to %s failed with error: %v", pkg.Version, ver, err) + continue + } } if c == 1 { repo = r - ver = p.Version + ver = pkg.Version + pri = q } } } - return + return ver, repo } // FindRepoLatest returns the latest version of a package along with its repo and arch. -func FindRepoLatest(pi goolib.PackageInfo, rm RepoMap, archs []string) (ver, repo, arch string, err error) { +// The archs are searched in order; if a matching package is found for any arch, it is +// returned immediately even if a later arch might have a later version. +func FindRepoLatest(pi goolib.PackageInfo, rm RepoMap, archs []string) (string, string, string, error) { psm := make(map[string][]*goolib.PkgSpec) + name := pi.Name if pi.Arch != "" { - for r, pl := range rm { - for _, p := range pl { - if p.PackageSpec.Name == pi.Name && p.PackageSpec.Arch == pi.Arch { - psm[r] = append(psm[r], p.PackageSpec) - } - } - } - if len(psm) != 0 { - v, r := latest(psm) - return v, r, pi.Arch, nil - } - return "", "", "", fmt.Errorf("no versions of package %s.%s found in any repo", pi.Name, pi.Arch) + archs = []string{pi.Arch} + name = fmt.Sprintf("%s.%s", pi.Name, pi.Arch) } - for _, a := range archs { - for r, pl := range rm { - for _, p := range pl { + for r, repo := range rm { + for _, p := range repo.Packages { if p.PackageSpec.Name == pi.Name && p.PackageSpec.Arch == a { psm[r] = append(psm[r], p.PackageSpec) } } } if len(psm) != 0 { - v, r := latest(psm) + v, r := latest(psm, rm) return v, r, a, nil } } - return "", "", "", fmt.Errorf("no versions of package %s found in any repo", pi.Name) + return "", "", "", fmt.Errorf("no versions of package %s found in any repo", name) } // WhatRepo returns what repo a package is in. // Name, Arch, and Ver fields of PackageInfo must be provided. func WhatRepo(pi goolib.PackageInfo, rm RepoMap) (string, error) { - for r, pl := range rm { - for _, p := range pl { + for r, repo := range rm { + for _, p := range repo.Packages { if p.PackageSpec.Name == pi.Name && p.PackageSpec.Arch == pi.Arch && p.PackageSpec.Version == pi.Ver { return r, nil } diff --git a/client/client_test.go b/client/client_test.go index 5562f40..04cfb31 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -96,12 +96,14 @@ func TestGetPackageStateNoMatch(t *testing.T) { func TestWhatRepo(t *testing.T) { rm := RepoMap{ - "foo_repo": []goolib.RepoSpec{ - { - PackageSpec: &goolib.PkgSpec{ - Name: "foo_pkg", - Version: "1.2.3@4", - Arch: "noarch", + "foo_repo": Repo{ + Packages: []goolib.RepoSpec{ + { + PackageSpec: &goolib.PkgSpec{ + Name: "foo_pkg", + Version: "1.2.3@4", + Arch: "noarch", + }, }, }, }, @@ -117,62 +119,123 @@ func TestWhatRepo(t *testing.T) { } func TestFindRepoLatest(t *testing.T) { - archs := []string{"noarch", "x86_64"} - rm := RepoMap{ - "foo_repo": []goolib.RepoSpec{ - { - PackageSpec: &goolib.PkgSpec{ - Name: "foo_pkg", - Version: "1.2.3@4", - Arch: "noarch", - }, + for _, tt := range []struct { + desc string + pi goolib.PackageInfo + archs []string + rm RepoMap + wantVersion string + wantArch string + wantRepo string + wantErr bool + }{ + { + desc: "name and arch", + pi: goolib.PackageInfo{Name: "foo_pkg", Arch: "noarch"}, + archs: []string{"noarch", "x86_64"}, + rm: RepoMap{ + "foo_repo": Repo{Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.2.3@4", Arch: "noarch"}}, + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "2.0.0@1", Arch: "x86_64"}}, + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.0.0@1", Arch: "noarch"}}, + {PackageSpec: &goolib.PkgSpec{Name: "bar_pkg", Version: "2.3.0@1", Arch: "noarch"}}, + }}, }, - { - PackageSpec: &goolib.PkgSpec{ - Name: "foo_pkg", - Version: "1.0.0@1", - Arch: "noarch", + wantVersion: "1.2.3@4", + wantArch: "noarch", + wantRepo: "foo_repo", + }, + { + desc: "name only", + pi: goolib.PackageInfo{Name: "foo_pkg"}, + archs: []string{"noarch", "x86_64"}, + rm: RepoMap{ + "foo_repo": Repo{Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.2.3@4", Arch: "noarch"}}, + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "2.0.0@1", Arch: "x86_64"}}, + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.0.0@1", Arch: "noarch"}}, + {PackageSpec: &goolib.PkgSpec{Name: "bar_pkg", Version: "2.3.0@1", Arch: "noarch"}}, + }}, + }, + wantVersion: "1.2.3@4", + wantArch: "noarch", + wantRepo: "foo_repo", + }, + { + desc: "specified arch not present", + pi: goolib.PackageInfo{Name: "foo_pkg", Arch: "x86_64"}, + archs: []string{"noarch", "x86_64"}, + rm: RepoMap{ + "foo_repo": Repo{Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.2.3@4", Arch: "noarch"}}, + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.0.0@1", Arch: "noarch"}}, + {PackageSpec: &goolib.PkgSpec{Name: "bar_pkg", Version: "2.3.0@1", Arch: "noarch"}}, + }}, + }, + wantErr: true, + }, + { + desc: "multiple repos with same priority", + pi: goolib.PackageInfo{Name: "foo_pkg", Arch: "noarch"}, + archs: []string{"noarch", "x86_64"}, + rm: RepoMap{ + "foo_repo": Repo{ + Priority: 500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.2.3@4", Arch: "noarch"}}, + }, + }, + "bar_repo": Repo{ + Priority: 500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "2.4.5@1", Arch: "noarch"}}, + }, }, }, - { - PackageSpec: &goolib.PkgSpec{ - Name: "bar_pkg", - Version: "1.0.0@1", - Arch: "noarch", + wantVersion: "2.4.5@1", + wantArch: "noarch", + wantRepo: "bar_repo", + }, + { + desc: "multiple repos with different priority", + pi: goolib.PackageInfo{Name: "foo_pkg", Arch: "noarch"}, + archs: []string{"noarch", "x86_64"}, + rm: RepoMap{ + "high_priority_repo": Repo{ + Priority: 1500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "1.2.3@4", Arch: "noarch"}}, + }, + }, + "low_priority_repo": Repo{ + Priority: 500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo_pkg", Version: "2.4.5@1", Arch: "noarch"}}, + }, }, }, + wantVersion: "1.2.3@4", + wantArch: "noarch", + wantRepo: "high_priority_repo", }, - } - - table := []struct { - pkg string - arch string - wVer string - wArch string - wRepo string - }{ - {"foo_pkg", "noarch", "1.2.3@4", "noarch", "foo_repo"}, - {"foo_pkg", "", "1.2.3@4", "noarch", "foo_repo"}, - } - for _, tt := range table { - gotVer, gotRepo, gotArch, err := FindRepoLatest(goolib.PackageInfo{Name: tt.pkg, Arch: tt.arch, Ver: ""}, rm, archs) - if err != nil { - t.Fatalf("FindRepoLatest failed: %v", err) - } - if gotVer != tt.wVer { - t.Errorf("FindRepoLatest for %q, %q returned version: %q, want %q", tt.pkg, tt.arch, gotVer, tt.wVer) - } - if gotArch != tt.wArch { - t.Errorf("FindRepoLatest for %q, %q returned arch: %q, want %q", tt.pkg, tt.arch, gotArch, tt.wArch) - } - if gotRepo != tt.wRepo { - t.Errorf("FindRepoLatest for %q, %q returned repo: %q, want %q", tt.pkg, tt.arch, gotRepo, tt.wRepo) - } - } - - werr := "no versions of package bar_pkg.x86_64 found in any repo" - if _, _, _, err := FindRepoLatest(goolib.PackageInfo{Name: "bar_pkg", Arch: "x86_64", Ver: ""}, rm, archs); err.Error() != werr { - t.Errorf("did not get expected error: got %q, want %q", err, werr) + } { + t.Run(tt.desc, func(t *testing.T) { + gotVersion, gotRepo, gotArch, err := FindRepoLatest(tt.pi, tt.rm, tt.archs) + if err != nil && !tt.wantErr { + t.Fatalf("FindRepoLatest(%v, %v, %v) failed: %v", tt.pi, tt.rm, tt.archs, err) + } else if err == nil && tt.wantErr { + t.Fatalf("FindRepoLatest(%v, %v, %v) got nil error, wanted non-nil", tt.pi, tt.rm, tt.archs) + } + if gotVersion != tt.wantVersion { + t.Errorf("FindRepoLatest(%v, %v, %v) got version: %q, want %q", tt.pi, tt.rm, tt.archs, gotVersion, tt.wantVersion) + } + if gotArch != tt.wantArch { + t.Errorf("FindRepoLatest(%v, %v, %v) got arch: %q, want %q", tt.pi, tt.rm, tt.archs, gotArch, tt.wantArch) + } + if gotRepo != tt.wantRepo { + t.Errorf("FindRepoLatest(%v, %v, %v) got repo: %q, want %q", tt.pi, tt.rm, tt.archs, gotRepo, tt.wantRepo) + } + }) } } @@ -298,12 +361,12 @@ func TestUnmarshalRepoPackagesCache(t *testing.T) { func TestFindRepoSpec(t *testing.T) { want := goolib.RepoSpec{PackageSpec: &goolib.PkgSpec{Name: "test"}} - rs := []goolib.RepoSpec{ + repo := Repo{Packages: []goolib.RepoSpec{ want, {PackageSpec: &goolib.PkgSpec{Name: "test2"}}, - } + }} - got, err := FindRepoSpec(goolib.PackageInfo{Name: "test", Arch: "", Ver: ""}, rs) + got, err := FindRepoSpec(goolib.PackageInfo{Name: "test", Arch: "", Ver: ""}, repo) if err != nil { t.Errorf("error running FindRepoSpec: %v", err) } @@ -313,9 +376,9 @@ func TestFindRepoSpec(t *testing.T) { } func TestFindRepoSpecNoMatch(t *testing.T) { - rs := []goolib.RepoSpec{{PackageSpec: &goolib.PkgSpec{Name: "test2"}}} + repo := Repo{Packages: []goolib.RepoSpec{{PackageSpec: &goolib.PkgSpec{Name: "test2"}}}} - if _, err := FindRepoSpec(goolib.PackageInfo{Name: "test", Arch: "", Ver: ""}, rs); err == nil { + if _, err := FindRepoSpec(goolib.PackageInfo{Name: "test", Arch: "", Ver: ""}, repo); err == nil { t.Error("did not get expected error when running FindRepoSpec") } } diff --git a/go.mod b/go.mod index b571afa..53bfc30 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/blang/semver v3.5.1+incompatible github.com/dustin/go-humanize v1.0.0 github.com/go-yaml/yaml v2.1.0+incompatible + github.com/google/go-cmp v0.6.0 github.com/google/logger v1.1.1 github.com/google/subcommands v1.2.0 github.com/olekukonko/tablewriter v0.0.5 diff --git a/go.sum b/go.sum index 2e120ef..10ecd26 100644 --- a/go.sum +++ b/go.sum @@ -112,8 +112,9 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/logger v1.1.1 h1:+6Z2geNxc9G+4D4oDO9njjjn2d0wN5d7uOo0vOIW1NQ= github.com/google/logger v1.1.1/go.mod h1:BkeJZ+1FhQ+/d087r4dzojEg1u2ZX+ZqG1jTUrLM+zQ= github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no= @@ -391,7 +392,6 @@ golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M= diff --git a/googet.go b/googet.go index 4ec7a50..a5a30c5 100644 --- a/googet.go +++ b/googet.go @@ -25,6 +25,7 @@ import ( "os" "path" "path/filepath" + "strconv" "strings" "time" @@ -38,13 +39,14 @@ import ( ) const ( - stateFile = "googet.state" - confFile = "googet.conf" - logFile = "googet.log" - cacheDir = "cache" - repoDir = "repos" - envVar = "GooGetRoot" - logSize = 10 * 1024 * 1024 + stateFile = "googet.state" + confFile = "googet.conf" + logFile = "googet.log" + cacheDir = "cache" + repoDir = "repos" + envVar = "GooGetRoot" + logSize = 10 * 1024 * 1024 + defaultPriority = 500 ) var ( @@ -82,6 +84,7 @@ type repoEntry struct { Name string URL string UseOAuth bool + Priority int } // UnmarshalYAML provides custom unmarshalling for repoEntry objects. @@ -98,6 +101,12 @@ func (r *repoEntry) UnmarshalYAML(unmarshal func(interface{}) error) error { r.URL = v case "useoauth": r.UseOAuth = strings.ToLower(v) == "true" + case "priority": + var err error + r.Priority, err = strconv.Atoi(v) + if err != nil { + return fmt.Errorf("invalid priority: %v", v) + } } } if r.URL == "" { @@ -163,44 +172,54 @@ func unmarshalConfFile(p string) (*conf, error) { return &cf, yaml.Unmarshal(b, &cf) } -func repoList(dir string) ([]string, error) { +// validateRepoURL uses the global allowUnsafeURL to determine if u should be checked for https or +// GCS status. +func validateRepoURL(u string) bool { + if allowUnsafeURL { + return true + } + gcs, _, _ := goolib.SplitGCSUrl(u) + parsed, err := url.Parse(u) + if err != nil { + logger.Errorf("Failed to parse URL '%s', skipping repo", u) + return false + } + if parsed.Scheme != "https" && !gcs { + logger.Errorf("%s will not be used as a repository, only https and Google Cloud Storage endpoints will be used unless 'allowunsafeurl' is set to 'true' in googet.conf", u) + return false + } + return true +} + +// repoList returns a deduped set of all repos listed in the repo config files contained in dir. +// 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) { rfs, err := repos(dir) if err != nil { return nil, err } - var rl []string + result := make(map[string]int) for _, rf := range rfs { for _, re := range rf.repoEntries { - switch { - case re.URL != "": - if re.UseOAuth { - rl = append(rl, "oauth-"+re.URL) - } else { - rl = append(rl, re.URL) - } - } - } - } - - if !allowUnsafeURL { - var srl []string - for _, r := range rl { - rTrimmed := strings.TrimPrefix(r, "oauth-") - isGCSURL, _, _ := goolib.SplitGCSUrl(rTrimmed) - parsed, err := url.Parse(rTrimmed) - if err != nil { - logger.Errorf("Failed to parse URL '%s', skipping repo", r) + u := re.URL + if u == "" || !validateRepoURL(u) { continue } - if parsed.Scheme != "https" && !isGCSURL { - logger.Errorf("%s will not be used as a repository, only https and Google Cloud Storage endpoints will be used unless 'allowunsafeurl' is set to 'true' in googet.conf", r) - continue + if re.UseOAuth { + u = "oauth-" + u + } + p := re.Priority + if p <= 0 { + p = defaultPriority + } + if q, ok := result[u]; !ok || p > q { + result[u] = p } - srl = append(srl, r) } - return srl, nil } - return rl, nil + return result, nil } func repos(dir string) ([]repoFile, error) { @@ -273,12 +292,15 @@ func readStateFromPath(sf string) (*client.GooGetState, error) { return client.UnmarshalState(b) } -func buildSources(s string) ([]string, error) { - if s != "" { - srcs := strings.Split(s, ",") - return srcs, nil +func buildSources(s string) (map[string]int, error) { + if s == "" { + return repoList(filepath.Join(rootDir, repoDir)) + } + m := make(map[string]int) + for _, src := range strings.Split(s, ",") { + m[src] = defaultPriority } - return repoList(filepath.Join(rootDir, repoDir)) + return m, nil } func confirmation(msg string) bool { diff --git a/googet_available.go b/googet_available.go index 17cb2da..53141ae 100644 --- a/googet_available.go +++ b/googet_available.go @@ -75,8 +75,8 @@ func (cmd *availableCmd) Execute(ctx context.Context, f *flag.FlagSet, _ ...inte m := make(map[string][]string) rm := client.AvailableVersions(ctx, repos, filepath.Join(rootDir, cacheDir), cacheLife, proxyServer) - for r, pl := range rm { - for _, p := range pl { + for r, repo := range rm { + for _, p := range repo.Packages { m[r] = append(m[r], p.PackageSpec.Name+"."+p.PackageSpec.Arch+"."+p.PackageSpec.Version) } } @@ -111,8 +111,8 @@ func (cmd *availableCmd) Execute(ctx context.Context, f *flag.FlagSet, _ ...inte } func repo(pi goolib.PackageInfo, rm client.RepoMap) { - for r, pl := range rm { - for _, p := range pl { + for r, repo := range rm { + for _, p := range repo.Packages { if p.PackageSpec.Name == pi.Name && p.PackageSpec.Arch == pi.Arch && p.PackageSpec.Version == pi.Ver { info(p.PackageSpec, r) return diff --git a/googet_test.go b/googet_test.go index 3b62afd..ca9a6c9 100644 --- a/googet_test.go +++ b/googet_test.go @@ -20,6 +20,8 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/google/googet/v2/client" "github.com/google/googet/v2/goolib" "github.com/google/googet/v2/oswrap" @@ -39,29 +41,30 @@ func TestRepoList(t *testing.T) { repoTests := []struct { content []byte - want []string + want map[string]int allowUnsafeURL bool }{ {[]byte("\n"), nil, false}, {[]byte("# This is just a comment"), nil, false}, - {[]byte("url: " + testRepo), []string{testRepo}, false}, - {[]byte("\n # Comment\nurl: " + testRepo), []string{testRepo}, false}, - {[]byte("- url: " + testRepo), []string{testRepo}, 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}, // The HTTP repo should be dropped. {[]byte("- url: " + testHTTPRepo), nil, false}, // The HTTP repo should not be dropped. - {[]byte("- url: " + testHTTPRepo), []string{testHTTPRepo}, true}, - {[]byte("- URL: " + testRepo), []string{testRepo}, false}, + {[]byte("- url: " + testHTTPRepo), map[string]int{testHTTPRepo: 500}, true}, + {[]byte("- URL: " + testRepo), map[string]int{testRepo: 500}, false}, // The HTTP repo should be dropped. - {[]byte("- url: " + testRepo + "\n\n- URL: " + testHTTPRepo), []string{testRepo}, false}, + {[]byte("- url: " + testRepo + "\n\n- URL: " + testHTTPRepo), map[string]int{testRepo: 500}, false}, // The HTTP repo should not be dropped. - {[]byte("- url: " + testRepo + "\n\n- URL: " + testHTTPRepo), []string{testRepo, testHTTPRepo}, true}, - {[]byte("- url: " + testRepo + "\n\n- URL: " + testRepo), []string{testRepo, testRepo}, false}, - {[]byte("- url: " + testRepo + "\n\n- url: " + testRepo), []string{testRepo, testRepo}, false}, + {[]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}, // Should contain oauth- prefix - {[]byte("- url: " + testRepo + "\n useoauth: true"), []string{"oauth-" + testRepo}, false}, + {[]byte("- url: " + testRepo + "\n useoauth: true"), map[string]int{"oauth-" + testRepo: 500}, false}, // Should not contain oauth- prefix - {[]byte("- url: " + testRepo + "\n useoauth: false"), []string{testRepo}, false}, + {[]byte("- url: " + testRepo + "\n useoauth: false"), map[string]int{testRepo: 500}, false}, + {[]byte("- url: " + testRepo + "\n priority: 1200"), map[string]int{testRepo: 1200}, false}, } for i, tt := range repoTests { @@ -73,8 +76,8 @@ func TestRepoList(t *testing.T) { if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("test case %d: returned repo does not match expected repo: got %q, want %q", i+1, got, tt.want) + if diff := cmp.Diff(tt.want, got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("test case %d: repoList unexpected diff (-want +got): %v", i+1, diff) } } } @@ -388,3 +391,82 @@ func TestCleanPackages(t *testing.T) { t.Errorf("cleanPackages did not remove notWantDir") } } + +func TestUpdates(t *testing.T) { + for _, tc := range []struct { + name string + pm packageMap + rm client.RepoMap + want []goolib.PackageInfo + }{ + { + name: "upgrade to later version", + pm: packageMap{ + "foo.x86_32": "1.0", + "bar.x86_32": "2.0", + }, + rm: client.RepoMap{ + "stable": client.Repo{ + Priority: 500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo", Version: "2.0", Arch: "x86_32"}}, + {PackageSpec: &goolib.PkgSpec{Name: "bar", Version: "2.0", Arch: "x86_32"}}, + }, + }, + }, + want: []goolib.PackageInfo{{Name: "foo", Arch: "x86_32", Ver: "2.0"}}, + }, + { + name: "rollback to earlier version", + pm: packageMap{ + "foo.x86_32": "2.0", + "bar.x86_32": "2.0", + }, + rm: client.RepoMap{ + "stable": client.Repo{ + Priority: 500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo", Version: "2.0", Arch: "x86_32"}}, + {PackageSpec: &goolib.PkgSpec{Name: "bar", Version: "2.0", Arch: "x86_32"}}, + }, + }, + "rollback": client.Repo{ + Priority: 1500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo", Version: "1.0", Arch: "x86_32"}}, + }, + }, + }, + want: []goolib.PackageInfo{{Name: "foo", Arch: "x86_32", Ver: "1.0"}}, + }, + { + name: "no change if rollback version already installed", + pm: packageMap{ + "foo.x86_32": "1.0", + }, + rm: client.RepoMap{ + "stable": client.Repo{ + Priority: 500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo", Version: "2.0", Arch: "x86_32"}}, + {PackageSpec: &goolib.PkgSpec{Name: "bar", Version: "2.0", Arch: "x86_32"}}, + }, + }, + "rollback": client.Repo{ + Priority: 1500, + Packages: []goolib.RepoSpec{ + {PackageSpec: &goolib.PkgSpec{Name: "foo", Version: "1.0", Arch: "x86_32"}}, + }, + }, + }, + want: nil, + }, + } { + t.Run(tc.name, func(t *testing.T) { + pi := updates(tc.pm, tc.rm) + if diff := cmp.Diff(tc.want, pi); diff != "" { + t.Errorf("update(%v, %v) got unexpected diff (-want +got):\n%v", tc.pm, tc.rm, diff) + } + }) + } +} diff --git a/googet_update.go b/googet_update.go index e6c75b7..f70b66f 100644 --- a/googet_update.go +++ b/googet_update.go @@ -112,18 +112,33 @@ func updates(pm packageMap, rm client.RepoMap) []goolib.PackageInfo { logger.Info(err) continue } - c, err := goolib.Compare(v, ver) + c, err := goolib.ComparePriorityVersion(rm[r].Priority, v, defaultPriority, ver) if err != nil { logger.Error(err) continue } - if c == 1 { - fmt.Printf(" %s, %s --> %s from %s\n", p, ver, v, r) - logger.Infof("Update for package %s, %s installed and %s available from %s.", p, ver, v, r) - ud = append(ud, goolib.PackageInfo{Name: pi.Name, Arch: pi.Arch, Ver: v}) + if c < 1 { + logger.Infof("%s - highest priority version already installed", p) continue } - logger.Infof("%s - latest version installed", p) + // The versions might actually be the same even though the priorities are different, + // so do another check to skip reinstall of the same version. + c, err = goolib.Compare(v, ver) + if err != nil { + logger.Error(err) + continue + } + if c == 0 { + logger.Infof("%s - same version installed", p) + continue + } + op := "Upgrade" + if c == -1 { + op = "Downgrade" + } + fmt.Printf(" %s, %s --> %s from %s\n", p, ver, v, r) + logger.Infof("%s for package %s, %s installed and %s available from %s.", op, p, ver, v, r) + ud = append(ud, goolib.PackageInfo{Name: pi.Name, Arch: pi.Arch, Ver: v}) } return ud } diff --git a/goolib/goospec.go b/goolib/goospec.go index 34e1e8b..33c5079 100644 --- a/goolib/goospec.go +++ b/goolib/goospec.go @@ -169,6 +169,17 @@ func fixVer(ver string) string { return strings.Join(out, ".") + suffix } +// ComparePriorityVersion compares (p1, v1) to (p2, v2) as priority-version tuples. +func ComparePriorityVersion(p1 int, v1 string, p2 int, v2 string) (int, error) { + if p1 < p2 { + return -1, nil + } + if p1 > p2 { + return 1, nil + } + return Compare(v1, v2) +} + // ParseVersion parses the string version into goospec.Version. ParseVersion // attempts to fix non-compliant Semver strings by removing leading zeros from // components, and replacing any missing components with zero values after diff --git a/goolib/goospec_test.go b/goolib/goospec_test.go index 318174f..532dcba 100644 --- a/goolib/goospec_test.go +++ b/goolib/goospec_test.go @@ -205,6 +205,38 @@ func TestBadCompare(t *testing.T) { } } +func TestComparePriorityVersion(t *testing.T) { + for _, tc := range []struct { + desc string + p1 int + v1 string + p2 int + 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}, + } { + t.Run(tc.desc, func(t *testing.T) { + if got, err := ComparePriorityVersion(tc.p1, tc.v1, tc.p2, tc.v2); err != nil { + t.Fatalf("ComparePriorityVersion(%v, %v, %v, %v): %v", tc.p1, tc.v1, tc.p2, tc.v2, err) + } else if got != tc.want { + t.Fatalf("ComparePriorityVersion(%v, %v, %v, %v) got: %v; want: %v", tc.p1, tc.v1, tc.p2, tc.v2, got, tc.want) + } + }) + } +} + func TestWritePackageSpec(t *testing.T) { es := &PkgSpec{ Name: "test", diff --git a/install/install.go b/install/install.go index cb54cc8..244b875 100644 --- a/install/install.go +++ b/install/install.go @@ -122,7 +122,7 @@ func installDeps(ctx context.Context, ps *goolib.PkgSpec, cache string, rm clien ins = true } if !ins { - return fmt.Errorf("cannot resolve dependancy, %s.%s version %s or greater not installed and not available in any repo", pi.Name, arch, ver) + return fmt.Errorf("cannot resolve dependency, %s.%s version %s or greater not installed and not available in any repo", pi.Name, arch, ver) } } return resolveReplacements(ctx, ps, state, dbOnly, proxyServer) @@ -130,14 +130,6 @@ func installDeps(ctx context.Context, ps *goolib.PkgSpec, cache string, rm clien // FromRepo installs a package and all dependencies from a repository. func FromRepo(ctx context.Context, pi goolib.PackageInfo, repo, cache string, rm client.RepoMap, archs []string, state *client.GooGetState, dbOnly bool, proxyServer string) error { - ni, err := NeedsInstallation(pi, *state) - if err != nil { - return err - } - if !ni { - return nil - } - logger.Infof("Starting install of %s.%s.%s", pi.Name, pi.Arch, pi.Ver) fmt.Printf("Installing %s.%s.%s and dependencies...\n", pi.Name, pi.Arch, pi.Ver) rs, err := client.FindRepoSpec(pi, rm[repo]) @@ -332,10 +324,7 @@ func copyPkg(src, dst string) (retErr error) { // NeedsInstallation checks if a package version needs installation. func NeedsInstallation(pi goolib.PackageInfo, state client.GooGetState) (bool, error) { for _, p := range state { - if p.PackageSpec.Name == pi.Name { - if p.PackageSpec.Arch != pi.Arch { - continue - } + if p.PackageSpec.Name == pi.Name && p.PackageSpec.Arch == pi.Arch { c, err := goolib.Compare(p.PackageSpec.Version, pi.Ver) if err != nil { return true, err