From d540cf24fa1893c88c632c845221aec952df94a4 Mon Sep 17 00:00:00 2001 From: Justin Date: Thu, 12 Oct 2023 17:26:47 -0400 Subject: [PATCH] fix: properly replace PATH instead of appending a new PATH (#626) Issue #, if available: *Description of changes:* - Fix env setting for limactl process on Windows *Testing done:* - Tested with new finch-core PR https://github.com/runfinch/finch-core/pull/187 - [x] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: Justin Alvarez --- deps/finch-core | 2 +- pkg/command/lima.go | 49 +++++++++++++++++++++++++------- pkg/command/lima_test.go | 2 -- pkg/command/lima_unix_test.go | 12 ++++++++ pkg/command/lima_windows_test.go | 12 ++++++++ 5 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 pkg/command/lima_unix_test.go create mode 100644 pkg/command/lima_windows_test.go diff --git a/deps/finch-core b/deps/finch-core index 033a53688..e43112ac8 160000 --- a/deps/finch-core +++ b/deps/finch-core @@ -1 +1 @@ -Subproject commit 033a53688c2acf7be855b90c1dbd3596d747368a +Subproject commit e43112ac87a55351e819fca1a3d3f15cfbd0a279 diff --git a/pkg/command/lima.go b/pkg/command/lima.go index 13f49d5f7..338fbd2de 100644 --- a/pkg/command/lima.go +++ b/pkg/command/lima.go @@ -7,6 +7,10 @@ import ( "bytes" "fmt" "io" + "runtime" + "strings" + + "golang.org/x/exp/slices" "github.com/runfinch/finch/pkg/flog" "github.com/runfinch/finch/pkg/system" @@ -14,7 +18,8 @@ import ( const ( envKeyLimaHome = "LIMA_HOME" - envKeyPath = "PATH" + envKeyUnixPath = "PATH" + envKeyWinPath = "Path" ) // LimaCmdCreator creates a limactl command. @@ -50,7 +55,7 @@ type limaCmdCreator struct { systemDeps LimaCmdCreatorSystemDeps limaHomePath string limactlPath string - qemuBinPath string + binPath string } var _ LimaCmdCreator = (*limaCmdCreator)(nil) @@ -70,7 +75,7 @@ type LimaCmdCreatorSystemDeps interface { func NewLimaCmdCreator( cmdCreator Creator, logger flog.Logger, - limaHomePath, limactlPath string, qemuBinPath string, + limaHomePath, limactlPath string, binPath string, systemDeps LimaCmdCreatorSystemDeps, ) LimaCmdCreator { return &limaCmdCreator{ @@ -78,7 +83,7 @@ func NewLimaCmdCreator( logger: logger, limaHomePath: limaHomePath, limactlPath: limactlPath, - qemuBinPath: qemuBinPath, + binPath: binPath, systemDeps: systemDeps, } } @@ -122,14 +127,38 @@ func (lcc *limaCmdCreator) create(stdin io.Reader, stdout, stderr io.Writer, arg lcc.logger.Debugf("Creating limactl command: ARGUMENTS: %v, %s: %s", args, envKeyLimaHome, lcc.limaHomePath) cmd := lcc.cmdCreator.Create(lcc.limactlPath, args...) limaHomeEnv := fmt.Sprintf("%s=%s", envKeyLimaHome, lcc.limaHomePath) - path := lcc.systemDeps.Env(envKeyPath) - // TODO: Refactor this; don't need qemuBinPath for windows - path = fmt.Sprintf("%s:%s", lcc.qemuBinPath, path) - pathEnv := fmt.Sprintf("%s=%s", envKeyPath, path) - env := append(lcc.systemDeps.Environ(), limaHomeEnv, pathEnv) - cmd.SetEnv(env) + var pathEnv string + var envKeyPath string + var path string + if runtime.GOOS == "windows" { + envKeyPath = envKeyWinPath + path = lcc.systemDeps.Env(envKeyPath) + path = fmt.Sprintf(`%s\;%s`, lcc.binPath, path) + pathEnv = fmt.Sprintf("%s=%s", envKeyPath, path) + } else { + envKeyPath = envKeyUnixPath + path = lcc.systemDeps.Env(envKeyPath) + path = fmt.Sprintf("%s:%s", lcc.binPath, path) + pathEnv = fmt.Sprintf("%s=%s", envKeyPath, path) + } + + newPathEnv := replaceOrAppend(lcc.systemDeps.Environ(), envKeyLimaHome, limaHomeEnv) + newPathEnv = replaceOrAppend(newPathEnv, envKeyPath, pathEnv) + + cmd.SetEnv(newPathEnv) cmd.SetStdin(stdin) cmd.SetStdout(stdout) cmd.SetStderr(stderr) return cmd } + +func replaceOrAppend(orig []string, varName, newVar string) []string { + envIdx := slices.IndexFunc(orig, func(envVar string) bool { + return strings.HasPrefix(envVar, fmt.Sprintf("%s=", varName)) + }) + + if envIdx != -1 { + return slices.Replace(orig, envIdx, envIdx+1, newVar) + } + return append(orig, newVar) +} diff --git a/pkg/command/lima_test.go b/pkg/command/lima_test.go index 5b1b14488..2dbcda6f4 100644 --- a/pkg/command/lima_test.go +++ b/pkg/command/lima_test.go @@ -27,8 +27,6 @@ const ( envKeyLimaHome = "LIMA_HOME" mockQemuBinPath = "/lima/bin" mockSystemPath = "/usr/bin" - envKeyPath = "PATH" - finalPath = mockQemuBinPath + ":" + mockSystemPath ) var mockArgs = []string{"shell", "finch"} diff --git a/pkg/command/lima_unix_test.go b/pkg/command/lima_unix_test.go new file mode 100644 index 000000000..d1b704313 --- /dev/null +++ b/pkg/command/lima_unix_test.go @@ -0,0 +1,12 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build !windows +// +build !windows + +package command_test + +const ( + envKeyPath = "PATH" + finalPath = mockQemuBinPath + ":" + mockSystemPath +) diff --git a/pkg/command/lima_windows_test.go b/pkg/command/lima_windows_test.go new file mode 100644 index 000000000..71e978ef8 --- /dev/null +++ b/pkg/command/lima_windows_test.go @@ -0,0 +1,12 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build windows +// +build windows + +package command_test + +const ( + envKeyPath = "Path" + finalPath = mockQemuBinPath + `\;` + mockSystemPath +)