Merge 1c5d2b6 into 40e2f98

cmd/cmd.go

@@ -11,6 +11,7 @@ import (
 	"github.com/buildpacks/pack/internal/commands"
 	"github.com/buildpacks/pack/internal/config"
 	imagewriter "github.com/buildpacks/pack/internal/inspectimage/writer"
+	ilogging "github.com/buildpacks/pack/internal/logging"
 	"github.com/buildpacks/pack/logging"
 )
 
@@ -41,9 +42,15 @@ func NewPackCommand(logger ConfigurableLogger) (*cobra.Command, error) {
 		Short: "CLI for building apps using Cloud Native Buildpacks",
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			if fs := cmd.Flags(); fs != nil {
-				if flag, err := fs.GetBool("no-color"); err == nil {
+				if flag, err := fs.GetBool("no-color"); err == nil && flag {
 					color.Disable(flag)
 				}
+
+				_, canDisplayColor := ilogging.IsTerminal(logging.GetWriterForLevel(logger, logging.InfoLevel))
+				if !canDisplayColor {
+					color.Disable(true)
+				}
+
 				if flag, err := fs.GetBool("quiet"); err == nil {
 					logger.WantQuiet(flag)
 				}

internal/image/fetcher.go

@@ -7,8 +7,6 @@ import (
 	"io"
 	"strings"
 
-	"github.com/buildpacks/pack/config"
-
 	"github.com/buildpacks/imgutil"
 	"github.com/buildpacks/imgutil/local"
 	"github.com/buildpacks/imgutil/remote"
@@ -18,8 +16,9 @@ import (
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/pkg/errors"
-	"golang.org/x/crypto/ssh/terminal"
 
+	"github.com/buildpacks/pack/config"
+	ilogging "github.com/buildpacks/pack/internal/logging"
 	"github.com/buildpacks/pack/internal/style"
 	"github.com/buildpacks/pack/logging"
 )
@@ -99,7 +98,7 @@ func (f *Fetcher) pullImage(ctx context.Context, imageID string) error {
 	}
 
 	writer := logging.GetWriterForLevel(f.logger, logging.InfoLevel)
-	termFd, isTerm := isTerminal(writer)
+	termFd, isTerm := ilogging.IsTerminal(writer)
 
 	err = jsonmessage.DisplayJSONMessagesStream(rc, &colorizedWriter{writer}, termFd, isTerm, nil)
 	if err != nil {
@@ -109,20 +108,6 @@ func (f *Fetcher) pullImage(ctx context.Context, imageID string) error {
 	return rc.Close()
 }
 
-func isTerminal(w io.Writer) (uintptr, bool) {
-	type descriptor interface {
-		Fd() uintptr
-	}
-
-	if f, ok := w.(descriptor); ok {
-		termFd := f.Fd()
-		isTerm := terminal.IsTerminal(int(termFd))
-		return termFd, isTerm
-	}
-
-	return 0, false
-}
-
 func registryAuth(ref string) (string, error) {
 	_, a, err := auth.ReferenceForRepoName(authn.DefaultKeychain, ref)
 	if err != nil {

internal/logging/log_writer.go

@@ -3,7 +3,6 @@ package logging
 import (
 	"fmt"
 	"io"
-	"os"
 	"regexp"
 	"sync"
 	"time"
@@ -57,15 +56,18 @@ func (tw *LogWriter) Fd() uintptr {
 	tw.Lock()
 	defer tw.Unlock()
 
-	file, ok := tw.out.(*os.File)
-	if ok {
+	if file, ok := tw.out.(hasDescriptor); ok {
 		return file.Fd()
 	}
 
-	return 0
+	return InvalidFileDescriptor
 }
 
 // Remove all ANSI color information.
 func stripColor(b []byte) []byte {
 	return colorCodeMatcher.ReplaceAll(b, []byte(""))
 }
+
+type hasDescriptor interface {
+	Fd() uintptr
+}

internal/logging/log_writer_test.go

@@ -20,7 +20,7 @@ const (
 )
 
 func TestLogWriter(t *testing.T) {
-	spec.Run(t, "LogWriter", testLogWriter, spec.Parallel(), spec.Report(report.Terminal{}))
+	spec.Run(t, "LogWriter", testLogWriter, spec.Sequential(), spec.Report(report.Terminal{}))
 }
 
 func testLogWriter(t *testing.T, when spec.G, it spec.S) {
@@ -66,10 +66,10 @@ func testLogWriter(t *testing.T, when spec.G, it spec.S) {
 		})
 
 		when("out is just a Writer", func() {
-			it("returns 0", func() {
+			it("returns the invalid file descriptor", func() {
 				var out *bytes.Buffer
 				writer = ilogging.NewLogWriter(out, clockFunc, true)
-				h.AssertEq(t, int(writer.Fd()), 0)
+				h.AssertEq(t, writer.Fd(), ilogging.InvalidFileDescriptor)
 			})
 		})
 	})

internal/logging/logging.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/apex/log"
+	"golang.org/x/crypto/ssh/terminal"
 
 	"github.com/buildpacks/pack/internal/style"
 	"github.com/buildpacks/pack/logging"
@@ -24,6 +25,8 @@ const (
 	verboseLevel = log.DebugLevel
 	// time format the out logging uses
 	timeFmt = "2006/01/02 15:04:05.000000"
+	// InvalidFileDescriptor based on https://golang.org/src/os/file_unix.go?s=2183:2210#L57
+	InvalidFileDescriptor = ^(uintptr(0))
 )
 
 // LogWithWriters is a logger used with the pack CLI, allowing users to print logs for various levels, including Info, Debug and Error
@@ -123,6 +126,17 @@ func (lw *LogWithWriters) IsVerbose() bool {
 	return lw.Level == log.DebugLevel
 }
 
+// IsTerminal returns whether a writer is a terminal
+func IsTerminal(w io.Writer) (uintptr, bool) {
+	if f, ok := w.(hasDescriptor); ok {
+		termFd := f.Fd()
+		isTerm := terminal.IsTerminal(int(termFd))
+		return termFd, isTerm
+	}
+
+	return InvalidFileDescriptor, false
+}
+
 func formatLevel(ll log.Level) string {
 	switch ll {
 	case log.ErrorLevel:

internal/logging/logging_test.go

@@ -1,6 +1,7 @@
 package logging
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -222,6 +223,20 @@ func testLogWithWriters(t *testing.T, when spec.G, it spec.S) {
 		expected := "\n"
 		h.AssertEq(t, fOut(), expected)
 	})
+
+	when("IsTerminal", func() {
+		it("returns false for a pipe", func() {
+			fd, isTerm := IsTerminal(logger.WriterForLevel(logging.InfoLevel))
+			h.AssertFalse(t, isTerm)
+			h.AssertNotEq(t, fd, InvalidFileDescriptor) //The mock writer is a pipe, and therefore has a file descriptor
+		})
+
+		it("returns InvalidFileDescriptor if passed a normal Writer", func() {
+			fd, isTerm := IsTerminal(&bytes.Buffer{})
+			h.AssertFalse(t, isTerm)
+			h.AssertEq(t, fd, InvalidFileDescriptor)
+		})
+	})
 }
 
 func assertLogWriterHasOut(t *testing.T, writer io.Writer, out io.Writer) {