dev: clean up Executor

pkg/commands/cache.go

@@ -1,13 +1,19 @@
 package commands
 
 import (
+	"bytes"
+	"crypto/sha256"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/spf13/cobra"
+	"gopkg.in/yaml.v3"
 
 	"github.com/golangci/golangci-lint/internal/cache"
+	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/fsutils"
 	"github.com/golangci/golangci-lint/pkg/logutils"
 )
@@ -60,6 +66,68 @@ func (e *Executor) executeCacheStatus(_ *cobra.Command, _ []string) {
 	}
 }
 
+func (e *Executor) initHashSalt(version string) error {
+	binSalt, err := computeBinarySalt(version)
+	if err != nil {
+		return fmt.Errorf("failed to calculate binary salt: %w", err)
+	}
+
+	configSalt, err := computeConfigSalt(e.cfg)
+	if err != nil {
+		return fmt.Errorf("failed to calculate config salt: %w", err)
+	}
+
+	b := bytes.NewBuffer(binSalt)
+	b.Write(configSalt)
+	cache.SetSalt(b.Bytes())
+	return nil
+}
+
+func computeBinarySalt(version string) ([]byte, error) {
+	if version != "" && version != "(devel)" {
+		return []byte(version), nil
+	}
+
+	if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) {
+		return []byte("debug"), nil
+	}
+
+	p, err := os.Executable()
+	if err != nil {
+		return nil, err
+	}
+	f, err := os.Open(p)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	h := sha256.New()
+	if _, err := io.Copy(h, f); err != nil {
+		return nil, err
+	}
+	return h.Sum(nil), nil
+}
+
+func computeConfigSalt(cfg *config.Config) ([]byte, error) {
+	// We don't hash all config fields to reduce meaningless cache
+	// invalidations. At least, it has a huge impact on tests speed.
+
+	lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings)
+	if err != nil {
+		return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err)
+	}
+
+	configData := bytes.NewBufferString("linters-settings=")
+	configData.Write(lintersSettingsBytes)
+	configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ","))
+
+	h := sha256.New()
+	if _, err := h.Write(configData.Bytes()); err != nil {
+		return nil, err
+	}
+	return h.Sum(nil), nil
+}
+
 func dirSizeBytes(path string) (int64, error) {
 	var size int64
 	err := filepath.Walk(path, func(_ string, info os.FileInfo, err error) error {

pkg/commands/executor.go

@@ -1,24 +1,15 @@
 package commands
 
 import (
-	"bytes"
-	"context"
-	"crypto/sha256"
 	"errors"
-	"fmt"
-	"io"
-	"os"
-	"path/filepath"
 	"strings"
 	"time"
 
 	"github.com/fatih/color"
 	"github.com/gofrs/flock"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	"gopkg.in/yaml.v3"
 
-	"github.com/golangci/golangci-lint/internal/cache"
 	"github.com/golangci/golangci-lint/internal/pkgcache"
 	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/fsutils"
@@ -163,103 +154,36 @@ func (e *Executor) Execute() error {
 	return e.rootCmd.Execute()
 }
 
-func (e *Executor) initHashSalt(version string) error {
-	binSalt, err := computeBinarySalt(version)
-	if err != nil {
-		return fmt.Errorf("failed to calculate binary salt: %w", err)
-	}
-
-	configSalt, err := computeConfigSalt(e.cfg)
-	if err != nil {
-		return fmt.Errorf("failed to calculate config salt: %w", err)
-	}
-
-	b := bytes.NewBuffer(binSalt)
-	b.Write(configSalt)
-	cache.SetSalt(b.Bytes())
-	return nil
-}
-
-func computeBinarySalt(version string) ([]byte, error) {
-	if version != "" && version != "(devel)" {
-		return []byte(version), nil
-	}
-
-	if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) {
-		return []byte("debug"), nil
-	}
-
-	p, err := os.Executable()
-	if err != nil {
-		return nil, err
-	}
-	f, err := os.Open(p)
-	if err != nil {
-		return nil, err
-	}
-	defer f.Close()
-	h := sha256.New()
-	if _, err := io.Copy(h, f); err != nil {
-		return nil, err
-	}
-	return h.Sum(nil), nil
-}
-
-func computeConfigSalt(cfg *config.Config) ([]byte, error) {
-	// We don't hash all config fields to reduce meaningless cache
-	// invalidations. At least, it has a huge impact on tests speed.
-
-	lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings)
-	if err != nil {
-		return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err)
-	}
-
-	configData := bytes.NewBufferString("linters-settings=")
-	configData.Write(lintersSettingsBytes)
-	configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ","))
-
-	h := sha256.New()
-	if _, err := h.Write(configData.Bytes()); err != nil {
-		return nil, err
-	}
-	return h.Sum(nil), nil
-}
+func fixSlicesFlags(fs *pflag.FlagSet) {
+	// It's a dirty hack to set flag.Changed to true for every string slice flag.
+	// It's necessary to merge config and command-line slices: otherwise command-line
+	// flags will always overwrite ones from the config.
+	fs.VisitAll(func(f *pflag.Flag) {
+		if f.Value.Type() != "stringSlice" {
+			return
+		}
 
-func (e *Executor) acquireFileLock() bool {
-	if e.cfg.Run.AllowParallelRunners {
-		e.debugf("Parallel runners are allowed, no locking")
-		return true
-	}
+		s, err := fs.GetStringSlice(f.Name)
+		if err != nil {
+			return
+		}
 
-	lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock")
-	e.debugf("Locking on file %s...", lockFile)
-	f := flock.New(lockFile)
-	const retryDelay = time.Second
+		if s == nil { // assume that every string slice flag has nil as the default
+			return
+		}
 
-	ctx := context.Background()
-	if !e.cfg.Run.AllowSerialRunners {
-		const totalTimeout = 5 * time.Second
-		var cancel context.CancelFunc
-		ctx, cancel = context.WithTimeout(ctx, totalTimeout)
-		defer cancel()
-	}
-	if ok, _ := f.TryLockContext(ctx, retryDelay); !ok {
-		return false
-	}
+		var safe []string
+		for _, v := range s {
+			// add quotes to escape comma because spf13/pflag use a CSV parser:
+			// https://github.com/spf13/pflag/blob/85dd5c8bc61cfa382fecd072378089d4e856579d/string_slice.go#L43
+			safe = append(safe, `"`+v+`"`)
+		}
 
-	e.flock = f
-	return true
+		// calling Set sets Changed to true: next Set calls will append, not overwrite
+		_ = f.Value.Set(strings.Join(safe, ","))
+	})
 }
 
-func (e *Executor) releaseFileLock() {
-	if e.cfg.Run.AllowParallelRunners {
-		return
-	}
-
-	if err := e.flock.Unlock(); err != nil {
-		e.debugf("Failed to unlock on file: %s", err)
-	}
-	if err := os.Remove(e.flock.Path()); err != nil {
-		e.debugf("Failed to remove lock file: %s", err)
-	}
+func wh(text string) string {
+	return color.GreenString(text)
 }

pkg/commands/help.go

@@ -33,33 +33,6 @@ func (e *Executor) initHelp() {
 	helpCmd.AddCommand(lintersHelpCmd)
 }
 
-func printLinterConfigs(lcs []*linter.Config) {
-	sort.Slice(lcs, func(i, j int) bool {
-		return lcs[i].Name() < lcs[j].Name()
-	})
-	for _, lc := range lcs {
-		altNamesStr := ""
-		if len(lc.AlternativeNames) != 0 {
-			altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", "))
-		}
-
-		// If the linter description spans multiple lines, truncate everything following the first newline
-		linterDescription := lc.Linter.Desc()
-		firstNewline := strings.IndexRune(linterDescription, '\n')
-		if firstNewline > 0 {
-			linterDescription = linterDescription[:firstNewline]
-		}
-
-		deprecatedMark := ""
-		if lc.IsDeprecated() {
-			deprecatedMark = " [" + color.RedString("deprecated") + "]"
-		}
-
-		fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", color.YellowString(lc.Name()),
-			altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix)
-	}
-}
-
 func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) {
 	var enabledLCs, disabledLCs []*linter.Config
 	for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() {
@@ -94,3 +67,30 @@ func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) {
 		fmt.Fprintf(logutils.StdOut, "%s: %s\n", color.YellowString(p), strings.Join(linterNames, ", "))
 	}
 }
+
+func printLinterConfigs(lcs []*linter.Config) {
+	sort.Slice(lcs, func(i, j int) bool {
+		return lcs[i].Name() < lcs[j].Name()
+	})
+	for _, lc := range lcs {
+		altNamesStr := ""
+		if len(lc.AlternativeNames) != 0 {
+			altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", "))
+		}
+
+		// If the linter description spans multiple lines, truncate everything following the first newline
+		linterDescription := lc.Linter.Desc()
+		firstNewline := strings.IndexRune(linterDescription, '\n')
+		if firstNewline > 0 {
+			linterDescription = linterDescription[:firstNewline]
+		}
+
+		deprecatedMark := ""
+		if lc.IsDeprecated() {
+			deprecatedMark = " [" + color.RedString("deprecated") + "]"
+		}
+
+		fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", color.YellowString(lc.Name()),
+			altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix)
+	}
+}