From ebf6a40f0a9377bc6f77f242d99a8b9a6803a1eb Mon Sep 17 00:00:00 2001 From: Caio Marcelo de Oliveira Filho Date: Fri, 5 Jan 2018 17:41:19 -0800 Subject: [PATCH] mixer: reduce noise when commands fail Only print errors once, not twice. Do not print usage information unless the command was called with invalid arguments. Because cobra already print its own errors, don't print them again in Execute function, just ensure we get the right exit code. Mixer errors will be handled by our code, instead of passing the errors to cobra and then later get them back. That way no usage is printed. Two helper functions were added fail/failf to perform Print+Exit. Signed-off-by: Caio Marcelo de Oliveira Filho --- mixer/cmd/build.go | 26 +++++++++++++------------- mixer/cmd/bundles.go | 9 +++------ mixer/cmd/root.go | 18 +++++++++++++++--- mixer/cmd/rpms.go | 14 ++++++++------ 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/mixer/cmd/build.go b/mixer/cmd/build.go index 50ca8923c..e44106881 100644 --- a/mixer/cmd/build.go +++ b/mixer/cmd/build.go @@ -72,9 +72,12 @@ var buildChrootsCmd = &cobra.Command{ Use: "chroots", Short: "Build the chroots for your mix", Long: `Build the chroots for your mix`, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { b := builder.NewFromConfig(config) - return buildChroots(b, buildFlags.noSigning) + err := buildChroots(b, buildFlags.noSigning) + if err != nil { + fail(err) + } }, } @@ -82,17 +85,16 @@ var buildUpdateCmd = &cobra.Command{ Use: "update", Short: "Build the update content for your mix", Long: `Build the update content for your mix`, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { b := builder.NewFromConfig(config) err := b.BuildUpdate(buildFlags.prefix, buildFlags.minVersion, buildFlags.format, buildFlags.noSigning, !buildFlags.noPublish, buildFlags.keepChroot) if err != nil { - return errors.Wrap(err, "couldn't build update") + failf("couldn't build update: %s", err) } if buildFlags.increment { b.UpdateMixVer() } - return nil }, } @@ -100,26 +102,25 @@ var buildAllCmd = &cobra.Command{ Use: "all", Short: "Build all content for mix with default options", Long: `Build all content for mix with default options`, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { b := builder.NewFromConfig(config) rpms, err := ioutil.ReadDir(b.RPMdir) if err == nil { err = b.AddRPMList(rpms) if err != nil { - return errors.Wrap(err, "couldn't add the RPMs") + failf("couldn't add the RPMs: %s", err) } } err = buildChroots(b, buildFlags.noSigning) if err != nil { - return errors.Wrap(err, "Error building chroots") + failf("couldn't build chroots: %s", err) } err = b.BuildUpdate(buildFlags.prefix, buildFlags.minVersion, buildFlags.format, buildFlags.noSigning, !buildFlags.noPublish, buildFlags.keepChroot) if err != nil { - return errors.Wrap(err, "Error building update") + failf("couldn't build update: %s", err) } b.UpdateMixVer() - return nil }, } @@ -127,13 +128,12 @@ var buildImageCmd = &cobra.Command{ Use: "image", Short: "Build an image from the mix content", Long: `Build an image from the mix content`, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { b := builder.NewFromConfig(config) err := b.BuildImage(buildFlags.format, buildFlags.template) if err != nil { - return errors.Wrap(err, "Error building image") + failf("couldn't build image: %s", err) } - return nil }, } diff --git a/mixer/cmd/bundles.go b/mixer/cmd/bundles.go index f23e1a822..680bb7b80 100644 --- a/mixer/cmd/bundles.go +++ b/mixer/cmd/bundles.go @@ -20,7 +20,6 @@ import ( "github.com/clearlinux/mixer-tools/builder" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -41,10 +40,10 @@ var addBundlesCmd = &cobra.Command{ Use: "add [bundle(s)]", Short: "Add clr-bundles to your mix", Long: `Add clr-bundles to your mix`, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { if bundleFlags.all == false { if len(args) <= 0 { - return errors.New("bundle add requires at least 1 argument if --all is not passed") + failf("bundle add requires at least 1 argument if --all is not passed") } } bundles := strings.Split(args[0], ",") @@ -52,7 +51,6 @@ var addBundlesCmd = &cobra.Command{ // TODO change this to return (int, error) numadded := b.AddBundles(bundles, bundleFlags.force, bundleFlags.all, bundleFlags.git) fmt.Println(numadded, " bundles were added") - return nil }, } @@ -60,12 +58,11 @@ var getBundlesCmd = &cobra.Command{ Use: "get", Short: "Get the clr-bundles from upstream", Long: `Get the clr-bundles from upstream`, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { b := builder.NewFromConfig(config) fmt.Println("Getting clr-bundles for version " + b.Clearver) // TODO change this to return an error b.UpdateRepo(b.Clearver, false) - return nil }, } diff --git a/mixer/cmd/root.go b/mixer/cmd/root.go index 61c545108..2415443bb 100644 --- a/mixer/cmd/root.go +++ b/mixer/cmd/root.go @@ -46,11 +46,14 @@ var initCmd = &cobra.Command{ Use: "init-mix", Short: "Initialize the mixer and workspace", Long: `Initialize the mixer and workspace`, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { b := builder.New() b.LoadBuilderConf(config) b.ReadBuilderConf() - return b.InitMix(strconv.Itoa(initFlags.clearver), strconv.Itoa(initFlags.mixver), initFlags.all, initFlags.upstreamurl) + err := b.InitMix(strconv.Itoa(initFlags.clearver), strconv.Itoa(initFlags.mixver), initFlags.all, initFlags.upstreamurl) + if err != nil { + fail(err) + } }, } @@ -58,7 +61,6 @@ var initCmd = &cobra.Command{ // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { if err := RootCmd.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "Mixer Error: %s\n", err) os.Exit(1) } } @@ -96,3 +98,13 @@ func init() { initCmd.Flags().StringVar(&config, "config", "", "Supply a specific builder.conf to use for mixing") initCmd.Flags().StringVar(&initFlags.upstreamurl, "upstream-url", "https://download.clearlinux.org", "Supply an upstream URL to use for mixing") } + +func fail(err error) { + fmt.Fprintf(os.Stderr, "ERROR: %s\n", err) + os.Exit(1) +} + +func failf(format string, a ...interface{}) { + fmt.Fprintf(os.Stderr, fmt.Sprintf("ERROR: %s\n", format), a...) + os.Exit(1) +} diff --git a/mixer/cmd/rpms.go b/mixer/cmd/rpms.go index cd8f4c1df..69b0c0886 100644 --- a/mixer/cmd/rpms.go +++ b/mixer/cmd/rpms.go @@ -19,7 +19,6 @@ import ( "github.com/clearlinux/mixer-tools/builder" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -27,7 +26,7 @@ var addRPMCmd = &cobra.Command{ Use: "add-rpms", Short: "Add RPMs to local yum repository", Long: `Add RPMS from the configured RPMDIR to local yum repository`, - RunE: runAddRPM, + Run: runAddRPM, } var rpmCmds = []*cobra.Command{ @@ -41,14 +40,17 @@ func init() { } } -func runAddRPM(cmd *cobra.Command, args []string) error { +func runAddRPM(cmd *cobra.Command, args []string) { b := builder.NewFromConfig(config) if b.RPMdir == "" { - return errors.Errorf("RPMDIR not set in configuration") + failf("RPMDIR not set in configuration") } rpms, err := ioutil.ReadDir(b.RPMdir) if err != nil { - return errors.Wrapf(err, "cannot read RPMDIR") + failf("cannot read RPMDIR: %s", err) + } + err = b.AddRPMList(rpms) + if err != nil { + fail(err) } - return b.AddRPMList(rpms) }