From 0f10032b3d64c040910156621667a6abcdd8b787 Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Fri, 10 Aug 2018 15:08:28 -0500 Subject: [PATCH] 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 | 5 +- common/download.go | 42 +++++--- common/progress.go | 61 ------------ common/step_download.go | 8 +- packer/rpc/ui.go | 8 +- packer/rpc/ui_test.go | 54 ++++++----- packer/ui.go | 95 ++++++++++++++++--- .../progress_posix.go => packer/ui_posix.go | 2 +- .../ui_windows.go | 2 +- provisioner/ansible-local/ui_stub.go | 6 +- provisioner/ansible/adapter_test.go | 4 +- provisioner/ansible/provisioner.go | 4 +- provisioner/file/provisioner.go | 4 +- provisioner/file/provisioner_test.go | 4 +- 14 files changed, 167 insertions(+), 132 deletions(-) delete mode 100644 common/progress.go rename common/progress_posix.go => packer/ui_posix.go (97%) rename common/progress_windows.go => packer/ui_windows.go (99%) diff --git a/common/config.go b/common/config.go index 1487ea292..1826e3a10 100644 --- a/common/config.go +++ b/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 diff --git a/common/download.go b/common/download.go index 62fcc908e..b80741f3f 100644 --- a/common/download.go +++ b/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 { diff --git a/common/progress.go b/common/progress.go deleted file mode 100644 index 476eff55d..000000000 --- a/common/progress.go +++ /dev/null @@ -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 -} diff --git a/common/step_download.go b/common/step_download.go index efd4607a7..e8520091d 100644 --- a/common/step_download.go +++ b/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) diff --git a/packer/rpc/ui.go b/packer/rpc/ui.go index 19553cc30..80508be82 100644 --- a/packer/rpc/ui.go +++ b/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 } diff --git a/packer/rpc/ui_test.go b/packer/rpc/ui_test.go index 42f9d6ae0..eef4be13c 100644 --- a/packer/rpc/ui_test.go +++ b/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 ui.getMinimumLengthValue != -1 { - t.Fatal("getminimumlength should be -1") + + if bar.Callback == nil { + t.Fatal("getprogressbar returned a bar with an empty callback") + } + bar.Callback("test") + if !ui.progessBarCallbackWasCalled { + t.Fatal("progressbarcallback should be called") } if ui.machineType != "foo" { diff --git a/packer/ui.go b/packer/ui.go index 73b07ea0e..cd88a1eab 100644 --- a/packer/ui.go +++ b/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 } diff --git a/common/progress_posix.go b/packer/ui_posix.go similarity index 97% rename from common/progress_posix.go rename to packer/ui_posix.go index 317307635..27c94e8ee 100644 --- a/common/progress_posix.go +++ b/packer/ui_posix.go @@ -1,6 +1,6 @@ // +build !windows -package common +package packer // Imports for determining terminal information across platforms import ( diff --git a/common/progress_windows.go b/packer/ui_windows.go similarity index 99% rename from common/progress_windows.go rename to packer/ui_windows.go index 676ab538d..ce99c3a35 100644 --- a/common/progress_windows.go +++ b/packer/ui_windows.go @@ -1,6 +1,6 @@ // +build windows -package common +package packer import ( "syscall" diff --git a/provisioner/ansible-local/ui_stub.go b/provisioner/ansible-local/ui_stub.go index 86d3cba86..bc2e40628 100644 --- a/provisioner/ansible-local/ui_stub.go +++ b/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() } diff --git a/provisioner/ansible/adapter_test.go b/provisioner/ansible/adapter_test.go index f8be403f5..92e669a97 100644 --- a/provisioner/ansible/adapter_test.go +++ b/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{} diff --git a/provisioner/ansible/provisioner.go b/provisioner/ansible/provisioner.go index 7b32e81ed..9cea3e435 100644 --- a/provisioner/ansible/provisioner.go +++ b/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() } diff --git a/provisioner/file/provisioner.go b/provisioner/file/provisioner.go index d734eaf06..616625a25 100644 --- a/provisioner/file/provisioner.go +++ b/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() diff --git a/provisioner/file/provisioner_test.go b/provisioner/file/provisioner_test.go index 6c749f1e5..b804f0e32 100644 --- a/provisioner/file/provisioner_test.go +++ b/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) {