tasklog: don't log progress status when stdout is not a tty

steffengodskesen · ttaylorr · commit 03aa2efcede9 · 2018-11-14T14:35:29.000-08:00
But make it overridable by either
 - setting lfs.forceprogress 1
 - setting GIT_LFS_FORCE_PROGRESS=1
diff --git a/commands/command_checkout.go b/commands/command_checkout.go
@@ -47,7 +47,9 @@ func checkoutCommand(cmd *cobra.Command, args []string) {
 
 	var totalBytes int64
 	var pointers []*lfs.WrappedPointer
-	logger := tasklog.NewLogger(os.Stdout)
+	logger := tasklog.NewLogger(os.Stdout,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	meter := tq.NewMeter()
 	meter.Direction = tq.Checkout
 	meter.Logger = meter.LoggerFromEnv(cfg.Os)
diff --git a/commands/command_fetch.go b/commands/command_fetch.go
@@ -230,7 +230,9 @@ func scanAll() []*lfs.WrappedPointer {
 	task := tasklog.NewSimpleTask()
 	defer task.Complete()
 
-	logger := tasklog.NewLogger(OutputWriter)
+	logger := tasklog.NewLogger(OutputWriter,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	logger.Enqueue(task)
 	var numObjs int64
 
@@ -324,7 +326,9 @@ func fetchAndReportToChan(allpointers []*lfs.WrappedPointer, filter *filepathfil
 }
 
 func readyAndMissingPointers(allpointers []*lfs.WrappedPointer, filter *filepathfilter.Filter) ([]*lfs.WrappedPointer, []*lfs.WrappedPointer, *tq.Meter) {
-	logger := tasklog.NewLogger(os.Stdout)
+	logger := tasklog.NewLogger(os.Stdout,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	meter := buildProgressMeter(false, tq.Download)
 	logger.Enqueue(meter)
 
diff --git a/commands/command_migrate_export.go b/commands/command_migrate_export.go
@@ -19,7 +19,9 @@ import (
 func migrateExportCommand(cmd *cobra.Command, args []string) {
 	ensureWorkingCopyClean(os.Stdin, os.Stderr)
 
-	l := tasklog.NewLogger(os.Stderr)
+	l := tasklog.NewLogger(os.Stderr,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	defer l.Close()
 
 	db, err := getObjectDatabase()
diff --git a/commands/command_migrate_import.go b/commands/command_migrate_import.go
@@ -24,7 +24,9 @@ import (
 func migrateImportCommand(cmd *cobra.Command, args []string) {
 	ensureWorkingCopyClean(os.Stdin, os.Stderr)
 
-	l := tasklog.NewLogger(os.Stderr)
+	l := tasklog.NewLogger(os.Stderr,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	defer l.Close()
 
 	db, err := getObjectDatabase()
diff --git a/commands/command_migrate_info.go b/commands/command_migrate_info.go
@@ -40,7 +40,9 @@ var (
 )
 
 func migrateInfoCommand(cmd *cobra.Command, args []string) {
-	l := tasklog.NewLogger(os.Stderr)
+	l := tasklog.NewLogger(os.Stderr,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 
 	db, err := getObjectDatabase()
 	if err != nil {
diff --git a/commands/command_prune.go b/commands/command_prune.go
@@ -57,7 +57,9 @@ func prune(fetchPruneConfig lfs.FetchPruneConfig, verifyRemote, dryRun, verbose
 	localObjects := make([]fs.Object, 0, 100)
 	retainedObjects := tools.NewStringSetWithCapacity(100)
 
-	logger := tasklog.NewLogger(OutputWriter)
+	logger := tasklog.NewLogger(OutputWriter,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	defer logger.Close()
 
 	var reachableObjects tools.StringSet
diff --git a/commands/command_pull.go b/commands/command_pull.go
@@ -38,7 +38,9 @@ func pull(filter *filepathfilter.Filter) {
 	}
 
 	pointers := newPointerMap()
-	logger := tasklog.NewLogger(os.Stdout)
+	logger := tasklog.NewLogger(os.Stdout,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	meter := tq.NewMeter()
 	meter.Logger = meter.LoggerFromEnv(cfg.Os)
 	logger.Enqueue(meter)
diff --git a/commands/uploader.go b/commands/uploader.go
@@ -111,7 +111,9 @@ func newUploadContext(dryRun bool) *uploadContext {
 		sink = ioutil.Discard
 	}
 
-	ctx.logger = tasklog.NewLogger(sink)
+	ctx.logger = tasklog.NewLogger(sink,
+		tasklog.ForceProgress(cfg.ForceProgress()),
+	)
 	ctx.meter = buildProgressMeter(ctx.DryRun, tq.Upload)
 	ctx.logger.Enqueue(ctx.meter)
 	ctx.committerName, ctx.committerEmail = cfg.CurrentCommitter()
diff --git a/config/config.go b/config/config.go
@@ -308,6 +308,10 @@ func (c *Configuration) SetLockableFilesReadOnly() bool {
 	return c.Os.Bool("GIT_LFS_SET_LOCKABLE_READONLY", true) && c.Git.Bool("lfs.setlockablereadonly", true)
 }
 
+func (c *Configuration) ForceProgress() bool {
+	return c.Os.Bool("GIT_LFS_FORCE_PROGRESS", false) || c.Git.Bool("lfs.forceprogress", false)
+}
+
 // HookDir returns the location of the hooks owned by this repository. If the
 // core.hooksPath configuration variable is supported, we prefer that and expand
 // paths appropriately.
diff --git a/docs/man/git-lfs-config.5.ronn b/docs/man/git-lfs-config.5.ronn
@@ -306,6 +306,16 @@ be scoped inside the configuration for a remote.
   * `total` The entire size of the file, in bytes.
   * `name` The name of the file.
 
+* `GIT_LFS_FORCE_PROGRESS`
+  `lfs.forceprogress`
+
+  Controls whether Git LFS will suppress progress status when the standard
+  output stream is not attached to a terminal. The default is `false` which
+  makes Git LFS detect whether stdout is a terminal and suppress progress when
+  it's not; you can disable this behaviour and force progress status even when
+  standard output stream is not a terminal by setting either variable to 1,
+  'yes' or 'true'.
+
 * `GIT_LFS_SET_LOCKABLE_READONLY`
   `lfs.setlockablereadonly`
 
diff --git a/git/githistory/rewriter.go b/git/githistory/rewriter.go
@@ -158,10 +158,12 @@ var (
 		}
 	}
 
-	// WithLoggerto logs updates caused by the *git/githistory.Rewriter to
+	// WithLoggerTo logs updates caused by the *git/githistory.Rewriter to
 	// the given io.Writer "sink".
-	WithLoggerTo = func(sink io.Writer) rewriterOption {
-		return WithLogger(tasklog.NewLogger(sink))
+	WithLoggerTo = func(sink io.Writer, forceProgress bool) rewriterOption {
+		return WithLogger(tasklog.NewLogger(sink,
+			tasklog.ForceProgress(forceProgress),
+		))
 	}
 
 	// WithLogger logs updates caused by the *git/githistory.Rewriter to the
diff --git a/go.mod b/go.mod
@@ -8,6 +8,7 @@ require (
 	github.com/git-lfs/wildmatch v1.0.0
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/kr/pty v0.0.0-20150511174710-5cf931ef8f76
+	github.com/mattn/go-isatty v0.0.4
 	github.com/olekukonko/ts v0.0.0-20171002115256-78ecb04241c0
 	github.com/pkg/errors v0.0.0-20170505043639-c605e284fe17
 	github.com/rubyist/tracerx v0.0.0-20170927163412-787959303086
diff --git a/go.sum b/go.sum
@@ -1,5 +1,6 @@
 github.com/ThomsonReutersEikon/go-ntlm v0.0.0-20151030004737-b00ec39bbdd0 h1:iBnLwXNz+Mz2K6MndIM9a4sLBb+TouHB8s0tvx16s4s=
 github.com/ThomsonReutersEikon/go-ntlm v0.0.0-20151030004737-b00ec39bbdd0/go.mod h1:0eFwySJYxbNw/r8cJ01IeztpJfwrgrmiLtYig80Yvrc=
+github.com/alexbrainman/sspi v0.0.0-20180125232955-4729b3d4d858 h1:OZQyEhf4BviydsRdmK4ryeJHotDLd7vL1X8+nnxXkfk=
 github.com/alexbrainman/sspi v0.0.0-20180125232955-4729b3d4d858/go.mod h1:976q2ETgjT2snVCf2ZaBnyBbVoPERGjUz+0sofzEfro=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -13,6 +14,8 @@ github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NH
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
 github.com/kr/pty v0.0.0-20150511174710-5cf931ef8f76 h1:i5TIRQpbCg4aJMUtVHIhkQnSw++Z405Z5pzqHqeNkdU=
 github.com/kr/pty v0.0.0-20150511174710-5cf931ef8f76/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
+github.com/mattn/go-isatty v0.0.4 h1:bnP0vzxcAdeI1zdubAl5PjU6zsERjGZb7raWodagDYs=
+github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
 github.com/olekukonko/ts v0.0.0-20171002115256-78ecb04241c0 h1:LiZB1h0GIcudcDci2bxbqI6DXV8bF8POAnArqvRrIyw=
 github.com/olekukonko/ts v0.0.0-20171002115256-78ecb04241c0/go.mod h1:F/7q8/HZz+TXjlsoZQQKVYvXTZaFH4QRa3y+j1p7MS0=
 github.com/pkg/errors v0.0.0-20170505043639-c605e284fe17 h1:chPfVn+gpAM5CTpTyVU9j8J+xgRGwmoDlNDLjKnJiYo=
@@ -33,4 +36,5 @@ github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHo
 github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
 github.com/xeipuuv/gojsonschema v0.0.0-20170210233622-6b67b3fab74d h1:BJPiQVOMMtJsJIkrF4T6K3RKbzqr7rkaybMk33dlGUo=
 github.com/xeipuuv/gojsonschema v0.0.0-20170210233622-6b67b3fab74d/go.mod h1:5yf86TLmAcydyeJq5YvxkGPE2fm/u4myDekKRoLuqhs=
+golang.org/x/sys v0.0.0-20180831094639-fa5fdf94c789 h1:T8D7l6WB3tLu+VpKvw06ieD/OhBi1XpJmG1U/FtttZg=
 golang.org/x/sys v0.0.0-20180831094639-fa5fdf94c789/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
diff --git a/t/git-lfs-test-server-api/main.go b/t/git-lfs-test-server-api/main.go
@@ -179,7 +179,9 @@ func buildTestData(repo *t.Repo, manifest *tq.Manifest) (oidsExist, oidsMissing
 	oidsMissing = make([]TestObject, 0, oidCount)
 
 	// just one commit
-	logger := tasklog.NewLogger(os.Stdout)
+	logger := tasklog.NewLogger(os.Stdout,
+		tasklog.ForceProgress(false),
+	)
 	meter := tq.NewMeter()
 	meter.Logger = meter.LoggerFromEnv(repo.OSEnv())
 	logger.Enqueue(meter)
diff --git a/t/testenv.sh b/t/testenv.sh
@@ -123,12 +123,14 @@ LFS_CLIENT_KEY_FILE_ENCRYPTED="$REMOTEDIR/client.enc.key"
 # the fake home dir used for the initial setup
 TESTHOME="$REMOTEDIR/home"
 
+GIT_LFS_FORCE_PROGRESS=1
 GIT_CONFIG_NOSYSTEM=1
 GIT_TERMINAL_PROMPT=0
 GIT_SSH=lfs-ssh-echo
 APPVEYOR_REPO_COMMIT_MESSAGE="test: env test should look for GIT_SSH too"
 
 export CREDSDIR
+export GIT_LFS_FORCE_PROGRESS
 export GIT_CONFIG_NOSYSTEM
 export GIT_SSH
 export APPVEYOR_REPO_COMMIT_MESSAGE
diff --git a/tasklog/log.go b/tasklog/log.go
@@ -4,10 +4,12 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"os"
 	"strings"
 	"sync"
 	"time"
 
+	isatty "github.com/mattn/go-isatty"
 	"github.com/olekukonko/ts"
 )
 
@@ -25,6 +27,9 @@ type Logger struct {
 	// this logger is running within.
 	widthFn func() int
 
+	// forceProgress forces progress status even when stdout is not a tty
+	forceProgress bool
+
 	// throttle is the minimum amount of time that must pass between each
 	// instant data is logged.
 	throttle time.Duration
@@ -38,9 +43,21 @@ type Logger struct {
 	wg *sync.WaitGroup
 }
 
+// Option is the type for
+type Option func(*Logger)
+
+// ForceProgress returns an options function that configures forced progress status
+// on the logger.
+func ForceProgress(v bool) Option {
+	return func(l *Logger) {
+		l.forceProgress = v
+	}
+}
+
 // NewLogger retuns a new *Logger instance that logs to "sink" and uses the
-// current terminal width as the width of the line.
-func NewLogger(sink io.Writer) *Logger {
+// current terminal width as the width of the line. Will log progress status if
+// stdout is a terminal or if forceProgress is true
+func NewLogger(sink io.Writer, options ...Option) *Logger {
 	if sink == nil {
 		sink = ioutil.Discard
 	}
@@ -60,6 +77,10 @@ func NewLogger(sink io.Writer) *Logger {
 		wg:    new(sync.WaitGroup),
 	}
 
+	for _, option := range options {
+		option(l)
+	}
+
 	go l.consume()
 
 	return l
@@ -214,6 +235,9 @@ func (l *Logger) logTask(task Task) {
 
 	var update *Update
 	for update = range task.Updates() {
+		if !isatty.IsTerminal(os.Stdout.Fd()) && !l.forceProgress {
+			continue
+		}
 		if logAll || l.throttle == 0 || !update.Throttled(last.Add(l.throttle)) {
 			l.logLine(update.S)
 			last = update.At
diff --git a/tasklog/log_test.go b/tasklog/log_test.go
@@ -31,7 +31,7 @@ func TestLoggerLogsTasks(t *testing.T) {
 		close(task)
 	}()
 
-	l := NewLogger(&buf)
+	l := NewLogger(&buf, ForceProgress(true))
 	l.throttle = 0
 	l.widthFn = func() int { return 0 }
 	l.Enqueue(ChanTask(task))
@@ -40,6 +40,25 @@ func TestLoggerLogsTasks(t *testing.T) {
 	assert.Equal(t, "first\rsecond\rsecond, done\n", buf.String())
 }
 
+func TestLoggerLogsSuppressesProgress(t *testing.T) {
+	var buf bytes.Buffer
+
+	task := make(chan *Update)
+	go func() {
+		task <- &Update{"first", time.Now(), false}
+		task <- &Update{"second", time.Now(), false}
+		close(task)
+	}()
+
+	l := NewLogger(&buf, ForceProgress(false))
+	l.throttle = 0
+	l.widthFn = func() int { return 0 }
+	l.Enqueue(ChanTask(task))
+	l.Close()
+
+	assert.Equal(t, "second, done\n", buf.String())
+}
+
 func TestLoggerLogsMultipleTasksInOrder(t *testing.T) {
 	var buf bytes.Buffer
 
@@ -56,7 +75,7 @@ func TestLoggerLogsMultipleTasksInOrder(t *testing.T) {
 		close(t2)
 	}()
 
-	l := NewLogger(&buf)
+	l := NewLogger(&buf, ForceProgress(true))
 	l.throttle = 0
 	l.widthFn = func() int { return 0 }
 	l.Enqueue(ChanTask(t1), ChanTask(t2))
@@ -75,7 +94,7 @@ func TestLoggerLogsMultipleTasksInOrder(t *testing.T) {
 func TestLoggerLogsMultipleTasksWithoutBlocking(t *testing.T) {
 	var buf bytes.Buffer
 
-	l := NewLogger(&buf)
+	l := NewLogger(&buf, ForceProgress(true))
 	l.throttle = 0
 	t1, t2 := make(chan *Update), make(chan *Update)
 
@@ -112,7 +131,7 @@ func TestLoggerThrottlesWrites(t *testing.T) {
 		close(t1)                                                        // t = 20+2ε ms, throttle is closed
 	}()
 
-	l := NewLogger(&buf)
+	l := NewLogger(&buf, ForceProgress(true))
 	l.widthFn = func() int { return 0 }
 	l.throttle = 15 * time.Millisecond
 
@@ -139,7 +158,7 @@ func TestLoggerThrottlesLastWrite(t *testing.T) {
 		close(t1)                                                        // t = 10+2ε ms, throttle is closed
 	}()
 
-	l := NewLogger(&buf)
+	l := NewLogger(&buf, ForceProgress(true))
 	l.widthFn = func() int { return 0 }
 	l.throttle = 15 * time.Millisecond
 
@@ -155,7 +174,7 @@ func TestLoggerThrottlesLastWrite(t *testing.T) {
 func TestLoggerLogsAllDurableUpdates(t *testing.T) {
 	var buf bytes.Buffer
 
-	l := NewLogger(&buf)
+	l := NewLogger(&buf, ForceProgress(true))
 	l.widthFn = func() int { return 0 }
 	l.throttle = 15 * time.Minute
 
@@ -182,7 +201,7 @@ func TestLoggerHandlesSilentTasks(t *testing.T) {
 	task := make(chan *Update)
 	close(task)
 
-	l := NewLogger(&buf)
+	l := NewLogger(&buf, ForceProgress(true))
 	l.Enqueue(ChanTask(task))
 	l.Close()
 

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,9 @@ var (`
`40`	`40`	`)`
`41`	`41`
`42`	`42`	`func migrateInfoCommand(cmd *cobra.Command, args []string) {`
`43`		`- l := tasklog.NewLogger(os.Stderr)`
	`43`	`+ l := tasklog.NewLogger(os.Stderr,`
	`44`	`+ tasklog.ForceProgress(cfg.ForceProgress()),`
	`45`	`+ )`
`44`	`46`
`45`	`47`	`db, err := getObjectDatabase()`
`46`	`48`	`if err != nil {`