From 221a1c499b6fe9918410c69545784d333559ee9b Mon Sep 17 00:00:00 2001 From: Steve Streeting Date: Fri, 6 May 2016 14:46:13 +0100 Subject: [PATCH] Refactor progress meter, spinner & log into own package --- commands/command_checkout.go | 3 +- commands/command_fetch.go | 3 +- commands/command_prune.go | 5 +- lfs/http.go | 54 ++++----- lfs/transfer_queue.go | 5 +- progress/logger.go | 64 +++++++++++ lfs/progress_meter.go => progress/meter.go | 123 +-------------------- progress/spinner.go | 65 +++++++++++ 8 files changed, 169 insertions(+), 153 deletions(-) create mode 100644 progress/logger.go rename lfs/progress_meter.go => progress/meter.go (58%) create mode 100644 progress/spinner.go diff --git a/commands/command_checkout.go b/commands/command_checkout.go index 45adcc90..fb4303b5 100644 --- a/commands/command_checkout.go +++ b/commands/command_checkout.go @@ -8,6 +8,7 @@ import ( "github.com/github/git-lfs/git" "github.com/github/git-lfs/lfs" + "github.com/github/git-lfs/progress" "github.com/github/git-lfs/vendor/_nuts/github.com/rubyist/tracerx" "github.com/github/git-lfs/vendor/_nuts/github.com/spf13/cobra" ) @@ -116,7 +117,7 @@ func checkoutWithIncludeExclude(include []string, exclude []string) { for _, pointer := range pointers { totalBytes += pointer.Size } - progress := lfs.NewProgressMeter(len(pointers), totalBytes, false) + progress := progress.NewProgressMeter(len(pointers), totalBytes, false, lfs.Config.Getenv("GIT_LFS_PROGRESS")) progress.Start() totalBytes = 0 for _, pointer := range pointers { diff --git a/commands/command_fetch.go b/commands/command_fetch.go index 1fc4cbe7..cbbc661a 100644 --- a/commands/command_fetch.go +++ b/commands/command_fetch.go @@ -6,6 +6,7 @@ import ( "github.com/github/git-lfs/git" "github.com/github/git-lfs/lfs" + "github.com/github/git-lfs/progress" "github.com/github/git-lfs/vendor/_nuts/github.com/rubyist/tracerx" "github.com/github/git-lfs/vendor/_nuts/github.com/spf13/cobra" ) @@ -214,7 +215,7 @@ func scanAll() []*lfs.WrappedPointer { // This could be a long process so use the chan version & report progress Print("Scanning for all objects ever referenced...") - spinner := lfs.NewSpinner() + spinner := progress.NewSpinner() var numObjs int64 pointerchan, err := lfs.ScanRefsToChan("", "", opts) if err != nil { diff --git a/commands/command_prune.go b/commands/command_prune.go index ed91c1a5..b4bd7f61 100644 --- a/commands/command_prune.go +++ b/commands/command_prune.go @@ -12,6 +12,7 @@ import ( "github.com/github/git-lfs/git" "github.com/github/git-lfs/lfs" "github.com/github/git-lfs/localstorage" + "github.com/github/git-lfs/progress" "github.com/github/git-lfs/vendor/_nuts/github.com/spf13/cobra" ) @@ -217,7 +218,7 @@ func pruneCheckErrors(taskErrors []error) { func pruneTaskDisplayProgress(progressChan PruneProgressChan, waitg *sync.WaitGroup) { defer waitg.Done() - spinner := lfs.NewSpinner() + spinner := progress.NewSpinner() localCount := 0 retainCount := 0 verifyCount := 0 @@ -262,7 +263,7 @@ func pruneTaskCollectErrors(outtaskErrors *[]error, errorChan chan error, errorw } func pruneDeleteFiles(prunableObjects []string) { - spinner := lfs.NewSpinner() + spinner := progress.NewSpinner() var problems bytes.Buffer // In case we fail to delete some var deletedFiles int diff --git a/lfs/http.go b/lfs/http.go index cf90e9e1..aa4f2df1 100644 --- a/lfs/http.go +++ b/lfs/http.go @@ -19,31 +19,31 @@ import ( "github.com/github/git-lfs/vendor/_nuts/github.com/rubyist/tracerx" ) -type transferStats struct { +type httpTransferStats struct { HeaderSize int BodySize int Start time.Time Stop time.Time } -type transfer struct { - requestStats *transferStats - responseStats *transferStats +type httpTransfer struct { + requestStats *httpTransferStats + responseStats *httpTransferStats } var ( // TODO should use some locks - transfers = make(map[*http.Response]*transfer) - transferBuckets = make(map[string][]*http.Response) - transfersLock sync.Mutex - transferBucketsLock sync.Mutex + httpTransfers = make(map[*http.Response]*httpTransfer) + httpTransferBuckets = make(map[string][]*http.Response) + httpTransfersLock sync.Mutex + httpTransferBucketsLock sync.Mutex ) func LogTransfer(key string, res *http.Response) { if Config.isLoggingStats { - transferBucketsLock.Lock() - transferBuckets[key] = append(transferBuckets[key], res) - transferBucketsLock.Unlock() + httpTransferBucketsLock.Lock() + httpTransferBuckets[key] = append(httpTransferBuckets[key], res) + httpTransferBucketsLock.Unlock() } } @@ -84,15 +84,15 @@ func (c *HttpClient) Do(req *http.Request) (*http.Response, error) { resHeaderSize = len(dump) } - reqstats := &transferStats{HeaderSize: reqHeaderSize, BodySize: crc.Count} + reqstats := &httpTransferStats{HeaderSize: reqHeaderSize, BodySize: crc.Count} // Response body size cannot be figured until it is read. Do not rely on a Content-Length // header because it may not exist or be -1 in the case of chunked responses. - resstats := &transferStats{HeaderSize: resHeaderSize, Start: start} - t := &transfer{requestStats: reqstats, responseStats: resstats} - transfersLock.Lock() - transfers[res] = t - transfersLock.Unlock() + resstats := &httpTransferStats{HeaderSize: resHeaderSize, Start: start} + t := &httpTransfer{requestStats: reqstats, responseStats: resstats} + httpTransfersLock.Lock() + httpTransfers[res] = t + httpTransfersLock.Unlock() } return res, err @@ -273,22 +273,22 @@ func (c *countingReadCloser) Read(b []byte) (int, error) { } if err == io.EOF && Config.isLoggingStats { - // This transfer is done, we're checking it this way so we can also - // catch transfers where the caller forgets to Close() the Body. + // This httpTransfer is done, we're checking it this way so we can also + // catch httpTransfers where the caller forgets to Close() the Body. if c.response != nil { - transfersLock.Lock() - if transfer, ok := transfers[c.response]; ok { - transfer.responseStats.BodySize = c.Count - transfer.responseStats.Stop = time.Now() + httpTransfersLock.Lock() + if httpTransfer, ok := httpTransfers[c.response]; ok { + httpTransfer.responseStats.BodySize = c.Count + httpTransfer.responseStats.Stop = time.Now() } - transfersLock.Unlock() + httpTransfersLock.Unlock() } } return n, err } // LogHttpStats is intended to be called after all HTTP operations for the -// commmand have finished. It dumps k/v logs, one line per transfer into +// commmand have finished. It dumps k/v logs, one line per httpTransfer into // a log file with the current timestamp. func LogHttpStats() { if !Config.isLoggingStats { @@ -303,9 +303,9 @@ func LogHttpStats() { fmt.Fprintf(file, "concurrent=%d batch=%v time=%d version=%s\n", Config.ConcurrentTransfers(), Config.BatchTransfer(), time.Now().Unix(), Version) - for key, responses := range transferBuckets { + for key, responses := range httpTransferBuckets { for _, response := range responses { - stats := transfers[response] + stats := httpTransfers[response] fmt.Fprintf(file, "key=%s reqheader=%d reqbody=%d resheader=%d resbody=%d restime=%d status=%d url=%s\n", key, stats.requestStats.HeaderSize, diff --git a/lfs/transfer_queue.go b/lfs/transfer_queue.go index 010233e8..2271bf4d 100644 --- a/lfs/transfer_queue.go +++ b/lfs/transfer_queue.go @@ -5,6 +5,7 @@ import ( "sync/atomic" "github.com/github/git-lfs/git" + "github.com/github/git-lfs/progress" "github.com/github/git-lfs/vendor/_nuts/github.com/rubyist/tracerx" ) @@ -25,7 +26,7 @@ type Transferable interface { // TransferQueue provides a queue that will allow concurrent transfers. type TransferQueue struct { retrying uint32 - meter *ProgressMeter + meter *progress.ProgressMeter workers int // Number of transfer workers to spawn transferKind string errors []error @@ -46,7 +47,7 @@ type TransferQueue struct { // newTransferQueue builds a TransferQueue, allowing `workers` concurrent transfers. func newTransferQueue(files int, size int64, dryRun bool) *TransferQueue { q := &TransferQueue{ - meter: NewProgressMeter(files, size, dryRun), + meter: progress.NewProgressMeter(files, size, dryRun, Config.Getenv("GIT_LFS_PROGRESS")), apic: make(chan Transferable, batchSize), transferc: make(chan Transferable, batchSize), retriesc: make(chan Transferable, batchSize), diff --git a/progress/logger.go b/progress/logger.go new file mode 100644 index 00000000..e2d98ac1 --- /dev/null +++ b/progress/logger.go @@ -0,0 +1,64 @@ +package progress + +import ( + "fmt" + "os" + "path/filepath" +) + +// progressLogger provides a wrapper around an os.File that can either +// write to the file or ignore all writes completely. +type progressLogger struct { + writeData bool + log *os.File +} + +// Write will write to the file and perform a Sync() if writing succeeds. +func (l *progressLogger) Write(b []byte) error { + if l.writeData { + if _, err := l.log.Write(b); err != nil { + return err + } + return l.log.Sync() + } + return nil +} + +// Close will call Close() on the underlying file +func (l *progressLogger) Close() error { + if l.log != nil { + return l.log.Close() + } + return nil +} + +// Shutdown will cause the logger to ignore any further writes. It should +// be used when writing causes an error. +func (l *progressLogger) Shutdown() { + l.writeData = false +} + +// newProgressLogger creates a progressLogger with a log file path. +// If a log file is able to be created, the logger will write to the file. If +// there is an err creating the file, the logger will ignore all writes. +func newProgressLogger(logPath string) (*progressLogger, error) { + + if len(logPath) == 0 { + return &progressLogger{}, nil + } + if !filepath.IsAbs(logPath) { + return &progressLogger{}, fmt.Errorf("GIT_LFS_PROGRESS must be an absolute path") + } + + cbDir := filepath.Dir(logPath) + if err := os.MkdirAll(cbDir, 0755); err != nil { + return &progressLogger{}, err + } + + file, err := os.OpenFile(logPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0666) + if err != nil { + return &progressLogger{}, err + } + + return &progressLogger{true, file}, nil +} diff --git a/lfs/progress_meter.go b/progress/meter.go similarity index 58% rename from lfs/progress_meter.go rename to progress/meter.go index 6b610e81..63cdd509 100644 --- a/lfs/progress_meter.go +++ b/progress/meter.go @@ -1,11 +1,8 @@ -package lfs +package progress import ( "fmt" - "io" "os" - "path/filepath" - "runtime" "strings" "sync" "sync/atomic" @@ -37,8 +34,8 @@ type ProgressMeter struct { // NewProgressMeter creates a new ProgressMeter for the number and size of // files given. -func NewProgressMeter(estFiles int, estBytes int64, dryRun bool) *ProgressMeter { - logger, err := newProgressLogger() +func NewProgressMeter(estFiles int, estBytes int64, dryRun bool, logPath string) *ProgressMeter { + logger, err := newProgressLogger(logPath) if err != nil { fmt.Fprintf(os.Stderr, "Error creating progress logger: %s\n", err) } @@ -154,65 +151,6 @@ func (p *ProgressMeter) update() { fmt.Fprintf(os.Stdout, out) } -// progressLogger provides a wrapper around an os.File that can either -// write to the file or ignore all writes completely. -type progressLogger struct { - writeData bool - log *os.File -} - -// Write will write to the file and perform a Sync() if writing succeeds. -func (l *progressLogger) Write(b []byte) error { - if l.writeData { - if _, err := l.log.Write(b); err != nil { - return err - } - return l.log.Sync() - } - return nil -} - -// Close will call Close() on the underlying file -func (l *progressLogger) Close() error { - if l.log != nil { - return l.log.Close() - } - return nil -} - -// Shutdown will cause the logger to ignore any further writes. It should -// be used when writing causes an error. -func (l *progressLogger) Shutdown() { - l.writeData = false -} - -// newProgressLogger creates a progressLogger based on the presence of -// the GIT_LFS_PROGRESS environment variable. If it is present and a log file -// is able to be created, the logger will write to the file. If it is absent, -// or there is an err creating the file, the logger will ignore all writes. -func newProgressLogger() (*progressLogger, error) { - logPath := Config.Getenv("GIT_LFS_PROGRESS") - - if len(logPath) == 0 { - return &progressLogger{}, nil - } - if !filepath.IsAbs(logPath) { - return &progressLogger{}, fmt.Errorf("GIT_LFS_PROGRESS must be an absolute path") - } - - cbDir := filepath.Dir(logPath) - if err := os.MkdirAll(cbDir, 0755); err != nil { - return &progressLogger{}, err - } - - file, err := os.OpenFile(logPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0666) - if err != nil { - return &progressLogger{}, err - } - - return &progressLogger{true, file}, nil -} - func formatBytes(i int64) string { switch { case i > 1099511627776: @@ -227,58 +165,3 @@ func formatBytes(i int64) string { return fmt.Sprintf("%d B", i) } - -// Indeterminate progress indicator 'spinner' -type Spinner struct { - stage int - msg string -} - -var spinnerChars = []byte{'|', '/', '-', '\\'} - -// Print a spinner (stage) to out followed by msg (no linefeed) -func (s *Spinner) Print(out io.Writer, msg string) { - s.msg = msg - s.Spin(out) -} - -// Just spin the spinner one more notch & use the last message -func (s *Spinner) Spin(out io.Writer) { - s.stage = (s.stage + 1) % len(spinnerChars) - s.update(out, string(spinnerChars[s.stage]), s.msg) -} - -// Finish the spinner with a completion message & newline -func (s *Spinner) Finish(out io.Writer, finishMsg string) { - s.msg = finishMsg - s.stage = 0 - var sym string - if runtime.GOOS == "windows" { - // Windows console sucks, can't do nice check mark except in ConEmu (not cmd or git bash) - // So play it safe & boring - sym = "*" - } else { - sym = fmt.Sprintf("%c", '\u2714') - } - s.update(out, sym, finishMsg) - out.Write([]byte{'\n'}) -} - -func (s *Spinner) update(out io.Writer, prefix, msg string) { - - str := fmt.Sprintf("%v %v", prefix, msg) - - width := 80 // default to 80 chars wide if ts.GetSize() fails - size, err := ts.GetSize() - if err == nil { - width = size.Col() - } - padding := strings.Repeat(" ", width-len(str)) - - fmt.Fprintf(out, "\r%v%v", str, padding) - -} - -func NewSpinner() *Spinner { - return &Spinner{} -} diff --git a/progress/spinner.go b/progress/spinner.go new file mode 100644 index 00000000..3af23095 --- /dev/null +++ b/progress/spinner.go @@ -0,0 +1,65 @@ +package progress + +import ( + "fmt" + "io" + "runtime" + "strings" + + "github.com/github/git-lfs/vendor/_nuts/github.com/olekukonko/ts" +) + +// Indeterminate progress indicator 'spinner' +type Spinner struct { + stage int + msg string +} + +var spinnerChars = []byte{'|', '/', '-', '\\'} + +// Print a spinner (stage) to out followed by msg (no linefeed) +func (s *Spinner) Print(out io.Writer, msg string) { + s.msg = msg + s.Spin(out) +} + +// Just spin the spinner one more notch & use the last message +func (s *Spinner) Spin(out io.Writer) { + s.stage = (s.stage + 1) % len(spinnerChars) + s.update(out, string(spinnerChars[s.stage]), s.msg) +} + +// Finish the spinner with a completion message & newline +func (s *Spinner) Finish(out io.Writer, finishMsg string) { + s.msg = finishMsg + s.stage = 0 + var sym string + if runtime.GOOS == "windows" { + // Windows console sucks, can't do nice check mark except in ConEmu (not cmd or git bash) + // So play it safe & boring + sym = "*" + } else { + sym = fmt.Sprintf("%c", '\u2714') + } + s.update(out, sym, finishMsg) + out.Write([]byte{'\n'}) +} + +func (s *Spinner) update(out io.Writer, prefix, msg string) { + + str := fmt.Sprintf("%v %v", prefix, msg) + + width := 80 // default to 80 chars wide if ts.GetSize() fails + size, err := ts.GetSize() + if err == nil { + width = size.Col() + } + padding := strings.Repeat(" ", width-len(str)) + + fmt.Fprintf(out, "\r%v%v", str, padding) + +} + +func NewSpinner() *Spinner { + return &Spinner{} +}