Refactor legacy CLI invocation

- Move 'init' responsibility to the legacy package
- Remove unnecessary Debug on wrapper
- Remove --phar-path option, but preserve {ENV_PREFIX}PHAR_PATH variable
- Remove unnecessary global vars
- Add a test

commands/completion.go

@@ -2,18 +2,13 @@ package commands
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
-	"os"
-	"os/exec"
-	"path"
+	"path/filepath"
 	"strings"
 
 	"github.com/spf13/cobra"
-	"github.com/spf13/viper"
 
 	"github.com/platformsh/cli/internal/config"
-	"github.com/platformsh/cli/internal/legacy"
 )
 
 func newCompletionCommand(cnf *config.Config) *cobra.Command {
@@ -28,42 +23,21 @@ func newCompletionCommand(cnf *config.Config) *cobra.Command {
 				completionArgs = append(completionArgs, "--shell-type", args[0])
 			}
 			var b bytes.Buffer
-			c := &legacy.CLIWrapper{
-				Config:             cnf,
-				Version:            version,
-				CustomPharPath:     viper.GetString("phar-path"),
-				Debug:              viper.GetBool("debug"),
-				DebugLogFunc:       debugLog,
-				DisableInteraction: viper.GetBool("no-interaction"),
-				Stdout:             &b,
-				Stderr:             cmd.ErrOrStderr(),
-				Stdin:              cmd.InOrStdin(),
-			}
-
-			if err := c.Init(); err != nil {
-				debugLog(err.Error())
-				os.Exit(1)
-				return
-			}
+			c := makeLegacyCLIWrapper(cnf, &b, cmd.ErrOrStderr(), cmd.InOrStdin())
 
 			if err := c.Exec(cmd.Context(), completionArgs...); err != nil {
-				debugLog(err.Error())
-				exitCode := 1
-				var execErr *exec.ExitError
-				if errors.As(err, &execErr) {
-					exitCode = execErr.ExitCode()
-				}
-				os.Exit(exitCode)
-				return
+				exitWithError(err)
 			}
 
+			pharPath := c.PharPath()
+
 			completions := strings.ReplaceAll(
 				strings.ReplaceAll(
 					b.String(),
-					c.PharPath(),
+					pharPath,
 					cnf.Application.Executable,
 				),
-				path.Base(c.PharPath()),
+				filepath.Base(pharPath),
 				cnf.Application.Executable,
 			)
 			fmt.Fprintln(cmd.OutOrStdout(), "#compdef "+cnf.Application.Executable)

commands/list.go

@@ -9,7 +9,6 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/platformsh/cli/internal/config"
-	"github.com/platformsh/cli/internal/legacy"
 )
 
 func newListCommand(cnf *config.Config) *cobra.Command {
@@ -18,39 +17,24 @@ func newListCommand(cnf *config.Config) *cobra.Command {
 		Short: "Lists commands",
 		Args:  cobra.MaximumNArgs(1),
 		Run: func(cmd *cobra.Command, args []string) {
-			var b bytes.Buffer
-			c := &legacy.CLIWrapper{
-				Config:             cnf,
-				Version:            version,
-				CustomPharPath:     viper.GetString("phar-path"),
-				Debug:              viper.GetBool("debug"),
-				DebugLogFunc:       debugLog,
-				DisableInteraction: viper.GetBool("no-interaction"),
-				Stdout:             &b,
-				Stderr:             cmd.ErrOrStderr(),
-				Stdin:              cmd.InOrStdin(),
-			}
-			if err := c.Init(); err != nil {
-				exitWithError(cmd, err)
-				return
-			}
-
 			arguments := []string{"list", "--format=json"}
 			if viper.GetBool("all") {
 				arguments = append(arguments, "--all")
 			}
 			if len(args) > 0 {
 				arguments = append(arguments, args[0])
 			}
+
+			var b bytes.Buffer
+			c := makeLegacyCLIWrapper(cnf, &b, cmd.ErrOrStderr(), cmd.InOrStdin())
+
 			if err := c.Exec(cmd.Context(), arguments...); err != nil {
-				exitWithError(cmd, err)
-				return
+				exitWithError(err)
 			}
 
 			var list List
 			if err := json.Unmarshal(b.Bytes(), &list); err != nil {
-				exitWithError(cmd, err)
-				return
+				exitWithError(err)
 			}
 
 			// Override the application name and executable with our own config.
@@ -88,15 +72,14 @@ func newListCommand(cnf *config.Config) *cobra.Command {
 				c.Stdout = cmd.OutOrStdout()
 				arguments := []string{"list", "--format=" + format}
 				if err := c.Exec(cmd.Context(), arguments...); err != nil {
-					exitWithError(cmd, err)
+					exitWithError(err)
 				}
 				return
 			}
 
 			result, err := formatter.Format(&list, config.FromContext(cmd.Context()))
 			if err != nil {
-				exitWithError(cmd, err)
-				return
+				exitWithError(err)
 			}
 
 			fmt.Fprintln(cmd.OutOrStdout(), string(result))

commands/root.go

@@ -76,7 +76,10 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
 			}
 		},
 		Run: func(cmd *cobra.Command, _ []string) {
-			runLegacyCLI(cmd.Context(), cnf, cmd.OutOrStdout(), cmd.ErrOrStderr(), cmd.InOrStdin(), os.Args[1:])
+			c := makeLegacyCLIWrapper(cnf, cmd.OutOrStdout(), cmd.ErrOrStderr(), cmd.InOrStdin())
+			if err := c.Exec(cmd.Context(), os.Args[1:]...); err != nil {
+				exitWithError(err)
+			}
 		},
 		PersistentPostRun: func(cmd *cobra.Command, _ []string) {
 			checkShellConfigLeftovers(cmd.ErrOrStderr(), cnf)
@@ -103,13 +106,13 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
 			args = []string{"help"}
 		}
 
-		runLegacyCLI(cmd.Context(), cnf, cmd.OutOrStdout(), cmd.ErrOrStderr(), cmd.InOrStdin(), args)
+		c := makeLegacyCLIWrapper(cnf, cmd.OutOrStdout(), cmd.ErrOrStderr(), cmd.InOrStdin())
+		if err := c.Exec(cmd.Context(), args...); err != nil {
+			exitWithError(err)
+		}
 	})
 
 	cmd.PersistentFlags().BoolP("version", "V", false, fmt.Sprintf("Displays the %s version", cnf.Application.Name))
