fix(sync): do not update vcs info of failed packages (#1901)

* extract srcinfo service to pkg

* take into account failed installs for vcs update. Fixes #1892

* fix tests

aur_install.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"sync"
 
 	"github.com/Jguer/yay/v11/pkg/db"
 	"github.com/Jguer/yay/v11/pkg/dep"
@@ -15,7 +14,6 @@ import (
 	"github.com/Jguer/yay/v11/pkg/text"
 	"github.com/Jguer/yay/v11/pkg/vcs"
 
-	gosrc "github.com/Morganamilo/go-srcinfo"
 	mapset "github.com/deckarep/golang-set/v2"
 	"github.com/leonelquinteros/gotext"
 )
@@ -23,33 +21,33 @@ import (
 type (
 	PostInstallHookFunc func(ctx context.Context) error
 	Installer           struct {
-		dbExecutor        db.Executor
-		postInstallHooks  []PostInstallHookFunc
-		failedAndIngnored map[string]error
-		exeCmd            exe.ICmdBuilder
-		vcsStore          vcs.Store
-		targetMode        parser.TargetMode
+		dbExecutor       db.Executor
+		postInstallHooks []PostInstallHookFunc
+		failedAndIgnored map[string]error
+		exeCmd           exe.ICmdBuilder
+		vcsStore         vcs.Store
+		targetMode       parser.TargetMode
 	}
 )
 
 func NewInstaller(dbExecutor db.Executor, exeCmd exe.ICmdBuilder, vcsStore vcs.Store, targetMode parser.TargetMode) *Installer {
 	return &Installer{
-		dbExecutor:        dbExecutor,
-		postInstallHooks:  []PostInstallHookFunc{},
-		failedAndIngnored: map[string]error{},
-		exeCmd:            exeCmd,
-		vcsStore:          vcsStore,
-		targetMode:        targetMode,
+		dbExecutor:       dbExecutor,
+		postInstallHooks: []PostInstallHookFunc{},
+		failedAndIgnored: map[string]error{},
+		exeCmd:           exeCmd,
+		vcsStore:         vcsStore,
+		targetMode:       targetMode,
 	}
 }
 
 func (installer *Installer) CompileFailedAndIgnored() error {
-	if len(installer.failedAndIngnored) == 0 {
+	if len(installer.failedAndIgnored) == 0 {
 		return nil
 	}
 
 	return &FailedIgnoredPkgError{
-		pkgErrors: installer.failedAndIngnored,
+		pkgErrors: installer.failedAndIgnored,
 	}
 }
 
@@ -77,11 +75,10 @@ func (installer *Installer) Install(ctx context.Context,
 	cmdArgs *parser.Arguments,
 	targets []map[string]*dep.InstallInfo,
 	pkgBuildDirs map[string]string,
-	srcinfos map[string]*gosrc.Srcinfo,
 ) error {
 	// Reorganize targets into layers of dependencies
 	for i := len(targets) - 1; i >= 0; i-- {
-		err := installer.handleLayer(ctx, cmdArgs, targets[i], pkgBuildDirs, srcinfos, i == 0)
+		err := installer.handleLayer(ctx, cmdArgs, targets[i], pkgBuildDirs, i == 0)
 		if err != nil {
 			// rollback
 			return err
@@ -95,7 +92,6 @@ func (installer *Installer) handleLayer(ctx context.Context,
 	cmdArgs *parser.Arguments,
 	layer map[string]*dep.InstallInfo,
 	pkgBuildDirs map[string]string,
-	srcinfos map[string]*gosrc.Srcinfo,
 	lastLayer bool,
 ) error {
 	// Install layer
@@ -142,7 +138,7 @@ func (installer *Installer) handleLayer(ctx context.Context,
 	}
 
 	errAur := installer.installAURPackages(ctx, cmdArgs, aurDeps, aurExp,
-		nameToBaseMap, pkgBuildDirs, true, srcinfos, lastLayer)
+		nameToBaseMap, pkgBuildDirs, true, lastLayer)
 
 	return errAur
 }
@@ -152,7 +148,6 @@ func (installer *Installer) installAURPackages(ctx context.Context,
 	aurDepNames, aurExpNames mapset.Set[string],
 	nameToBase, pkgBuildDirsByBase map[string]string,
 	installIncompatible bool,
-	srcinfos map[string]*gosrc.Srcinfo,
 	lastLayer bool,
 ) error {
 	all := aurDepNames.Union(aurExpNames).ToSlice()
@@ -163,8 +158,6 @@ func (installer *Installer) installAURPackages(ctx context.Context,
 	deps, exps := make([]string, 0, aurDepNames.Cardinality()), make([]string, 0, aurExpNames.Cardinality())
 	pkgArchives := make([]string, 0, len(exps)+len(deps))
 
-	var wg sync.WaitGroup
-
 	for _, name := range all {
 		base := nameToBase[name]
 		dir := pkgBuildDirsByBase[base]
@@ -175,7 +168,7 @@ func (installer *Installer) installAURPackages(ctx context.Context,
 				return fmt.Errorf("%s - %w", gotext.Get("error making: %s", base), errMake)
 			}
 
-			installer.failedAndIngnored[name] = errMake
+			installer.failedAndIgnored[name] = errMake
 			text.Errorln(gotext.Get("error making: %s", base), "-", errMake)
 			continue
 		}
@@ -201,17 +194,8 @@ func (installer *Installer) installAURPackages(ctx context.Context,
 		if hasDebug {
 			deps = append(deps, name+"-debug")
 		}
-
-		srcinfo := srcinfos[base]
-		wg.Add(1)
-		go func(name string) {
-			installer.vcsStore.Update(ctx, name, srcinfo.Source)
-			wg.Done()
-		}(name)
 	}
 
-	wg.Wait()
-
 	if err := installPkgArchive(ctx, installer.exeCmd, installer.targetMode, installer.vcsStore, cmdArgs, pkgArchives); err != nil {
 		return fmt.Errorf("%s - %w", fmt.Sprintf(gotext.Get("error installing:")+" %v", pkgArchives), err)
 	}

aur_install_test.go

@@ -9,7 +9,6 @@ import (
 	"strings"
 	"testing"
 
-	gosrc "github.com/Morganamilo/go-srcinfo"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -138,8 +137,6 @@ func TestInstaller_InstallNeeded(t *testing.T) {
 				"yay": tmpDir,
 			}
 
-			srcInfos := map[string]*gosrc.Srcinfo{"yay": {}}
-
 			targets := []map[string]*dep.InstallInfo{
 				{
 					"yay": {
@@ -152,7 +149,7 @@ func TestInstaller_InstallNeeded(t *testing.T) {
 				},
 			}
 
-			errI := installer.Install(context.Background(), cmdArgs, targets, pkgBuildDirs, srcInfos)
+			errI := installer.Install(context.Background(), cmdArgs, targets, pkgBuildDirs)
 			require.NoError(td, errI)
 
 			require.Len(td, mockRunner.ShowCalls, len(tc.wantShow))
@@ -414,9 +411,7 @@ func TestInstaller_InstallMixedSourcesAndLayers(t *testing.T) {
 				"jellyfin": tmpDirJfin,
 			}
 
-			srcInfos := map[string]*gosrc.Srcinfo{"yay": {}, "jellyfin": {}}
-
-			errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs, srcInfos)
+			errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs)
 			require.NoError(td, errI)
 
 			require.Len(td, mockRunner.ShowCalls, len(tc.wantShow))
@@ -570,8 +565,7 @@ func TestInstaller_CompileFailed(t *testing.T) {
 				"yay": tmpDir,
 			}
 
-			srcInfos := map[string]*gosrc.Srcinfo{"yay": {}}
-			errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs, srcInfos)
+			errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs)
 			if tc.lastLayer {
 				require.NoError(td, errI) // last layer error
 			} else {
@@ -728,9 +722,7 @@ func TestInstaller_InstallSplitPackage(t *testing.T) {
 				"jellyfin": tmpDir,
 			}
 
-			srcInfos := map[string]*gosrc.Srcinfo{"jellyfin": {}}
-
-			errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs, srcInfos)
+			errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs)
 			require.NoError(td, errI)
 
 			require.Len(td, mockRunner.ShowCalls, len(tc.wantShow))

install.go

@@ -25,6 +25,7 @@ import (
 	"github.com/Jguer/yay/v11/pkg/settings"
 	"github.com/Jguer/yay/v11/pkg/settings/exe"
 	"github.com/Jguer/yay/v11/pkg/settings/parser"
+	"github.com/Jguer/yay/v11/pkg/srcinfo"
 	"github.com/Jguer/yay/v11/pkg/stringset"
 	"github.com/Jguer/yay/v11/pkg/text"
 	"github.com/Jguer/yay/v11/pkg/vcs"
@@ -282,7 +283,7 @@ func install(ctx context.Context, cmdArgs *parser.Arguments, dbExecutor db.Execu
 		return errM
 	}
 
-	srcinfos, err = parseSrcinfoFiles(pkgbuildDirs, true)
+	srcinfos, err = srcinfo.ParseSrcinfoFilesByBase(pkgbuildDirs, true)
 	if err != nil {
 		return err
 	}
@@ -537,30 +538,6 @@ func parsePackageList(ctx context.Context, cmdBuilder exe.ICmdBuilder,
 	return pkgdests, pkgVersion, nil
 }
 
-func parseSrcinfoFiles(pkgBuildDirs map[string]string, errIsFatal bool) (map[string]*gosrc.Srcinfo, error) {
-	srcinfos := make(map[string]*gosrc.Srcinfo)
-
-	k := 0
-	for base, dir := range pkgBuildDirs {
-		text.OperationInfoln(gotext.Get("(%d/%d) Parsing SRCINFO: %s", k+1, len(pkgBuildDirs), text.Cyan(base)))
-
-		pkgbuild, err := gosrc.ParseFile(filepath.Join(dir, ".SRCINFO"))
-		if err != nil {
-			if !errIsFatal {
-				text.Warnln(gotext.Get("failed to parse %s -- skipping: %s", base, err))
-				continue
-			}
-
-			return nil, errors.New(gotext.Get("failed to parse %s: %s", base, err))
-		}
-
-		srcinfos[base] = pkgbuild
-		k++
-	}
-
-	return srcinfos, nil
-}
-
 func pkgbuildsToSkip(bases []dep.Base, targets stringset.StringSet) stringset.StringSet {
 	toSkip := make(stringset.StringSet)
 
@@ -682,7 +659,7 @@ func buildInstallPkgbuilds(
 			}
 		}
 
-		srcinfo := srcinfos[pkg]
+		srcInfo := srcinfos[pkg]
 
 		args := []string{"--nobuild", "-fC"}
 
@@ -797,7 +774,7 @@ func buildInstallPkgbuilds(
 		var wg sync.WaitGroup
 
 		for _, pkg := range base {
-			if srcinfo == nil {
+			if srcInfo == nil {
 				text.Errorln(gotext.Get("could not find srcinfo for: %s", pkg.Name))
 				break
 			}
@@ -806,7 +783,7 @@ func buildInstallPkgbuilds(
 
 			text.Debugln("checking vcs store for:", pkg.Name)
 			go func(name string) {
-				config.Runtime.VCSStore.Update(ctx, name, srcinfo.Source)
+				config.Runtime.VCSStore.Update(ctx, name, srcInfo.Source)
 				wg.Done()
 			}(pkg.Name)
 		}

local_install_test.go

@@ -134,6 +134,7 @@ func TestIntegrationLocalInstall(t *testing.T) {
 	}
 
 	config := &settings.Configuration{
+		RemoveMake: "no",
 		Runtime: &settings.Runtime{
 			CmdBuilder: cmdBuilder,
 			VCSStore:   &vcs.Mock{},

pkg/srcinfo/service.go

@@ -0,0 +1,125 @@
+package srcinfo
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"path/filepath"
+
+	gosrc "github.com/Morganamilo/go-srcinfo"
+	"github.com/leonelquinteros/gotext"
+
+	"github.com/Jguer/yay/v11/pkg/db"
+	"github.com/Jguer/yay/v11/pkg/dep"
+	"github.com/Jguer/yay/v11/pkg/pgp"
+	"github.com/Jguer/yay/v11/pkg/settings"
+	"github.com/Jguer/yay/v11/pkg/settings/exe"
+	"github.com/Jguer/yay/v11/pkg/text"
+	"github.com/Jguer/yay/v11/pkg/vcs"
+)
+
+// TODO: add tests
+type Service struct {
+	dbExecutor db.Executor
+	cfg        *settings.Configuration
+	cmdBuilder exe.ICmdBuilder
+	vcsStore   vcs.Store
+
+	pkgBuildDirs map[string]string
+	srcInfos     map[string]*gosrc.Srcinfo
+}
+
+func NewService(dbExecutor db.Executor, cfg *settings.Configuration,
+	cmdBuilder exe.ICmdBuilder, vcsStore vcs.Store, pkgBuildDirs map[string]string,
+) (*Service, error) {
+	srcinfos, err := ParseSrcinfoFilesByBase(pkgBuildDirs, true)
+	if err != nil {
+		panic(err)
+	}
+	return &Service{
+		dbExecutor:   dbExecutor,
+		cfg:          cfg,
+		cmdBuilder:   cmdBuilder,
+		vcsStore:     vcsStore,
+		pkgBuildDirs: pkgBuildDirs,
+		srcInfos:     srcinfos,
+	}, nil
+}
+
+func (s *Service) IncompatiblePkgs(ctx context.Context) ([]string, error) {
+	incompatible := []string{}
+
+	alpmArch, err := s.dbExecutor.AlpmArchitectures()
+	if err != nil {
+		return nil, err
+	}
+
+nextpkg:
+	for base, srcinfo := range s.srcInfos {
+		for _, arch := range srcinfo.Arch {
+			if db.ArchIsSupported(alpmArch, arch) {
+				continue nextpkg
+			}
+		}
+		incompatible = append(incompatible, base)
+	}
+
+	return incompatible, nil
+}
+
+func (s *Service) CheckPGPKeys(ctx context.Context) error {
+	_, errCPK := pgp.CheckPgpKeys(ctx, s.pkgBuildDirs, s.srcInfos, s.cmdBuilder, settings.NoConfirm)
+	return errCPK
+}
+
+func (s *Service) UpdateVCSStore(ctx context.Context, targets []map[string]*dep.InstallInfo, ignore map[string]error,
+) error {
+	for _, srcinfo := range s.srcInfos {
+		if srcinfo.Source == nil {
+			continue
+		}
+
+		// TODO: high complexity - refactor
+		for i := range srcinfo.Packages {
+			for j := range targets {
+				if _, ok := targets[j][srcinfo.Packages[i].Pkgname]; !ok {
+					text.Debugln("skipping VCS update for", srcinfo.Packages[i].Pkgname, "not in targets")
+					continue
+				}
+				if _, ok := ignore[srcinfo.Packages[i].Pkgname]; ok {
+					text.Debugln("skipping VCS update for", srcinfo.Packages[i].Pkgname, "due to install error")
+					continue
+				}
+
+				text.Debugln("updating VCS entry for", srcinfo.Packages[i].Pkgname, fmt.Sprintf("source: %v", srcinfo.Source))
+				s.vcsStore.Update(ctx, srcinfo.Packages[i].Pkgname, srcinfo.Source)
+			}
+		}
+	}
+
+	return nil
+}
+
+func ParseSrcinfoFilesByBase(pkgBuildDirs map[string]string, errIsFatal bool) (map[string]*gosrc.Srcinfo, error) {
+	srcinfos := make(map[string]*gosrc.Srcinfo)
+
+	k := 0
+	for base, dir := range pkgBuildDirs {
+		text.OperationInfoln(gotext.Get("(%d/%d) Parsing SRCINFO: %s", k+1, len(pkgBuildDirs), text.Cyan(base)))
+
+		pkgbuild, err := gosrc.ParseFile(filepath.Join(dir, ".SRCINFO"))
+		if err != nil {
+			if !errIsFatal {
+				text.Warnln(gotext.Get("failed to parse %s -- skipping: %s", base, err))
+				continue
+			}
+
+			return nil, errors.New(gotext.Get("failed to parse %s: %s", base, err))
+		}
+
+		srcinfos[base] = pkgbuild
+		k++
+	}
+
+	return srcinfos, nil
+}

srcinfo.go

@@ -1,37 +0,0 @@
-package main
-
-import (
-	"context"
-
-	"github.com/Jguer/yay/v11/pkg/db"
-	"github.com/Jguer/yay/v11/pkg/pgp"
-	"github.com/Jguer/yay/v11/pkg/settings"
-	"github.com/Jguer/yay/v11/pkg/settings/exe"
-
-	gosrc "github.com/Morganamilo/go-srcinfo"
-)
-
-type srcinfoOperator struct {
-	dbExecutor db.Executor
-	cfg        *settings.Configuration
-	cmdBuilder exe.ICmdBuilder
-}
-
-func (s *srcinfoOperator) Run(ctx context.Context, pkgbuildDirs map[string]string) (map[string]*gosrc.Srcinfo, error) {
-	srcinfos, err := parseSrcinfoFiles(pkgbuildDirs, true)
-	if err != nil {
-		return nil, err
-	}
-
-	if err := confirmIncompatibleInstall(srcinfos, s.dbExecutor); err != nil {
-		return nil, err
-	}
-
-	if s.cfg.PGPFetch {
-		if _, errCPK := pgp.CheckPgpKeys(ctx, pkgbuildDirs, srcinfos, s.cmdBuilder, settings.NoConfirm); errCPK != nil {
-			return nil, errCPK
-		}
-	}
-
-	return srcinfos, nil
-}

sync.go

@@ -13,6 +13,7 @@ import (
 	"github.com/Jguer/yay/v11/pkg/multierror"
 	"github.com/Jguer/yay/v11/pkg/settings"
 	"github.com/Jguer/yay/v11/pkg/settings/parser"
+	"github.com/Jguer/yay/v11/pkg/srcinfo"
 	"github.com/Jguer/yay/v11/pkg/text"
 
 	"github.com/leonelquinteros/gotext"
@@ -99,9 +100,9 @@ func (o *OperationService) Run(ctx context.Context,
 	preparer := NewPreparer(o.dbExecutor, o.cfg.Runtime.CmdBuilder, o.cfg)
 	installer := NewInstaller(o.dbExecutor, o.cfg.Runtime.CmdBuilder, o.cfg.Runtime.VCSStore, o.cfg.Runtime.Mode)
 
-	pkgBuildDirs, err := preparer.Run(ctx, os.Stdout, targets)
-	if err != nil {
-		return err
+	pkgBuildDirs, errInstall := preparer.Run(ctx, os.Stdout, targets)
+	if errInstall != nil {
+		return errInstall
 	}
 
 	cleanFunc := preparer.ShouldCleanMakeDeps()
@@ -113,28 +114,34 @@ func (o *OperationService) Run(ctx context.Context,
 		installer.AddPostInstallHook(cleanAURDirsFunc)
 	}
 
-	srcinfoOp := srcinfoOperator{
-		dbExecutor: o.dbExecutor,
-		cfg:        o.cfg,
-		cmdBuilder: installer.exeCmd,
-	}
-	srcinfos, err := srcinfoOp.Run(ctx, pkgBuildDirs)
-	if err != nil {
-		return err
-	}
-
 	go func() {
-		_ = completion.Update(ctx, o.cfg.Runtime.HTTPClient, o.dbExecutor,
+		errComp := completion.Update(ctx, o.cfg.Runtime.HTTPClient, o.dbExecutor,
 			o.cfg.AURURL, o.cfg.Runtime.CompletionPath, o.cfg.CompletionInterval, false)
+		if errComp != nil {
+			text.Warnln(errComp)
+		}
 	}()
 
-	err = installer.Install(ctx, cmdArgs, targets, pkgBuildDirs, srcinfos)
-	if err != nil {
-		if errHook := installer.RunPostInstallHooks(ctx); errHook != nil {
-			text.Errorln(errHook)
-		}
+	srcInfo, errInstall := srcinfo.NewService(o.dbExecutor, o.cfg, o.cfg.Runtime.CmdBuilder, o.cfg.Runtime.VCSStore, pkgBuildDirs)
+	if errInstall != nil {
+		return errInstall
+	}
 
-		return err
+	incompatible, errInstall := srcInfo.IncompatiblePkgs(ctx)
+	if errInstall != nil {
+		return errInstall
+	}
+
+	if errIncompatible := confirmIncompatible(incompatible); errIncompatible != nil {
+		return errIncompatible
+	}
+
+	if errPGP := srcInfo.CheckPGPKeys(ctx); errPGP != nil {
+		return errPGP
+	}
+
+	if errInstall := installer.Install(ctx, cmdArgs, targets, pkgBuildDirs); errInstall != nil {
+		return errInstall
 	}
 
 	var multiErr multierror.MultiError
@@ -143,9 +150,31 @@ func (o *OperationService) Run(ctx context.Context,
 		multiErr.Add(err)
 	}
 
+	if err := srcInfo.UpdateVCSStore(ctx, targets, installer.failedAndIgnored); err != nil {
+		text.Warnln(err)
+	}
+
 	if err := installer.RunPostInstallHooks(ctx); err != nil {
 		multiErr.Add(err)
 	}
 
 	return multiErr.Return()
 }
+
+func confirmIncompatible(incompatible []string) error {
+	if len(incompatible) > 0 {
+		text.Warnln(gotext.Get("The following packages are not compatible with your architecture:"))
+
+		for _, pkg := range incompatible {
+			fmt.Print("  " + text.Cyan(pkg))
+		}
+
+		fmt.Println()
+
+		if !text.ContinueTask(os.Stdin, gotext.Get("Try to build them anyway?"), true, settings.NoConfirm) {
+			return &settings.ErrUserAbort{}
+		}
+	}
+
+	return nil
+}

vcs.go

@@ -13,6 +13,7 @@ import (
 	"github.com/Jguer/yay/v11/pkg/download"
 	"github.com/Jguer/yay/v11/pkg/query"
 	"github.com/Jguer/yay/v11/pkg/settings"
+	"github.com/Jguer/yay/v11/pkg/srcinfo"
 	"github.com/Jguer/yay/v11/pkg/stringset"
 	"github.com/Jguer/yay/v11/pkg/text"
 )
@@ -51,7 +52,7 @@ func createDevelDB(ctx context.Context, config *settings.Configuration, dbExecut
 		return err
 	}
 
-	srcinfos, err := parseSrcinfoFiles(pkgBuildDirsByBase, false)
+	srcinfos, err := srcinfo.ParseSrcinfoFilesByBase(pkgBuildDirsByBase, false)
 	if err != nil {
 		return err
 	}