From 917d0935adfed8297a2dd5dbb6dbf80ab657c746 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Fri, 22 Oct 2021 12:35:03 +0200 Subject: [PATCH] Show progressbar only if terminal is big enough Fixes #259 The bug is caused by https://github.com/pterm/pterm/blob/4c725e56bfd9eb38e1c7b9dec187b50b93baa8bd/progressbar_printer.go#L190 which is not taking into account when barCurrentLength is <=0. As a workaround display it only if the terminal width is big enough. Now the context drives this kind of runtime state, so we keep everything tight and we inject only that into the workers --- pkg/api/core/types/context.go | 38 +++++++++++++++++++++++++++++++-- pkg/installer/client/http.go | 33 +++++++++++++++++++---------- pkg/installer/installer.go | 40 +++++++++++++++++++---------------- 3 files changed, 80 insertions(+), 31 deletions(-) diff --git a/pkg/api/core/types/context.go b/pkg/api/core/types/context.go index 7943aa13..f89fdb3a 100644 --- a/pkg/api/core/types/context.go +++ b/pkg/api/core/types/context.go @@ -28,9 +28,11 @@ import ( "github.com/kyokomi/emoji" "github.com/mudler/luet/pkg/helpers/terminal" + "github.com/pkg/errors" "github.com/pterm/pterm" "go.uber.org/zap" "go.uber.org/zap/zapcore" + "golang.org/x/term" ) const ( @@ -48,13 +50,16 @@ type Context struct { NoSpinner bool s *pterm.SpinnerPrinter - spinnerLock sync.Mutex + spinnerLock *sync.Mutex z *zap.Logger + AreaPrinter *pterm.AreaPrinter + ProgressBar *pterm.ProgressbarPrinter } func NewContext() *Context { return &Context{ - IsTerminal: terminal.IsTerminal(os.Stdout), + spinnerLock: &sync.Mutex{}, + IsTerminal: terminal.IsTerminal(os.Stdout), Config: &LuetConfig{ ConfigFromHost: true, Logging: LuetLoggingConfig{}, @@ -68,6 +73,35 @@ func NewContext() *Context { } } +func (c *Context) Copy() *Context { + + configCopy := *c.Config + configCopy.System = *c.Config.GetSystem() + configCopy.General = *c.Config.GetGeneral() + configCopy.Logging = *c.Config.GetLogging() + + ctx := *c + ctxCopy := &ctx + ctxCopy.Config = &configCopy + + return ctxCopy +} + +// GetTerminalSize returns the width and the height of the active terminal. +func (c *Context) GetTerminalSize() (width, height int, err error) { + w, h, err := term.GetSize(int(os.Stdout.Fd())) + if w <= 0 { + w = 0 + } + if h <= 0 { + h = 0 + } + if err != nil { + err = errors.New("size not detectable") + } + return w, h, err +} + func (c *Context) Init() (err error) { if c.IsTerminal { if !c.Config.Logging.Color { diff --git a/pkg/installer/client/http.go b/pkg/installer/client/http.go index 9d134a3d..af307a18 100644 --- a/pkg/installer/client/http.go +++ b/pkg/installer/client/http.go @@ -38,8 +38,6 @@ type HttpClient struct { RepoData RepoData Cache *artifact.ArtifactCache context *types.Context - - ProgressBarArea *pterm.AreaPrinter } func NewHttpClient(r RepoData, ctx *types.Context) *HttpClient { @@ -126,11 +124,16 @@ func (c *HttpClient) DownloadFile(p string) (string, error) { } resp := client.Do(req) - pb := pterm.DefaultProgressbar.WithTotal(int(resp.Size())) - if c.ProgressBarArea != nil { - pb = pb.WithPrintTogether(c.ProgressBarArea) + + // Initialize a progressbar only if we have one in the current context + var pb *pterm.ProgressbarPrinter + if c.context.ProgressBar != nil { + pb = pterm.DefaultProgressbar.WithTotal(int(resp.Size())) + if c.context.AreaPrinter != nil { + pb = pb.WithPrintTogether(c.context.AreaPrinter) + } + pb, _ = pb.WithTitle(filepath.Base(resp.Request.HTTPRequest.URL.RequestURI())).Start() } - pb, _ = pb.WithTitle(filepath.Base(resp.Request.HTTPRequest.URL.RequestURI())).Start() // start download loop t := time.NewTicker(500 * time.Millisecond) defer t.Stop() @@ -140,11 +143,15 @@ func (c *HttpClient) DownloadFile(p string) (string, error) { for { select { case <-t.C: - // bar.Set64(resp.BytesComplete()) - //pb.Increment() - pb.Increment().Current = int(resp.BytesComplete()) + // update the progress bar + if pb != nil { + pb.Increment().Current = int(resp.BytesComplete()) + } case <-resp.Done: - pb.Increment().Current = int(resp.BytesComplete()) + // update the progress bar + if pb != nil { + pb.Increment().Current = int(resp.BytesComplete()) + } // download is complete break download_loop } @@ -157,7 +164,11 @@ func (c *HttpClient) DownloadFile(p string) (string, error) { c.context.Info("Downloaded", p, "of", fmt.Sprintf("%.2f", (float64(resp.BytesComplete())/1000)/1000), "MB (", fmt.Sprintf("%.2f", (float64(resp.BytesPerSecond())/1024)/1024), "MiB/s )") - pb.Stop() + + if pb != nil { + // stop the progressbar if active + pb.Stop() + } //bar.Finish() downloaded = true break diff --git a/pkg/installer/installer.go b/pkg/installer/installer.go index d537bbd6..7fb30519 100644 --- a/pkg/installer/installer.go +++ b/pkg/installer/installer.go @@ -32,7 +32,6 @@ import ( "github.com/mudler/luet/pkg/helpers" fileHelper "github.com/mudler/luet/pkg/helpers/file" "github.com/mudler/luet/pkg/helpers/match" - "github.com/mudler/luet/pkg/installer/client" pkg "github.com/mudler/luet/pkg/package" "github.com/mudler/luet/pkg/solver" "github.com/pterm/pterm" @@ -566,21 +565,30 @@ func (l *LuetInstaller) download(syncedRepos Repositories, toDownload map[string var wg = new(sync.WaitGroup) - area, _ := pterm.DefaultArea.Start() + ctx := l.Options.Context.Copy() - p, _ := pterm.DefaultProgressbar.WithPrintTogether(area).WithTotal(len(toDownload)).WithTitle("Downloading packages").Start() + // Check if the terminal is big enough to display a progress bar + // https://github.com/pterm/pterm/blob/4c725e56bfd9eb38e1c7b9dec187b50b93baa8bd/progressbar_printer.go#L190 + w, _, err := ctx.GetTerminalSize() + if ctx.IsTerminal && err == nil && w > 100 { + area, _ := pterm.DefaultArea.Start() + p, _ := pterm.DefaultProgressbar.WithPrintTogether(area).WithTotal(len(toDownload)).WithTitle("Downloading packages").Start() + + ctx.AreaPrinter = area + ctx.ProgressBar = p + defer area.Stop() + } // Download for i := 0; i < l.Options.Concurrency; i++ { wg.Add(1) - go l.downloadWorker(i, wg, all, p, area) + go l.downloadWorker(i, wg, all, ctx) } for _, c := range toDownload { all <- c } close(all) wg.Wait() - area.Stop() return nil } @@ -772,7 +780,7 @@ func (l *LuetInstaller) checkFileconflicts(toInstall map[string]ArtifactMatch, c filesToInstall := []string{} for _, m := range toInstall { - a, err := l.downloadPackage(m, nil) + a, err := l.downloadPackage(m, l.Options.Context) if err != nil && !l.Options.Force { return errors.Wrap(err, "Failed downloading package") } @@ -867,14 +875,9 @@ func (l *LuetInstaller) install(o Option, syncedRepos Repositories, toInstall ma return s.ExecuteFinalizers(l.Options.Context, toFinalize) } -func (l *LuetInstaller) downloadPackage(a ArtifactMatch, area *pterm.AreaPrinter) (*artifact.PackageArtifact, error) { +func (l *LuetInstaller) downloadPackage(a ArtifactMatch, ctx *types.Context) (*artifact.PackageArtifact, error) { - cli := a.Repository.Client(l.Options.Context) - - switch v := cli.(type) { - case *client.HttpClient: - v.ProgressBarArea = area - } + cli := a.Repository.Client(ctx) artifact, err := cli.DownloadArtifact(a.Artifact) if err != nil { @@ -890,7 +893,7 @@ func (l *LuetInstaller) downloadPackage(a ArtifactMatch, area *pterm.AreaPrinter func (l *LuetInstaller) installPackage(m ArtifactMatch, s *System) error { - a, err := l.downloadPackage(m, nil) + a, err := l.downloadPackage(m, l.Options.Context) if err != nil && !l.Options.Force { return errors.Wrap(err, "Failed downloading package") } @@ -910,20 +913,21 @@ func (l *LuetInstaller) installPackage(m ArtifactMatch, s *System) error { return s.Database.SetPackageFiles(&pkg.PackageFile{PackageFingerprint: m.Package.GetFingerPrint(), Files: files}) } -func (l *LuetInstaller) downloadWorker(i int, wg *sync.WaitGroup, c <-chan ArtifactMatch, pb *pterm.ProgressbarPrinter, area *pterm.AreaPrinter) error { +func (l *LuetInstaller) downloadWorker(i int, wg *sync.WaitGroup, c <-chan ArtifactMatch, ctx *types.Context) error { defer wg.Done() for p := range c { // TODO: Keep trace of what was added from the tar, and save it into system - _, err := l.downloadPackage(p, area) + _, err := l.downloadPackage(p, ctx) if err != nil { l.Options.Context.Error("Failed downloading package "+p.Package.GetName(), err.Error()) return errors.Wrap(err, "Failed downloading package "+p.Package.GetName()) } else { l.Options.Context.Success(":package: Package ", p.Package.HumanReadableString(), "downloaded") } - pb.Increment() - + if ctx.ProgressBar != nil { + ctx.ProgressBar.Increment() + } } return nil