From ddd96c513b9a3f86e2bd16b61dbda5d610624c38 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Tue, 4 Sep 2018 17:57:21 +0200 Subject: [PATCH 01/22] first draft at self refreshing loading bar centralized/controlled by Ui --- common/config.go | 6 ++- common/download.go | 57 +++++++++------------ common/download_test.go | 24 +++++---- common/progress.go | 7 +-- common/step_download.go | 5 +- packer/progressbar.go | 39 ++++++++++++++ packer/rpc/server.go | 3 +- packer/rpc/ui.go | 82 +++++++++++++++++++++++++++++- packer/ui.go | 29 +++++++++++ provisioner/ansible/provisioner.go | 5 ++ provisioner/file/provisioner.go | 12 ++--- 11 files changed, 211 insertions(+), 58 deletions(-) create mode 100644 packer/progressbar.go diff --git a/common/config.go b/common/config.go index 24c6d4233..9527af55b 100644 --- a/common/config.go +++ b/common/config.go @@ -9,6 +9,8 @@ import ( "runtime" "strings" "time" + + "github.com/hashicorp/packer/packer" ) // PackerKeyEnv is used to specify the key interval (delay) between keystrokes @@ -41,7 +43,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{}, nil) + cli := NewDownloadClient(&DownloadConfig{}, new(packer.NoopProgressBar)) // Iterate through each downloader to see if a protocol was found. ok := false @@ -173,7 +175,7 @@ func FileExistsLocally(original string) bool { // First create a dummy downloader so we can figure out which // protocol to use. - cli := NewDownloadClient(&DownloadConfig{}, nil) + cli := NewDownloadClient(&DownloadConfig{}, new(packer.NoopProgressBar)) d, ok := cli.config.DownloaderMap[u.Scheme] if !ok { return false diff --git a/common/download.go b/common/download.go index f0e88420b..21ff25165 100644 --- a/common/download.go +++ b/common/download.go @@ -10,19 +10,17 @@ import ( "errors" "fmt" "hash" + "io" "log" + "net/http" "net/url" "os" "path" + "path/filepath" "runtime" "strings" -) -// imports related to each Downloader implementation -import ( - "io" - "net/http" - "path/filepath" + "github.com/hashicorp/packer/packer" // imports related to each Downloader implementation ) // DownloadConfig is the configuration given to instantiate a new @@ -81,14 +79,9 @@ func HashForType(t string) hash.Hash { // NewDownloadClient returns a new DownloadClient for the given // configuration. -func NewDownloadClient(c *DownloadConfig, bar 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 = GetDummyProgressBar() - } - // Create downloader map if it hasn't been specified already. if c.DownloaderMap == nil { c.DownloaderMap = map[string]Downloader{ @@ -237,7 +230,7 @@ type HTTPDownloader struct { total uint64 userAgent string - progress ProgressBar + progress packer.ProgressBar } func (d *HTTPDownloader) Cancel() { @@ -331,9 +324,10 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { d.total = d.current + uint64(resp.ContentLength) bar := d.progress - bar.SetTotal64(int64(d.total)) - progressBar := bar.Start() - progressBar.Set64(int64(d.current)) + log.Printf("this %#v", bar) + log.Printf("that") + bar.Start(d.total) + bar.Set(d.current) var buffer [4096]byte for { @@ -343,7 +337,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { } d.current += uint64(n) - progressBar.Set64(int64(d.current)) + bar.Set(d.current) if _, werr := dst.Write(buffer[:n]); werr != nil { return werr @@ -353,7 +347,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { break } } - progressBar.Finish() + bar.Finish() return nil } @@ -374,7 +368,7 @@ type FileDownloader struct { current uint64 total uint64 - progress ProgressBar + progress packer.ProgressBar } func (d *FileDownloader) Progress() uint64 { @@ -466,9 +460,9 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { d.total = uint64(fi.Size()) bar := d.progress - bar.SetTotal64(int64(d.total)) - progressBar := bar.Start() - progressBar.Set64(int64(d.current)) + + bar.Start(d.total) + bar.Set(d.current) // no bufferSize specified, so copy synchronously. if d.bufferSize == nil { @@ -477,7 +471,7 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { d.active = false d.current += uint64(n) - progressBar.Set64(int64(d.current)) + bar.Set(d.current) // use a goro in case someone else wants to enable cancel/resume } else { @@ -490,7 +484,7 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { } d.current += uint64(n) - progressBar.Set64(int64(d.current)) + bar.Set(d.current) } d.active = false e <- err @@ -499,7 +493,7 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { // ...and we spin until it's done err = <-errch } - progressBar.Finish() + bar.Finish() f.Close() return err } @@ -513,7 +507,7 @@ type SMBDownloader struct { current uint64 total uint64 - progress ProgressBar + progress packer.ProgressBar } func (d *SMBDownloader) Progress() uint64 { @@ -587,9 +581,8 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { d.total = uint64(fi.Size()) bar := d.progress - bar.SetTotal64(int64(d.total)) - progressBar := bar.Start() - progressBar.Set64(int64(d.current)) + + bar.Start(d.current) // no bufferSize specified, so copy synchronously. if d.bufferSize == nil { @@ -598,7 +591,7 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { d.active = false d.current += uint64(n) - progressBar.Set64(int64(d.current)) + bar.Set(d.current) // use a goro in case someone else wants to enable cancel/resume } else { @@ -611,7 +604,7 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { } d.current += uint64(n) - progressBar.Set64(int64(d.current)) + bar.Set(d.current) } d.active = false e <- err @@ -620,7 +613,7 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { // ...and as usual we spin until it's done err = <-errch } - progressBar.Finish() + bar.Finish() f.Close() return err } diff --git a/common/download_test.go b/common/download_test.go index 153e8daf2..ea6d7585e 100644 --- a/common/download_test.go +++ b/common/download_test.go @@ -12,6 +12,8 @@ import ( "runtime" "strings" "testing" + + "github.com/hashicorp/packer/packer" ) func TestDownloadClientVerifyChecksum(t *testing.T) { @@ -36,7 +38,7 @@ func TestDownloadClientVerifyChecksum(t *testing.T) { Checksum: checksum, } - d := NewDownloadClient(config, nil) + d := NewDownloadClient(config, new(packer.NoopProgressBar)) result, err := d.VerifyChecksum(tf.Name()) if err != nil { t.Fatalf("Verify err: %s", err) @@ -59,7 +61,7 @@ func TestDownloadClient_basic(t *testing.T) { Url: ts.URL + "/basic.txt", TargetPath: tf.Name(), CopyFile: true, - }, nil) + }, new(packer.NoopProgressBar)) path, err := client.Get() if err != nil { @@ -95,7 +97,7 @@ func TestDownloadClient_checksumBad(t *testing.T) { Hash: HashForType("md5"), Checksum: checksum, CopyFile: true, - }, nil) + }, new(packer.NoopProgressBar)) if _, err := client.Get(); err == nil { t.Fatal("should error") @@ -121,7 +123,7 @@ func TestDownloadClient_checksumGood(t *testing.T) { Hash: HashForType("md5"), Checksum: checksum, CopyFile: true, - }, nil) + }, new(packer.NoopProgressBar)) path, err := client.Get() if err != nil { @@ -153,7 +155,7 @@ func TestDownloadClient_checksumNoDownload(t *testing.T) { Hash: HashForType("md5"), Checksum: checksum, CopyFile: true, - }, nil) + }, new(packer.NoopProgressBar)) path, err := client.Get() if err != nil { t.Fatalf("err: %s", err) @@ -183,7 +185,7 @@ func TestDownloadClient_notFound(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL + "/not-found.txt", TargetPath: tf.Name(), - }, nil) + }, new(packer.NoopProgressBar)) if _, err := client.Get(); err == nil { t.Fatal("should error") @@ -211,7 +213,7 @@ func TestDownloadClient_resume(t *testing.T) { Url: ts.URL, TargetPath: tf.Name(), CopyFile: true, - }, nil) + }, new(packer.NoopProgressBar)) path, err := client.Get() if err != nil { @@ -273,7 +275,7 @@ func TestDownloadClient_usesDefaultUserAgent(t *testing.T) { CopyFile: true, } - client := NewDownloadClient(config, nil) + client := NewDownloadClient(config, new(packer.NoopProgressBar)) _, err = client.Get() if err != nil { t.Fatal(err) @@ -306,7 +308,7 @@ func TestDownloadClient_setsUserAgent(t *testing.T) { CopyFile: true, } - client := NewDownloadClient(config, nil) + client := NewDownloadClient(config, new(packer.NoopProgressBar)) _, err = client.Get() if err != nil { t.Fatal(err) @@ -405,7 +407,7 @@ func TestDownloadFileUrl(t *testing.T) { CopyFile: false, } - client := NewDownloadClient(config, nil) + client := NewDownloadClient(config, new(packer.NoopProgressBar)) // Verify that we fail to match the checksum _, err = client.Get() @@ -436,7 +438,7 @@ func SimulateFileUriDownload(t *testing.T, uri string) (string, error) { } // go go go - client := NewDownloadClient(config, nil) + client := NewDownloadClient(config, new(packer.NoopProgressBar)) path, err := client.Get() // ignore any non-important checksum errors if it's not a unc path diff --git a/common/progress.go b/common/progress.go index 83b6d0e2b..6cb91b08b 100644 --- a/common/progress.go +++ b/common/progress.go @@ -2,13 +2,14 @@ package common import ( "fmt" + "log" + "reflect" + "time" + "github.com/cheggaaa/pb" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/packer/rpc" - "log" - "reflect" - "time" ) // This is the arrow from packer/ui.go -> TargetedUI.prefixLines diff --git a/common/step_download.go b/common/step_download.go index c55e12632..26f6d6b65 100644 --- a/common/step_download.go +++ b/common/step_download.go @@ -64,7 +64,7 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste ui.Say(fmt.Sprintf("Retrieving %s", s.Description)) // Get a progress bar from the ui so we can hand it off to the download client - bar := GetProgressBar(ui, GetPackerConfigFromStateBag(state)) + bar := ui.ProgressBar() // First try to use any already downloaded file // If it fails, proceed to regular download logic @@ -144,7 +144,8 @@ func (s *StepDownload) download(config *DownloadConfig, state multistep.StateBag ui := state.Get("ui").(packer.Ui) // Get a progress bar and hand it off to the download client - bar := GetProgressBar(ui, GetPackerConfigFromStateBag(state)) + bar := ui.ProgressBar() + log.Printf("new progress bar: %#v, %t", bar, bar == nil) // Create download client with config and progress bar download := NewDownloadClient(config, bar) diff --git a/packer/progressbar.go b/packer/progressbar.go new file mode 100644 index 000000000..1f3784fc1 --- /dev/null +++ b/packer/progressbar.go @@ -0,0 +1,39 @@ +package packer + +import ( + "github.com/cheggaaa/pb" +) + +// ProgressBar allows to graphically display +// a self refreshing progress bar. +// No-op When in machine readable mode. +type ProgressBar interface { + Start(total uint64) + Set(current uint64) + Finish() +} + +type BasicProgressBar struct { + *pb.ProgressBar +} + +func (bpb *BasicProgressBar) Start(total uint64) { + bpb.SetTotal64(int64(total)) + bpb.ProgressBar.Start() +} + +func (bpb *BasicProgressBar) Set(current uint64) { + bpb.ProgressBar.Set64(int64(current)) +} + +var _ ProgressBar = new(BasicProgressBar) + +// NoopProgressBar is a silent progress bar +type NoopProgressBar struct { +} + +func (bpb *NoopProgressBar) Start(_ uint64) {} +func (bpb *NoopProgressBar) Set(_ uint64) {} +func (bpb *NoopProgressBar) Finish() {} + +var _ ProgressBar = new(NoopProgressBar) diff --git a/packer/rpc/server.go b/packer/rpc/server.go index 8982ec0e7..dff8854e2 100644 --- a/packer/rpc/server.go +++ b/packer/rpc/server.go @@ -114,7 +114,8 @@ func (s *Server) RegisterProvisioner(p packer.Provisioner) { func (s *Server) RegisterUi(ui packer.Ui) { s.server.RegisterName(DefaultUiEndpoint, &UiServer{ - ui: ui, + ui: ui, + register: s.server.RegisterName, }) } diff --git a/packer/rpc/ui.go b/packer/rpc/ui.go index 1c6356a65..50dce61fa 100644 --- a/packer/rpc/ui.go +++ b/packer/rpc/ui.go @@ -2,6 +2,7 @@ package rpc import ( "log" + "math/rand" "net/rpc" "github.com/hashicorp/packer/packer" @@ -14,10 +15,13 @@ type Ui struct { endpoint string } +var _ packer.Ui = new(Ui) + // UiServer wraps a packer.Ui implementation and makes it exportable // as part of a Golang RPC server. type UiServer struct { - ui packer.Ui + ui packer.Ui + register func(name string, rcvr interface{}) error } // The arguments sent to Ui.Machine @@ -60,6 +64,38 @@ func (u *Ui) Say(message string) { } } +func (u *Ui) ProgressBar() packer.ProgressBar { + var callMeMaybe string + if err := u.client.Call("Ui.ProgressBar", nil, &callMeMaybe); err != nil { + log.Printf("Error in Ui RPC call: %s", err) + return new(packer.NoopProgressBar) + } + + return &RemoteProgressBarClient{ + id: callMeMaybe, + client: u.client, + } +} + +type RemoteProgressBarClient struct { + id string + client *rpc.Client +} + +var _ packer.ProgressBar = new(RemoteProgressBarClient) + +func (pb *RemoteProgressBarClient) Start(total uint64) { + pb.client.Call(pb.id+".Start", total, new(interface{})) +} + +func (pb *RemoteProgressBarClient) Set(current uint64) { + pb.client.Call(pb.id+".Set", current, new(interface{})) +} + +func (pb *RemoteProgressBarClient) Finish() { + pb.client.Call(pb.id+".Finish", nil, new(interface{})) +} + func (u *UiServer) Ask(query string, reply *string) (err error) { *reply, err = u.ui.Ask(query) return @@ -91,3 +127,47 @@ func (u *UiServer) Say(message *string, reply *interface{}) error { *reply = nil return nil } + +func RandStringBytes(n int) string { + const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" + + b := make([]byte, n) + for i := range b { + b[i] = letterBytes[rand.Intn(len(letterBytes))] + } + return string(b) +} + +func (u *UiServer) ProgressBar(_ *string, reply *interface{}) error { + bar := u.ui.ProgressBar() + + callbackName := RandStringBytes(6) + + log.Printf("registering progressbar %s", callbackName) + err := u.register(callbackName, &RemoteProgressBarServer{bar}) + if err != nil { + log.Printf("failed to register a new progress bar rpc server, %s", err) + return err + } + *reply = callbackName + return nil +} + +type RemoteProgressBarServer struct { + pb packer.ProgressBar +} + +func (pb *RemoteProgressBarServer) Finish(_ string, _ *interface{}) error { + pb.pb.Finish() + return nil +} + +func (pb *RemoteProgressBarServer) Start(total uint64, _ *interface{}) error { + pb.pb.Start(total) + return nil +} + +func (pb *RemoteProgressBarServer) Set(current uint64, _ *interface{}) error { + pb.pb.Set(current) + return nil +} diff --git a/packer/ui.go b/packer/ui.go index c16a2eae0..e6ceba0e3 100644 --- a/packer/ui.go +++ b/packer/ui.go @@ -15,6 +15,8 @@ import ( "syscall" "time" "unicode" + + "github.com/cheggaaa/pb" ) type UiColor uint @@ -37,6 +39,7 @@ type Ui interface { Message(string) Error(string) Machine(string, ...string) + ProgressBar() ProgressBar } // ColoredUi is a UI that is colored using terminal colors. @@ -46,6 +49,8 @@ type ColoredUi struct { Ui Ui } +var _ Ui = new(ColoredUi) + // TargetedUI is a UI that wraps another UI implementation and modifies // the output to indicate a specific target. Specifically, all Say output // is prefixed with the target name. Message output is not prefixed but @@ -56,6 +61,8 @@ type TargetedUI struct { Ui Ui } +var _ Ui = new(TargetedUI) + // The BasicUI is a UI that reads and writes from a standard Go reader // and writer. It is safe to be called from multiple goroutines. Machine // readable output is simply logged for this UI. @@ -68,12 +75,21 @@ type BasicUi struct { scanner *bufio.Scanner } +var _ Ui = new(BasicUi) + +func (bu *BasicUi) ProgressBar() ProgressBar { + log.Printf("hehey !") + return &BasicProgressBar{ProgressBar: pb.New(0)} +} + // MachineReadableUi is a UI that only outputs machine-readable output // to the given Writer. type MachineReadableUi struct { Writer io.Writer } +var _ Ui = new(MachineReadableUi) + func (u *ColoredUi) Ask(query string) (string, error) { return u.Ui.Ask(u.colorize(query, u.Color, true)) } @@ -100,6 +116,10 @@ func (u *ColoredUi) Machine(t string, args ...string) { u.Ui.Machine(t, args...) } +func (u *ColoredUi) ProgressBar() ProgressBar { + return u.Ui.ProgressBar() //TODO(adrien): color me +} + func (u *ColoredUi) colorize(message string, color UiColor, bold bool) string { if !u.supportsColors() { return message @@ -153,6 +173,10 @@ func (u *TargetedUI) Machine(t string, args ...string) { u.Ui.Machine(fmt.Sprintf("%s,%s", u.Target, t), args...) } +func (u *TargetedUI) ProgressBar() ProgressBar { + return u.Ui.ProgressBar() +} + func (u *TargetedUI) prefixLines(arrow bool, message string) string { arrowText := "==>" if !arrow { @@ -305,3 +329,8 @@ func (u *MachineReadableUi) Machine(category string, args ...string) { } } } + +func (u *MachineReadableUi) ProgressBar() ProgressBar { + panic("MachineReadableUi") + return nil // no-op +} diff --git a/provisioner/ansible/provisioner.go b/provisioner/ansible/provisioner.go index 574ec2b39..d8d82289f 100644 --- a/provisioner/ansible/provisioner.go +++ b/provisioner/ansible/provisioner.go @@ -612,3 +612,8 @@ func (ui *Ui) Machine(t string, args ...string) { ui.ui.Machine(t, args...) <-ui.sem } + +func (ui *Ui) ProgressBar() packer.ProgressBar { + panic("to implement") + return nil // TODO +} diff --git a/provisioner/file/provisioner.go b/provisioner/file/provisioner.go index 13e2aa8e6..a05c2ed9c 100644 --- a/provisioner/file/provisioner.go +++ b/provisioner/file/provisioner.go @@ -127,12 +127,12 @@ func (p *Provisioner) ProvisionDownload(ui packer.Ui, comm packer.Communicator) defer f.Close() // Get a default progress bar - pb := common.GetProgressBar(ui, &p.config.PackerConfig) - bar := pb.Start() - defer bar.Finish() + pb := packer.NoopProgressBar{} + pb.Start(0) + defer pb.Finish() // Create MultiWriter for the current progress - pf := io.MultiWriter(f, bar) + pf := io.MultiWriter(f) // Download the file if err = comm.Download(src, pf); err != nil { @@ -176,8 +176,8 @@ func (p *Provisioner) ProvisionUpload(ui packer.Ui, comm packer.Communicator) er } // Get a default progress bar - pb := common.GetProgressBar(ui, &p.config.PackerConfig) - bar := pb.Start() + bar := ui.ProgressBar() + bar.Start(0) defer bar.Finish() // Create ProxyReader for the current progress From fd7cb47adcf8d8c40204dfdb65ca5cf9da2303ed Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 12:50:53 +0200 Subject: [PATCH 02/22] use proxy reader for download progress & stop storing total/current in downloaders --- common/download.go | 118 ++++++++++---------------------- packer/progressbar.go | 50 ++++++++++++-- packer/rpc/ui.go | 15 ++-- provisioner/file/provisioner.go | 2 +- 4 files changed, 90 insertions(+), 95 deletions(-) diff --git a/common/download.go b/common/download.go index 21ff25165..de018f9bd 100644 --- a/common/download.go +++ b/common/download.go @@ -85,10 +85,10 @@ func NewDownloadClient(c *DownloadConfig, bar packer.ProgressBar) *DownloadClien // 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{progressBar: bar, bufferSize: nil}, + "http": &HTTPDownloader{progressBar: bar, userAgent: c.UserAgent}, + "https": &HTTPDownloader{progressBar: bar, userAgent: c.UserAgent}, + "smb": &SMBDownloader{progressBar: bar, bufferSize: nil}, } } return &DownloadClient{config: c} @@ -99,8 +99,7 @@ func NewDownloadClient(c *DownloadConfig, bar packer.ProgressBar) *DownloadClien type Downloader interface { Resume() Cancel() - Progress() uint64 - Total() uint64 + ProgressBar() packer.ProgressBar } // A LocalDownloader is responsible for converting a uri to a local path @@ -226,11 +225,9 @@ func (d *DownloadClient) VerifyChecksum(path string) (bool, error) { // HTTPDownloader is an implementation of Downloader that downloads // files over HTTP. type HTTPDownloader struct { - current uint64 - total uint64 userAgent string - progress packer.ProgressBar + progressBar packer.ProgressBar } func (d *HTTPDownloader) Cancel() { @@ -249,8 +246,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return err } - // Reset our progress - d.current = 0 + var current uint64 // Make the request. We first make a HEAD request so we can check // if the server supports range queries. If the server/URL doesn't @@ -294,7 +290,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { if _, err = dst.Seek(0, os.SEEK_END); err == nil { req.Header.Set("Range", fmt.Sprintf("bytes=%d-", fi.Size())) - d.current = uint64(fi.Size()) + current = uint64(fi.Size()) } } } @@ -321,24 +317,21 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return fmt.Errorf("HTTP error: %s", err.Error()) } - d.total = d.current + uint64(resp.ContentLength) + total := current + uint64(resp.ContentLength) - bar := d.progress - log.Printf("this %#v", bar) - log.Printf("that") - bar.Start(d.total) - bar.Set(d.current) + bar := d.ProgressBar() + bar.Start(total) + bar.Add(current) + + body := bar.NewProxyReader(resp.Body) var buffer [4096]byte for { - n, err := resp.Body.Read(buffer[:]) + n, err := body.Read(buffer[:]) if err != nil && err != io.EOF { return err } - d.current += uint64(n) - bar.Set(d.current) - if _, werr := dst.Write(buffer[:n]); werr != nil { return werr } @@ -351,32 +344,13 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return nil } -func (d *HTTPDownloader) Progress() uint64 { - return d.current -} - -func (d *HTTPDownloader) Total() uint64 { - return d.total -} - // FileDownloader is an implementation of Downloader that downloads // files using the regular filesystem. type FileDownloader struct { bufferSize *uint - active bool - current uint64 - total uint64 - - progress packer.ProgressBar -} - -func (d *FileDownloader) Progress() uint64 { - return d.current -} - -func (d *FileDownloader) Total() uint64 { - return d.total + active bool + progressBar packer.ProgressBar } func (d *FileDownloader) Cancel() { @@ -443,7 +417,6 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { } /* download the file using the operating system's facilities */ - d.current = 0 d.active = true f, err := os.Open(realpath) @@ -457,38 +430,31 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { if err != nil { return err } - d.total = uint64(fi.Size()) - bar := d.progress + bar := d.ProgressBar() - bar.Start(d.total) - bar.Set(d.current) + bar.Start(uint64(fi.Size())) + fProxy := bar.NewProxyReader(f) // no bufferSize specified, so copy synchronously. if d.bufferSize == nil { - var n int64 - n, err = io.Copy(dst, f) + _, err = io.Copy(dst, fProxy) d.active = false - d.current += uint64(n) - bar.Set(d.current) - // use a goro in case someone else wants to enable cancel/resume } else { errch := make(chan error) go func(d *FileDownloader, r io.Reader, w io.Writer, e chan error) { for d.active { - n, err := io.CopyN(w, r, int64(*d.bufferSize)) + _, err := io.CopyN(w, r, int64(*d.bufferSize)) if err != nil { break } - d.current += uint64(n) - bar.Set(d.current) } d.active = false e <- err - }(d, f, dst, errch) + }(d, fProxy, dst, errch) // ...and we spin until it's done err = <-errch @@ -503,19 +469,8 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { type SMBDownloader struct { bufferSize *uint - active bool - current uint64 - total uint64 - - progress packer.ProgressBar -} - -func (d *SMBDownloader) Progress() uint64 { - return d.current -} - -func (d *SMBDownloader) Total() uint64 { - return d.total + active bool + progressBar packer.ProgressBar } func (d *SMBDownloader) Cancel() { @@ -564,7 +519,6 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { } /* Open up the "\\"-prefixed path using the Windows filesystem */ - d.current = 0 d.active = true f, err := os.Open(realpath) @@ -578,37 +532,31 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { if err != nil { return err } - d.total = uint64(fi.Size()) - bar := d.progress + bar := d.ProgressBar() - bar.Start(d.current) + bar.Start(uint64(fi.Size())) + fProxy := bar.NewProxyReader(f) // no bufferSize specified, so copy synchronously. if d.bufferSize == nil { - var n int64 - n, err = io.Copy(dst, f) + _, err = io.Copy(dst, fProxy) d.active = false - d.current += uint64(n) - bar.Set(d.current) - // use a goro in case someone else wants to enable cancel/resume } else { errch := make(chan error) go func(d *SMBDownloader, r io.Reader, w io.Writer, e chan error) { for d.active { - n, err := io.CopyN(w, r, int64(*d.bufferSize)) + _, err := io.CopyN(w, r, int64(*d.bufferSize)) if err != nil { break } - d.current += uint64(n) - bar.Set(d.current) } d.active = false e <- err - }(d, f, dst, errch) + }(d, fProxy, dst, errch) // ...and as usual we spin until it's done err = <-errch @@ -617,3 +565,7 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { f.Close() return err } + +func (d *HTTPDownloader) ProgressBar() packer.ProgressBar { return d.progressBar } +func (d *FileDownloader) ProgressBar() packer.ProgressBar { return d.progressBar } +func (d *SMBDownloader) ProgressBar() packer.ProgressBar { return d.progressBar } diff --git a/packer/progressbar.go b/packer/progressbar.go index 1f3784fc1..5a70f25a4 100644 --- a/packer/progressbar.go +++ b/packer/progressbar.go @@ -1,6 +1,8 @@ package packer import ( + "io" + "github.com/cheggaaa/pb" ) @@ -9,7 +11,8 @@ import ( // No-op When in machine readable mode. type ProgressBar interface { Start(total uint64) - Set(current uint64) + Add(current uint64) + NewProxyReader(r io.Reader) (proxy io.Reader) Finish() } @@ -22,8 +25,20 @@ func (bpb *BasicProgressBar) Start(total uint64) { bpb.ProgressBar.Start() } -func (bpb *BasicProgressBar) Set(current uint64) { - bpb.ProgressBar.Set64(int64(current)) +func (bpb *BasicProgressBar) Add(current uint64) { + bpb.ProgressBar.Add64(int64(current)) +} +func (bpb *BasicProgressBar) NewProxyReader(r io.Reader) io.Reader { + return &ProxyReader{ + Reader: r, + ProgressBar: bpb, + } +} +func (bpb *BasicProgressBar) NewProxyReadCloser(r io.ReadCloser) io.ReadCloser { + return &ProxyReader{ + Reader: r, + ProgressBar: bpb, + } } var _ ProgressBar = new(BasicProgressBar) @@ -32,8 +47,31 @@ var _ ProgressBar = new(BasicProgressBar) type NoopProgressBar struct { } -func (bpb *NoopProgressBar) Start(_ uint64) {} -func (bpb *NoopProgressBar) Set(_ uint64) {} -func (bpb *NoopProgressBar) Finish() {} +func (npb *NoopProgressBar) Start(uint64) {} +func (npb *NoopProgressBar) Add(uint64) {} +func (npb *NoopProgressBar) Finish() {} +func (npb *NoopProgressBar) NewProxyReader(r io.Reader) io.Reader { return r } +func (npb *NoopProgressBar) NewProxyReadCloser(r io.ReadCloser) io.ReadCloser { return r } var _ ProgressBar = new(NoopProgressBar) + +// ProxyReader implements io.ReadCloser but sends +// count of read bytes to progress bar +type ProxyReader struct { + io.Reader + ProgressBar +} + +func (r *ProxyReader) Read(p []byte) (n int, err error) { + n, err = r.Reader.Read(p) + r.ProgressBar.Add(uint64(n)) + return +} + +// Close the reader if it implements io.Closer +func (r *ProxyReader) Close() (err error) { + if closer, ok := r.Reader.(io.Closer); ok { + return closer.Close() + } + return +} diff --git a/packer/rpc/ui.go b/packer/rpc/ui.go index 50dce61fa..c1ed2435f 100644 --- a/packer/rpc/ui.go +++ b/packer/rpc/ui.go @@ -1,6 +1,7 @@ package rpc import ( + "io" "log" "math/rand" "net/rpc" @@ -88,14 +89,18 @@ func (pb *RemoteProgressBarClient) Start(total uint64) { pb.client.Call(pb.id+".Start", total, new(interface{})) } -func (pb *RemoteProgressBarClient) Set(current uint64) { - pb.client.Call(pb.id+".Set", current, new(interface{})) +func (pb *RemoteProgressBarClient) Add(current uint64) { + pb.client.Call(pb.id+".Add", current, new(interface{})) } func (pb *RemoteProgressBarClient) Finish() { pb.client.Call(pb.id+".Finish", nil, new(interface{})) } +func (pb *RemoteProgressBarClient) NewProxyReader(r io.Reader) io.Reader { + return &packer.ProxyReader{Reader: r, ProgressBar: pb} +} + func (u *UiServer) Ask(query string, reply *string) (err error) { *reply, err = u.ui.Ask(query) return @@ -128,7 +133,7 @@ func (u *UiServer) Say(message *string, reply *interface{}) error { return nil } -func RandStringBytes(n int) string { +func RandStringBytes(n int) string { // TODO(azr): remove before merging const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" b := make([]byte, n) @@ -167,7 +172,7 @@ func (pb *RemoteProgressBarServer) Start(total uint64, _ *interface{}) error { return nil } -func (pb *RemoteProgressBarServer) Set(current uint64, _ *interface{}) error { - pb.pb.Set(current) +func (pb *RemoteProgressBarServer) Add(current uint64, _ *interface{}) error { + pb.pb.Add(current) return nil } diff --git a/provisioner/file/provisioner.go b/provisioner/file/provisioner.go index a05c2ed9c..474e87596 100644 --- a/provisioner/file/provisioner.go +++ b/provisioner/file/provisioner.go @@ -177,7 +177,7 @@ func (p *Provisioner) ProvisionUpload(ui packer.Ui, comm packer.Communicator) er // Get a default progress bar bar := ui.ProgressBar() - bar.Start(0) + bar.Start(uint64(info.Size())) defer bar.Finish() // Create ProxyReader for the current progress From 9b07d7670ed5dff98365df03330410700cdcbe1a Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 14:02:09 +0200 Subject: [PATCH 03/22] use no ops for ansible ui & MachineReadableUi --- packer/ui.go | 3 +-- provisioner/ansible/provisioner.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packer/ui.go b/packer/ui.go index e6ceba0e3..e9a98b24c 100644 --- a/packer/ui.go +++ b/packer/ui.go @@ -331,6 +331,5 @@ func (u *MachineReadableUi) Machine(category string, args ...string) { } func (u *MachineReadableUi) ProgressBar() ProgressBar { - panic("MachineReadableUi") - return nil // no-op + return new(NoopProgressBar) } diff --git a/provisioner/ansible/provisioner.go b/provisioner/ansible/provisioner.go index d8d82289f..d2528218d 100644 --- a/provisioner/ansible/provisioner.go +++ b/provisioner/ansible/provisioner.go @@ -614,6 +614,5 @@ func (ui *Ui) Machine(t string, args ...string) { } func (ui *Ui) ProgressBar() packer.ProgressBar { - panic("to implement") - return nil // TODO + return new(packer.NoopProgressBar) } From 541c68aed556052da94119d8ef710375210e9d94 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 17:15:54 +0200 Subject: [PATCH 04/22] add StackableProgressBar struct that will refresh/show dl status for multiple files * simplified the downloader interface, and removed the total/current values from them * downloaders use a proxy reader that will add all read bytes to progress * removed unused const mtu * DownloadClient doesn't need a downloader, so I removed it too --- common/config.go | 4 ++-- common/download.go | 45 +++++++++++++++++++---------------------- common/step_download.go | 21 ++++++++----------- packer/progressbar.go | 45 ++++++++++++++++++++++++++++++++++++----- packer/ui.go | 17 ++++++++++++---- 5 files changed, 84 insertions(+), 48 deletions(-) diff --git a/common/config.go b/common/config.go index 9527af55b..70ebc8452 100644 --- a/common/config.go +++ b/common/config.go @@ -43,7 +43,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{}, new(packer.NoopProgressBar)) + cli := NewDownloadClient(&DownloadConfig{}, new(packer.NoopUi)) // Iterate through each downloader to see if a protocol was found. ok := false @@ -175,7 +175,7 @@ func FileExistsLocally(original string) bool { // First create a dummy downloader so we can figure out which // protocol to use. - cli := NewDownloadClient(&DownloadConfig{}, new(packer.NoopProgressBar)) + cli := NewDownloadClient(&DownloadConfig{}, new(packer.NoopUi)) d, ok := cli.config.DownloaderMap[u.Scheme] if !ok { return false diff --git a/common/download.go b/common/download.go index de018f9bd..39ffa5e17 100644 --- a/common/download.go +++ b/common/download.go @@ -56,8 +56,7 @@ type DownloadConfig struct { // A DownloadClient helps download, verify checksums, etc. type DownloadClient struct { - config *DownloadConfig - downloader Downloader + config *DownloadConfig } // HashForType returns the Hash implementation for the given string @@ -79,23 +78,21 @@ func HashForType(t string) hash.Hash { // NewDownloadClient returns a new DownloadClient for the given // configuration. -func NewDownloadClient(c *DownloadConfig, bar packer.ProgressBar) *DownloadClient { - const mtu = 1500 /* ethernet */ - 20 /* ipv4 */ - 20 /* tcp */ - +func NewDownloadClient(c *DownloadConfig, ui packer.Ui) *DownloadClient { // Create downloader map if it hasn't been specified already. if c.DownloaderMap == nil { + log.Printf("instantiating. ui: %#v", ui) c.DownloaderMap = map[string]Downloader{ - "file": &FileDownloader{progressBar: bar, bufferSize: nil}, - "http": &HTTPDownloader{progressBar: bar, userAgent: c.UserAgent}, - "https": &HTTPDownloader{progressBar: bar, userAgent: c.UserAgent}, - "smb": &SMBDownloader{progressBar: bar, bufferSize: nil}, + "file": &FileDownloader{Ui: ui, bufferSize: nil}, + "http": &HTTPDownloader{Ui: ui, userAgent: c.UserAgent}, + "https": &HTTPDownloader{Ui: ui, userAgent: c.UserAgent}, + "smb": &SMBDownloader{Ui: ui, bufferSize: nil}, } } return &DownloadClient{config: c} } -// A downloader implements the ability to transfer a file, and cancel or resume -// it. +// Downloader defines what capabilities a downloader should have. type Downloader interface { Resume() Cancel() @@ -142,17 +139,18 @@ func (d *DownloadClient) Get() (string, error) { var finalPath string var ok bool - d.downloader, ok = d.config.DownloaderMap[u.Scheme] + downloader, ok := d.config.DownloaderMap[u.Scheme] if !ok { return "", fmt.Errorf("No downloader for scheme: %s", u.Scheme) } + log.Printf("downloader: %#v", downloader) - remote, ok := d.downloader.(RemoteDownloader) + remote, ok := downloader.(RemoteDownloader) if !ok { - return "", fmt.Errorf("Unable to treat uri scheme %s as a Downloader. : %T", u.Scheme, d.downloader) + return "", fmt.Errorf("Unable to treat uri scheme %s as a Downloader. : %T", u.Scheme, downloader) } - local, ok := d.downloader.(LocalDownloader) + local, ok := downloader.(LocalDownloader) if !ok && !d.config.CopyFile { d.config.CopyFile = true } @@ -167,7 +165,6 @@ func (d *DownloadClient) Get() (string, error) { return "", err } - log.Printf("[DEBUG] Downloading: %s", u.String()) err = remote.Download(f, u) f.Close() if err != nil { @@ -227,7 +224,7 @@ func (d *DownloadClient) VerifyChecksum(path string) (bool, error) { type HTTPDownloader struct { userAgent string - progressBar packer.ProgressBar + Ui packer.Ui } func (d *HTTPDownloader) Cancel() { @@ -349,8 +346,8 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { type FileDownloader struct { bufferSize *uint - active bool - progressBar packer.ProgressBar + active bool + Ui packer.Ui } func (d *FileDownloader) Cancel() { @@ -469,8 +466,8 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { type SMBDownloader struct { bufferSize *uint - active bool - progressBar packer.ProgressBar + active bool + Ui packer.Ui } func (d *SMBDownloader) Cancel() { @@ -566,6 +563,6 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { return err } -func (d *HTTPDownloader) ProgressBar() packer.ProgressBar { return d.progressBar } -func (d *FileDownloader) ProgressBar() packer.ProgressBar { return d.progressBar } -func (d *SMBDownloader) ProgressBar() packer.ProgressBar { return d.progressBar } +func (d *HTTPDownloader) ProgressBar() packer.ProgressBar { return d.Ui.ProgressBar() } +func (d *FileDownloader) ProgressBar() packer.ProgressBar { return d.Ui.ProgressBar() } +func (d *SMBDownloader) ProgressBar() packer.ProgressBar { return d.Ui.ProgressBar() } diff --git a/common/step_download.go b/common/step_download.go index 26f6d6b65..16da1626f 100644 --- a/common/step_download.go +++ b/common/step_download.go @@ -63,9 +63,6 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste ui.Say(fmt.Sprintf("Retrieving %s", s.Description)) - // Get a progress bar from the ui so we can hand it off to the download client - bar := ui.ProgressBar() - // First try to use any already downloaded file // If it fails, proceed to regular download logic @@ -99,7 +96,7 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste } downloadConfigs[i] = config - if match, _ := NewDownloadClient(config, bar).VerifyChecksum(config.TargetPath); match { + if match, _ := NewDownloadClient(config, ui).VerifyChecksum(config.TargetPath); match { ui.Message(fmt.Sprintf("Found already downloaded, initial checksum matched, no download needed: %s", url)) finalPath = config.TargetPath break @@ -141,14 +138,14 @@ func (s *StepDownload) Cleanup(multistep.StateBag) {} func (s *StepDownload) download(config *DownloadConfig, state multistep.StateBag) (string, error, bool) { var path string - ui := state.Get("ui").(packer.Ui) + v, ok := state.GetOk("ui") + if !ok { + return "", nil, false + } + ui := v.(packer.Ui) - // Get a progress bar and hand it off to the download client - bar := ui.ProgressBar() - log.Printf("new progress bar: %#v, %t", bar, bar == nil) - - // Create download client with config and progress bar - download := NewDownloadClient(config, bar) + // Create download client with config + download := NewDownloadClient(config, ui) downloadCompleteCh := make(chan error, 1) go func() { @@ -160,7 +157,6 @@ func (s *StepDownload) download(config *DownloadConfig, state multistep.StateBag for { select { case err := <-downloadCompleteCh: - bar.Finish() if err != nil { return "", err, true @@ -175,7 +171,6 @@ func (s *StepDownload) download(config *DownloadConfig, state multistep.StateBag case <-time.After(1 * time.Second): if _, ok := state.GetOk(multistep.StateCancelled); ok { - bar.Finish() ui.Say("Interrupt received. Cancelling download...") return "", nil, false } diff --git a/packer/progressbar.go b/packer/progressbar.go index 5a70f25a4..2a1ed97d6 100644 --- a/packer/progressbar.go +++ b/packer/progressbar.go @@ -2,13 +2,14 @@ package packer import ( "io" + "sync" + "sync/atomic" "github.com/cheggaaa/pb" ) // ProgressBar allows to graphically display // a self refreshing progress bar. -// No-op When in machine readable mode. type ProgressBar interface { Start(total uint64) Add(current uint64) @@ -16,10 +17,46 @@ type ProgressBar interface { Finish() } +type StackableProgressBar struct { + total uint64 + started bool + BasicProgressBar + startOnce sync.Once + group sync.WaitGroup +} + +var _ ProgressBar = new(StackableProgressBar) + +func (spb *StackableProgressBar) start() { + spb.BasicProgressBar.ProgressBar = pb.New(0) + spb.BasicProgressBar.ProgressBar.SetUnits(pb.U_BYTES) + + spb.BasicProgressBar.ProgressBar.Start() + go func() { + spb.group.Wait() + spb.BasicProgressBar.ProgressBar.Finish() + spb.startOnce = sync.Once{} + spb.BasicProgressBar.ProgressBar = nil + }() +} + +func (spb *StackableProgressBar) Start(total uint64) { + atomic.AddUint64(&spb.total, total) + spb.group.Add(1) + spb.startOnce.Do(spb.start) + spb.SetTotal64(int64(spb.total)) +} + +func (spb *StackableProgressBar) Finish() { + spb.group.Done() +} + type BasicProgressBar struct { *pb.ProgressBar } +var _ ProgressBar = new(BasicProgressBar) + func (bpb *BasicProgressBar) Start(total uint64) { bpb.SetTotal64(int64(total)) bpb.ProgressBar.Start() @@ -41,20 +78,18 @@ func (bpb *BasicProgressBar) NewProxyReadCloser(r io.ReadCloser) io.ReadCloser { } } -var _ ProgressBar = new(BasicProgressBar) - // NoopProgressBar is a silent progress bar type NoopProgressBar struct { } +var _ ProgressBar = new(NoopProgressBar) + func (npb *NoopProgressBar) Start(uint64) {} func (npb *NoopProgressBar) Add(uint64) {} func (npb *NoopProgressBar) Finish() {} func (npb *NoopProgressBar) NewProxyReader(r io.Reader) io.Reader { return r } func (npb *NoopProgressBar) NewProxyReadCloser(r io.ReadCloser) io.ReadCloser { return r } -var _ ProgressBar = new(NoopProgressBar) - // ProxyReader implements io.ReadCloser but sends // count of read bytes to progress bar type ProxyReader struct { diff --git a/packer/ui.go b/packer/ui.go index e9a98b24c..340c56827 100644 --- a/packer/ui.go +++ b/packer/ui.go @@ -15,8 +15,6 @@ import ( "syscall" "time" "unicode" - - "github.com/cheggaaa/pb" ) type UiColor uint @@ -42,6 +40,17 @@ type Ui interface { ProgressBar() ProgressBar } +type NoopUi struct{} + +var _ Ui = new(NoopUi) + +func (*NoopUi) Ask(string) (string, error) { return "", errors.New("this is a noop ui") } +func (*NoopUi) Say(string) { return } +func (*NoopUi) Message(string) { return } +func (*NoopUi) Error(string) { return } +func (*NoopUi) Machine(string, ...string) { return } +func (*NoopUi) ProgressBar() ProgressBar { return new(NoopProgressBar) } + // ColoredUi is a UI that is colored using terminal colors. type ColoredUi struct { Color UiColor @@ -73,13 +82,13 @@ type BasicUi struct { l sync.Mutex interrupted bool scanner *bufio.Scanner + StackableProgressBar } var _ Ui = new(BasicUi) func (bu *BasicUi) ProgressBar() ProgressBar { - log.Printf("hehey !") - return &BasicProgressBar{ProgressBar: pb.New(0)} + return &bu.StackableProgressBar } // MachineReadableUi is a UI that only outputs machine-readable output From 408578507f70883162298b7da66e339629036077 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 18:22:45 +0200 Subject: [PATCH 05/22] also prefix bar with number of items processing --- packer/progressbar.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packer/progressbar.go b/packer/progressbar.go index 2a1ed97d6..ac181757e 100644 --- a/packer/progressbar.go +++ b/packer/progressbar.go @@ -1,6 +1,7 @@ package packer import ( + "fmt" "io" "sync" "sync/atomic" @@ -18,6 +19,7 @@ type ProgressBar interface { } type StackableProgressBar struct { + items int32 total uint64 started bool BasicProgressBar @@ -42,13 +44,21 @@ func (spb *StackableProgressBar) start() { func (spb *StackableProgressBar) Start(total uint64) { atomic.AddUint64(&spb.total, total) + atomic.AddInt32(&spb.items, 1) spb.group.Add(1) spb.startOnce.Do(spb.start) - spb.SetTotal64(int64(spb.total)) + spb.SetTotal64(int64(atomic.LoadUint64(&spb.total))) + spb.prefix() +} + +func (spb *StackableProgressBar) prefix() { + spb.BasicProgressBar.ProgressBar.Prefix(fmt.Sprintf("%d items: ", atomic.LoadInt32(&spb.items))) } func (spb *StackableProgressBar) Finish() { + atomic.AddInt32(&spb.items, -1) spb.group.Done() + spb.prefix() } type BasicProgressBar struct { From a9d302def8c14a5575f57958caf3c541fdda414c Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 18:31:10 +0200 Subject: [PATCH 06/22] removed debug logs --- common/download.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/download.go b/common/download.go index 39ffa5e17..3580555cf 100644 --- a/common/download.go +++ b/common/download.go @@ -81,7 +81,6 @@ func HashForType(t string) hash.Hash { func NewDownloadClient(c *DownloadConfig, ui packer.Ui) *DownloadClient { // Create downloader map if it hasn't been specified already. if c.DownloaderMap == nil { - log.Printf("instantiating. ui: %#v", ui) c.DownloaderMap = map[string]Downloader{ "file": &FileDownloader{Ui: ui, bufferSize: nil}, "http": &HTTPDownloader{Ui: ui, userAgent: c.UserAgent}, @@ -143,7 +142,6 @@ func (d *DownloadClient) Get() (string, error) { if !ok { return "", fmt.Errorf("No downloader for scheme: %s", u.Scheme) } - log.Printf("downloader: %#v", downloader) remote, ok := downloader.(RemoteDownloader) if !ok { From a11f985e3c19764abe02c1674d0972cc0a9f216d Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 18:31:20 +0200 Subject: [PATCH 07/22] revert unecessary check --- common/step_download.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/common/step_download.go b/common/step_download.go index 16da1626f..61f7baf85 100644 --- a/common/step_download.go +++ b/common/step_download.go @@ -138,11 +138,7 @@ func (s *StepDownload) Cleanup(multistep.StateBag) {} func (s *StepDownload) download(config *DownloadConfig, state multistep.StateBag) (string, error, bool) { var path string - v, ok := state.GetOk("ui") - if !ok { - return "", nil, false - } - ui := v.(packer.Ui) + ui := state.Get("ui").(packer.Ui) // Create download client with config download := NewDownloadClient(config, ui) From 7efe3cac3d5e728ee471bbccfd1043e06e4dcafe Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 18:31:27 +0200 Subject: [PATCH 08/22] todos --- packer/rpc/ui.go | 2 +- provisioner/file/provisioner.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packer/rpc/ui.go b/packer/rpc/ui.go index c1ed2435f..afd73fd38 100644 --- a/packer/rpc/ui.go +++ b/packer/rpc/ui.go @@ -79,7 +79,7 @@ func (u *Ui) ProgressBar() packer.ProgressBar { } type RemoteProgressBarClient struct { - id string + id string // TODO(azr): don't need an id any more since bar is a singleton client *rpc.Client } diff --git a/provisioner/file/provisioner.go b/provisioner/file/provisioner.go index 474e87596..7ae675f26 100644 --- a/provisioner/file/provisioner.go +++ b/provisioner/file/provisioner.go @@ -128,7 +128,7 @@ func (p *Provisioner) ProvisionDownload(ui packer.Ui, comm packer.Communicator) // Get a default progress bar pb := packer.NoopProgressBar{} - pb.Start(0) + pb.Start(0) // TODO: find size ? Remove ? defer pb.Finish() // Create MultiWriter for the current progress From 059e14fe42f5dfc420a79b3a2eda779111b8b21d Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 5 Sep 2018 21:03:33 +0200 Subject: [PATCH 09/22] put back usefull debug --- common/download.go | 1 + 1 file changed, 1 insertion(+) diff --git a/common/download.go b/common/download.go index 3580555cf..b1081b6e9 100644 --- a/common/download.go +++ b/common/download.go @@ -163,6 +163,7 @@ func (d *DownloadClient) Get() (string, error) { return "", err } + log.Printf("[DEBUG] Downloading: %s", u.String()) err = remote.Download(f, u) f.Close() if err != nil { From 7f50228080c30e33a5f1eebaca8a40a95be7471c Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 15:50:05 +0200 Subject: [PATCH 10/22] remove old/unused progress code --- common/progress.go | 146 ---------------------------------------- common/progress_test.go | 142 -------------------------------------- 2 files changed, 288 deletions(-) delete mode 100644 common/progress.go delete mode 100644 common/progress_test.go diff --git a/common/progress.go b/common/progress.go deleted file mode 100644 index 6cb91b08b..000000000 --- a/common/progress.go +++ /dev/null @@ -1,146 +0,0 @@ -package common - -import ( - "fmt" - "log" - "reflect" - "time" - - "github.com/cheggaaa/pb" - "github.com/hashicorp/packer/helper/multistep" - "github.com/hashicorp/packer/packer" - "github.com/hashicorp/packer/packer/rpc" -) - -// This is the arrow from packer/ui.go -> TargetedUI.prefixLines -const targetedUIArrowText = "==>" - -// 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 - -// 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 -} - -// Get a progress bar with the default appearance -func GetDefaultProgressBar() 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(5 * time.Second) - return bar -} - -// Return a dummy progress bar that doesn't do anything -func GetDummyProgressBar() ProgressBar { - bar := pb.New64(0) - bar.ManualUpdate = true - return bar -} - -// Given a packer.Ui, calculate the number of characters that a packer.Ui will -// prefix a message with. Then we can use this to calculate the progress bar's width. -func calculateUiPrefixLength(ui packer.Ui) int { - var recursiveCalculateUiPrefixLength func(packer.Ui, int) int - - // Define a recursive closure that traverses through all the known packer.Ui types - // and aggregates the length of the message prefix from each particular type - recursiveCalculateUiPrefixLength = func(ui packer.Ui, agg int) int { - switch ui.(type) { - - case *packer.ColoredUi: - // packer.ColoredUi is simply a wrapper around .Ui - u := ui.(*packer.ColoredUi) - return recursiveCalculateUiPrefixLength(u.Ui, agg) - - case *packer.TargetedUI: - // A TargetedUI adds the .Target and an arrow by default - u := ui.(*packer.TargetedUI) - res := fmt.Sprintf("%s %s: ", targetedUIArrowText, u.Target) - return recursiveCalculateUiPrefixLength(u.Ui, agg+len(res)) - - case *packer.BasicUi: - // The standard BasicUi appends only a newline - return agg + len("\n") - - // packer.rpc.Ui returns 0 here to trigger the hack described later - case *rpc.Ui: - return 0 - - case *packer.MachineReadableUi: - // MachineReadableUi doesn't emit anything...like at all - return 0 - } - - log.Printf("Calculating the message prefix length for packer.Ui type (%T) is not implemented. Using the current aggregated length of %d.", ui, agg) - return agg - } - return recursiveCalculateUiPrefixLength(ui, 0) -} - -func GetPackerConfigFromStateBag(state multistep.StateBag) *PackerConfig { - config := state.Get("config") - rConfig := reflect.Indirect(reflect.ValueOf(config)) - iPackerConfig := rConfig.FieldByName("PackerConfig").Interface() - packerConfig := iPackerConfig.(PackerConfig) - return &packerConfig -} - -func GetProgressBar(ui packer.Ui, config *PackerConfig) ProgressBar { - // Figure out the prefix length by quering the UI - uiPrefixLength := calculateUiPrefixLength(ui) - - // hack to deal with packer.rpc.Ui courtesy of @Swampdragons - if _, ok := ui.(*rpc.Ui); uiPrefixLength == 0 && config != nil && ok { - res := fmt.Sprintf("%s %s: \n", targetedUIArrowText, config.PackerBuildName) - uiPrefixLength = len(res) - } - - // Now we can use the prefix length to calculate the progress bar width - width := calculateProgressBarWidth(uiPrefixLength) - - log.Printf("ProgressBar: Using progress bar width: %d\n", width) - - // Get a default progress bar and set some output defaults - bar := GetDefaultProgressBar() - bar.SetWidth(width) - bar.Callback = func(message string) { - ui.Message(message) - } - return bar -} diff --git a/common/progress_test.go b/common/progress_test.go deleted file mode 100644 index 028492f88..000000000 --- a/common/progress_test.go +++ /dev/null @@ -1,142 +0,0 @@ -package common - -import ( - "github.com/hashicorp/packer/packer" - "testing" -) - -// test packer.Ui implementation to verify that progress bar is being written -type testProgressBarUi struct { - messageCalled bool - messageMessage string -} - -func (u *testProgressBarUi) Say(string) {} -func (u *testProgressBarUi) Error(string) {} -func (u *testProgressBarUi) Machine(string, ...string) {} - -func (u *testProgressBarUi) Ask(string) (string, error) { - return "", nil -} -func (u *testProgressBarUi) Message(message string) { - u.messageCalled = true - u.messageMessage = message -} - -// ..and now let's begin our actual tests -func TestCalculateUiPrefixLength_Unknown(t *testing.T) { - ui := &testProgressBarUi{} - - expected := 0 - if res := calculateUiPrefixLength(ui); res != expected { - t.Fatalf("calculateUiPrefixLength should have returned a length of %d", expected) - } -} - -func TestCalculateUiPrefixLength_BasicUi(t *testing.T) { - ui := &packer.BasicUi{} - - expected := 1 - if res := calculateUiPrefixLength(ui); res != expected { - t.Fatalf("calculateUiPrefixLength should have returned a length of %d", expected) - } -} - -func TestCalculateUiPrefixLength_TargetedUI(t *testing.T) { - ui := &packer.TargetedUI{} - ui.Target = "TestTarget" - arrowText := "==>" - - expected := len(arrowText + " " + ui.Target + ": ") - if res := calculateUiPrefixLength(ui); res != expected { - t.Fatalf("calculateUiPrefixLength should have returned a length of %d", expected) - } -} - -func TestCalculateUiPrefixLength_TargetedUIWrappingBasicUi(t *testing.T) { - ui := &packer.TargetedUI{} - ui.Target = "TestTarget" - ui.Ui = &packer.BasicUi{} - arrowText := "==>" - - expected := len(arrowText + " " + ui.Target + ": " + "\n") - if res := calculateUiPrefixLength(ui); res != expected { - t.Fatalf("calculateUiPrefixLength should have returned a length of %d", expected) - } -} - -func TestCalculateUiPrefixLength_TargetedUIWrappingMachineUi(t *testing.T) { - ui := &packer.TargetedUI{} - ui.Target = "TestTarget" - ui.Ui = &packer.MachineReadableUi{} - - expected := 0 - if res := calculateUiPrefixLength(ui); res != expected { - t.Fatalf("calculateUiPrefixLength should have returned a length of %d", expected) - } -} -func TestDefaultProgressBar(t *testing.T) { - var callbackCalled bool - - // Initialize the default progress bar - bar := GetDefaultProgressBar() - bar.Callback = func(state string) { - callbackCalled = true - t.Logf("TestDefaultProgressBar emitted %#v", state) - } - bar.SetTotal64(1) - - // Set it off - progressBar := bar.Start() - progressBar.Set64(1) - - // Check to see that the callback was hit - if !callbackCalled { - t.Fatalf("TestDefaultProgressBar.Callback should be called") - } -} - -func TestDummyProgressBar(t *testing.T) { - var callbackCalled bool - - // Initialize the dummy progress bar - bar := GetDummyProgressBar() - bar.Callback = func(state string) { - callbackCalled = true - t.Logf("TestDummyProgressBar emitted %#v", state) - } - bar.SetTotal64(1) - - // Now we can go - progressBar := bar.Start() - progressBar.Set64(1) - - // Check to see that the callback was hit - if callbackCalled { - t.Fatalf("TestDummyProgressBar.Callback should not be called") - } -} - -func TestUiProgressBar(t *testing.T) { - - ui := &testProgressBarUi{} - - // Initialize the Ui progress bar - bar := GetProgressBar(ui, nil) - bar.SetTotal64(1) - - // Ensure that callback has been set to something - if bar.Callback == nil { - t.Fatalf("TestUiProgressBar.Callback should be initialized") - } - - // Now we can go - progressBar := bar.Start() - progressBar.Set64(1) - - // Check to see that the callback was hit - if !ui.messageCalled { - t.Fatalf("TestUiProgressBar.messageCalled should be called") - } - t.Logf("TestUiProgressBar emitted %#v", ui.messageMessage) -} From 5b66069da08a4206b133a0b688e220720f38cd02 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 15:52:28 +0200 Subject: [PATCH 11/22] tests: remove Ui stubs to use packer.NoopUi to deduplicate code --- provisioner/ansible-local/provisioner_test.go | 7 ++-- provisioner/ansible-local/ui_stub.go | 15 ------- provisioner/ansible/adapter_test.go | 32 +-------------- provisioner/file/provisioner_test.go | 40 ++++++------------- 4 files changed, 18 insertions(+), 76 deletions(-) delete mode 100644 provisioner/ansible-local/ui_stub.go diff --git a/provisioner/ansible-local/provisioner_test.go b/provisioner/ansible-local/provisioner_test.go index e72c6c01a..741bba235 100644 --- a/provisioner/ansible-local/provisioner_test.go +++ b/provisioner/ansible-local/provisioner_test.go @@ -8,11 +8,12 @@ import ( "testing" "fmt" + "os/exec" + "github.com/hashicorp/packer/builder/docker" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/provisioner/file" "github.com/hashicorp/packer/template" - "os/exec" ) func TestProvisioner_Impl(t *testing.T) { @@ -132,7 +133,7 @@ func TestProvisionerProvision_PlaybookFiles(t *testing.T) { } comm := &communicatorMock{} - if err := p.Provision(&uiStub{}, comm); err != nil { + if err := p.Provision(new(packer.NoopUi), comm); err != nil { t.Fatalf("err: %s", err) } @@ -166,7 +167,7 @@ func TestProvisionerProvision_PlaybookFilesWithPlaybookDir(t *testing.T) { } comm := &communicatorMock{} - if err := p.Provision(&uiStub{}, comm); err != nil { + if err := p.Provision(new(packer.NoopUi), comm); err != nil { t.Fatalf("err: %s", err) } diff --git a/provisioner/ansible-local/ui_stub.go b/provisioner/ansible-local/ui_stub.go deleted file mode 100644 index 4faa2a215..000000000 --- a/provisioner/ansible-local/ui_stub.go +++ /dev/null @@ -1,15 +0,0 @@ -package ansiblelocal - -type uiStub struct{} - -func (su *uiStub) Ask(string) (string, error) { - return "", nil -} - -func (su *uiStub) Error(string) {} - -func (su *uiStub) Machine(string, ...string) {} - -func (su *uiStub) Message(string) {} - -func (su *uiStub) Say(msg string) {} diff --git a/provisioner/ansible/adapter_test.go b/provisioner/ansible/adapter_test.go index 2ab2d350b..29667cbe2 100644 --- a/provisioner/ansible/adapter_test.go +++ b/provisioner/ansible/adapter_test.go @@ -24,7 +24,7 @@ func TestAdapter_Serve(t *testing.T) { config := &ssh.ServerConfig{} - ui := new(ui) + ui := new(packer.NoopUi) sut := newAdapter(done, &l, config, "", newUi(ui), communicator{}) go func() { @@ -93,36 +93,6 @@ func (a addr) String() string { return "test" } -type ui int - -func (u *ui) Ask(s string) (string, error) { - *u++ - return s, nil -} - -func (u *ui) Say(s string) { - *u++ - log.Println(s) -} - -func (u *ui) Message(s string) { - *u++ - log.Println(s) -} - -func (u *ui) Error(s string) { - *u++ - log.Println(s) -} - -func (u *ui) Machine(s1 string, s2 ...string) { - *u++ - log.Println(s1) - for _, s := range s2 { - log.Println(s) - } -} - type communicator struct{} func (c communicator) Start(*packer.RemoteCmd) error { diff --git a/provisioner/file/provisioner_test.go b/provisioner/file/provisioner_test.go index 175082aae..732bba3ff 100644 --- a/provisioner/file/provisioner_test.go +++ b/provisioner/file/provisioner_test.go @@ -1,6 +1,7 @@ package file import ( + "bytes" "io/ioutil" "os" "path/filepath" @@ -99,27 +100,6 @@ func TestProvisionerPrepare_EmptyDestination(t *testing.T) { } } -type stubUi struct { - sayMessages string -} - -func (su *stubUi) Ask(string) (string, error) { - return "", nil -} - -func (su *stubUi) Error(string) { -} - -func (su *stubUi) Machine(string, ...string) { -} - -func (su *stubUi) Message(string) { -} - -func (su *stubUi) Say(msg string) { - su.sayMessages += msg -} - func TestProvisionerProvision_SendsFile(t *testing.T) { var p Provisioner tf, err := ioutil.TempFile("", "packer") @@ -141,18 +121,21 @@ func TestProvisionerProvision_SendsFile(t *testing.T) { t.Fatalf("err: %s", err) } - ui := &stubUi{} + b := bytes.NewBuffer(nil) + ui := &packer.BasicUi{ + Writer: b, + } comm := &packer.MockCommunicator{} err = p.Provision(ui, comm) if err != nil { t.Fatalf("should successfully provision: %s", err) } - if !strings.Contains(ui.sayMessages, tf.Name()) { + if !strings.Contains(b.String(), tf.Name()) { t.Fatalf("should print source filename") } - if !strings.Contains(ui.sayMessages, "something") { + if !strings.Contains(b.String(), "something") { t.Fatalf("should print destination filename") } @@ -197,18 +180,21 @@ func TestProvisionDownloadMkdirAll(t *testing.T) { if err := p.Prepare(config); err != nil { t.Fatalf("err: %s", err) } - ui := &stubUi{} + b := bytes.NewBuffer(nil) + ui := &packer.BasicUi{ + Writer: b, + } comm := &packer.MockCommunicator{} err = p.ProvisionDownload(ui, comm) if err != nil { t.Fatalf("should successfully provision: %s", err) } - if !strings.Contains(ui.sayMessages, tf.Name()) { + if !strings.Contains(b.String(), tf.Name()) { t.Fatalf("should print source filename") } - if !strings.Contains(ui.sayMessages, "something") { + if !strings.Contains(b.String(), "something") { t.Fatalf("should print destination filename") } From 42561cf7771cfb4a98e4d00476ba24735c789e03 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 15:52:46 +0200 Subject: [PATCH 12/22] packer/rpc/ui_test.go: test progress bar too --- packer/rpc/ui_test.go | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/packer/rpc/ui_test.go b/packer/rpc/ui_test.go index 4dd2d7036..7b0489012 100644 --- a/packer/rpc/ui_test.go +++ b/packer/rpc/ui_test.go @@ -3,20 +3,23 @@ package rpc import ( "reflect" "testing" + + "github.com/hashicorp/packer/packer" ) 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 + askCalled bool + askQuery string + errorCalled bool + errorMessage string + machineCalled bool + machineType string + machineArgs []string + messageCalled bool + messageMessage string + sayCalled bool + sayMessage string + progressBarCalled bool } func (u *testUi) Ask(query string) (string, error) { @@ -46,6 +49,11 @@ func (u *testUi) Say(message string) { u.sayMessage = message } +func (u *testUi) ProgressBar() packer.ProgressBar { + u.progressBarCalled = true + return new(packer.NoopProgressBar) +} + func TestUiRPC(t *testing.T) { // Create the UI to test ui := new(testUi) @@ -87,6 +95,10 @@ func TestUiRPC(t *testing.T) { if ui.sayMessage != "message" { t.Fatalf("bad: %#v", ui.errorMessage) } + uiClient.ProgressBar() + if ui.progressBarCalled != true { + t.Fatalf("ProgressBar not called.") + } uiClient.Machine("foo", "bar", "baz") if !ui.machineCalled { From 23762a181037c4e9871cb9280edddcf7493ef46f Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 15:53:07 +0200 Subject: [PATCH 13/22] fix tests for common/download_test.go --- common/download_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/common/download_test.go b/common/download_test.go index ea6d7585e..dd41d6cc1 100644 --- a/common/download_test.go +++ b/common/download_test.go @@ -38,7 +38,7 @@ func TestDownloadClientVerifyChecksum(t *testing.T) { Checksum: checksum, } - d := NewDownloadClient(config, new(packer.NoopProgressBar)) + d := NewDownloadClient(config, new(packer.NoopUi)) result, err := d.VerifyChecksum(tf.Name()) if err != nil { t.Fatalf("Verify err: %s", err) @@ -61,7 +61,7 @@ func TestDownloadClient_basic(t *testing.T) { Url: ts.URL + "/basic.txt", TargetPath: tf.Name(), CopyFile: true, - }, new(packer.NoopProgressBar)) + }, new(packer.NoopUi)) path, err := client.Get() if err != nil { @@ -97,7 +97,7 @@ func TestDownloadClient_checksumBad(t *testing.T) { Hash: HashForType("md5"), Checksum: checksum, CopyFile: true, - }, new(packer.NoopProgressBar)) + }, new(packer.NoopUi)) if _, err := client.Get(); err == nil { t.Fatal("should error") @@ -123,7 +123,7 @@ func TestDownloadClient_checksumGood(t *testing.T) { Hash: HashForType("md5"), Checksum: checksum, CopyFile: true, - }, new(packer.NoopProgressBar)) + }, new(packer.NoopUi)) path, err := client.Get() if err != nil { @@ -155,7 +155,7 @@ func TestDownloadClient_checksumNoDownload(t *testing.T) { Hash: HashForType("md5"), Checksum: checksum, CopyFile: true, - }, new(packer.NoopProgressBar)) + }, new(packer.NoopUi)) path, err := client.Get() if err != nil { t.Fatalf("err: %s", err) @@ -185,7 +185,7 @@ func TestDownloadClient_notFound(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL + "/not-found.txt", TargetPath: tf.Name(), - }, new(packer.NoopProgressBar)) + }, new(packer.NoopUi)) if _, err := client.Get(); err == nil { t.Fatal("should error") @@ -213,7 +213,7 @@ func TestDownloadClient_resume(t *testing.T) { Url: ts.URL, TargetPath: tf.Name(), CopyFile: true, - }, new(packer.NoopProgressBar)) + }, new(packer.NoopUi)) path, err := client.Get() if err != nil { @@ -275,7 +275,7 @@ func TestDownloadClient_usesDefaultUserAgent(t *testing.T) { CopyFile: true, } - client := NewDownloadClient(config, new(packer.NoopProgressBar)) + client := NewDownloadClient(config, new(packer.NoopUi)) _, err = client.Get() if err != nil { t.Fatal(err) @@ -308,7 +308,7 @@ func TestDownloadClient_setsUserAgent(t *testing.T) { CopyFile: true, } - client := NewDownloadClient(config, new(packer.NoopProgressBar)) + client := NewDownloadClient(config, new(packer.NoopUi)) _, err = client.Get() if err != nil { t.Fatal(err) @@ -407,7 +407,7 @@ func TestDownloadFileUrl(t *testing.T) { CopyFile: false, } - client := NewDownloadClient(config, new(packer.NoopProgressBar)) + client := NewDownloadClient(config, new(packer.NoopUi)) // Verify that we fail to match the checksum _, err = client.Get() @@ -438,7 +438,7 @@ func SimulateFileUriDownload(t *testing.T, uri string) (string, error) { } // go go go - client := NewDownloadClient(config, new(packer.NoopProgressBar)) + client := NewDownloadClient(config, new(packer.NoopUi)) path, err := client.Get() // ignore any non-important checksum errors if it's not a unc path From d89e1133c3304ee22ad30bd09d4aa7131e57d281 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 15:55:19 +0200 Subject: [PATCH 14/22] use freshly merged random.AlphaNum instead of our own random --- packer/rpc/ui.go | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/packer/rpc/ui.go b/packer/rpc/ui.go index afd73fd38..8678726ad 100644 --- a/packer/rpc/ui.go +++ b/packer/rpc/ui.go @@ -3,9 +3,10 @@ package rpc import ( "io" "log" - "math/rand" "net/rpc" + "github.com/hashicorp/packer/common/random" + "github.com/hashicorp/packer/packer" ) @@ -133,20 +134,10 @@ func (u *UiServer) Say(message *string, reply *interface{}) error { return nil } -func RandStringBytes(n int) string { // TODO(azr): remove before merging - const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" - - b := make([]byte, n) - for i := range b { - b[i] = letterBytes[rand.Intn(len(letterBytes))] - } - return string(b) -} - func (u *UiServer) ProgressBar(_ *string, reply *interface{}) error { bar := u.ui.ProgressBar() - callbackName := RandStringBytes(6) + callbackName := random.AlphaNum(6) log.Printf("registering progressbar %s", callbackName) err := u.register(callbackName, &RemoteProgressBarServer{bar}) From 6d3e36e6ea5a090a8e39627f023adfa857bf7354 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 16:14:59 +0200 Subject: [PATCH 15/22] simplify remote progress bar as we are using a single instance --- packer/rpc/ui.go | 66 +++++++++++++++--------------------------------- 1 file changed, 21 insertions(+), 45 deletions(-) diff --git a/packer/rpc/ui.go b/packer/rpc/ui.go index 8678726ad..c06329772 100644 --- a/packer/rpc/ui.go +++ b/packer/rpc/ui.go @@ -5,8 +5,6 @@ import ( "log" "net/rpc" - "github.com/hashicorp/packer/common/random" - "github.com/hashicorp/packer/packer" ) @@ -67,38 +65,27 @@ func (u *Ui) Say(message string) { } func (u *Ui) ProgressBar() packer.ProgressBar { - var callMeMaybe string - if err := u.client.Call("Ui.ProgressBar", nil, &callMeMaybe); err != nil { + if err := u.client.Call("Ui.ProgressBar", new(interface{}), new(interface{})); err != nil { log.Printf("Error in Ui RPC call: %s", err) - return new(packer.NoopProgressBar) - } - - return &RemoteProgressBarClient{ - id: callMeMaybe, - client: u.client, } + return u // Ui is also a progress bar !! } -type RemoteProgressBarClient struct { - id string // TODO(azr): don't need an id any more since bar is a singleton - client *rpc.Client +var _ packer.ProgressBar = new(Ui) + +func (pb *Ui) Start(total uint64) { + pb.client.Call("Ui.Start", total, new(interface{})) } -var _ packer.ProgressBar = new(RemoteProgressBarClient) - -func (pb *RemoteProgressBarClient) Start(total uint64) { - pb.client.Call(pb.id+".Start", total, new(interface{})) +func (pb *Ui) Add(current uint64) { + pb.client.Call("Ui.Add", current, new(interface{})) } -func (pb *RemoteProgressBarClient) Add(current uint64) { - pb.client.Call(pb.id+".Add", current, new(interface{})) +func (pb *Ui) Finish() { + pb.client.Call("Ui.Finish", nil, new(interface{})) } -func (pb *RemoteProgressBarClient) Finish() { - pb.client.Call(pb.id+".Finish", nil, new(interface{})) -} - -func (pb *RemoteProgressBarClient) NewProxyReader(r io.Reader) io.Reader { +func (pb *Ui) NewProxyReader(r io.Reader) io.Reader { return &packer.ProxyReader{Reader: r, ProgressBar: pb} } @@ -135,35 +122,24 @@ func (u *UiServer) Say(message *string, reply *interface{}) error { } func (u *UiServer) ProgressBar(_ *string, reply *interface{}) error { - bar := u.ui.ProgressBar() - - callbackName := random.AlphaNum(6) - - log.Printf("registering progressbar %s", callbackName) - err := u.register(callbackName, &RemoteProgressBarServer{bar}) - if err != nil { - log.Printf("failed to register a new progress bar rpc server, %s", err) - return err - } - *reply = callbackName + // No-op for now, this function might be + // used in the future if we want to use + // different progress bars with identifiers. + u.ui.ProgressBar() return nil } -type RemoteProgressBarServer struct { - pb packer.ProgressBar -} - -func (pb *RemoteProgressBarServer) Finish(_ string, _ *interface{}) error { - pb.pb.Finish() +func (pb *UiServer) Finish(_ string, _ *interface{}) error { + pb.ui.ProgressBar().Finish() return nil } -func (pb *RemoteProgressBarServer) Start(total uint64, _ *interface{}) error { - pb.pb.Start(total) +func (pb *UiServer) Start(total uint64, _ *interface{}) error { + pb.ui.ProgressBar().Start(total) return nil } -func (pb *RemoteProgressBarServer) Add(current uint64, _ *interface{}) error { - pb.pb.Add(current) +func (pb *UiServer) Add(current uint64, _ *interface{}) error { + pb.ui.ProgressBar().Add(current) return nil } From f3c923c47dea42ff8b9031a5e2ab954466eaa54a Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 16:15:09 +0200 Subject: [PATCH 16/22] add tests for progress bar rpc calls --- packer/rpc/ui_test.go | 69 +++++++++++++++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 15 deletions(-) diff --git a/packer/rpc/ui_test.go b/packer/rpc/ui_test.go index 7b0489012..42c68292f 100644 --- a/packer/rpc/ui_test.go +++ b/packer/rpc/ui_test.go @@ -1,6 +1,7 @@ package rpc import ( + "io" "reflect" "testing" @@ -8,18 +9,23 @@ import ( ) 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 - progressBarCalled bool + askCalled bool + askQuery string + errorCalled bool + errorMessage string + machineCalled bool + machineType string + machineArgs []string + messageCalled bool + messageMessage string + sayCalled bool + sayMessage string + + progressBarCalled bool + progressBarStartCalled bool + progressBarAddCalled bool + progressBarFinishCalled bool + progressBarNewProxyReaderCalled bool } func (u *testUi) Ask(query string) (string, error) { @@ -51,7 +57,24 @@ func (u *testUi) Say(message string) { func (u *testUi) ProgressBar() packer.ProgressBar { u.progressBarCalled = true - return new(packer.NoopProgressBar) + return u +} + +func (u *testUi) Start(uint64) { + u.progressBarStartCalled = true +} + +func (u *testUi) Add(uint64) { + u.progressBarAddCalled = true +} + +func (u *testUi) Finish() { + u.progressBarFinishCalled = true +} + +func (u *testUi) NewProxyReader(r io.Reader) io.Reader { + u.progressBarNewProxyReaderCalled = true + return r } func TestUiRPC(t *testing.T) { @@ -95,9 +118,25 @@ func TestUiRPC(t *testing.T) { if ui.sayMessage != "message" { t.Fatalf("bad: %#v", ui.errorMessage) } - uiClient.ProgressBar() + + bar := uiClient.ProgressBar() if ui.progressBarCalled != true { - t.Fatalf("ProgressBar not called.") + t.Errorf("ProgressBar not called.") + } + + bar.Start(100) + if ui.progressBarStartCalled != true { + t.Errorf("progressBar.Start not called.") + } + + bar.Add(1) + if ui.progressBarAddCalled != true { + t.Errorf("progressBar.Add not called.") + } + + bar.Finish() + if ui.progressBarFinishCalled != true { + t.Errorf("progressBar.Finish not called.") } uiClient.Machine("foo", "bar", "baz") From 3842f85eb42d84243a7eafad3ff6a303559bc1ee Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 16:30:35 +0200 Subject: [PATCH 17/22] download: remove close calls that are already defered --- common/download.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/download.go b/common/download.go index b1081b6e9..508f15c24 100644 --- a/common/download.go +++ b/common/download.go @@ -456,7 +456,7 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { err = <-errch } bar.Finish() - f.Close() + return err } @@ -558,7 +558,6 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { err = <-errch } bar.Finish() - f.Close() return err } From f9c58e2b1f42a7aef855fecfd89a5e122335633f Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 16:38:28 +0200 Subject: [PATCH 18/22] download: defer progress bar Finish --- common/download.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/download.go b/common/download.go index 508f15c24..0d3e9fc0e 100644 --- a/common/download.go +++ b/common/download.go @@ -317,6 +317,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { bar := d.ProgressBar() bar.Start(total) + defer bar.Finish() bar.Add(current) body := bar.NewProxyReader(resp.Body) @@ -336,7 +337,6 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { break } } - bar.Finish() return nil } @@ -430,6 +430,7 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { bar := d.ProgressBar() bar.Start(uint64(fi.Size())) + defer bar.Finish() fProxy := bar.NewProxyReader(f) // no bufferSize specified, so copy synchronously. @@ -455,7 +456,6 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { // ...and we spin until it's done err = <-errch } - bar.Finish() return err } @@ -532,6 +532,7 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { bar := d.ProgressBar() bar.Start(uint64(fi.Size())) + defer bar.Finish() fProxy := bar.NewProxyReader(f) // no bufferSize specified, so copy synchronously. @@ -557,7 +558,6 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { // ...and as usual we spin until it's done err = <-errch } - bar.Finish() return err } From bb59a70e8f41fc2fa6c3dc89552e788bcdf37359 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 16:49:15 +0200 Subject: [PATCH 19/22] progressbar: use int64 instead of uint64 * it's what's used for file sizes and used lib --- common/download.go | 10 +++++----- packer/progressbar.go | 22 +++++++++++----------- packer/rpc/ui.go | 8 ++++---- packer/rpc/ui_test.go | 4 ++-- provisioner/file/provisioner.go | 2 +- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/common/download.go b/common/download.go index 0d3e9fc0e..592cd2975 100644 --- a/common/download.go +++ b/common/download.go @@ -242,7 +242,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return err } - var current uint64 + var current int64 // Make the request. We first make a HEAD request so we can check // if the server supports range queries. If the server/URL doesn't @@ -286,7 +286,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { if _, err = dst.Seek(0, os.SEEK_END); err == nil { req.Header.Set("Range", fmt.Sprintf("bytes=%d-", fi.Size())) - current = uint64(fi.Size()) + current = fi.Size() } } } @@ -313,7 +313,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return fmt.Errorf("HTTP error: %s", err.Error()) } - total := current + uint64(resp.ContentLength) + total := current + resp.ContentLength bar := d.ProgressBar() bar.Start(total) @@ -429,7 +429,7 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { bar := d.ProgressBar() - bar.Start(uint64(fi.Size())) + bar.Start(fi.Size()) defer bar.Finish() fProxy := bar.NewProxyReader(f) @@ -531,7 +531,7 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { bar := d.ProgressBar() - bar.Start(uint64(fi.Size())) + bar.Start(fi.Size()) defer bar.Finish() fProxy := bar.NewProxyReader(f) diff --git a/packer/progressbar.go b/packer/progressbar.go index ac181757e..c9b8d7d4b 100644 --- a/packer/progressbar.go +++ b/packer/progressbar.go @@ -12,15 +12,15 @@ import ( // ProgressBar allows to graphically display // a self refreshing progress bar. type ProgressBar interface { - Start(total uint64) - Add(current uint64) + Start(total int64) + Add(current int64) NewProxyReader(r io.Reader) (proxy io.Reader) Finish() } type StackableProgressBar struct { items int32 - total uint64 + total int64 started bool BasicProgressBar startOnce sync.Once @@ -42,12 +42,12 @@ func (spb *StackableProgressBar) start() { }() } -func (spb *StackableProgressBar) Start(total uint64) { - atomic.AddUint64(&spb.total, total) +func (spb *StackableProgressBar) Start(total int64) { + atomic.AddInt64(&spb.total, total) atomic.AddInt32(&spb.items, 1) spb.group.Add(1) spb.startOnce.Do(spb.start) - spb.SetTotal64(int64(atomic.LoadUint64(&spb.total))) + spb.SetTotal64(atomic.LoadInt64(&spb.total)) spb.prefix() } @@ -67,12 +67,12 @@ type BasicProgressBar struct { var _ ProgressBar = new(BasicProgressBar) -func (bpb *BasicProgressBar) Start(total uint64) { +func (bpb *BasicProgressBar) Start(total int64) { bpb.SetTotal64(int64(total)) bpb.ProgressBar.Start() } -func (bpb *BasicProgressBar) Add(current uint64) { +func (bpb *BasicProgressBar) Add(current int64) { bpb.ProgressBar.Add64(int64(current)) } func (bpb *BasicProgressBar) NewProxyReader(r io.Reader) io.Reader { @@ -94,8 +94,8 @@ type NoopProgressBar struct { var _ ProgressBar = new(NoopProgressBar) -func (npb *NoopProgressBar) Start(uint64) {} -func (npb *NoopProgressBar) Add(uint64) {} +func (npb *NoopProgressBar) Start(int64) {} +func (npb *NoopProgressBar) Add(int64) {} func (npb *NoopProgressBar) Finish() {} func (npb *NoopProgressBar) NewProxyReader(r io.Reader) io.Reader { return r } func (npb *NoopProgressBar) NewProxyReadCloser(r io.ReadCloser) io.ReadCloser { return r } @@ -109,7 +109,7 @@ type ProxyReader struct { func (r *ProxyReader) Read(p []byte) (n int, err error) { n, err = r.Reader.Read(p) - r.ProgressBar.Add(uint64(n)) + r.ProgressBar.Add(int64(n)) return } diff --git a/packer/rpc/ui.go b/packer/rpc/ui.go index c06329772..c9d3ef176 100644 --- a/packer/rpc/ui.go +++ b/packer/rpc/ui.go @@ -73,11 +73,11 @@ func (u *Ui) ProgressBar() packer.ProgressBar { var _ packer.ProgressBar = new(Ui) -func (pb *Ui) Start(total uint64) { +func (pb *Ui) Start(total int64) { pb.client.Call("Ui.Start", total, new(interface{})) } -func (pb *Ui) Add(current uint64) { +func (pb *Ui) Add(current int64) { pb.client.Call("Ui.Add", current, new(interface{})) } @@ -134,12 +134,12 @@ func (pb *UiServer) Finish(_ string, _ *interface{}) error { return nil } -func (pb *UiServer) Start(total uint64, _ *interface{}) error { +func (pb *UiServer) Start(total int64, _ *interface{}) error { pb.ui.ProgressBar().Start(total) return nil } -func (pb *UiServer) Add(current uint64, _ *interface{}) error { +func (pb *UiServer) Add(current int64, _ *interface{}) error { pb.ui.ProgressBar().Add(current) return nil } diff --git a/packer/rpc/ui_test.go b/packer/rpc/ui_test.go index 42c68292f..078b85e44 100644 --- a/packer/rpc/ui_test.go +++ b/packer/rpc/ui_test.go @@ -60,11 +60,11 @@ func (u *testUi) ProgressBar() packer.ProgressBar { return u } -func (u *testUi) Start(uint64) { +func (u *testUi) Start(int64) { u.progressBarStartCalled = true } -func (u *testUi) Add(uint64) { +func (u *testUi) Add(int64) { u.progressBarAddCalled = true } diff --git a/provisioner/file/provisioner.go b/provisioner/file/provisioner.go index 7ae675f26..1be9f1885 100644 --- a/provisioner/file/provisioner.go +++ b/provisioner/file/provisioner.go @@ -177,7 +177,7 @@ func (p *Provisioner) ProvisionUpload(ui packer.Ui, comm packer.Communicator) er // Get a default progress bar bar := ui.ProgressBar() - bar.Start(uint64(info.Size())) + bar.Start(info.Size()) defer bar.Finish() // Create ProxyReader for the current progress From 32cacad273ee2e80dbffaffb8a397332b5b12c02 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 16:50:47 +0200 Subject: [PATCH 20/22] remove comment that is not true anymore --- common/download.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/download.go b/common/download.go index 592cd2975..78f31f25d 100644 --- a/common/download.go +++ b/common/download.go @@ -20,7 +20,7 @@ import ( "runtime" "strings" - "github.com/hashicorp/packer/packer" // imports related to each Downloader implementation + "github.com/hashicorp/packer/packer" ) // DownloadConfig is the configuration given to instantiate a new From 8a851efcc8cd7a3d2b9f092968f8e5d766d4c2f1 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 17:01:14 +0200 Subject: [PATCH 21/22] progressbar: more godocs --- packer/progressbar.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packer/progressbar.go b/packer/progressbar.go index c9b8d7d4b..aa92b9690 100644 --- a/packer/progressbar.go +++ b/packer/progressbar.go @@ -18,6 +18,16 @@ type ProgressBar interface { Finish() } +// StackableProgressBar is a progress bar that +// allows to track multiple downloads at once. +// Every call to Start increments a counter that +// will display the number of current loadings. +// Every call to Start will add total to an internal +// total that is the total displayed. +// +// When all active downloads are finished +// StackableProgressBar will clean itself to a default +// state. type StackableProgressBar struct { items int32 total int64 @@ -61,6 +71,9 @@ func (spb *StackableProgressBar) Finish() { spb.prefix() } +// BasicProgressBar is packer's basic progress bar. +// Current implementation will always try to keep +// itself at the bottom of a terminal. type BasicProgressBar struct { *pb.ProgressBar } @@ -88,7 +101,7 @@ func (bpb *BasicProgressBar) NewProxyReadCloser(r io.ReadCloser) io.ReadCloser { } } -// NoopProgressBar is a silent progress bar +// NoopProgressBar is a silent progress bar. type NoopProgressBar struct { } @@ -101,7 +114,7 @@ func (npb *NoopProgressBar) NewProxyReader(r io.Reader) io.Reader { func (npb *NoopProgressBar) NewProxyReadCloser(r io.ReadCloser) io.ReadCloser { return r } // ProxyReader implements io.ReadCloser but sends -// count of read bytes to progress bar +// count of read bytes to a progress bar type ProxyReader struct { io.Reader ProgressBar From 0ac7b6436448419eb80f56677a4f0cf8ca4e27ea Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 6 Sep 2018 18:12:15 +0200 Subject: [PATCH 22/22] fix panic of the future using a mutex instead of some atomic calls + more docs --- packer/progressbar.go | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/packer/progressbar.go b/packer/progressbar.go index aa92b9690..e2b229d07 100644 --- a/packer/progressbar.go +++ b/packer/progressbar.go @@ -24,17 +24,19 @@ type ProgressBar interface { // will display the number of current loadings. // Every call to Start will add total to an internal // total that is the total displayed. -// +// First call to Start will start a goroutine +// that is waiting for every download to be finished. +// Last call to Finish triggers a cleanup. // When all active downloads are finished // StackableProgressBar will clean itself to a default // state. type StackableProgressBar struct { - items int32 - total int64 - started bool + mtx sync.Mutex // locks in Start & Finish BasicProgressBar - startOnce sync.Once - group sync.WaitGroup + items int32 + total int64 + + started bool } var _ ProgressBar = new(StackableProgressBar) @@ -44,21 +46,21 @@ func (spb *StackableProgressBar) start() { spb.BasicProgressBar.ProgressBar.SetUnits(pb.U_BYTES) spb.BasicProgressBar.ProgressBar.Start() - go func() { - spb.group.Wait() - spb.BasicProgressBar.ProgressBar.Finish() - spb.startOnce = sync.Once{} - spb.BasicProgressBar.ProgressBar = nil - }() + spb.started = true } func (spb *StackableProgressBar) Start(total int64) { - atomic.AddInt64(&spb.total, total) - atomic.AddInt32(&spb.items, 1) - spb.group.Add(1) - spb.startOnce.Do(spb.start) - spb.SetTotal64(atomic.LoadInt64(&spb.total)) + spb.mtx.Lock() + + spb.total += total + spb.items++ + + if !spb.started { + spb.start() + } + spb.SetTotal64(spb.total) spb.prefix() + spb.mtx.Unlock() } func (spb *StackableProgressBar) prefix() { @@ -66,9 +68,19 @@ func (spb *StackableProgressBar) prefix() { } func (spb *StackableProgressBar) Finish() { - atomic.AddInt32(&spb.items, -1) - spb.group.Done() + spb.mtx.Lock() + + spb.items-- + if spb.items == 0 { + // slef cleanup + spb.BasicProgressBar.ProgressBar.Finish() + spb.BasicProgressBar.ProgressBar = nil + spb.started = false + spb.total = 0 + return + } spb.prefix() + spb.mtx.Unlock() } // BasicProgressBar is packer's basic progress bar. @@ -81,12 +93,12 @@ type BasicProgressBar struct { var _ ProgressBar = new(BasicProgressBar) func (bpb *BasicProgressBar) Start(total int64) { - bpb.SetTotal64(int64(total)) + bpb.SetTotal64(total) bpb.ProgressBar.Start() } func (bpb *BasicProgressBar) Add(current int64) { - bpb.ProgressBar.Add64(int64(current)) + bpb.ProgressBar.Add64(current) } func (bpb *BasicProgressBar) NewProxyReader(r io.Reader) io.Reader { return &ProxyReader{