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

* reduce complexity of devel upgrade gathering

* clean orphans devel

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

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

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

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

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,8 +13,8 @@ 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
 		}
 	}
 
@@ -21,12 +22,12 @@ func (ae *AlpmExecutor) getPackageNamesBySource() {
 		"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 {

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
 		}

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"}},

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) {
+}

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)
+}

pkg/vcs/vcs_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/Jguer/yay/v11/pkg/db"
 	"github.com/Jguer/yay/v11/pkg/settings/exe"
 )
 
@@ -113,7 +114,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 		name   string
 		fields fields
 		args   args
-		want   []string
+		want   bool
 	}{
 		{
 			name: "simple-has_update",
@@ -128,7 +129,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 					Returned: []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa	HEAD"},
 				}},
 			},
-			want: []string{"yay"},
+			want: true,
 		},
 		{
 			name: "double-has_update",
@@ -151,7 +152,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 					},
 				}},
 			},
-			want: []string{"yay"},
+			want: true,
 		},
 		{
 			name: "simple-no_update",
@@ -166,7 +167,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 					Returned: []string{"991c5b4146fd27f4aacf4e3111258a848934aaa1	HEAD"},
 				}},
 			},
-			want: []string{},
+			want: false,
 		},
 		{
 			name: "simple-no_split",
@@ -181,7 +182,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 					Returned: []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
 				}},
 			},
-			want: []string{},
+			want: false,
 		},
 		{
 			name: "simple-error",
@@ -199,7 +200,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 					},
 				},
 			},
-			want: []string{},
+			want: false,
 		},
 		{
 			name: "simple-no protocol",
@@ -214,7 +215,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 					Returned: []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
 				}},
 			},
-			want: []string{},
+			want: false,
 		},
 	}
 	for _, tt := range tests {
@@ -227,7 +228,7 @@ func TestInfoStoreToUpgrade(t *testing.T) {
 					"yay": tt.args.infos,
 				},
 			}
-			got := v.ToUpgrade(context.Background())
+			got := v.ToUpgrade(context.Background(), "yay")
 			assert.Equal(t, tt.want, got)
 		})
 	}
@@ -468,10 +469,57 @@ func TestInfoStore_Remove(t *testing.T) {
 				OriginsByPackage: tt.fields.OriginsByPackage,
 				FilePath:         filePath,
 			}
-			v.RemovePackage(tt.args.pkgs)
+			v.RemovePackages(tt.args.pkgs)
 			assert.Len(t, tt.fields.OriginsByPackage, 2)
 		})
 	}
 
 	require.NoError(t, os.Remove(filePath))
 }
+
+func TestInfoStore_CleanOrphans(t *testing.T) {
+	t.Parallel()
+	type fields struct {
+		OriginsByPackage map[string]OriginInfoByURL
+	}
+	type args struct {
+		pkgs map[string]db.IPackage
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		args   args
+	}{
+		{
+			name: "simple",
+			args: args{pkgs: map[string]db.IPackage{"a": nil, "b": nil, "d": nil}},
+			fields: fields{
+				OriginsByPackage: map[string]OriginInfoByURL{
+					"a": {},
+					"b": {},
+					"c": {},
+					"d": {},
+				},
+			},
+		},
+	}
+
+	file, err := os.CreateTemp("/tmp", "yay-vcs-*-test")
+	filePath := file.Name()
+	require.NoError(t, err)
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			v := &InfoStore{
+				OriginsByPackage: tt.fields.OriginsByPackage,
+				FilePath:         filePath,
+			}
+			v.CleanOrphans(tt.args.pkgs)
+			assert.Len(t, tt.fields.OriginsByPackage, 3)
+		})
+	}
+
+	require.NoError(t, os.Remove(filePath))
+}

upgrade.go

@@ -103,6 +103,7 @@ func upList(ctx context.Context, aurCache settings.AURCache,
 				go func() {
 					develUp = upgrade.UpDevel(ctx, remote, aurdata, config.Runtime.VCSStore)
 
+					config.Runtime.VCSStore.CleanOrphans(remote)
 					wg.Done()
 				}()
 			}
@@ -137,10 +138,10 @@ func upList(ctx context.Context, aurCache settings.AURCache,
 }
 
 func printLocalNewerThanAUR(
-	remote []alpm.IPackage, aurdata map[string]*aur.Pkg,
+	remote map[string]alpm.IPackage, aurdata map[string]*aur.Pkg,
 ) {
-	for _, pkg := range remote {
-		aurPkg, ok := aurdata[pkg.Name()]
+	for name, pkg := range remote {
+		aurPkg, ok := aurdata[name]
 		if !ok {
 			continue
 		}
@@ -149,7 +150,7 @@ func printLocalNewerThanAUR(
 
 		if !isDevelPackage(pkg) && db.VerCmp(pkg.Version(), aurPkg.Version) > 0 {
 			text.Warnln(gotext.Get("%s: local (%s) is newer than AUR (%s)",
-				text.Cyan(pkg.Name()),
+				text.Cyan(name),
 				left, right,
 			))
 		}