From 49267b9cd96e3749dc059bd970470154b2fc92f0 Mon Sep 17 00:00:00 2001 From: Jo Date: Thu, 27 Apr 2023 09:20:21 +0200 Subject: [PATCH] feat(upgrade): separate menu for pulled along dependencies (#2141) try separate menu for pulled along use installed as term fix order gap fix tests add aur db + aur scenario --- cmd.go | 5 +- cmd_test.go | 137 +++++++++++++++++++++++++++++++++ pkg/db/mock/executor.go | 4 + pkg/query/query_builder.go | 12 +-- pkg/settings/runtime.go | 2 + pkg/upgrade/service.go | 40 +++++++--- pkg/upgrade/service_test.go | 14 ++-- pkg/upgrade/upgrade.go | 36 ++++++++- query_test.go | 147 +++++++++++++++++++++++++++++++++++- 9 files changed, 364 insertions(+), 33 deletions(-) create mode 100644 cmd_test.go diff --git a/cmd.go b/cmd.go index b13a2cf2..d6ed78f1 100644 --- a/cmd.go +++ b/cmd.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "net/http" - "os" "strings" alpm "github.com/Jguer/go-alpm/v2" @@ -403,9 +402,9 @@ func displayNumberMenu(ctx context.Context, cfg *settings.Configuration, pkgS [] return nil } - text.Infoln(gotext.Get("Packages to install (eg: 1 2 3, 1-3 or ^4)")) + cfg.Runtime.Logger.Infoln(gotext.Get("Packages to install (eg: 1 2 3, 1-3 or ^4)")) - numberBuf, err := text.GetInput(os.Stdin, "", false) + numberBuf, err := cfg.Runtime.Logger.GetInput("", false) if err != nil { return err } diff --git a/cmd_test.go b/cmd_test.go new file mode 100644 index 00000000..68fb8901 --- /dev/null +++ b/cmd_test.go @@ -0,0 +1,137 @@ +package main + +import ( + "context" + "fmt" + "io" + "os" + "os/exec" + "strings" + "testing" + + "github.com/Jguer/aur" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Jguer/yay/v12/pkg/db/mock" + mockaur "github.com/Jguer/yay/v12/pkg/dep/mock" + "github.com/Jguer/yay/v12/pkg/query" + "github.com/Jguer/yay/v12/pkg/settings" + "github.com/Jguer/yay/v12/pkg/settings/exe" + "github.com/Jguer/yay/v12/pkg/settings/parser" + "github.com/Jguer/yay/v12/pkg/text" + "github.com/Jguer/yay/v12/pkg/vcs" +) + +func TestYogurtMenuAURDB(t *testing.T) { + t.Skip("skip until Operation service is an interface") + t.Parallel() + makepkgBin := t.TempDir() + "/makepkg" + pacmanBin := t.TempDir() + "/pacman" + gitBin := t.TempDir() + "/git" + 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()) + + f, err = os.OpenFile(gitBin, os.O_RDONLY|os.O_CREATE, 0o755) + require.NoError(t, err) + require.NoError(t, f.Close()) + + captureOverride := func(cmd *exec.Cmd) (stdout string, stderr string, err error) { + return "", "", nil + } + + showOverride := func(cmd *exec.Cmd) error { + return nil + } + + mockRunner := &exe.MockRunner{CaptureFn: captureOverride, ShowFn: showOverride} + cmdBuilder := &exe.CmdBuilder{ + MakepkgBin: makepkgBin, + SudoBin: "su", + PacmanBin: pacmanBin, + PacmanConfigPath: "/etc/pacman.conf", + GitBin: "git", + Runner: mockRunner, + SudoLoopEnabled: false, + } + + cmdArgs := parser.MakeArguments() + cmdArgs.AddArg("Y") + cmdArgs.AddTarget("yay") + + db := &mock.DBExecutor{ + AlpmArchitecturesFn: func() ([]string, error) { + return []string{"x86_64"}, nil + }, + RefreshHandleFn: func() error { + return nil + }, + ReposFn: func() []string { + return []string{"aur"} + }, + SyncPackagesFn: func(s ...string) []mock.IPackage { + return []mock.IPackage{ + &mock.Package{ + PName: "yay", + PBase: "yay", + PVersion: "10.0.0", + PDB: mock.NewDB("aur"), + }, + } + }, + LocalPackageFn: func(s string) mock.IPackage { + return nil + }, + } + aurCache := &mockaur.MockAUR{ + GetFn: func(ctx context.Context, query *aur.Query) ([]aur.Pkg, error) { + return []aur.Pkg{ + { + Name: "yay", + PackageBase: "yay", + Version: "10.0.0", + }, + }, nil + }, + } + logger := text.NewLogger(io.Discard, os.Stderr, strings.NewReader("1\n"), true, "test") + cfg := &settings.Configuration{ + NewInstallEngine: true, + RemoveMake: "no", + Runtime: &settings.Runtime{ + Logger: logger, + CmdBuilder: cmdBuilder, + VCSStore: &vcs.Mock{}, + QueryBuilder: query.NewSourceQueryBuilder(aurCache, logger, "votes", parser.ModeAny, "name", + true, false, true), + AURCache: aurCache, + }, + } + + err = handleCmd(context.Background(), cfg, cmdArgs, db) + require.NoError(t, err) + + wantCapture := []string{} + wantShow := []string{ + "pacman -S -y --config /etc/pacman.conf --", + "pacman -S -y -u --config /etc/pacman.conf --", + } + + require.Len(t, mockRunner.ShowCalls, len(wantShow)) + require.Len(t, mockRunner.CaptureCalls, len(wantCapture)) + + for i, call := range mockRunner.ShowCalls { + show := call.Args[0].(*exec.Cmd).String() + show = strings.ReplaceAll(show, makepkgBin, "makepkg") + show = strings.ReplaceAll(show, pacmanBin, "pacman") + show = strings.ReplaceAll(show, gitBin, "pacman") + + // options are in a different order on different systems and on CI root user is used + assert.Subset(t, strings.Split(show, " "), strings.Split(wantShow[i], " "), fmt.Sprintf("%d - %s", i, show)) + } +} diff --git a/pkg/db/mock/executor.go b/pkg/db/mock/executor.go index baa3d7f8..7e7c5d60 100644 --- a/pkg/db/mock/executor.go +++ b/pkg/db/mock/executor.go @@ -30,6 +30,7 @@ type DBExecutor struct { RefreshHandleFn func() error ReposFn func() []string SyncPackageFn func(string) IPackage + SyncPackagesFn func(...string) []IPackage SyncSatisfierFn func(string) IPackage SyncUpgradesFn func(bool) (map[string]db.SyncUpgrade, error) } @@ -170,6 +171,9 @@ func (t *DBExecutor) SyncPackage(s string) IPackage { } func (t *DBExecutor) SyncPackages(s ...string) []IPackage { + if t.SyncPackagesFn != nil { + return t.SyncPackagesFn(s...) + } panic("implement me") } diff --git a/pkg/query/query_builder.go b/pkg/query/query_builder.go index 176ae910..d371c706 100644 --- a/pkg/query/query_builder.go +++ b/pkg/query/query_builder.go @@ -211,12 +211,12 @@ func (s *SourceQueryBuilder) Results(dbExecutor db.Executor, verboseSearch Searc } pkg := s.queryMap[s.results[i].source][s.results[i].name] - if s.results[i].source == sourceAUR { - aurPkg := pkg.(aur.Pkg) - toPrint += aurPkgSearchString(&aurPkg, dbExecutor, s.singleLineResults) - } else { - syncPkg := pkg.(alpm.IPackage) - toPrint += syncPkgSearchString(syncPkg, dbExecutor, s.singleLineResults) + + switch pPkg := pkg.(type) { + case aur.Pkg: + toPrint += aurPkgSearchString(&pPkg, dbExecutor, s.singleLineResults) + case alpm.IPackage: + toPrint += syncPkgSearchString(pPkg, dbExecutor, s.singleLineResults) } s.logger.Println(toPrint) diff --git a/pkg/settings/runtime.go b/pkg/settings/runtime.go index 275da79c..722671a7 100644 --- a/pkg/settings/runtime.go +++ b/pkg/settings/runtime.go @@ -6,6 +6,7 @@ import ( "net/http" "os" "path/filepath" + "time" "github.com/leonelquinteros/gotext" @@ -68,6 +69,7 @@ func BuildRuntime(cfg *Configuration, cmdArgs *parser.Arguments, version string) metadata.WithCacheFilePath(filepath.Join(cfg.BuildDir, "aur.json")), metadata.WithRequestEditorFn(userAgentFn), metadata.WithBaseURL(cfg.AURURL), + metadata.WithCustomCacheValidity(100000*time.Hour), metadata.WithDebugLogger(logger.Debugln), ) if errAURCache != nil { diff --git a/pkg/upgrade/service.go b/pkg/upgrade/service.go index e2a74d0a..61d63c48 100644 --- a/pkg/upgrade/service.go +++ b/pkg/upgrade/service.go @@ -262,8 +262,7 @@ func (u *UpgradeService) GraphUpgrades(ctx context.Context, // userExcludeUpgrades asks the user which packages to exclude from the upgrade and // removes them from the graph func (u *UpgradeService) UserExcludeUpgrades(graph *topo.Graph[string, *dep.InstallInfo]) ([]string, error) { - allUpLen := graph.Len() - if allUpLen == 0 { + if graph.Len() == 0 { return []string{}, nil } aurUp, repoUp := u.graphToUpSlice(graph) @@ -271,12 +270,35 @@ func (u *UpgradeService) UserExcludeUpgrades(graph *topo.Graph[string, *dep.Inst sort.Sort(repoUp) sort.Sort(aurUp) - allUp := UpSlice{Up: append(repoUp.Up, aurUp.Up...), Repos: append(repoUp.Repos, aurUp.Repos...)} + allUp := UpSlice{Repos: append(repoUp.Repos, aurUp.Repos...)} + for _, up := range repoUp.Up { + if up.LocalVersion == "" && up.Reason != alpm.PkgReasonExplicit { + allUp.PulledDeps = append(allUp.PulledDeps, up) + } else { + allUp.Up = append(allUp.Up, up) + } + } - u.log.Printf("%s"+text.Bold(" %d ")+"%s\n", text.Bold(text.Cyan("::")), allUpLen, text.Bold(gotext.Get("Packages to upgrade/install."))) + for _, up := range aurUp.Up { + if up.LocalVersion == "" && up.Reason != alpm.PkgReasonExplicit { + allUp.PulledDeps = append(allUp.PulledDeps, up) + } else { + allUp.Up = append(allUp.Up, up) + } + } + + if len(allUp.PulledDeps) > 0 { + u.log.Printf("%s"+text.Bold(" %d ")+"%s\n", text.Bold(text.Cyan("::")), + len(allUp.PulledDeps), text.Bold(gotext.Get("%s will also be installed for this operation", + gotext.GetN("dependency", "dependencies", len(allUp.PulledDeps))))) + allUp.PrintDeps(u.log) + } + + u.log.Printf("%s"+text.Bold(" %d ")+"%s\n", text.Bold(text.Cyan("::")), + len(allUp.Up), text.Bold(gotext.Get("%s to upgrade/install.", gotext.GetN("package", "packages", len(allUp.Up))))) allUp.Print(u.log) - u.log.Infoln(gotext.Get("Packages to exclude: (eg: \"1 2 3\", \"1-3\", \"^4\" or repo name)")) + u.log.Infoln(gotext.Get("%s to exclude: (eg: \"1 2 3\", \"1-3\", \"^4\" or repo name)", gotext.GetN("package", "packages", len(allUp.Up)))) u.log.Warnln(gotext.Get("Excluding packages may cause partial upgrades and break systems")) numbers, err := u.log.GetInput(u.cfg.AnswerUpgrade, settings.NoConfirm) @@ -292,10 +314,6 @@ func (u *UpgradeService) UserExcludeUpgrades(graph *topo.Graph[string, *dep.Inst excluded := make([]string, 0) for i := range allUp.Up { up := &allUp.Up[i] - // choices do not apply to non-installed packages - if up.LocalVersion == "" { - continue - } if isInclude && otherExclude.Get(up.Repository) { u.log.Debugln("pruning", up.Name) @@ -303,13 +321,13 @@ func (u *UpgradeService) UserExcludeUpgrades(graph *topo.Graph[string, *dep.Inst continue } - if isInclude && exclude.Get(allUpLen-i) { + if isInclude && exclude.Get(len(allUp.Up)-i) { u.log.Debugln("pruning", up.Name) excluded = append(excluded, graph.Prune(up.Name)...) continue } - if !isInclude && !(include.Get(allUpLen-i) || otherInclude.Get(up.Repository)) { + if !isInclude && !(include.Get(len(allUp.Up)-i) || otherInclude.Get(up.Repository)) { u.log.Debugln("pruning", up.Name) excluded = append(excluded, graph.Prune(up.Name)...) continue diff --git a/pkg/upgrade/service_test.go b/pkg/upgrade/service_test.go index a2c3a3d0..dfa97816 100644 --- a/pkg/upgrade/service_test.go +++ b/pkg/upgrade/service_test.go @@ -281,7 +281,7 @@ func TestUpgradeService_GraphUpgrades(t *testing.T) { { name: "exclude linux", fields: fields{ - input: strings.NewReader("4\n"), + input: strings.NewReader("3\n"), output: io.Discard, noConfirm: false, }, @@ -301,7 +301,7 @@ func TestUpgradeService_GraphUpgrades(t *testing.T) { { name: "only linux", fields: fields{ - input: strings.NewReader("^4\n"), + input: strings.NewReader("^3\n"), output: io.Discard, noConfirm: false, }, @@ -642,7 +642,6 @@ func TestUpgradeService_GraphUpgrades_zfs_dkms(t *testing.T) { } type fields struct { input io.Reader - output io.Writer noConfirm bool devel bool } @@ -664,7 +663,6 @@ func TestUpgradeService_GraphUpgrades_zfs_dkms(t *testing.T) { name: "no input", fields: fields{ input: strings.NewReader("\n"), - output: io.Discard, noConfirm: false, }, args: args{ @@ -684,7 +682,6 @@ func TestUpgradeService_GraphUpgrades_zfs_dkms(t *testing.T) { name: "no input - inverted order", fields: fields{ input: strings.NewReader("\n"), - output: io.Discard, noConfirm: false, }, args: args{ @@ -742,16 +739,15 @@ func TestUpgradeService_GraphUpgrades_zfs_dkms(t *testing.T) { ReposFn: func() []string { return []string{"core"} }, } + logger := text.NewLogger(io.Discard, os.Stderr, + tt.fields.input, true, "test") grapher := dep.NewGrapher(dbExe, mockAUR, - false, true, false, false, false, text.NewLogger(tt.fields.output, os.Stderr, - tt.fields.input, true, "test")) + false, true, false, false, false, logger) cfg := &settings.Configuration{ Devel: tt.fields.devel, Mode: parser.ModeAny, } - logger := text.NewLogger(tt.fields.output, os.Stderr, - tt.fields.input, true, "test") u := &UpgradeService{ log: logger, grapher: grapher, diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index cf6e43ab..85954c07 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -22,8 +22,9 @@ func StylizedNameWithRepository(u *Upgrade) string { // upSlice is a slice of Upgrades. type UpSlice struct { - Up []Upgrade - Repos []string + Up []Upgrade + Repos []string + PulledDeps []Upgrade } func (u UpSlice) Len() int { return len(u.Up) } @@ -84,3 +85,34 @@ func (u UpSlice) Print(logger *text.Logger) { } } } + +func (u UpSlice) PrintDeps(logger *text.Logger) { + longestName, longestVersion := 0, 0 + + for k := range u.PulledDeps { + upgrade := &u.PulledDeps[k] + packNameLen := len(StylizedNameWithRepository(upgrade)) + packVersion, _ := query.GetVersionDiff(upgrade.LocalVersion, upgrade.RemoteVersion) + packVersionLen := len(packVersion) + longestName = intrange.Max(packNameLen, longestName) + longestVersion = intrange.Max(packVersionLen, longestVersion) + } + + lenUp := len(u.PulledDeps) + longestNumber := len(fmt.Sprintf("%v", lenUp)) + namePadding := fmt.Sprintf(" %s%%-%ds ", strings.Repeat(" ", longestNumber), longestName) + versionPadding := fmt.Sprintf("%%-%ds", longestVersion) + + for k := range u.PulledDeps { + upgrade := &u.PulledDeps[k] + left, right := query.GetVersionDiff(upgrade.LocalVersion, upgrade.RemoteVersion) + + logger.Printf(namePadding, StylizedNameWithRepository(upgrade)) + logger.Printf("%s -> %s\n", fmt.Sprintf(versionPadding, left), right) + if upgrade.Extra != "" { + logger.Println(strings.Repeat(" ", longestNumber), strings.ToLower(upgrade.Extra)) + } + } + + logger.Println() +} diff --git a/query_test.go b/query_test.go index 4461cea4..fe71e622 100644 --- a/query_test.go +++ b/query_test.go @@ -12,6 +12,7 @@ import ( "github.com/Jguer/yay/v12/pkg/db/mock" mockaur "github.com/Jguer/yay/v12/pkg/dep/mock" + "github.com/Jguer/yay/v12/pkg/query" "github.com/Jguer/yay/v12/pkg/settings" "github.com/Jguer/yay/v12/pkg/settings/exe" "github.com/Jguer/yay/v12/pkg/settings/parser" @@ -38,9 +39,8 @@ func getFromFile(t *testing.T, filePath string) mockaur.GetFunc { } func TestSyncInfo(t *testing.T) { - pacmanBin := t.TempDir() + "/pacman" - t.Parallel() + pacmanBin := t.TempDir() + "/pacman" testCases := []struct { name string @@ -160,3 +160,146 @@ func TestSyncInfo(t *testing.T) { }) } } + +// Should not error when there is a DB called aur +func TestSyncSearchAURDB(t *testing.T) { + t.Parallel() + + pacmanBin := t.TempDir() + "/pacman" + testCases := []struct { + name string + args []string + targets []string + wantShow []string + wantErr bool + bottomUp bool + singleLine bool + mixed bool + }{ + { + name: "Ss jellyfin false false", + args: []string{"S", "s"}, + targets: []string{"jellyfin"}, + wantShow: []string{}, + }, + { + name: "Ss jellyfin true false", + args: []string{"S", "s"}, + targets: []string{"jellyfin"}, + wantShow: []string{}, + singleLine: true, + }, + { + name: "Ss jellyfin true true", + args: []string{"S", "s"}, + targets: []string{"jellyfin"}, + wantShow: []string{}, + singleLine: true, + mixed: true, + }, + { + name: "Ss jellyfin false true", + args: []string{"S", "s"}, + targets: []string{"jellyfin"}, + wantShow: []string{}, + singleLine: false, + mixed: true, + }, + { + name: "Ss jellyfin true true - bottomup", + args: []string{"S", "s"}, + targets: []string{"jellyfin"}, + wantShow: []string{}, + singleLine: true, + mixed: true, + bottomUp: true, + }, + } + + dbExc := &mock.DBExecutor{ + SyncPackagesFn: func(s ...string) []mock.IPackage { + return []mock.IPackage{ + &mock.Package{ + PName: "jellyfin", + PBase: "jellyfin", + PDB: mock.NewDB("aur"), + }, + } + }, + LocalPackageFn: func(s string) mock.IPackage { + return &mock.Package{ + PName: "jellyfin", + PBase: "jellyfin", + PDB: mock.NewDB("aur"), + } + }, + PackagesFromGroupFn: func(s string) []mock.IPackage { + return nil + }, + } + + mockAUR := &mockaur.MockAUR{GetFn: func(ctx context.Context, query *aur.Query) ([]aur.Pkg, error) { + if query.Needles[0] == "jellyfin" { + jfinFn := getFromFile(t, "pkg/dep/testdata/jellyfin.json") + return jfinFn(ctx, query) + } + + return nil, fmt.Errorf("not found") + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + mockRunner := &exe.MockRunner{ + CaptureFn: func(cmd *exec.Cmd) (stdout string, stderr string, err error) { + return "", "", nil + }, + ShowFn: func(cmd *exec.Cmd) error { return nil }, + } + cmdBuilder := &exe.CmdBuilder{ + SudoBin: "su", + PacmanBin: pacmanBin, + PacmanConfigPath: "/etc/pacman.conf", + GitBin: "git", + Runner: mockRunner, + SudoLoopEnabled: false, + } + cfg := &settings.Configuration{ + Runtime: &settings.Runtime{ + CmdBuilder: cmdBuilder, + AURCache: mockAUR, + QueryBuilder: query.NewSourceQueryBuilder(mockAUR, NewTestLogger(), "votes", parser.ModeAny, "name", + tc.bottomUp, tc.singleLine, tc.mixed), + Logger: NewTestLogger(), + }, + } + + cmdArgs := parser.MakeArguments() + cmdArgs.AddArg(tc.args...) + cmdArgs.AddTarget(tc.targets...) + + err := handleCmd(context.Background(), + cfg, cmdArgs, dbExc, + ) + + if tc.wantErr { + require.Error(t, err) + assert.EqualError(t, err, "") + } else { + require.NoError(t, err) + } + if len(tc.wantShow) == 0 { + assert.Empty(t, mockRunner.ShowCalls) + return + } + for i, call := range mockRunner.ShowCalls { + show := call.Args[0].(*exec.Cmd).String() + show = strings.ReplaceAll(show, pacmanBin, "pacman") + + // options are in a different order on different systems and on CI root user is used + assert.Subset(t, strings.Split(show, " "), + strings.Split(tc.wantShow[i], " "), + fmt.Sprintf("%d - %s", i, show)) + } + }) + } +}