Skip to content

Commit

Permalink
fix: properly replace PATH instead of appending a new PATH (#626)
Browse files Browse the repository at this point in the history
Issue #, if available:

*Description of changes:*
  - Fix env setting for limactl process on Windows

*Testing done:*
- Tested with new finch-core PR
runfinch/finch-core#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 <[email protected]>
Signed-off-by: Vishwas Siravara <[email protected]>
  • Loading branch information
pendo324 authored and vsiravar committed Oct 17, 2023
1 parent d1b7513 commit 5389714
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 13 deletions.
2 changes: 1 addition & 1 deletion deps/finch-core
Submodule finch-core updated 3 files
+1 −0 .gitignore
+95 −22 Makefile
+18 −0 verify_hash.ps1
49 changes: 39 additions & 10 deletions pkg/command/lima.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,19 @@ import (
"bytes"
"fmt"
"io"
"runtime"
"strings"

"golang.org/x/exp/slices"

"github.com/runfinch/finch/pkg/flog"
"github.com/runfinch/finch/pkg/system"
)

const (
envKeyLimaHome = "LIMA_HOME"
envKeyPath = "PATH"
envKeyUnixPath = "PATH"
envKeyWinPath = "Path"
)

// LimaCmdCreator creates a limactl command.
Expand Down Expand Up @@ -50,7 +55,7 @@ type limaCmdCreator struct {
systemDeps LimaCmdCreatorSystemDeps
limaHomePath string
limactlPath string
qemuBinPath string
binPath string
}

var _ LimaCmdCreator = (*limaCmdCreator)(nil)
Expand All @@ -70,15 +75,15 @@ 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{
cmdCreator: cmdCreator,
logger: logger,
limaHomePath: limaHomePath,
limactlPath: limactlPath,
qemuBinPath: qemuBinPath,
binPath: binPath,
systemDeps: systemDeps,
}
}
Expand Down Expand Up @@ -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)
}
2 changes: 0 additions & 2 deletions pkg/command/lima_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ const (
envKeyLimaHome = "LIMA_HOME"
mockQemuBinPath = "/lima/bin"
mockSystemPath = "/usr/bin"
envKeyPath = "PATH"
finalPath = mockQemuBinPath + ":" + mockSystemPath
)

var mockArgs = []string{"shell", "finch"}
Expand Down
12 changes: 12 additions & 0 deletions pkg/command/lima_unix_test.go
Original file line number Diff line number Diff line change
@@ -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
)
12 changes: 12 additions & 0 deletions pkg/command/lima_windows_test.go
Original file line number Diff line number Diff line change
@@ -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
)

0 comments on commit 5389714

Please sign in to comment.