Skip to content

Commit

Permalink
fix(new_engine): Improve partial upgrade protection and pinned deps (J…
Browse files Browse the repository at this point in the history 
…guer#1945)

* fix dep graph, existing in graph

* do not change from same dep reason

* roll up layer installs in case of fail

* re-use pacman exclude mechanism

should finish the reimplementation of the missing guards from the legacy
engine.

* include update in debug log

* test rollups
  • Loading branch information
@Jguer
Jguer authored Mar 5, 2023
1 parent 0387dfd commit 8b8d600
Show file tree
Hide file tree
Showing 12 changed files with 785 additions and 86 deletions.
55 changes: 45 additions & 10 deletions aur_install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@ type (
vcsStore vcs.Store
targetMode parser.TargetMode
downloadOnly bool
log *text.Logger
}
)

func NewInstaller(dbExecutor db.Executor,
exeCmd exe.ICmdBuilder, vcsStore vcs.Store, targetMode parser.TargetMode,
downloadOnly bool,
downloadOnly bool, logger *text.Logger,
) *Installer {
return &Installer{
dbExecutor: dbExecutor,
Expand All @@ -43,6 +44,7 @@ func NewInstaller(dbExecutor db.Executor,
vcsStore: vcsStore,
targetMode: targetMode,
downloadOnly: downloadOnly,
log: logger,
}
}

Expand Down Expand Up @@ -80,30 +82,54 @@ func (installer *Installer) Install(ctx context.Context,
cmdArgs *parser.Arguments,
targets []map[string]*dep.InstallInfo,
pkgBuildDirs map[string]string,
excluded []string,
) error {
// Reorganize targets into layers of dependencies
var errMulti multierror.MultiError
for i := len(targets) - 1; i >= 0; i-- {
err := installer.handleLayer(ctx, cmdArgs, targets[i], pkgBuildDirs, i == 0)
if err != nil {
// rollback
return err
lastLayer := i == 0
errI := installer.handleLayer(ctx, cmdArgs, targets[i], pkgBuildDirs, lastLayer, excluded)
if errI == nil && lastLayer {
// success after rollups
return nil
}

if errI != nil {
errMulti.Add(errI)
if lastLayer {
break
}

// rollup
installer.log.Warnln(gotext.Get("Failed to install layer, rolling up to next layer."), "error:", errI)
targets[i-1] = mergeLayers(targets[i-1], targets[i])
}
}

return nil
return errMulti.Return()
}

func mergeLayers(layer1, layer2 map[string]*dep.InstallInfo) map[string]*dep.InstallInfo {
for name, info := range layer2 {
layer1[name] = info
}

return layer1
}

func (installer *Installer) handleLayer(ctx context.Context,
cmdArgs *parser.Arguments,
layer map[string]*dep.InstallInfo,
pkgBuildDirs map[string]string,
lastLayer bool,
excluded []string,
) error {
// Install layer
nameToBaseMap := make(map[string]string, 0)
syncDeps, syncExp := mapset.NewThreadUnsafeSet[string](), mapset.NewThreadUnsafeSet[string]()
aurDeps, aurExp := mapset.NewThreadUnsafeSet[string](), mapset.NewThreadUnsafeSet[string]()

upgradeSync := false
for name, info := range layer {
switch info.Source {
case dep.AUR, dep.SrcInfo:
Expand All @@ -120,6 +146,10 @@ func (installer *Installer) handleLayer(ctx context.Context,
aurDeps.Add(name)
}
case dep.Sync:
if info.Upgrade {
upgradeSync = true
continue // do not add to targets, let pacman handle it
}
compositePkgName := fmt.Sprintf("%s/%s", *info.SyncDBName, name)

switch info.Reason {
Expand All @@ -135,9 +165,10 @@ func (installer *Installer) handleLayer(ctx context.Context,
}
}

text.Debugln("syncDeps", syncDeps, "SyncExp", syncExp, "aurDeps", aurDeps, "aurExp", aurExp)
text.Debugln("syncDeps", syncDeps, "SyncExp", syncExp,
"aurDeps", aurDeps, "aurExp", aurExp, "upgrade", upgradeSync)

errShow := installer.installSyncPackages(ctx, cmdArgs, syncDeps, syncExp)
errShow := installer.installSyncPackages(ctx, cmdArgs, syncDeps, syncExp, excluded, upgradeSync)
if errShow != nil {
return ErrInstallRepoPkgs
}
Expand Down Expand Up @@ -326,20 +357,24 @@ func (installer *Installer) getNewTargets(pkgdests map[string]string, name strin
func (installer *Installer) installSyncPackages(ctx context.Context, cmdArgs *parser.Arguments,
syncDeps, // repo targets that are deps
syncExp mapset.Set[string], // repo targets that are exp
excluded []string,
upgrade bool, // run even without targets
) error {
repoTargets := syncDeps.Union(syncExp).ToSlice()
if len(repoTargets) == 0 {
if len(repoTargets) == 0 && !upgrade {
return nil
}

arguments := cmdArgs.Copy()
arguments.DelArg("asdeps", "asdep")
arguments.DelArg("asexplicit", "asexp")
arguments.DelArg("i", "install")
arguments.DelArg("u", "upgrade")
arguments.Op = "S"
arguments.ClearTargets()
arguments.AddTarget(repoTargets...)
if len(excluded) > 0 {
arguments.CreateOrAppendOption("ignore", excluded...)
}

errShow := installer.exeCmd.Show(installer.exeCmd.BuildPacmanCmd(ctx,
arguments, installer.targetMode, settings.NoConfirm))
Expand Down
81 changes: 57 additions & 24 deletions aur_install_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"io"
"os"
"os/exec"
"strings"
Expand All @@ -16,9 +17,12 @@ import (
"github.com/Jguer/yay/v11/pkg/dep"
"github.com/Jguer/yay/v11/pkg/settings/exe"
"github.com/Jguer/yay/v11/pkg/settings/parser"
"github.com/Jguer/yay/v11/pkg/text"
"github.com/Jguer/yay/v11/pkg/vcs"
)

var testLogger = text.NewLogger(io.Discard, strings.NewReader(""), true, "test")

func ptrString(s string) *string {
return &s
}
Expand Down Expand Up @@ -127,7 +131,7 @@ func TestInstaller_InstallNeeded(t *testing.T) {

cmdBuilder.Runner = mockRunner

installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false)
installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)

cmdArgs := parser.MakeArguments()
cmdArgs.AddArg("needed")
Expand All @@ -149,7 +153,7 @@ func TestInstaller_InstallNeeded(t *testing.T) {
},
}

errI := installer.Install(context.Background(), cmdArgs, targets, pkgBuildDirs)
errI := installer.Install(context.Background(), cmdArgs, targets, pkgBuildDirs, []string{})
require.NoError(td, errI)

require.Len(td, mockRunner.ShowCalls, len(tc.wantShow))
Expand Down Expand Up @@ -401,7 +405,7 @@ func TestInstaller_InstallMixedSourcesAndLayers(t *testing.T) {

cmdBuilder.Runner = mockRunner

installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false)
installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)

cmdArgs := parser.MakeArguments()
cmdArgs.AddTarget("yay")
Expand All @@ -411,7 +415,7 @@ func TestInstaller_InstallMixedSourcesAndLayers(t *testing.T) {
"jellyfin": tmpDirJfin,
}

errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs)
errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs, []string{})
require.NoError(td, errI)

require.Len(td, mockRunner.ShowCalls, len(tc.wantShow))
Expand Down Expand Up @@ -454,7 +458,7 @@ func TestInstaller_RunPostHooks(t *testing.T) {

cmdBuilder.Runner = mockRunner

installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false)
installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)

called := false
hook := func(ctx context.Context) error {
Expand Down Expand Up @@ -482,17 +486,41 @@ func TestInstaller_CompileFailed(t *testing.T) {
require.NoError(t, f.Close())

type testCase struct {
desc string
targets []map[string]*dep.InstallInfo
lastLayer bool
desc string
targets []map[string]*dep.InstallInfo
wantErrInstall bool
wantErrCompile bool
failBuild bool
failPkgInstall bool
}

tmpDir := t.TempDir()

testCases := []testCase{
{
desc: "last layer",
lastLayer: true,
desc: "one layer",
wantErrInstall: false,
wantErrCompile: true,
failBuild: true,
failPkgInstall: false,
targets: []map[string]*dep.InstallInfo{
{
"yay": {
Source: dep.AUR,
Reason: dep.Explicit,
Version: "91.0.0-1",
SrcinfoPath: ptrString(tmpDir + "/.SRCINFO"),
AURBase: ptrString("yay"),
},
},
},
},
{
desc: "one layer -- fail install",
wantErrInstall: true,
wantErrCompile: false,
failBuild: false,
failPkgInstall: true,
targets: []map[string]*dep.InstallInfo{
{
"yay": {
Expand All @@ -506,10 +534,15 @@ func TestInstaller_CompileFailed(t *testing.T) {
},
},
{
desc: "not last layer",
lastLayer: false,
desc: "two layers",
wantErrInstall: false,
wantErrCompile: true,
failBuild: true,
failPkgInstall: false,
targets: []map[string]*dep.InstallInfo{
{"bob": {}},
{"bob": {
AURBase: ptrString("yay"),
}},
{
"yay": {
Source: dep.AUR,
Expand All @@ -533,7 +566,7 @@ func TestInstaller_CompileFailed(t *testing.T) {
}

showOverride := func(cmd *exec.Cmd) error {
if strings.Contains(cmd.String(), "makepkg -cf --noconfirm") && cmd.Dir == tmpDir {
if tc.failBuild && strings.Contains(cmd.String(), "makepkg -cf --noconfirm") && cmd.Dir == tmpDir {
return errors.New("makepkg failed")
}
return nil
Expand All @@ -555,7 +588,7 @@ func TestInstaller_CompileFailed(t *testing.T) {

cmdBuilder.Runner = mockRunner

installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false)
installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)

cmdArgs := parser.MakeArguments()
cmdArgs.AddArg("needed")
Expand All @@ -565,14 +598,14 @@ func TestInstaller_CompileFailed(t *testing.T) {
"yay": tmpDir,
}

errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs)
if tc.lastLayer {
require.NoError(td, errI) // last layer error
} else {
errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs, []string{})
if tc.wantErrInstall {
require.Error(td, errI)
} else {
require.NoError(td, errI)
}
err := installer.CompileFailedAndIgnored()
if tc.lastLayer {
if tc.wantErrCompile {
require.Error(td, err)
assert.ErrorContains(td, err, "yay")
} else {
Expand Down Expand Up @@ -713,7 +746,7 @@ func TestInstaller_InstallSplitPackage(t *testing.T) {

cmdBuilder.Runner = mockRunner

installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false)
installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)

cmdArgs := parser.MakeArguments()
cmdArgs.AddTarget("jellyfin")
Expand All @@ -722,7 +755,7 @@ func TestInstaller_InstallSplitPackage(t *testing.T) {
"jellyfin": tmpDir,
}

errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs)
errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs, []string{})
require.NoError(td, errI)

require.Len(td, mockRunner.ShowCalls, len(tc.wantShow))
Expand Down Expand Up @@ -851,7 +884,7 @@ func TestInstaller_InstallDownloadOnly(t *testing.T) {

cmdBuilder.Runner = mockRunner

installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, true)
installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, true, testLogger)

cmdArgs := parser.MakeArguments()
cmdArgs.AddTarget("yay")
Expand All @@ -872,7 +905,7 @@ func TestInstaller_InstallDownloadOnly(t *testing.T) {
},
}

errI := installer.Install(context.Background(), cmdArgs, targets, pkgBuildDirs)
errI := installer.Install(context.Background(), cmdArgs, targets, pkgBuildDirs, []string{})
require.NoError(td, errI)

require.Len(td, mockRunner.ShowCalls, len(tc.wantShow))
Expand Down
24 changes: 12 additions & 12 deletions cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func handleCmd(ctx context.Context, cfg *settings.Configuration, cmdArgs *parser
case "R", "remove":
return handleRemove(ctx, cmdArgs, cfg.Runtime.VCSStore)
case "S", "sync":
return handleSync(ctx, cmdArgs, dbExecutor)
return handleSync(ctx, cfg, cmdArgs, dbExecutor)
case "T", "deptest":
return cfg.Runtime.CmdBuilder.Show(cfg.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
cmdArgs, cfg.Runtime.Mode, settings.NoConfirm))
Expand Down Expand Up @@ -349,33 +349,33 @@ func handleBuild(ctx context.Context,
return nil
}

func handleSync(ctx context.Context, cmdArgs *parser.Arguments, dbExecutor db.Executor) error {
func handleSync(ctx context.Context, cfg *settings.Configuration, cmdArgs *parser.Arguments, dbExecutor db.Executor) error {
targets := cmdArgs.Targets

switch {
case cmdArgs.ExistsArg("s", "search"):
return syncSearch(ctx, targets, dbExecutor, config.Runtime.QueryBuilder, !cmdArgs.ExistsArg("q", "quiet"))
return syncSearch(ctx, targets, dbExecutor, cfg.Runtime.QueryBuilder, !cmdArgs.ExistsArg("q", "quiet"))
case cmdArgs.ExistsArg("p", "print", "print-format"):
return config.Runtime.CmdBuilder.Show(config.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
cmdArgs, config.Runtime.Mode, settings.NoConfirm))
return cfg.Runtime.CmdBuilder.Show(cfg.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
cmdArgs, cfg.Runtime.Mode, settings.NoConfirm))
case cmdArgs.ExistsArg("c", "clean"):
return syncClean(ctx, cmdArgs, dbExecutor)
case cmdArgs.ExistsArg("l", "list"):
return syncList(ctx, config.Runtime.HTTPClient, cmdArgs, dbExecutor)
return syncList(ctx, cfg.Runtime.HTTPClient, cmdArgs, dbExecutor)
case cmdArgs.ExistsArg("g", "groups"):
return config.Runtime.CmdBuilder.Show(config.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
cmdArgs, config.Runtime.Mode, settings.NoConfirm))
return cfg.Runtime.CmdBuilder.Show(cfg.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
cmdArgs, cfg.Runtime.Mode, settings.NoConfirm))
case cmdArgs.ExistsArg("i", "info"):
return syncInfo(ctx, cmdArgs, targets, dbExecutor)
case cmdArgs.ExistsArg("u", "sysupgrade") || len(cmdArgs.Targets) > 0:
if config.NewInstallEngine {
return syncInstall(ctx, config, cmdArgs, dbExecutor)
if cfg.NewInstallEngine {
return syncInstall(ctx, cfg, cmdArgs, dbExecutor)
}

return install(ctx, cmdArgs, dbExecutor, false)
case cmdArgs.ExistsArg("y", "refresh"):
return config.Runtime.CmdBuilder.Show(config.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
cmdArgs, config.Runtime.Mode, settings.NoConfirm))
return cfg.Runtime.CmdBuilder.Show(cfg.Runtime.CmdBuilder.BuildPacmanCmd(ctx,
cmdArgs, cfg.Runtime.Mode, settings.NoConfirm))
}

return nil
Expand Down
Loading

0 comments on commit 8b8d600

Please sign in to comment.