Skip to content

Commit

Permalink
Use build-specific settings for effect across windows and unix
Browse files Browse the repository at this point in the history
- On Windows, we use the standard os/exec functionality. As Windows doesn't really have the concept of a PTY.
- On Unix, we use a PTY. This allows commands started through effect to appear like a user is actually running them. In many cases, this doesn't matter, but some commands will change their output if there is a user or pipe. The PTY fakes the spawned process into thinking it's a user. This will generally cause the spawned process to emite human-friendly output.
- This change is due to the fact that recent Windows builds of paketo-buildpacks/procfile have been failing. This traces back to creack/pty#50. It's not clear why this is happening now, since the issue is a few years old. With recent changes in libpak, we're now affected by this.
- Initial tests with paketo-buildpacks/procfile show that making the PTY stuff conditional on the OS, allows this to work as expected.

Signed-off-by: Daniel Mikusa <[email protected]>
  • Loading branch information
Daniel Mikusa committed Nov 22, 2021
1 parent 6b070f4 commit d0f38ec
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 56 deletions.
56 changes: 0 additions & 56 deletions effect/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,8 @@
package effect

import (
"fmt"
"io"
"os"
"os/exec"
"syscall"

"github.com/creack/pty"
)

// Execution is information about a command to run.
Expand Down Expand Up @@ -80,54 +75,3 @@ func (CommandExecutor) Execute(execution Execution) error {

return cmd.Run()
}

// TTYExecutor is an implementation of Executor that uses exec.Command and runs the command with a TTY.
type TTYExecutor struct{}

func (t TTYExecutor) Execute(execution Execution) error {
cmd := exec.Command(execution.Command, execution.Args...)

if execution.Dir != "" {
cmd.Dir = execution.Dir
}

if len(execution.Env) > 0 {
cmd.Env = execution.Env
}

cmd.Stdin = execution.Stdin

f, err := pty.Start(cmd)
if err != nil {
return fmt.Errorf("unable to start PTY\n%w", err)
}
defer f.Close()

if _, err := io.Copy(execution.Stdout, f); err != nil {
if !t.isEIO(err) {
return fmt.Errorf("unable to write output\n%w", err)
}
}

return cmd.Wait()
}

func (TTYExecutor) isEIO(err error) bool {
pe, ok := err.(*os.PathError)
if !ok {
return false
}

return pe.Err == syscall.EIO
}

// NewExecutor creates a new Executor. If the buildpack is currently running in a TTY, returns a TTY-aware Executor.
func NewExecutor() Executor {
// TODO: Remove once TTY support is in place
return TTYExecutor{}
// if isatty.IsTerminal(os.Stdout.Fd()) {
// return TTYExecutor{}
// } else {
// return CommandExecutor{}
// }
}
81 changes: 81 additions & 0 deletions effect/executor_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//go:build aix && darwin && dragonfly && freebsd && linux && netbsd && openbsd && solaris
// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris

/*
* Copyright 2018-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package effect

import (
"fmt"
"io"
"os"
"os/exec"
"syscall"

"github.com/creack/pty"
)

// TTYExecutor is an implementation of Executor that uses exec.Command and runs the command with a TTY.
type TTYExecutor struct{}

func (t TTYExecutor) Execute(execution Execution) error {
cmd := exec.Command(execution.Command, execution.Args...)

if execution.Dir != "" {
cmd.Dir = execution.Dir
}

if len(execution.Env) > 0 {
cmd.Env = execution.Env
}

cmd.Stdin = execution.Stdin

f, err := pty.Start(cmd)
if err != nil {
return fmt.Errorf("unable to start PTY\n%w", err)
}
defer f.Close()

if _, err := io.Copy(execution.Stdout, f); err != nil {
if !t.isEIO(err) {
return fmt.Errorf("unable to write output\n%w", err)
}
}

return cmd.Wait()
}

func (TTYExecutor) isEIO(err error) bool {
pe, ok := err.(*os.PathError)
if !ok {
return false
}

return pe.Err == syscall.EIO
}

// NewExecutor creates a new Executor. If the buildpack is currently running in a TTY, returns a TTY-aware Executor.
func NewExecutor() Executor {
// TODO: Remove once TTY support is in place
return TTYExecutor{}
// if isatty.IsTerminal(os.Stdout.Fd()) {
// return TTYExecutor{}
// } else {
// return CommandExecutor{}
// }
}
25 changes: 25 additions & 0 deletions effect/executor_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//go:build windows
// +build windows

/*
* Copyright 2018-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package effect

// NewExecutor creates a new Executor.
func NewExecutor() Executor {
return CommandExecutor{}
}

0 comments on commit d0f38ec

Please sign in to comment.