Skip to content

Commit

Permalink
Refactor code following suggestions from review
Browse files Browse the repository at this point in the history
  • Loading branch information
@artmello
artmello committed Jan 21, 2020
1 parent a880a40 commit bea76ce
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 145 deletions.
159 changes: 43 additions & 116 deletions cmd/krew/cmd/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
package cmd

import (
"bytes"
"fmt"
"io"
"os"
"path/filepath"
"sort"
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand All @@ -28,20 +29,9 @@ import (
"sigs.k8s.io/krew/internal/index/indexscanner"
"sigs.k8s.io/krew/internal/installation"
"sigs.k8s.io/krew/pkg/constants"
"sigs.k8s.io/krew/pkg/index"
)

type newPlugin struct {
name string
version string
}

type newVersionPlugin struct {
name string
installed bool
oldVersion string
newVersion string
}

// updateCmd represents the update command
var updateCmd = &cobra.Command{
Use: "update",
Expand All @@ -57,124 +47,64 @@ Remarks:
RunE: ensureIndexUpdated,
}

func retrieveUpdatedPluginList() ([]string, error) {
modifiedFiles, err := gitutil.ListModifiedFiles(constants.IndexURI, paths.IndexPluginsPath())
if err != nil {
return []string{}, err
}
func showUpdatedPlugins(out io.Writer, preUpdate []index.Plugin, posUpdate []index.Plugin, installedPlugins map[string]string) {
var newPlugins []index.Plugin
var updatedPlugins []index.Plugin

var plugins []string
for _, f := range modifiedFiles {
filename := filepath.Base(f)
extension := filepath.Ext(filename)
name := filename[0 : len(filename)-len(extension)]
plugins = append(plugins, name)
oldIndex := make(map[string]index.Plugin)
for _, p := range preUpdate {
oldIndex[p.Name] = p
}

return plugins, nil
}

func retrievePluginNameVersionMap(names []string) map[string]string {
m := make(map[string]string, len(names))
for _, n := range names {
plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(), n)
if err != nil {
for _, p := range posUpdate {
old, ok := oldIndex[p.Name]
if !ok {
newPlugins = append(newPlugins, p)
continue
}

m[n] = plugin.Spec.Version
}

return m
}

func retrieveInstalledPluginMap() (map[string]bool, error) {
plugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
if err != nil {
return map[string]bool{}, err
}
m := make(map[string]bool, len(plugins))
for name := range plugins {
m[name] = true
}

return m, nil
}

func filterAndSortUpdatedPlugins(old, updated map[string]string, installed map[string]bool) ([]newPlugin, []newVersionPlugin) {
var newPluginList []newPlugin
var newVersionList []newVersionPlugin

for name, version := range updated {
oldVersion, ok := old[name]
if !ok {
newPluginList = append(newPluginList, newPlugin{
name: name,
version: version,
})
if _, ok := installedPlugins[p.Name]; !ok {
continue
}

if version != oldVersion {
_, installed := installed[name]
newVersionList = append(newVersionList, newVersionPlugin{
name: name,
installed: installed,
oldVersion: oldVersion,
newVersion: version,
})
continue
if old.Spec.Version != p.Spec.Version {
updatedPlugins = append(updatedPlugins, p)
}
}

sort.Slice(newPluginList, func(i, j int) bool {
return newPluginList[i].name < newPluginList[j].name
})
if len(newPlugins) > 0 {
var b bytes.Buffer
b.WriteString(" New plugins available: ")

sort.Slice(newVersionList, func(i, j int) bool {
if newVersionList[i].installed && !newVersionList[j].installed {
return true
var s []string
for _, p := range newPlugins {
s = append(s, fmt.Sprintf("%s %s", p.Name, p.Spec.Version))
}
b.WriteString(strings.Join(s, ", "))

if !newVersionList[i].installed && newVersionList[j].installed {
return false
}
fmt.Fprintln(out, b.String())

return newVersionList[i].name < newVersionList[j].name
})
}

return newPluginList, newVersionList
}
if len(updatedPlugins) > 0 {
var b bytes.Buffer
b.WriteString(" Upgrades available: ")

func showUpdatedPlugins(newPluginList []newPlugin, newVersionList []newVersionPlugin) {
if len(newPluginList) > 0 {
fmt.Fprintln(os.Stderr, " New plugins available:")
for _, np := range newPluginList {
fmt.Fprintf(os.Stderr, " * %s %s\n", np.name, np.version)
var s []string
for _, p := range updatedPlugins {
old := oldIndex[p.Name]
s = append(s, fmt.Sprintf("%s %s -> %s", p.Name, old.Spec.Version, p.Spec.Version))
}
}
b.WriteString(strings.Join(s, ", "))

if len(newVersionList) > 0 {
fmt.Fprintln(os.Stderr, " The following plugins have new version:")
for _, np := range newVersionList {
if np.installed {
fmt.Fprintf(os.Stderr, " * %s %s -> %s (!)\n", np.name, np.oldVersion, np.newVersion)
continue
}
fmt.Fprintf(os.Stderr, " * %s %s -> %s\n", np.name, np.oldVersion, np.newVersion)
}
fmt.Fprintln(out, b.String())
}
}

func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
updatedPlugins, err := retrieveUpdatedPluginList()
preUpdateIndex, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath())
if err != nil {
return errors.Wrap(err, "failed to load the list of updated plugins from the index")
}

var oldMap map[string]string
if len(updatedPlugins) > 0 {
oldMap = retrievePluginNameVersionMap(updatedPlugins)
return errors.Wrap(err, "failed to load plugin index before update")
}

klog.V(1).Infof("Updating the local copy of plugin index (%s)", paths.IndexPath())
Expand All @@ -183,20 +113,17 @@ func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
}
fmt.Fprintln(os.Stderr, "Updated the local copy of plugin index.")

if len(updatedPlugins) < 0 {
return nil
posUpdateIndex, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath())
if err != nil {
return errors.Wrap(err, "failed to load plugin index after update")
}

updatedMap := retrievePluginNameVersionMap(updatedPlugins)

installedMap, err := retrieveInstalledPluginMap()
installedPlugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
if err != nil {
return errors.Wrap(err, "failed to find all installed versions")
return errors.Wrap(err, "failed to load installed plugins list after update")
}

newPluginList, newVersionList := filterAndSortUpdatedPlugins(oldMap, updatedMap, installedMap)

showUpdatedPlugins(newPluginList, newVersionList)
showUpdatedPlugins(os.Stderr, preUpdateIndex, posUpdateIndex, installedPlugins)

return nil
}
Expand Down
36 changes: 7 additions & 29 deletions internal/gitutil/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ func EnsureCloned(uri, destinationPath string) error {
if ok, err := IsGitCloned(destinationPath); err != nil {
return err
} else if !ok {
_, err := exec("", "clone", "-v", uri, destinationPath)
return err
return exec("", "clone", "-v", uri, destinationPath)
}
return nil
}
Expand All @@ -50,15 +49,15 @@ func IsGitCloned(gitPath string) (bool, error) {
// and also will create a pristine working directory by removing
// untracked files and directories.
func updateAndCleanUntracked(destinationPath string) error {
if _, err := exec(destinationPath, "fetch", "-v"); err != nil {
if err := exec(destinationPath, "fetch", "-v"); err != nil {
return errors.Wrapf(err, "fetch index at %q failed", destinationPath)
}

if _, err := exec(destinationPath, "reset", "--hard", "@{upstream}"); err != nil {
if err := exec(destinationPath, "reset", "--hard", "@{upstream}"); err != nil {
return errors.Wrapf(err, "reset index at %q failed", destinationPath)
}

_, err := exec(destinationPath, "clean", "-xfd")
err := exec(destinationPath, "clean", "-xfd")
return errors.Wrapf(err, "clean index at %q failed", destinationPath)
}

Expand All @@ -70,27 +69,7 @@ func EnsureUpdated(uri, destinationPath string) error {
return updateAndCleanUntracked(destinationPath)
}

// ListModifiedFiles will fetch origin and list modified files
func ListModifiedFiles(uri, destinationPath string) ([]string, error) {
if _, err := exec(destinationPath, "fetch", "-v"); err != nil {
return []string{}, errors.Wrapf(err, "fetch index at %q failed", destinationPath)
}

output, err := exec(destinationPath, "diff", "--name-only", "@{upstream}", "--", ".")
if err != nil {
return []string{}, errors.Wrapf(err, "diff index at %q failed", destinationPath)
}

var modifiedFiles []string
for _, f := range strings.Split(output, "\n") {
if len(f) > 0 {
modifiedFiles = append(modifiedFiles, f)
}
}
return modifiedFiles, nil
}

func exec(pwd string, args ...string) (string, error) {
func exec(pwd string, args ...string) error {
klog.V(4).Infof("Going to run git %s", strings.Join(args, " "))
cmd := osexec.Command("git", args...)
cmd.Dir = pwd
Expand All @@ -101,8 +80,7 @@ func exec(pwd string, args ...string) (string, error) {
}
cmd.Stdout, cmd.Stderr = w, w
if err := cmd.Run(); err != nil {
output := buf.String()
return output, errors.Wrapf(err, "command execution failure, output=%q", output)
return errors.Wrapf(err, "command execution failure, output=%q", buf.String())
}
return buf.String(), nil
return nil
}

0 comments on commit bea76ce

Please sign in to comment.