From 56d1b7ed1c890907fa982ff61e1f9be848290aa4 Mon Sep 17 00:00:00 2001 From: smolx <97612210+smolx@users.noreply.github.com> Date: Tue, 23 May 2023 23:16:27 +0200 Subject: [PATCH] Fix --rebuild option (#2163) * Reimplement --rebuild option in the new engine (#2153) * Refactor --rebuild option * Fix comment formatting --- aur_install.go | 33 +++- aur_install_test.go | 227 +++++++++++++++++++++++++++- install.go | 2 +- pkg/settings/args.go | 8 +- pkg/settings/config.go | 8 +- pkg/settings/parser/rebuild_mode.go | 10 ++ sync.go | 2 +- 7 files changed, 269 insertions(+), 21 deletions(-) create mode 100644 pkg/settings/parser/rebuild_mode.go diff --git a/aur_install.go b/aur_install.go index 9d6ec3dd..2f9753d9 100644 --- a/aur_install.go +++ b/aur_install.go @@ -27,6 +27,8 @@ type ( exeCmd exe.ICmdBuilder vcsStore vcs.Store targetMode parser.TargetMode + rebuildMode parser.RebuildMode + origTargets mapset.Set[string] downloadOnly bool log *text.Logger @@ -36,7 +38,7 @@ type ( func NewInstaller(dbExecutor db.Executor, exeCmd exe.ICmdBuilder, vcsStore vcs.Store, targetMode parser.TargetMode, - downloadOnly bool, logger *text.Logger, + rebuildMode parser.RebuildMode, downloadOnly bool, logger *text.Logger, ) *Installer { return &Installer{ dbExecutor: dbExecutor, @@ -45,6 +47,7 @@ func NewInstaller(dbExecutor db.Executor, exeCmd: exeCmd, vcsStore: vcsStore, targetMode: targetMode, + rebuildMode: rebuildMode, downloadOnly: downloadOnly, log: logger, manualConfirmRequired: true, @@ -90,6 +93,13 @@ func (installer *Installer) Install(ctx context.Context, ) error { installer.log.Debugln("manualConfirmRequired:", manualConfirmRequired) installer.manualConfirmRequired = manualConfirmRequired + + installer.origTargets = mapset.NewThreadUnsafeSet[string]() + for _, targetString := range cmdArgs.Targets { + installer.origTargets.Add(dep.ToTarget(targetString).Name) + } + installer.log.Debugln("origTargets:", installer.origTargets) + // Reorganize targets into layers of dependencies var errMulti multierror.MultiError for i := len(targets) - 1; i >= 0; i-- { @@ -216,7 +226,8 @@ func (installer *Installer) installAURPackages(ctx context.Context, base := nameToBase[name] dir := pkgBuildDirsByBase[base] - pkgdests, errMake := installer.buildPkg(ctx, dir, base, installIncompatible, cmdArgs.ExistsArg("needed")) + pkgdests, errMake := installer.buildPkg(ctx, dir, base, + installIncompatible, cmdArgs.ExistsArg("needed"), installer.origTargets.Contains(name)) if errMake != nil { if !lastLayer { return fmt.Errorf("%s - %w", gotext.Get("error making: %s", base), errMake) @@ -264,7 +275,7 @@ func (installer *Installer) installAURPackages(ctx context.Context, func (installer *Installer) buildPkg(ctx context.Context, dir, base string, - installIncompatible, needed bool, + installIncompatible, needed, isTarget bool, ) (map[string]string, error) { args := []string{"--nobuild", "-fC"} @@ -288,7 +299,7 @@ func (installer *Installer) buildPkg(ctx context.Context, args = []string{"-c", "--nobuild", "--noextract", "--ignorearch"} pkgdests = map[string]string{} text.Warnln(gotext.Get("%s is up to date -- skipping", text.Cyan(base+"-"+pkgVersion))) - case pkgsAreBuilt(pkgdests): + case installer.skipAlreadyBuiltPkg(isTarget, pkgdests): args = []string{"-c", "--nobuild", "--noextract", "--ignorearch"} text.Warnln(gotext.Get("%s already made -- skipping build", text.Cyan(base+"-"+pkgVersion))) default: @@ -333,6 +344,20 @@ func pkgsAreBuilt(pkgdests map[string]string) bool { return true } +func (installer *Installer) skipAlreadyBuiltPkg(isTarget bool, pkgdests map[string]string) bool { + switch installer.rebuildMode { + case parser.RebuildModeNo: + return pkgsAreBuilt(pkgdests) + case parser.RebuildModeYes: + return !isTarget && pkgsAreBuilt(pkgdests) + // case parser.RebuildModeTree: // TODO + // case parser.RebuildModeAll: // TODO + default: + // same as RebuildModeNo + return pkgsAreBuilt(pkgdests) + } +} + func (*Installer) isDep(cmdArgs *parser.Arguments, aurExpNames mapset.Set[string], name string) bool { switch { case cmdArgs.ExistsArg("asdeps", "asdep"): diff --git a/aur_install_test.go b/aur_install_test.go index f40c67d8..94e57396 100644 --- a/aur_install_test.go +++ b/aur_install_test.go @@ -133,7 +133,8 @@ func TestInstaller_InstallNeeded(t *testing.T) { cmdBuilder.Runner = mockRunner - installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger()) + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, + parser.RebuildModeNo, false, NewTestLogger()) cmdArgs := parser.MakeArguments() cmdArgs.AddArg("needed") @@ -407,7 +408,7 @@ func TestInstaller_InstallMixedSourcesAndLayers(t *testing.T) { cmdBuilder.Runner = mockRunner - installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger()) + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, parser.RebuildModeNo, false, NewTestLogger()) cmdArgs := parser.MakeArguments() cmdArgs.AddTarget("yay") @@ -460,7 +461,8 @@ func TestInstaller_RunPostHooks(t *testing.T) { cmdBuilder.Runner = mockRunner - installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger()) + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, + parser.RebuildModeNo, false, NewTestLogger()) called := false hook := func(ctx context.Context) error { @@ -590,7 +592,8 @@ func TestInstaller_CompileFailed(t *testing.T) { cmdBuilder.Runner = mockRunner - installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger()) + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, + parser.RebuildModeNo, false, NewTestLogger()) cmdArgs := parser.MakeArguments() cmdArgs.AddArg("needed") @@ -748,7 +751,8 @@ func TestInstaller_InstallSplitPackage(t *testing.T) { cmdBuilder.Runner = mockRunner - installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger()) + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, + parser.RebuildModeNo, false, NewTestLogger()) cmdArgs := parser.MakeArguments() cmdArgs.AddTarget("jellyfin") @@ -886,7 +890,8 @@ func TestInstaller_InstallDownloadOnly(t *testing.T) { cmdBuilder.Runner = mockRunner - installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, true, NewTestLogger()) + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, + parser.RebuildModeNo, true, NewTestLogger()) cmdArgs := parser.MakeArguments() cmdArgs.AddTarget("yay") @@ -989,7 +994,8 @@ func TestInstaller_InstallGroup(t *testing.T) { cmdBuilder.Runner = mockRunner - installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, true, NewTestLogger()) + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, + parser.RebuildModeNo, true, NewTestLogger()) cmdArgs := parser.MakeArguments() cmdArgs.AddTarget("kubernetes-tools") @@ -1035,3 +1041,210 @@ func TestInstaller_InstallGroup(t *testing.T) { }) } } + +func TestInstaller_InstallRebuild(t *testing.T) { + t.Parallel() + + makepkgBin := t.TempDir() + "/makepkg" + pacmanBin := t.TempDir() + "/pacman" + f, err := os.OpenFile(makepkgBin, os.O_RDONLY|os.O_CREATE, 0o755) + require.NoError(t, err) + require.NoError(t, f.Close()) + + f, err = os.OpenFile(pacmanBin, os.O_RDONLY|os.O_CREATE, 0o755) + require.NoError(t, err) + require.NoError(t, f.Close()) + + type testCase struct { + desc string + rebuildOption parser.RebuildMode + isInstalled bool + isBuilt bool + wantShow []string + wantCapture []string + targets []map[string]*dep.InstallInfo + } + + tmpDir := t.TempDir() + + testCases := []testCase{ + { + desc: "--norebuild(default) when built and not installed", + rebuildOption: parser.RebuildModeNo, + isBuilt: true, + isInstalled: false, + wantShow: []string{ + "makepkg --nobuild -fC --ignorearch", + "makepkg -c --nobuild --noextract --ignorearch", + "pacman -U --config -- /testdir/yay-91.0.0-1-x86_64.pkg.tar.zst", + "pacman -D -q --asexplicit --config -- yay", + }, + wantCapture: []string{"makepkg --packagelist"}, + 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: "--rebuild when built and not installed", + rebuildOption: parser.RebuildModeYes, + isBuilt: true, + isInstalled: false, + wantShow: []string{ + "makepkg --nobuild -fC --ignorearch", + "makepkg -cf --noconfirm --noextract --noprepare --holdver --ignorearch", + "pacman -U --config -- /testdir/yay-91.0.0-1-x86_64.pkg.tar.zst", + "pacman -D -q --asexplicit --config -- yay", + }, + wantCapture: []string{"makepkg --packagelist"}, + 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: "--rebuild when built and installed", + rebuildOption: parser.RebuildModeYes, + isInstalled: true, + isBuilt: true, + wantShow: []string{ + "makepkg --nobuild -fC --ignorearch", + "makepkg -cf --noconfirm --noextract --noprepare --holdver --ignorearch", + "pacman -U --config -- /testdir/yay-91.0.0-1-x86_64.pkg.tar.zst", + "pacman -D -q --asexplicit --config -- yay", + }, + wantCapture: []string{"makepkg --packagelist"}, + 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: "--rebuild when built and installed previously as dep", + rebuildOption: parser.RebuildModeYes, + isInstalled: true, + isBuilt: true, + wantShow: []string{ + "makepkg --nobuild -fC --ignorearch", + "makepkg -cf --noconfirm --noextract --noprepare --holdver --ignorearch", + "pacman -U --config -- /testdir/yay-91.0.0-1-x86_64.pkg.tar.zst", + "pacman -D -q --asdeps --config -- yay", + }, + wantCapture: []string{"makepkg --packagelist"}, + targets: []map[string]*dep.InstallInfo{ + { + "yay": { + Source: dep.AUR, + Reason: dep.Dep, + Version: "91.0.0-1", + SrcinfoPath: ptrString(tmpDir + "/.SRCINFO"), + AURBase: ptrString("yay"), + }, + }, + }, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.desc, func(td *testing.T) { + tmpDir := td.TempDir() + pkgTar := tmpDir + "/yay-91.0.0-1-x86_64.pkg.tar.zst" + + captureOverride := func(cmd *exec.Cmd) (stdout string, stderr string, err error) { + return pkgTar, "", nil + } + + i := 0 + showOverride := func(cmd *exec.Cmd) error { + i++ + if i == 2 { + if !tc.isBuilt { + f, err := os.OpenFile(pkgTar, os.O_RDONLY|os.O_CREATE, 0o666) + require.NoError(td, err) + require.NoError(td, f.Close()) + } + } + return nil + } + + // create a mock file + if tc.isBuilt { + f, err := os.OpenFile(pkgTar, os.O_RDONLY|os.O_CREATE, 0o666) + require.NoError(td, err) + require.NoError(td, f.Close()) + } + + isCorrectInstalledOverride := func(string, string) bool { + return tc.isInstalled + } + + mockDB := &mock.DBExecutor{IsCorrectVersionInstalledFn: isCorrectInstalledOverride} + mockRunner := &exe.MockRunner{CaptureFn: captureOverride, ShowFn: showOverride} + cmdBuilder := &exe.CmdBuilder{ + MakepkgBin: makepkgBin, + SudoBin: "su", + PacmanBin: pacmanBin, + Runner: mockRunner, + SudoLoopEnabled: false, + } + + cmdBuilder.Runner = mockRunner + + installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, + tc.rebuildOption, false, NewTestLogger()) + + cmdArgs := parser.MakeArguments() + cmdArgs.AddTarget("yay") + + pkgBuildDirs := map[string]string{ + "yay": tmpDir, + } + + errI := installer.Install(context.Background(), cmdArgs, tc.targets, pkgBuildDirs, []string{}, false) + require.NoError(td, errI) + + require.Len(td, mockRunner.ShowCalls, len(tc.wantShow)) + require.Len(td, mockRunner.CaptureCalls, len(tc.wantCapture)) + + for i, call := range mockRunner.ShowCalls { + show := call.Args[0].(*exec.Cmd).String() + show = strings.ReplaceAll(show, tmpDir, "/testdir") // replace the temp dir with a static path + show = strings.ReplaceAll(show, makepkgBin, "makepkg") + show = strings.ReplaceAll(show, pacmanBin, "pacman") + + // options are in a different order on different systems and on CI root user is used + assert.Subset(td, strings.Split(show, " "), strings.Split(tc.wantShow[i], " "), show) + } + + for i, call := range mockRunner.CaptureCalls { + capture := call.Args[0].(*exec.Cmd).String() + capture = strings.ReplaceAll(capture, tmpDir, "/testdir") // replace the temp dir with a static path + capture = strings.ReplaceAll(capture, makepkgBin, "makepkg") + capture = strings.ReplaceAll(capture, pacmanBin, "pacman") + assert.Subset(td, strings.Split(capture, " "), strings.Split(tc.wantCapture[i], " "), capture) + } + }) + } +} diff --git a/install.go b/install.go index 8072376a..4e4c7c94 100644 --- a/install.go +++ b/install.go @@ -157,7 +157,7 @@ func install(ctx context.Context, cfg *settings.Configuration, dp, err := dep.GetPool(ctx, requestTargets, warnings, dbExecutor, cfg.Runtime.AURClient, cfg.Mode, - ignoreProviders, settings.NoConfirm, cfg.Provides, cfg.ReBuild, cfg.RequestSplitN, noDeps, noCheck, assumeInstalled) + ignoreProviders, settings.NoConfirm, cfg.Provides, string(cfg.ReBuild), cfg.RequestSplitN, noDeps, noCheck, assumeInstalled) if err != nil { return err } diff --git a/pkg/settings/args.go b/pkg/settings/args.go index 8d994544..104ad062 100644 --- a/pkg/settings/args.go +++ b/pkg/settings/args.go @@ -97,13 +97,13 @@ func (c *Configuration) handleOption(option, value string) bool { case "noredownload": c.ReDownload = "no" case "rebuild": - c.ReBuild = "yes" + c.ReBuild = parser.RebuildModeYes case "rebuildall": - c.ReBuild = "all" + c.ReBuild = parser.RebuildModeAll case "rebuildtree": - c.ReBuild = "tree" + c.ReBuild = parser.RebuildModeTree case "norebuild": - c.ReBuild = "no" + c.ReBuild = parser.RebuildModeNo case "batchinstall": c.BatchInstall = true case "nobatchinstall": diff --git a/pkg/settings/config.go b/pkg/settings/config.go index bae3e884..5086bb38 100644 --- a/pkg/settings/config.go +++ b/pkg/settings/config.go @@ -35,7 +35,6 @@ type Configuration struct { PacmanBin string `json:"pacmanbin"` PacmanConf string `json:"pacmanconf"` ReDownload string `json:"redownload"` - ReBuild string `json:"rebuild"` AnswerClean string `json:"answerclean"` AnswerDiff string `json:"answerdiff"` AnswerEdit string `json:"answeredit"` @@ -78,8 +77,9 @@ type Configuration struct { CompletionPath string `json:"-"` VCSFilePath string `json:"-"` // ConfigPath string `json:"-"` - SaveConfig bool `json:"-"` - Mode parser.TargetMode `json:"-"` + SaveConfig bool `json:"-"` + Mode parser.TargetMode `json:"-"` + ReBuild parser.RebuildMode `json:"rebuild"` } // SaveConfig writes yay config to file. @@ -133,7 +133,7 @@ func (c *Configuration) expandEnv() { c.SudoBin = expandEnvOrHome(c.SudoBin) c.SudoFlags = os.ExpandEnv(c.SudoFlags) c.ReDownload = os.ExpandEnv(c.ReDownload) - c.ReBuild = os.ExpandEnv(c.ReBuild) + c.ReBuild = parser.RebuildMode(os.ExpandEnv(string(c.ReBuild))) c.AnswerClean = os.ExpandEnv(c.AnswerClean) c.AnswerDiff = os.ExpandEnv(c.AnswerDiff) c.AnswerEdit = os.ExpandEnv(c.AnswerEdit) diff --git a/pkg/settings/parser/rebuild_mode.go b/pkg/settings/parser/rebuild_mode.go new file mode 100644 index 00000000..9ce5544e --- /dev/null +++ b/pkg/settings/parser/rebuild_mode.go @@ -0,0 +1,10 @@ +package parser + +type RebuildMode string + +const ( + RebuildModeNo RebuildMode = "no" + RebuildModeYes RebuildMode = "yes" + RebuildModeTree RebuildMode = "tree" + RebuildModeAll RebuildMode = "all" +) diff --git a/sync.go b/sync.go index 2af0d413..c215184f 100644 --- a/sync.go +++ b/sync.go @@ -115,7 +115,7 @@ 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.Mode, + o.cfg.Runtime.VCSStore, o.cfg.Mode, o.cfg.ReBuild, cmdArgs.ExistsArg("w", "downloadonly"), o.cfg.Runtime.Logger.Child("installer")) pkgBuildDirs, errInstall := preparer.Run(ctx, os.Stdout, targets)