From 7d4b68a26f911ebdbc3b8066d63eaaf2d860dc97 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 9 Jan 2025 10:22:16 +0100 Subject: [PATCH] main: add --verbose,-v persistent flag that increases verbosity This commit adds `--verbose,-v` which will increase the verbosity of logrus and also switch the --progress to "verbose". This is addressing the feedback we got in https://github.com/osbuild/bootc-image-builder/pull/765 and a followup for #776 The new `-v` clashes unfortunately with cobras default for version, so there is no single dash flag for version anymore. Most unix tools (e.g. cp,rsync,mv,curl,ssh,tar) use "-v" for "--verbose" so IMHO we should follow suite. Unfortuantely there is no consistency in linux, e.g. git,gcc are counter examples where it means version). I would still go with -v for verbose as ssh,tar,curl are probably used more often to get verbose output. --- bib/cmd/bootc-image-builder/main.go | 27 +++++++++----- bib/cmd/bootc-image-builder/main_test.go | 46 ++++++++++++++++++++++++ test/test_opts.py | 2 +- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/bib/cmd/bootc-image-builder/main.go b/bib/cmd/bootc-image-builder/main.go index 1aac2420..bc734f5c 100644 --- a/bib/cmd/bootc-image-builder/main.go +++ b/bib/cmd/bootc-image-builder/main.go @@ -546,18 +546,26 @@ func chownR(path string, chown string) error { var rootLogLevel string func rootPreRunE(cmd *cobra.Command, _ []string) error { - if rootLogLevel == "" { + verbose, _ := cmd.Flags().GetBool("verbose") + progress, _ := cmd.Flags().GetString("progress") + switch { + case rootLogLevel != "": + level, err := logrus.ParseLevel(rootLogLevel) + if err != nil { + return err + } + logrus.SetLevel(level) + case verbose: + logrus.SetLevel(logrus.InfoLevel) + default: logrus.SetLevel(logrus.ErrorLevel) - return nil } - - level, err := logrus.ParseLevel(rootLogLevel) - if err != nil { - return err + if verbose && progress == "auto" { + if err := cmd.Flags().Set("progress", "verbose"); err != nil { + return err + } } - logrus.SetLevel(level) - return nil } @@ -607,6 +615,7 @@ func buildCobraCmdline() (*cobra.Command, error) { rootCmd.SetVersionTemplate(version) rootCmd.PersistentFlags().StringVar(&rootLogLevel, "log-level", "", "logging level (debug, info, error); default error") + rootCmd.PersistentFlags().BoolP("verbose", "v", false, `Switch to verbose mode`) buildCmd := &cobra.Command{ Use: "build IMAGE_NAME", @@ -620,7 +629,7 @@ func buildCobraCmdline() (*cobra.Command, error) { SilenceUsage: true, Example: rootCmd.Use + " build quay.io/centos-bootc/centos-bootc:stream9\n" + rootCmd.Use + " quay.io/centos-bootc/centos-bootc:stream9\n", - Version: rootCmd.Version, + Version: rootCmd.Version, } buildCmd.SetVersionTemplate(version) diff --git a/bib/cmd/bootc-image-builder/main_test.go b/bib/cmd/bootc-image-builder/main_test.go index bfc16601..5cb5d97b 100644 --- a/bib/cmd/bootc-image-builder/main_test.go +++ b/bib/cmd/bootc-image-builder/main_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -573,3 +574,48 @@ func TestCobraCmdline(t *testing.T) { }) } } + +func TestCobraCmdlineVerbose(t *testing.T) { + for _, tc := range []struct { + cmdline []string + expectedProgress string + expectedLogrusLevel logrus.Level + }{ + { + []string{"quay.io..."}, + "auto", + logrus.ErrorLevel, + }, + { + []string{"-v", "quay.io..."}, + "verbose", + logrus.InfoLevel, + }, + } { + restore := mockOsArgs(tc.cmdline) + defer restore() + + rootCmd, err := main.BuildCobraCmdline() + assert.NoError(t, err) + + // collect progressFlag value + var progressFlag string + for _, cmd := range rootCmd.Commands() { + cmd.RunE = func(cmd *cobra.Command, args []string) error { + if progressFlag != "" { + t.Error("progressFlag set twice") + } + progressFlag, err = cmd.Flags().GetString("progress") + assert.NoError(t, err) + return nil + } + } + + t.Run(tc.expectedProgress, func(t *testing.T) { + err = rootCmd.Execute() + assert.NoError(t, err) + assert.Equal(t, tc.expectedProgress, progressFlag) + assert.Equal(t, tc.expectedLogrusLevel, logrus.GetLevel()) + }) + } +} diff --git a/test/test_opts.py b/test/test_opts.py index 94cb5efe..28641f13 100644 --- a/test/test_opts.py +++ b/test/test_opts.py @@ -148,7 +148,7 @@ def test_bib_errors_only_once(tmp_path, container_storage, build_fake_container) assert res.stderr.count(needle) == 1 -@pytest.mark.parametrize("version_argument", ["version", "--version", "-v"]) +@pytest.mark.parametrize("version_argument", ["version", "--version"]) def test_bib_version(tmp_path, container_storage, build_fake_container, version_argument): output_path = tmp_path / "output" output_path.mkdir(exist_ok=True)