diff --git a/bib/cmd/bootc-image-builder/main.go b/bib/cmd/bootc-image-builder/main.go index 80e39dc2..e466a004 100644 --- a/bib/cmd/bootc-image-builder/main.go +++ b/bib/cmd/bootc-image-builder/main.go @@ -244,9 +244,7 @@ func manifestFromCobra(cmd *cobra.Command, args []string, pbar progress.Progress } pbar.SetPulseMsgf("Manifest generation step") - if err := pbar.Start(); err != nil { - return nil, nil, fmt.Errorf("cannot start progress: %v", err) - } + pbar.Start() if err := setup.ValidateHasContainerTags(imgref); err != nil { return nil, nil, err @@ -434,11 +432,7 @@ func cmdBuild(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("cannto create progress bar: %w", err) } - defer func() { - if err := pbar.Stop(); err != nil { - logrus.Warnf("progressbar stopping failed: %v", err) - } - }() + defer pbar.Stop() manifest_fname := fmt.Sprintf("manifest-%s.json", strings.Join(imgTypes, "-")) pbar.SetMessagef("Generating manifest %s", manifest_fname) diff --git a/bib/internal/progress/progress.go b/bib/internal/progress/progress.go index e5ac2a80..e4d79113 100644 --- a/bib/internal/progress/progress.go +++ b/bib/internal/progress/progress.go @@ -38,6 +38,10 @@ type ProgressBar interface { // SetProgress sets the progress details at the given "level". // Levels should start with "0" and increase as the nesting // gets deeper. + // + // Note that reducing depth is currently not supported, once + // a sub-progress is added it cannot be removed/hidden + // (but if required it can be added, its a SMOP) SetProgress(level int, msg string, done int, total int) error // The high-level message that is displayed in a spinner @@ -52,8 +56,13 @@ type ProgressBar interface { // For us this usually comes from the stages and has information // like "Starting module org.osbuild.selinux" SetMessagef(fmt string, args ...interface{}) - Start() error - Stop() error + + // Start will start rendering the progress information + Start() + + // Stop will stop rendering the progress information, the + // screen is not cleared, the last few lines will be visible + Stop() } // New creates a new progressbar based on the requested type @@ -165,21 +174,19 @@ func (b *terminalProgressBar) renderLoop() { } } -func (b *terminalProgressBar) Start() error { +func (b *terminalProgressBar) Start() { // render() already running if b.shutdownCh != nil { - return nil + return } fmt.Fprintf(b.out, "%s", CURSOR_HIDE) b.shutdownCh = make(chan bool) go b.renderLoop() - - return nil } -func (b *terminalProgressBar) Stop() (err error) { +func (b *terminalProgressBar) Stop() { if b.shutdownCh == nil { - return nil + return } // request shutdown b.shutdownCh <- true @@ -188,10 +195,12 @@ func (b *terminalProgressBar) Stop() (err error) { case <-b.shutdownCh: // shudown complete case <-time.After(1 * time.Second): + // I cannot think of how this could happen, i.e. why + // closing would not work but lets be conservative - + // without a timeout we hang here forever logrus.Warnf("no progress channel shutdown after 1sec") } b.shutdownCh = nil - return nil } type plainProgressBar struct { @@ -213,12 +222,10 @@ func (b *plainProgressBar) SetMessagef(msg string, args ...interface{}) { fmt.Fprintf(b.w, msg, args...) } -func (b *plainProgressBar) Start() (err error) { - return nil +func (b *plainProgressBar) Start() { } -func (b *plainProgressBar) Stop() (err error) { - return nil +func (b *plainProgressBar) Stop() { } func (b *plainProgressBar) SetProgress(subLevel int, msg string, done int, total int) error { @@ -250,14 +257,12 @@ func (b *debugProgressBar) SetMessagef(msg string, args ...interface{}) { fmt.Fprintf(b.w, "\n") } -func (b *debugProgressBar) Start() (err error) { +func (b *debugProgressBar) Start() { fmt.Fprintf(b.w, "Start progressbar\n") - return nil } -func (b *debugProgressBar) Stop() (err error) { +func (b *debugProgressBar) Stop() { fmt.Fprintf(b.w, "Stop progressbar\n") - return nil } func (b *debugProgressBar) SetProgress(subLevel int, msg string, done int, total int) error { diff --git a/bib/internal/progress/progress_test.go b/bib/internal/progress/progress_test.go index 364d96a9..b7efca13 100644 --- a/bib/internal/progress/progress_test.go +++ b/bib/internal/progress/progress_test.go @@ -54,11 +54,9 @@ func TestPlainProgress(t *testing.T) { assert.Equal(t, "message", buf.String()) buf.Reset() - err = pbar.Start() - assert.NoError(t, err) + pbar.Start() assert.Equal(t, "", buf.String()) - err = pbar.Stop() - assert.NoError(t, err) + pbar.Stop() assert.Equal(t, "", buf.String()) } @@ -82,13 +80,11 @@ func TestDebugProgress(t *testing.T) { assert.Equal(t, "msg: some-message\n", buf.String()) buf.Reset() - err = pbar.Start() - assert.NoError(t, err) + pbar.Start() assert.Equal(t, "Start progressbar\n", buf.String()) buf.Reset() - err = pbar.Stop() - assert.NoError(t, err) + pbar.Stop() assert.Equal(t, "Stop progressbar\n", buf.String()) buf.Reset() } @@ -101,14 +97,12 @@ func TestTermProgress(t *testing.T) { pbar, err := progress.NewTerminalProgressBar() assert.NoError(t, err) - err = pbar.Start() - assert.NoError(t, err) + pbar.Start() pbar.SetPulseMsgf("pulse-msg") pbar.SetMessagef("some-message") err = pbar.SetProgress(0, "set-progress-msg", 0, 5) assert.NoError(t, err) - err = pbar.Stop() - assert.NoError(t, err) + pbar.Stop() assert.Contains(t, buf.String(), "[1 / 6] set-progress-msg") assert.Contains(t, buf.String(), "[|] pulse-msg\n")