Skip to content

Commit

Permalink
Fix plugin execution for Windows
Browse files Browse the repository at this point in the history
Fixes #737

* Use exec.Command instend of syscall.Exec for windows.
* Fix a bug in plugin handler test when running it on windows.
* Fix typo.
  • Loading branch information
MIBc committed Mar 20, 2020
1 parent 062aeb9 commit bc8d398
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 7 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
| Fix trigger create --filter flag to be optional
| https://github.com/knative/client/pull/745[#745]

| 🐛
| Fix plugin execution for Windows.
| https://github.com/knative/client/pull/738[#738]

## v0.13.0 (2020-03-11)

[cols="1,10,3", options="header", width="100%"]
Expand Down
2 changes: 1 addition & 1 deletion docs/plugins/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Plugins follow a similar architecture to
with some small differences. One key difference is that `kn` plugins can either
live in your `PATH` or in a chosen and specified directory.
[Kn plugins](https://github.com/knative/client/tree/master/docs/cmd/kn_plugin.md)
shows how to install and create new plugins as well as gives some examples and
show how to install and create new plugins as well as gives some examples and
best practices.

To see what plugins are installed on your machine, you can use the
Expand Down
13 changes: 13 additions & 0 deletions pkg/kn/commands/plugin/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"syscall"

Expand Down Expand Up @@ -93,6 +94,18 @@ func (h *DefaultPluginHandler) Lookup(name string) (string, bool) {

// Execute implements PluginHandler
func (h *DefaultPluginHandler) Execute(executablePath string, cmdArgs, environment []string) error {
if runtime.GOOS == "windows" {
cmd := exec.Command(executablePath, cmdArgs...)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.Stdin = os.Stdin
cmd.Env = environment
err := cmd.Run()
if err == nil {
os.Exit(0)
}
return err
}
return syscall.Exec(executablePath, cmdArgs, environment)
}

Expand Down
12 changes: 12 additions & 0 deletions pkg/kn/commands/plugin/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"testing"

"gotest.tools/assert"
Expand All @@ -45,6 +46,9 @@ func TestPluginHandler(t *testing.T) {

beforeEach := func(t *testing.T) {
pluginName = "fake"
if runtime.GOOS == "windows" {
pluginName += ".bat"
}
pluginPath = CreateTestPluginInPath(t, "kn-"+pluginName, KnTestPluginScript, FileModeExecutable, tmpPathDir)
assert.Assert(t, pluginPath != "")

Expand Down Expand Up @@ -134,6 +138,14 @@ func TestPluginHandler(t *testing.T) {
err = pluginHandler.Execute(bogusPath, []string{bogusPath}, os.Environ())
assert.Assert(t, err != nil, fmt.Sprintf("bogus plugin in path %s unexpectedly executed OK", bogusPath))
})
t.Run("executing fake plugin successfully", func(t *testing.T) {
setup(t)
defer cleanup(t)
beforeEach(t)

err = pluginHandler.Execute(pluginPath, []string{}, os.Environ())
assert.Assert(t, err == nil, fmt.Sprintf("fail to execute fake plugin in path %s ", pluginPath))
})
})

t.Run("HandlePluginCommand", func(t *testing.T) {
Expand Down
7 changes: 1 addition & 6 deletions pkg/kn/commands/plugin/plugin_test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"testing"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -58,13 +57,9 @@ func CreateTestPlugin(t *testing.T, name, script string, fileMode os.FileMode) s
// CreateTestPluginInPath with name, path, script, and fileMode and return the tmp random path
func CreateTestPluginInPath(t *testing.T, name, script string, fileMode os.FileMode, path string) string {
fullPath := filepath.Join(path, name)
if runtime.GOOS == "windows" {
fullPath += ".bat"
}
err := ioutil.WriteFile(fullPath, []byte(script), fileMode)
assert.NilError(t, err)

return filepath.Join(path, name)
return fullPath
}

// DeleteTestPlugin with path
Expand Down

0 comments on commit bc8d398

Please sign in to comment.