-	cmd.PersistentFlags().String("phar-path", "",
-		fmt.Sprintf("Uses a local .phar file for the Legacy %s", cnf.Application.Name),
-	)
 	cmd.PersistentFlags().Bool("debug", false, "Enable debug logging")
 	cmd.PersistentFlags().Bool("no-interaction", false, "Enable non-interactive mode")
 	cmd.PersistentFlags().BoolP("verbose", "v", false, "Enable verbose output")
@@ -239,40 +242,27 @@ func debugLog(format string, v ...any) {
 	fmt.Fprintf(color.Error, prefix+" "+strings.TrimSpace(format)+"\n", v...)
 }
 
-func exitWithError(cmd *cobra.Command, err error) {
-	cmd.PrintErrln(color.RedString(err.Error()))
-	exitCode := 1
+func exitWithError(err error) {
 	var execErr *exec.ExitError
 	if errors.As(err, &execErr) {
-		exitCode = execErr.ExitCode()
+		exitCode := execErr.ExitCode()
+		debugLog(err.Error())
+		os.Exit(exitCode)
+	}
+	if !viper.GetBool("quiet") {
+		fmt.Fprintln(color.Error, color.RedString(err.Error()))
 	}
-	os.Exit(exitCode)
+	os.Exit(1)
 }
 
-func runLegacyCLI(ctx context.Context, cnf *config.Config, stdout, stderr io.Writer, stdin io.Reader, args []string) {
-	c := &legacy.CLIWrapper{
+func makeLegacyCLIWrapper(cnf *config.Config, stdout, stderr io.Writer, stdin io.Reader) *legacy.CLIWrapper {
+	return &legacy.CLIWrapper{
 		Config:             cnf,
 		Version:            version,
-		CustomPharPath:     viper.GetString("phar-path"),
-		Debug:              viper.GetBool("debug"),
 		DebugLogFunc:       debugLog,
 		DisableInteraction: viper.GetBool("no-interaction"),
 		Stdout:             stdout,
 		Stderr:             stderr,
 		Stdin:              stdin,
 	}
-	if err := c.Init(); err != nil {
-		fmt.Fprintln(stderr, color.RedString(err.Error()))
-		os.Exit(1)
-	}
-
-	if err := c.Exec(ctx, args...); err != nil {
-		debugLog("%s\n", color.RedString(err.Error()))
-		exitCode := 1
-		var execErr *exec.ExitError
-		if errors.As(err, &execErr) {
-			exitCode = execErr.ExitCode()
-		}
-		os.Exit(exitCode)
-	}
 }

internal/config/config.go

@@ -33,6 +33,7 @@ func FromYAML(b []byte) (*Config, error) {
 		return nil, fmt.Errorf("invalid config: %w", err)
 	}
 	c.applyDynamicDefaults()
+	c.raw = b
 	return c, nil
 }
 

internal/config/schema.go

@@ -1,8 +1,11 @@
 package config
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
+
+	"gopkg.in/yaml.v3"
 )
 
 // Config provides YAML configuration for the CLI.
@@ -60,6 +63,8 @@ type Config struct {
 	SSH struct {
 		DomainWildcards []string `validate:"required" yaml:"domain_wildcards"` // e.g. ["*.platform.sh"]
 	} `validate:"required"`
+
+	raw []byte `yaml:"-"`
 }
 
 // applyDefaults applies defaults to config before parsing.
@@ -99,3 +104,15 @@ func (c *Config) WritableUserDir() (string, error) {
 
 	return path, nil
 }
+
+// Raw returns the config before it was unmarshalled, or a marshaled version if that is not available.
+func (c *Config) Raw() ([]byte, error) {
+	if len(c.raw) == 0 {
+		b, err := yaml.Marshal(c)
+		if err != nil {
+			return nil, fmt.Errorf("could not load raw config: %w", err)
+		}
+		c.raw = b
+	}
+	return c.raw, nil
+}

internal/legacy/legacy.go

@@ -9,6 +9,8 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"sync"
+	"time"
 
 	"github.com/gofrs/flock"
 
@@ -23,9 +25,6 @@ var (
 	PHPVersion       = "0.0.0"
 )
 
-var phpPath = fmt.Sprintf("php-%s", PHPVersion)
-var pharPath = fmt.Sprintf("phar-%s", LegacyCLIVersion)
-
 // copyFile from the given bytes to destination
 func copyFile(destination string, fin []byte) error {
 	if _, err := os.Stat(destination); err != nil && !os.IsNotExist(err) {
@@ -73,10 +72,11 @@ type CLIWrapper struct {
 	Stdin              io.Reader
 	Config             *config.Config
 	Version            string
-	CustomPharPath     string
 	Debug              bool
 	DisableInteraction bool
 	DebugLogFunc       func(string, ...any)
+
+	initOnce sync.Once
 }
 
 func (c *CLIWrapper) debug(msg string, args ...any) {
@@ -89,37 +89,43 @@ func (c *CLIWrapper) cacheDir() string {
 	return path.Join(os.TempDir(), fmt.Sprintf("%s-%s-%s", c.Config.Application.Slug, PHPVersion, LegacyCLIVersion))
 }
 
-// Init the CLI wrapper, creating a temporary directory and copying over files
-func (c *CLIWrapper) Init() error {
+// runInitOnce runs the init method, only once for this object.
+func (c *CLIWrapper) runInitOnce() error {
+	var err error
+	c.initOnce.Do(func() { err = c.init() })
+	return err
+}
+
+// init initializes the CLI wrapper, creating a temporary directory and copying over files.
+func (c *CLIWrapper) init() error {
+	preInit := time.Now()
+
 	if _, err := os.Stat(c.cacheDir()); os.IsNotExist(err) {
 		c.debug("Cache directory does not exist, creating: %s", c.cacheDir())
 		if err := os.Mkdir(c.cacheDir(), 0o700); err != nil {
 			return fmt.Errorf("could not create temporary directory: %w", err)
 		}
 	}
+	preLock := time.Now()
 	fileLock := flock.New(path.Join(c.cacheDir(), ".lock"))
 	if err := fileLock.Lock(); err != nil {
 		return fmt.Errorf("could not acquire lock: %w", err)
 	}
-	c.debug("Lock acquired: %s", fileLock.Path())
+	c.debug("Lock acquired (%s): %s", time.Since(preLock), fileLock.Path())
 	//nolint:errcheck
 	defer fileLock.Unlock()
 
 	if _, err := os.Stat(c.PharPath()); os.IsNotExist(err) {
-		if c.CustomPharPath != "" {
-			return fmt.Errorf("legacy CLI phar file not found: %w", err)
-		}
-
 		c.debug("Phar file does not exist, copying: %s", c.PharPath())
 		if err := copyFile(c.PharPath(), phar); err != nil {
 			return fmt.Errorf("could not copy phar file: %w", err)
 		}
 	}
 
 	// Always write the config.yaml file if it changed.
-	configContent, err := config.LoadYAML()
+	configContent, err := c.Config.Raw()
 	if err != nil {
-		return fmt.Errorf("could not load config for checking: %w", err)
+		return err
 	}
 	changed, err := fileChanged(c.ConfigPath(), configContent)
 	if err != nil {
@@ -141,11 +147,17 @@ func (c *CLIWrapper) Init() error {
 		}
 	}
 
+	c.debug("Initialized PHP CLI (%s)", time.Since(preInit))
+
 	return nil
 }
 
 // Exec a legacy CLI command with the given arguments
 func (c *CLIWrapper) Exec(ctx context.Context, args ...string) error {
+	if err := c.runInitOnce(); err != nil {
+		return fmt.Errorf("failed to initialize PHP CLI: %w", err)
+	}
+
 	cmd := c.makeCmd(ctx, args)
 	if c.Stdin != nil {
 		cmd.Stdin = c.Stdin
@@ -173,9 +185,6 @@ func (c *CLIWrapper) Exec(ctx context.Context, args ...string) error {
 		envPrefix+"WRAPPED=1",
 		envPrefix+"APPLICATION_VERSION="+c.Version,
 	)
-	if c.Debug {
-		cmd.Env = append(cmd.Env, envPrefix+"CLI_DEBUG=1")
-	}
 	if c.DisableInteraction {
 		cmd.Env = append(cmd.Env, envPrefix+"NO_INTERACTION=1")
 	}
@@ -210,11 +219,11 @@ func (c *CLIWrapper) makeCmd(ctx context.Context, args []string) *exec.Cmd {
 
 // PharPath returns the path to the legacy CLI's Phar file.
 func (c *CLIWrapper) PharPath() string {
-	if c.CustomPharPath != "" {
-		return c.CustomPharPath
+	if customPath := os.Getenv(c.Config.Application.EnvPrefix + "PHAR_PATH"); customPath != "" {
+		return customPath
 	}
 
-	return path.Join(c.cacheDir(), pharPath)
+	return path.Join(c.cacheDir(), c.Config.Application.Executable+".phar")
 }
 
 // ConfigPath returns the path to the YAML config file that will be provided to the legacy CLI.