Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bring back error coloring #5718

Merged
merged 9 commits into from
May 4, 2021
7 changes: 3 additions & 4 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const (
HouseKeepingMessagesAllowedAnnotation = "skaffold_annotation_housekeeping_allowed"
)

func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command {
updateMsg := make(chan string, 1)
surveyPrompt := make(chan bool, 1)
metricsPrompt := make(chan bool, 1)
Expand All @@ -75,8 +75,7 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {

opts.Command = cmd.Use
instrumentation.SetCommand(cmd.Use)

out = color.SetupColors(out, defaultColor, forceColors)
out := color.SetupColors(out, defaultColor, forceColors)
if timestamps {
l := logrus.New()
l.SetOutput(out)
Expand All @@ -88,7 +87,7 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
cmd.Root().SetOutput(out)

// Setup logs
if err := setUpLogs(err, v, timestamps); err != nil {
if err := setUpLogs(errOut, v, timestamps); err != nil {
return err
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/skaffold/skaffold.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ func main() {
if errors.Is(err, context.Canceled) {
logrus.Debugln("ignore error since context is cancelled:", err)
} else {
color.Red.Fprintln(os.Stderr, err)
errOut := color.SetupColors(os.Stderr, color.DefaultColorCode, false)
color.Red.Fprintln(errOut, err)
code = exitCode(err)
}
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/skaffold/color/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ func init() {
// SetupColors conditionally wraps the input `Writer` with a color enabled `Writer`.
func SetupColors(out io.Writer, defaultColor int, forceColors bool) io.Writer {
_, isTerm := util.IsTerminal(out)
useColors := isTerm || forceColors
supportsColor, _ := util.SupportsColor()

useColors := (isTerm && supportsColor) || forceColors
if useColors {
// Use EnableColorsStdout to enable use of color on Windows
useColors = false // value is updated if color-enablement is successful
Expand Down
2 changes: 2 additions & 0 deletions pkg/skaffold/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ const (
LeeroyAppResponse = "leeroooooy app!!\n"

GithubIssueLink = "https://github.com/GoogleContainerTools/skaffold/issues/new"

Windows = "windows"
)

type Phase string
Expand Down
4 changes: 3 additions & 1 deletion pkg/skaffold/util/tar.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"time"

"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
)

type headerModifier func(*tar.Header)
Expand Down Expand Up @@ -145,7 +147,7 @@ func addFileToTar(root string, src string, dst string, tw *tar.Writer, hm header
}

// Code copied from https://github.com/moby/moby/blob/master/pkg/archive/archive_windows.go
if runtime.GOOS == "windows" {
if runtime.GOOS == constants.Windows {
header.Mode = int64(chmodTarEntry(os.FileMode(header.Mode)))
}
if hm != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/util/tar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"testing"
"time"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand Down Expand Up @@ -234,7 +235,7 @@ func TestCreateTarWithAbsolutePaths(t *testing.T) {
}

func TestAddFileToTarSymlinks(t *testing.T) {
if runtime.GOOS == "windows" {
if runtime.GOOS == constants.Windows {
t.Skip("creating symlinks requires extra privileges on Windows")
}

Expand Down
31 changes: 31 additions & 0 deletions pkg/skaffold/util/term.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,20 @@ limitations under the License.
package util

import (
"fmt"
"io"
"os/exec"
"runtime"
"strconv"
"strings"

"golang.org/x/term"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
)

var colors = tputColors

func IsTerminal(w io.Writer) (uintptr, bool) {
type descriptor interface {
Fd() uintptr
Expand All @@ -35,3 +44,25 @@ func IsTerminal(w io.Writer) (uintptr, bool) {

return 0, false
}

func SupportsColor() (bool, error) {
if runtime.GOOS == constants.Windows {
return true, nil
}

res, err := colors()
if err != nil {
return false, fmt.Errorf("checking terminal colors: %w", err)
}

numColors, err := strconv.Atoi(strings.TrimSpace(string(res)))
if err != nil {
return false, err
}

return numColors != 0, nil
}

func tputColors() ([]byte, error) {
return exec.Command("tput", "colors").Output()
}
48 changes: 48 additions & 0 deletions pkg/skaffold/util/term_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ package util

import (
"bytes"
"errors"
"runtime"
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand All @@ -31,3 +34,48 @@ func TestIsNotTerminal(t *testing.T) {
testutil.CheckDeepEqual(t, uintptr(0x00), termFd)
testutil.CheckDeepEqual(t, false, isTerm)
}

func TestSupportsColor(t *testing.T) {
tests := []struct {
description string
colorsOutput []byte
shouldErr bool
expected bool
}{
{
description: "Supports 256 colors",
colorsOutput: []byte("256"),
expected: true,
},
{
description: "Supports 0 colors",
colorsOutput: []byte("0"),
expected: false,
},
{
description: "Errors",
colorsOutput: []byte("-1"),
shouldErr: true,
expected: false,
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&colors, func() ([]byte, error) {
if test.shouldErr {
return nil, errors.New("error")
}

return test.colorsOutput, nil
})
if runtime.GOOS == constants.Windows {
test.expected = true
test.shouldErr = false
}

supportsColors, err := SupportsColor()
t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, supportsColors)
})
}
}