diff --git a/bib/internal/progress/export_test.go b/bib/internal/progress/export_test.go index c8318242..c6d8e17e 100644 --- a/bib/internal/progress/export_test.go +++ b/bib/internal/progress/export_test.go @@ -17,3 +17,11 @@ func MockOsStderr(w io.Writer) (restore func()) { osStderr = saved } } + +func MockIsattyIsTerminal(fn func(uintptr) bool) (restore func()) { + saved := isattyIsTerminal + isattyIsTerminal = fn + return func() { + isattyIsTerminal = saved + } +} diff --git a/bib/internal/progress/progress.go b/bib/internal/progress/progress.go index 7e6eefa9..66313078 100644 --- a/bib/internal/progress/progress.go +++ b/bib/internal/progress/progress.go @@ -11,6 +11,7 @@ import ( "time" "github.com/cheggaaa/pb/v3" + "github.com/mattn/go-isatty" "github.com/sirupsen/logrus" "github.com/osbuild/images/pkg/osbuild" @@ -66,12 +67,19 @@ type ProgressBar interface { Stop() } +var isattyIsTerminal = isatty.IsTerminal + // New creates a new progressbar based on the requested type func New(typ string) (ProgressBar, error) { switch typ { - // XXX: autoseelct based on PS1 value (i.e. use term in - // interactive shells only?) - case "", "plain": + case "": + // autoselect based on if we are on an interactive + // terminal, use plain progress for scripts + if isattyIsTerminal(os.Stdin.Fd()) { + return NewTerminalProgressBar() + } + return NewPlainProgressBar() + case "plain": return NewPlainProgressBar() case "term": return NewTerminalProgressBar() diff --git a/bib/internal/progress/progress_test.go b/bib/internal/progress/progress_test.go index 211fc221..326513f0 100644 --- a/bib/internal/progress/progress_test.go +++ b/bib/internal/progress/progress_test.go @@ -111,3 +111,22 @@ func TestTermProgress(t *testing.T) { // check shutdown assert.Contains(t, buf.String(), progress.CURSOR_SHOW) } + +func TestProgressNewAutoselect(t *testing.T) { + for _, tc := range []struct { + onTerm bool + expected interface{} + }{ + {false, &progress.PlainProgressBar{}}, + {true, &progress.TerminalProgressBar{}}, + } { + restore := progress.MockIsattyIsTerminal(func(uintptr) bool { + return tc.onTerm + }) + defer restore() + + pb, err := progress.New("") + assert.NoError(t, err) + assert.Equal(t, reflect.TypeOf(pb), reflect.TypeOf(tc.expected), fmt.Sprintf("[%v] %T not the expected %T", tc.onTerm, pb, tc.expected)) + } +} diff --git a/test/test_progress.py b/test/test_progress.py index b531eb92..d83cad97 100644 --- a/test/test_progress.py +++ b/test/test_progress.py @@ -5,21 +5,18 @@ from containerbuild import build_container_fixture, build_fake_container_fixture -def test_progress_debug(tmp_path, container_storage, build_fake_container): +def test_progress_debug(tmp_path, build_fake_container): output_path = tmp_path / "output" output_path.mkdir(exist_ok=True) cmdline = [ "podman", "run", "--rm", "--privileged", - "--security-opt", "label=type:unconfined_t", - "-v", f"{container_storage}:/var/lib/containers/storage", - "-v", f"{output_path}:/output", build_fake_container, "build", + "--progress=debug", "quay.io/centos-bootc/centos-bootc:stream9", ] - cmdline.append("--progress=debug") res = subprocess.run(cmdline, capture_output=True, check=True, text=True) assert res.stderr.count("Start progressbar") == 1 assert res.stderr.count("Manifest generation step") == 1 @@ -29,22 +26,41 @@ def test_progress_debug(tmp_path, container_storage, build_fake_container): assert res.stdout.strip() == "" -def test_progress_term(tmp_path, container_storage, build_fake_container): +def test_progress_term_works_without_tty(tmp_path, build_fake_container): output_path = tmp_path / "output" output_path.mkdir(exist_ok=True) cmdline = [ "podman", "run", "--rm", + # note that "-t" is missing here "--privileged", - "--security-opt", "label=type:unconfined_t", - "-v", f"{container_storage}:/var/lib/containers/storage", - "-v", f"{output_path}:/output", build_fake_container, "build", - # explicitly select term progress + # explicitly selecting term progress works even when there is no tty + # (i.e. we just need ansi terminal support) "--progress=term", "quay.io/centos-bootc/centos-bootc:stream9", ] res = subprocess.run(cmdline, capture_output=True, text=True, check=False) assert res.returncode == 0 assert "[|] Manifest generation step" in res.stderr + + +def test_progress_term_autoselect(tmp_path, build_fake_container): + output_path = tmp_path / "output" + output_path.mkdir(exist_ok=True) + + cmdline = [ + "podman", "run", "--rm", + # we have a terminal + "-t", + "--privileged", + build_fake_container, + "build", + # note that we do not select a --progress here so auto-select is used + "quay.io/centos-bootc/centos-bootc:stream9", + ] + res = subprocess.run(cmdline, capture_output=True, text=True, check=False) + assert res.returncode == 0 + # its curious that we get the output on stdout here, podman weirdness? + assert "[|] Manifest generation step" in res.stdout