fix(vcs): do not vcs update gather orphan info (Jguer#1902)

* reduce complexity of devel upgrade gathering * clean orphans devel
KarolosLykos · Jan 23, 2023 · 4626a04 · 4626a04
1 parent 1bfbd01
commit 4626a04
Show file tree

Hide file tree

Showing 11 changed files with 157 additions and 82 deletions.
diff --git a/cmd.go b/cmd.go
@@ -383,7 +383,7 @@ func handleRemove(ctx context.Context, cmdArgs *parser.Arguments, localCache vcs
 	err := config.Runtime.CmdBuilder.Show(config.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
 		cmdArgs, config.Runtime.Mode, settings.NoConfirm))
 	if err == nil {
-		localCache.RemovePackage(cmdArgs.Targets)
+		localCache.RemovePackages(cmdArgs.Targets)
 	}
 
 	return err

diff --git a/install.go b/install.go
@@ -648,7 +648,7 @@ func buildInstallPkgbuilds(
 
 			if errArchive != nil || errReason != nil {
 				if i != 0 {
-					go config.Runtime.VCSStore.RemovePackage([]string{do.Aur[i-1].String()})
+					go config.Runtime.VCSStore.RemovePackages([]string{do.Aur[i-1].String()})
 				}
 
 				if errArchive != nil {
@@ -794,12 +794,12 @@ func buildInstallPkgbuilds(
 	text.Debugln("installing archives:", pkgArchives)
 	errArchive := installPkgArchive(ctx, config.Runtime.CmdBuilder, config.Runtime.Mode, config.Runtime.VCSStore, cmdArgs, pkgArchives)
 	if errArchive != nil {
-		go config.Runtime.VCSStore.RemovePackage([]string{do.Aur[len(do.Aur)-1].String()})
+		go config.Runtime.VCSStore.RemovePackages([]string{do.Aur[len(do.Aur)-1].String()})
 	}
 
 	errReason := setInstallReason(ctx, config.Runtime.CmdBuilder, config.Runtime.Mode, cmdArgs, deps, exp)
 	if errReason != nil {
-		go config.Runtime.VCSStore.RemovePackage([]string{do.Aur[len(do.Aur)-1].String()})
+		go config.Runtime.VCSStore.RemovePackages([]string{do.Aur[len(do.Aur)-1].String()})
 	}
 
 	settings.NoConfirm = oldConfirm

diff --git a/pkg/db/executor.go b/pkg/db/executor.go
@@ -31,7 +31,7 @@ type Executor interface {
 	BiggestPackages() []IPackage
 	Cleanup()
 	InstalledRemotePackageNames() []string
-	InstalledRemotePackages() []IPackage
+	InstalledRemotePackages() map[string]IPackage
 	InstalledSyncPackageNames() []string
 	IsCorrectVersionInstalled(string, string) bool
 	LastBuildTime() time.Time

diff --git a/pkg/db/ialpm/alpm.go b/pkg/db/ialpm/alpm.go
@@ -24,8 +24,8 @@ type AlpmExecutor struct {
 	syncDBsCache []alpm.IDB
 	conf         *pacmanconf.Config
 
-	installedRemotePkgs     []alpm.IPackage
 	installedRemotePkgNames []string
+	installedRemotePkgMap   map[string]alpm.IPackage
 	installedSyncPkgNames   []string
 }
 
@@ -36,8 +36,8 @@ func NewExecutor(pacmanConf *pacmanconf.Config) (*AlpmExecutor, error) {
 		syncDB:                  nil,
 		syncDBsCache:            []alpm.IDB{},
 		conf:                    pacmanConf,
-		installedRemotePkgs:     nil,
 		installedRemotePkgNames: nil,
+		installedRemotePkgMap:   map[string]alpm.IPackage{},
 		installedSyncPkgNames:   nil,
 	}
 

diff --git a/pkg/db/ialpm/high_level.go b/pkg/db/ialpm/high_level.go
@@ -1,7 +1,8 @@
 package ialpm
 
 import (
-	"github.com/Jguer/yay/v11/pkg/db"
+	alpm "github.com/Jguer/go-alpm/v2"
+
 	"github.com/Jguer/yay/v11/pkg/text"
 )
 
@@ -12,21 +13,21 @@ func (ae *AlpmExecutor) getPackageNamesBySource() {
 		if ae.SyncPackage(pkgName) != nil {
 			ae.installedSyncPkgNames = append(ae.installedSyncPkgNames, pkgName)
 		} else {
-			ae.installedRemotePkgs = append(ae.installedRemotePkgs, localpkg)
 			ae.installedRemotePkgNames = append(ae.installedRemotePkgNames, pkgName)
+			ae.installedRemotePkgMap[pkgName] = localpkg
 		}
 	}
 
 	text.Debugln("populating db executor package caches.",
 		"sync_len", len(ae.installedSyncPkgNames), "remote_len", len(ae.installedRemotePkgNames))
 }
 
-func (ae *AlpmExecutor) InstalledRemotePackages() []db.IPackage {
-	if ae.installedRemotePkgs == nil {
+func (ae *AlpmExecutor) InstalledRemotePackages() map[string]alpm.IPackage {
+	if ae.installedRemotePkgMap == nil {
 		ae.getPackageNamesBySource()
 	}
 
-	return ae.installedRemotePkgs
+	return ae.installedRemotePkgMap
 }
 
 func (ae *AlpmExecutor) InstalledRemotePackageNames() []string {

diff --git a/pkg/upgrade/sources.go b/pkg/upgrade/sources.go
@@ -13,31 +13,25 @@ import (
 
 func UpDevel(
 	ctx context.Context,
-	remote []db.IPackage, // should be a map
+	remote map[string]db.IPackage,
 	aurdata map[string]*query.Pkg,
 	localCache vcs.Store,
 ) UpSlice {
-	toUpdate := make([]db.IPackage, 0, len(aurdata))
 	toRemove := make([]string, 0)
+	toUpgrade := UpSlice{Up: make([]Upgrade, 0), Repos: []string{"devel"}}
 
-	for _, pkgName := range localCache.ToUpgrade(ctx) {
-		if _, ok := aurdata[pkgName]; ok {
-			for _, pkg := range remote {
-				if pkg.Name() == pkgName {
-					toUpdate = append(toUpdate, pkg)
-				}
+	for pkgName, pkg := range remote {
+		if localCache.ToUpgrade(ctx, pkgName) {
+			if _, ok := aurdata[pkgName]; !ok {
+				text.Warnln(gotext.Get("ignoring package devel upgrade (no AUR info found):"), pkgName)
+				continue
 			}
-		} else {
-			toRemove = append(toRemove, pkgName)
-		}
-	}
 
-	toUpgrade := UpSlice{Up: make([]Upgrade, 0, len(toUpdate)), Repos: []string{"devel"}}
+			if pkg.ShouldIgnore() {
+				printIgnoringPackage(pkg, "latest-commit")
+				continue
+			}
 
-	for _, pkg := range toUpdate {
-		if pkg.ShouldIgnore() {
-			printIgnoringPackage(pkg, "latest-commit")
-		} else {
 			toUpgrade.Up = append(toUpgrade.Up,
 				Upgrade{
 					Name:          pkg.Name(),
@@ -50,7 +44,7 @@ func UpDevel(
 		}
 	}
 
-	localCache.RemovePackage(toRemove)
+	localCache.RemovePackages(toRemove)
 
 	return toUpgrade
 }
@@ -66,11 +60,11 @@ func printIgnoringPackage(pkg db.IPackage, newPkgVersion string) {
 
 // UpAUR gathers foreign packages and checks if they have new versions.
 // Output: Upgrade type package list.
-func UpAUR(remote []db.IPackage, aurdata map[string]*query.Pkg, timeUpdate bool) UpSlice {
+func UpAUR(remote map[string]db.IPackage, aurdata map[string]*query.Pkg, timeUpdate bool) UpSlice {
 	toUpgrade := UpSlice{Up: make([]Upgrade, 0), Repos: []string{"aur"}}
 
-	for _, pkg := range remote {
-		aurPkg, ok := aurdata[pkg.Name()]
+	for name, pkg := range remote {
+		aurPkg, ok := aurdata[name]
 		if !ok {
 			continue
 		}

diff --git a/pkg/upgrade/sources_test.go b/pkg/upgrade/sources_test.go
@@ -18,7 +18,7 @@ func Test_upAUR(t *testing.T) {
 	t.Parallel()
 
 	type args struct {
-		remote     []alpm.IPackage
+		remote     map[string]alpm.IPackage
 		aurdata    map[string]*aur.Pkg
 		timeUpdate bool
 	}
@@ -30,10 +30,10 @@ func Test_upAUR(t *testing.T) {
 		{
 			name: "No Updates",
 			args: args{
-				remote: []alpm.IPackage{
-					&mock.Package{PName: "hello", PVersion: "2.0.0"},
-					&mock.Package{PName: "local_pkg", PVersion: "1.1.0"},
-					&mock.Package{PName: "ignored", PVersion: "1.0.0", PShouldIgnore: true},
+				remote: map[string]alpm.IPackage{
+					"hello":     &mock.Package{PName: "hello", PVersion: "2.0.0"},
+					"local_pkg": &mock.Package{PName: "local_pkg", PVersion: "1.1.0"},
+					"ignored":   &mock.Package{PName: "ignored", PVersion: "1.0.0", PShouldIgnore: true},
 				},
 				aurdata: map[string]*aur.Pkg{
 					"hello":   {Version: "2.0.0", Name: "hello"},
@@ -46,7 +46,9 @@ func Test_upAUR(t *testing.T) {
 		{
 			name: "Simple Update",
 			args: args{
-				remote:     []alpm.IPackage{&mock.Package{PName: "hello", PVersion: "2.0.0"}},
+				remote: map[string]alpm.IPackage{
+					"hello": &mock.Package{PName: "hello", PVersion: "2.0.0"},
+				},
 				aurdata:    map[string]*aur.Pkg{"hello": {Version: "2.1.0", Name: "hello"}},
 				timeUpdate: false,
 			},
@@ -55,7 +57,9 @@ func Test_upAUR(t *testing.T) {
 		{
 			name: "Time Update",
 			args: args{
-				remote:     []alpm.IPackage{&mock.Package{PName: "hello", PVersion: "2.0.0", PBuildDate: time.Now()}},
+				remote: map[string]alpm.IPackage{
+					"hello": &mock.Package{PName: "hello", PVersion: "2.0.0", PBuildDate: time.Now()},
+				},
 				aurdata:    map[string]*aur.Pkg{"hello": {Version: "2.0.0", Name: "hello", LastModified: int(time.Now().AddDate(0, 0, 2).Unix())}},
 				timeUpdate: true,
 			},
@@ -66,6 +70,7 @@ func Test_upAUR(t *testing.T) {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
+
 			got := UpAUR(tt.args.remote, tt.args.aurdata, tt.args.timeUpdate)
 			assert.EqualValues(t, tt.want, got)
 		})
@@ -76,7 +81,7 @@ func Test_upDevel(t *testing.T) {
 	t.Parallel()
 
 	type args struct {
-		remote  []alpm.IPackage
+		remote  map[string]alpm.IPackage
 		aurdata map[string]*aur.Pkg
 		cached  vcs.Store
 	}
@@ -90,10 +95,10 @@ func Test_upDevel(t *testing.T) {
 			name: "No Updates",
 			args: args{
 				cached: &vcs.Mock{},
-				remote: []alpm.IPackage{
-					&mock.Package{PName: "hello", PVersion: "2.0.0"},
-					&mock.Package{PName: "local_pkg", PVersion: "1.1.0"},
-					&mock.Package{PName: "ignored", PVersion: "1.0.0", PShouldIgnore: true},
+				remote: map[string]alpm.IPackage{
+					"hello":     &mock.Package{PName: "hello", PVersion: "2.0.0"},
+					"local_pkg": &mock.Package{PName: "local_pkg", PVersion: "1.1.0"},
+					"ignored":   &mock.Package{PName: "ignored", PVersion: "1.0.0", PShouldIgnore: true},
 				},
 				aurdata: map[string]*aur.Pkg{
 					"hello":   {Version: "2.0.0", Name: "hello"},
@@ -109,10 +114,10 @@ func Test_upDevel(t *testing.T) {
 				cached: &vcs.Mock{
 					ToUpgradeReturn: []string{"hello", "hello4"},
 				},
-				remote: []alpm.IPackage{
-					&mock.Package{PName: "hello", PVersion: "2.0.0"},
-					&mock.Package{PName: "hello2", PVersion: "3.0.0"},
-					&mock.Package{PName: "hello4", PVersion: "4.0.0"},
+				remote: map[string]alpm.IPackage{
+					"hello":  &mock.Package{PName: "hello", PVersion: "2.0.0"},
+					"hello2": &mock.Package{PName: "hello2", PVersion: "3.0.0"},
+					"hello4": &mock.Package{PName: "hello4", PVersion: "4.0.0"},
 				},
 				aurdata: map[string]*aur.Pkg{
 					"hello":  {Version: "2.0.0", Name: "hello"},
@@ -141,8 +146,10 @@ func Test_upDevel(t *testing.T) {
 			name:     "No update returned",
 			finalLen: 1,
 			args: args{
-				cached:  &vcs.Mock{ToUpgradeReturn: []string{}},
-				remote:  []alpm.IPackage{&mock.Package{PName: "hello", PVersion: "2.0.0"}},
+				cached: &vcs.Mock{ToUpgradeReturn: []string{}},
+				remote: map[string]alpm.IPackage{
+					"hello": &mock.Package{PName: "hello", PVersion: "2.0.0"},
+				},
 				aurdata: map[string]*aur.Pkg{"hello": {Version: "2.0.0", Name: "hello"}},
 			},
 			want: UpSlice{Repos: []string{"devel"}},
@@ -154,7 +161,9 @@ func Test_upDevel(t *testing.T) {
 				cached: &vcs.Mock{
 					ToUpgradeReturn: []string{"hello"},
 				},
-				remote:  []alpm.IPackage{&mock.Package{PName: "hello", PVersion: "2.0.0", PShouldIgnore: true}},
+				remote: map[string]alpm.IPackage{
+					"hello": &mock.Package{PName: "hello", PVersion: "2.0.0", PShouldIgnore: true},
+				},
 				aurdata: map[string]*aur.Pkg{"hello": {Version: "2.0.0", Name: "hello"}},
 			},
 			want: UpSlice{Repos: []string{"devel"}},

diff --git a/pkg/vcs/mock.go b/pkg/vcs/mock.go
@@ -3,6 +3,7 @@ package vcs
 import (
 	"context"
 
+	"github.com/Jguer/go-alpm/v2"
 	gosrc "github.com/Morganamilo/go-srcinfo"
 )
 
@@ -11,8 +12,13 @@ type Mock struct {
 	ToUpgradeReturn  []string
 }
 
-func (m *Mock) ToUpgrade(ctx context.Context) []string {
-	return m.ToUpgradeReturn
+func (m *Mock) ToUpgrade(ctx context.Context, pkgName string) bool {
+	for _, pkg := range m.ToUpgradeReturn {
+		if pkg == pkgName {
+			return true
+		}
+	}
+	return false
 }
 
 func (m *Mock) Update(ctx context.Context, pkgName string, sources []gosrc.ArchString) {
@@ -22,9 +28,12 @@ func (m *Mock) Save() error {
 	return nil
 }
 
-func (m *Mock) RemovePackage(pkgs []string) {
+func (m *Mock) RemovePackages(pkgs []string) {
 }
 
 func (m *Mock) Load() error {
 	return nil
 }
+
+func (m *Mock) CleanOrphans(pkgs map[string]alpm.IPackage) {
+}
diff --git a/pkg/vcs/vcs.go b/pkg/vcs/vcs.go
@@ -11,6 +11,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/Jguer/go-alpm/v2"
 	gosrc "github.com/Morganamilo/go-srcinfo"
 	"github.com/leonelquinteros/gotext"
 
@@ -19,16 +20,18 @@ import (
 )
 
 type Store interface {
-	// ToUpgrade returns a list of packages that need to be updated.
-	ToUpgrade(ctx context.Context) []string
+	// ToUpgrade returns true if the package needs to be updated.
+	ToUpgrade(ctx context.Context, pkgName string) bool
 	// Update updates the VCS info of a package.
 	Update(ctx context.Context, pkgName string, sources []gosrc.ArchString)
-	// Save saves the VCS info to disk.
-	Save() error
-	// RemovePackage removes the VCS info of a package.
-	RemovePackage(pkgs []string)
+	// RemovePackages removes the VCS info of the packages given as arg if they exist.
+	RemovePackages(pkgs []string)
+	// Clean orphaned VCS info.
+	CleanOrphans(pkgs map[string]alpm.IPackage)
 	// Load loads the VCS info from disk.
 	Load() error
+	// Save saves the VCS info to disk.
+	Save() error
 }
 
 // InfoStore is a collection of OriginInfoByURL by Package.
@@ -200,15 +203,12 @@ func parseSource(source string) (url, branch string, protocols []string) {
 	return url, branch, protocols
 }
 
-func (v *InfoStore) ToUpgrade(ctx context.Context) []string {
-	pkgs := make([]string, 0, len(v.OriginsByPackage))
-	for pkgName, infos := range v.OriginsByPackage {
-		if v.needsUpdate(ctx, infos) {
-			pkgs = append(pkgs, pkgName)
-		}
+func (v *InfoStore) ToUpgrade(ctx context.Context, pkgName string) bool {
+	if infos, ok := v.OriginsByPackage[pkgName]; ok {
+		return v.needsUpdate(ctx, infos)
 	}
 
-	return pkgs
+	return false
 }
 
 func (v *InfoStore) needsUpdate(ctx context.Context, infos OriginInfoByURL) bool {
@@ -278,7 +278,7 @@ func (v *InfoStore) Save() error {
 }
 
 // RemovePackage removes package from VCS information.
-func (v *InfoStore) RemovePackage(pkgs []string) {
+func (v *InfoStore) RemovePackages(pkgs []string) {
 	updated := false
 
 	for _, pkgName := range pkgs {
@@ -314,3 +314,16 @@ func (v *InfoStore) Load() error {
 
 	return nil
 }
+
+func (v *InfoStore) CleanOrphans(pkgs map[string]alpm.IPackage) {
+	missing := make([]string, 0)
+
+	for pkgName := range v.OriginsByPackage {
+		if _, ok := pkgs[pkgName]; !ok {
+			text.Debugln("removing orphaned vcs package:", pkgName)
+			missing = append(missing, pkgName)
+		}
+	}
+
+	v.RemovePackages(missing)
+}