Moved the progress bar from common to the packer.UI interface and refactored it so that the terminal width is calculated based on each interface which returns a custom progressbar specific to its ui.

common/config.go

@@ -2,7 +2,6 @@ package common
 
 import (
 	"fmt"
-	"github.com/cheggaaa/pb"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -55,7 +54,7 @@ func SupportedProtocol(u *url.URL) bool {
 
 	// build a dummy NewDownloadClient since this is the only place that valid
 	// protocols are actually exposed.
-	cli := NewDownloadClient(&DownloadConfig{}, *pb.New(0))
+	cli := NewDownloadClient(&DownloadConfig{}, nil)
 
 	// Iterate through each downloader to see if a protocol was found.
 	ok := false
@@ -187,7 +186,7 @@ func FileExistsLocally(original string) bool {
 
 	// First create a dummy downloader so we can figure out which
 	// protocol to use.
-	cli := NewDownloadClient(&DownloadConfig{}, *pb.New(0))
+	cli := NewDownloadClient(&DownloadConfig{}, nil)
 	d, ok := cli.config.DownloaderMap[u.Scheme]
 	if !ok {
 		return false

common/download.go

@@ -19,7 +19,9 @@ import (
 )
 
 // required import for progress-bar
-import "github.com/cheggaaa/pb"
+import (
+	"github.com/hashicorp/packer/packer"
+)
 
 // imports related to each Downloader implementation
 import (
@@ -84,16 +86,21 @@ func HashForType(t string) hash.Hash {
 
 // NewDownloadClient returns a new DownloadClient for the given
 // configuration.
-func NewDownloadClient(c *DownloadConfig, bar pb.ProgressBar) *DownloadClient {
+func NewDownloadClient(c *DownloadConfig, bar packer.ProgressBar) *DownloadClient {
 	const mtu = 1500 /* ethernet */ - 20 /* ipv4 */ - 20 /* tcp */
 
+	// If bar is nil, then use a dummy progress bar that doesn't do anything
+	if bar == nil {
+		bar = packer.GetDummyProgressBar()
+	}
+
 	// Create downloader map if it hasn't been specified already.
 	if c.DownloaderMap == nil {
 		c.DownloaderMap = map[string]Downloader{
-			"file":  &FileDownloader{progress: &bar, bufferSize: nil},
-			"http":  &HTTPDownloader{progress: &bar, userAgent: c.UserAgent},
-			"https": &HTTPDownloader{progress: &bar, userAgent: c.UserAgent},
-			"smb":   &SMBDownloader{progress: &bar, bufferSize: nil},
+			"file":  &FileDownloader{progress: bar, bufferSize: nil},
+			"http":  &HTTPDownloader{progress: bar, userAgent: c.UserAgent},
+			"https": &HTTPDownloader{progress: bar, userAgent: c.UserAgent},
+			"smb":   &SMBDownloader{progress: bar, bufferSize: nil},
 		}
 	}
 	return &DownloadClient{config: c}
@@ -235,7 +242,7 @@ type HTTPDownloader struct {
 	total     uint64
 	userAgent string
 
-	progress *pb.ProgressBar
+	progress packer.ProgressBar
 }
 
 func (d *HTTPDownloader) Cancel() {
@@ -328,8 +335,9 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error {
 
 	d.total = d.current + uint64(resp.ContentLength)
 
-	d.progress.Total = int64(d.total)
-	progressBar := d.progress.Start()
+	bar := d.progress
+	bar.Total = int64(d.total)
+	progressBar := bar.Start()
 	progressBar.Set64(int64(d.current))
 
 	var buffer [4096]byte
@@ -371,7 +379,7 @@ type FileDownloader struct {
 	current uint64
 	total   uint64
 
-	progress *pb.ProgressBar
+	progress packer.ProgressBar
 }
 
 func (d *FileDownloader) Progress() uint64 {
@@ -462,8 +470,10 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error {
 	}
 	d.total = uint64(fi.Size())
 
-	d.progress.Total = int64(d.total)
-	progressBar := d.progress.Start()
+	bar := d.progress
+	bar.Total = int64(d.total)
+	progressBar := bar.Start()
+	progressBar.Set64(int64(d.current))
 
 	// no bufferSize specified, so copy synchronously.
 	if d.bufferSize == nil {
@@ -508,7 +518,7 @@ type SMBDownloader struct {
 	current uint64
 	total   uint64
 
-	progress *pb.ProgressBar
+	progress packer.ProgressBar
 }
 
 func (d *SMBDownloader) Progress() uint64 {
@@ -581,8 +591,10 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error {
 	}
 	d.total = uint64(fi.Size())
 
-	d.progress.Total = int64(d.total)
-	progressBar := d.progress.Start()
+	bar := d.progress
+	bar.Total = int64(d.total)
+	progressBar := bar.Start()
+	progressBar.Set64(int64(d.current))
 
 	// no bufferSize specified, so copy synchronously.
 	if d.bufferSize == nil {

common/progress.go

@@ -1,61 +0,0 @@
-package common
-
-import (
-	"fmt"
-	"github.com/cheggaaa/pb"
-	"github.com/hashicorp/packer/packer"
-	"log"
-	"time"
-)
-
-// Default progress bar appearance
-func GetNewProgressBar(ui *packer.Ui) pb.ProgressBar {
-	bar := pb.New64(0)
-	bar.ShowPercent = true
-	bar.ShowCounters = true
-	bar.ShowSpeed = false
-	bar.ShowBar = true
-	bar.ShowTimeLeft = false
-	bar.ShowFinalTime = false
-	bar.SetUnits(pb.U_BYTES)
-	bar.Format("[=>-]")
-	bar.SetRefreshRate(1 * time.Second)
-	bar.SetWidth(80)
-
-	// If there's no UI set, then the progress bar doesn't need anything else
-	if ui == nil {
-		return *bar
-	}
-	UI := *ui
-
-	// Now check the UI's width to adjust the progress bar
-	uiWidth := UI.GetMinimumLength() + len("\n")
-
-	// If the UI's width is signed, then this interface doesn't really
-	// benefit from a progress bar
-	if uiWidth < 0 {
-		log.Println("Refusing to render progress-bar for unsupported UI.")
-		return *bar
-	}
-	bar.Callback = UI.Message
-
-	// Figure out the terminal width if possible
-	width, _, err := GetTerminalDimensions()
-	if err != nil {
-		newerr := fmt.Errorf("Unable to determine terminal dimensions: %v", err)
-		log.Printf("Using default width (%d) for progress-bar due to error: %s", bar.GetWidth(), newerr)
-		return *bar
-	}
-
-	// Adjust the progress bar's width according to the terminal size
-	// and whatever width is returned from the UI
-	if width > uiWidth {
-		width -= uiWidth
-		bar.SetWidth(width)
-	} else {
-		newerr := fmt.Errorf("Terminal width (%d) is smaller than UI message width (%d).", width, uiWidth)
-		log.Printf("Using default width (%d) for progress-bar due to error: %s", bar.GetWidth(), newerr)
-	}
-
-	return *bar
-}

common/step_download.go

@@ -63,8 +63,8 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste
 
 	ui.Say(fmt.Sprintf("Retrieving %s", s.Description))
 
-	// Get a default-looking progress bar and connect it to the ui.
-	bar := GetNewProgressBar(&ui)
+	// Get a progress bar from the ui so we can hand it off to the download client
+	bar := ui.GetProgressBar()
 
 	// First try to use any already downloaded file
 	// If it fails, proceed to regular download logic
@@ -143,8 +143,8 @@ func (s *StepDownload) download(config *DownloadConfig, state multistep.StateBag
 	var path string
 	ui := state.Get("ui").(packer.Ui)
 
-	// Get a default-looking progress bar and connect it to the ui.
-	bar := GetNewProgressBar(&ui)
+	// Get a progress bar and hand it off to the download client
+	bar := ui.GetProgressBar()
 
 	// Create download client with config and progress bar
 	download := NewDownloadClient(config, bar)

packer/rpc/ui.go

@@ -60,8 +60,8 @@ func (u *Ui) Say(message string) {
 	}
 }
 
-func (u *Ui) GetMinimumLength() (result int) {
-	if err := u.client.Call("Ui.GetMinimumLength", nil, &result); err != nil {
+func (u *Ui) GetProgressBar() (result packer.ProgressBar) {
+	if err := u.client.Call("Ui.GetProgressBar", nil, &result); err != nil {
 		log.Printf("Error in Ui RPC call: %s", err)
 	}
 	return
@@ -99,8 +99,8 @@ func (u *UiServer) Say(message *string, reply *interface{}) error {
 	return nil
 }
 
-func (u *UiServer) GetMinimumLength(noargs *interface{}, reply *int) error {
-	res := u.ui.GetMinimumLength()
+func (u *UiServer) GetProgressBar(noargs *interface{}, reply *packer.ProgressBar) error {
+	res := u.ui.GetProgressBar()
 	*reply = res
 	return nil
 }

packer/rpc/ui_test.go

@@ -1,24 +1,26 @@
 package rpc
 
 import (
+	"github.com/hashicorp/packer/packer"
 	"reflect"
 	"testing"
 )
 
 type testUi struct {
-	askCalled              bool
-	askQuery               string
-	errorCalled            bool
-	errorMessage           string
-	machineCalled          bool
-	machineType            string
-	machineArgs            []string
-	messageCalled          bool
-	messageMessage         string
-	sayCalled              bool
-	sayMessage             string
-	getMinimumLengthCalled bool
-	getMinimumLengthValue  int
+	askCalled                   bool
+	askQuery                    string
+	errorCalled                 bool
+	errorMessage                string
+	machineCalled               bool
+	machineType                 string
+	machineArgs                 []string
+	messageCalled               bool
+	messageMessage              string
+	sayCalled                   bool
+	sayMessage                  string
+	getProgressBarCalled        bool
+	getProgressBarValue         packer.ProgressBar
+	progessBarCallbackWasCalled bool
 }
 
 func (u *testUi) Ask(query string) (string, error) {
@@ -48,10 +50,13 @@ func (u *testUi) Say(message string) {
 	u.sayMessage = message
 }
 
-func (u *testUi) GetMinimumLength() int {
-	u.getMinimumLengthCalled = true
-	u.getMinimumLengthValue = -1
-	return u.getMinimumLengthValue
+func (u *testUi) GetProgressBar() packer.ProgressBar {
+	u.getProgressBarCalled = true
+	u.getProgressBarValue = packer.GetDummyProgressBar()
+	u.getProgressBarValue.Callback = func(string) {
+		u.progessBarCallbackWasCalled = true
+	}
+	return u.getProgressBarValue
 }
 
 func TestUiRPC(t *testing.T) {
@@ -101,12 +106,17 @@ func TestUiRPC(t *testing.T) {
 		t.Fatal("machine should be called")
 	}
 
-	uiClient.GetMinimumLength()
-	if !ui.getMinimumLengthCalled {
-		t.Fatal("getminimumlength should be called")
+	bar := uiClient.GetProgressBar()
+	if !ui.getProgressBarCalled {
+		t.Fatal("getprogressbar should be called")
+	}
+
+	if bar.Callback == nil {
+		t.Fatal("getprogressbar returned a bar with an empty callback")
 	}
-	if ui.getMinimumLengthValue != -1 {
-		t.Fatal("getminimumlength should be -1")
+	bar.Callback("test")
+	if !ui.progessBarCallbackWasCalled {
+		t.Fatal("progressbarcallback should be called")
 	}
 
 	if ui.machineType != "foo" {

packer/ui.go

@@ -15,6 +15,8 @@ import (
 	"syscall"
 	"time"
 	"unicode"
+
+	"github.com/cheggaaa/pb"
 )
 
 type UiColor uint
@@ -28,6 +30,15 @@ const (
 	UiColorCyan            = 36
 )
 
+// The ProgressBar interface is used for abstracting cheggaaa's progress-
+// bar, or any other progress bar. If a UI does not support a progress-
+// bar, then it must return a null progress bar.
+const (
+	DefaultProgressBarWidth = 80
+)
+
+type ProgressBar = *pb.ProgressBar
+
 // The Ui interface handles all communication for Packer with the outside
 // world. This sort of control allows us to strictly control how output
 // is formatted and various levels of output.
@@ -37,7 +48,7 @@ type Ui interface {
 	Message(string)
 	Error(string)
 	Machine(string, ...string)
-	GetMinimumLength() int
+	GetProgressBar() ProgressBar
 }
 
 // ColoredUi is a UI that is colored using terminal colors.
@@ -133,8 +144,9 @@ func (u *ColoredUi) supportsColors() bool {
 	return cygwin
 }
 
-func (u *ColoredUi) GetMinimumLength() int {
-	return u.Ui.GetMinimumLength()
+func (u *ColoredUi) GetProgressBar() ProgressBar {
+	log.Printf("ColoredUi: Using wrapped UI for progress bar.\n")
+	return u.Ui.GetProgressBar()
 }
 
 func (u *TargetedUI) Ask(query string) (string, error) {
@@ -173,10 +185,9 @@ func (u *TargetedUI) prefixLines(arrow bool, message string) string {
 	return strings.TrimRightFunc(result.String(), unicode.IsSpace)
 }
 
-func (u *TargetedUI) GetMinimumLength() int {
-	var dummy string
-	dummy = u.prefixLines(false, "")
-	return len(dummy) + len("\n")
+func (u *TargetedUI) GetProgressBar() ProgressBar {
+	log.Printf("TargetedUI: Using wrapped UI for progress bar.\n")
+	return u.Ui.GetProgressBar()
 }
 
 func (rw *BasicUi) Ask(query string) (string, error) {
@@ -271,8 +282,21 @@ func (rw *BasicUi) Machine(t string, args ...string) {
 	log.Printf("machine readable: %s %#v", t, args)
 }
 
-func (u *BasicUi) GetMinimumLength() int {
-	return 0
+func (rw *BasicUi) GetProgressBar() ProgressBar {
+	width := calculateProgressBarWidth(0)
+
+	log.Printf("BasicUi: Using progress bar width: %d\n", width)
+
+	bar := GetDefaultProgressBar()
+	bar.SetWidth(width)
+	bar.Callback = func(message string) {
+		rw.l.Lock()
+		defer rw.l.Unlock()
+
+		// Discard the error here so we don't emit an error everytime the progress-bar fails to update
+		fmt.Fprint(rw.Writer, message)
+	}
+	return bar
 }
 
 func (u *MachineReadableUi) Ask(query string) (string, error) {
@@ -321,6 +345,55 @@ func (u *MachineReadableUi) Machine(category string, args ...string) {
 	}
 }
 
-func (u *MachineReadableUi) GetMinimumLength() int {
-	return -1
+func (u *MachineReadableUi) GetProgressBar() ProgressBar {
+	log.Printf("MachineReadableUi: Using dummy progress bar.\n")
+	bar := GetDummyProgressBar()
+	return bar
+}
+
+// Return a dummy progress bar that doesn't do anything
+func GetDummyProgressBar() *pb.ProgressBar {
+	return pb.New64(0)
+}
+
+// Get a progress bar with the default appearance
+func GetDefaultProgressBar() *pb.ProgressBar {
+	bar := pb.New64(0)
+	bar.ShowPercent = true
+	bar.ShowCounters = true
+	bar.ShowSpeed = false
+	bar.ShowBar = true
+	bar.ShowTimeLeft = false
+	bar.ShowFinalTime = false
+	bar.SetUnits(pb.U_BYTES)
+	bar.Format("[=>-]")
+	bar.SetRefreshRate(1 * time.Second)
+	return bar
+}
+
+// Figure out the terminal dimensions and use it to calculate the available rendering space
+func calculateProgressBarWidth(length int) int {
+	// If the UI's width is signed, then this is an interface that doesn't really benefit from a progress bar
+	if length < 0 {
+		log.Println("Refusing to render progress-bar for unsupported UI.")
+		return length
+	}
+
+	// Figure out the terminal width if possible
+	width, _, err := GetTerminalDimensions()
+	if err != nil {
+		newerr := fmt.Errorf("Unable to determine terminal dimensions: %v", err)
+		log.Printf("Using default width (%d) for progress-bar due to error: %s", DefaultProgressBarWidth, newerr)
+		return DefaultProgressBarWidth
+	}
+
+	// If the terminal width is smaller than the requested length, then complain
+	if width < length {
+		newerr := fmt.Errorf("Terminal width (%d) is smaller than UI message width (%d).", width, length)
+		log.Printf("Using default width (%d) for progress-bar due to error: %s", DefaultProgressBarWidth, newerr)
+		return DefaultProgressBarWidth
+	}
+
+	// Otherwise subtract the minimum length and return it
+	return width - length
 }

packer/ui_posix.go

@@ -1,6 +1,6 @@
 // +build !windows
 
-package common
+package packer
 
 // Imports for determining terminal information across platforms
 import (

packer/ui_windows.go

@@ -1,6 +1,6 @@
 // +build windows
 
-package common
+package packer
 
 import (
 	"syscall"

provisioner/ansible-local/ui_stub.go

@@ -1,5 +1,7 @@
 package ansiblelocal
 
+import "github.com/hashicorp/packer/packer"
+
 type uiStub struct{}
 
 func (su *uiStub) Ask(string) (string, error) {
@@ -13,6 +15,6 @@ func (su *uiStub) Machine(string, ...string) {}
 func (su *uiStub) Message(string) {}
 
 func (su *uiStub) Say(msg string) {}
-func (su *uiStub) GetMinimumLength() int {
-	return -1
+func (su *uiStub) GetProcessBar() packer.ProgressBar {
+	return packer.GetDummyProgressBar()
 }

provisioner/ansible/adapter_test.go

@@ -123,8 +123,8 @@ func (u *ui) Machine(s1 string, s2 ...string) {
 	}
 }
 
-func (u *ui) GetMinimumLength() int {
-	return -1
+func (u *ui) GetProgressBar() packer.ProgressBar {
+	return packer.GetDummyProgressBar()
 }
 
 type communicator struct{}

provisioner/ansible/provisioner.go

@@ -600,6 +600,6 @@ func (ui *Ui) Machine(t string, args ...string) {
 	<-ui.sem
 }
 
-func (ui *Ui) GetMinimumLength() int {
-	return -1
+func (ui *Ui) GetProgressBar() packer.ProgressBar {
+	return packer.GetDummyProgressBar()
 }

provisioner/file/provisioner.go

@@ -127,7 +127,7 @@ func (p *Provisioner) ProvisionDownload(ui packer.Ui, comm packer.Communicator)
 		defer f.Close()
 
 		// Get a default progress bar
-		pb := common.GetNewProgressBar(&ui)
+		pb := ui.GetProgressBar()
 		bar := pb.Start()
 		defer bar.Finish()
 
@@ -176,7 +176,7 @@ func (p *Provisioner) ProvisionUpload(ui packer.Ui, comm packer.Communicator) er
 		}
 
 		// Get a default progress bar
-		pb := common.GetNewProgressBar(&ui)
+		pb := ui.GetProgressBar()
 		bar := pb.Start()
 		defer bar.Finish()
 

provisioner/file/provisioner_test.go

@@ -120,8 +120,8 @@ func (su *stubUi) Say(msg string) {
 	su.sayMessages += msg
 }
 
-func (su *stubUi) GetMinimumLength() int {
-	return -1
+func (su *stubUi) GetProgressBar() packer.ProgressBar {
+	return packer.GetDummyProgressBar()
 }
 
 func TestProvisionerProvision_SendsFile(t *testing.T) {