[question] How to dynamically discover sub-commands and execute ?

riton commented 1 year ago

Context

I'm trying to create a build helper for multiple tools. The command hierarchy would be such as:

$ mycmd build tool1
$ mycmd build tool2

I would like to have a special subcommand of the build command, named all. Ideally, this all subcommand would discover all the subcommands of the build command and execute them. Like if mycmd build tool1 && mycmd build tool2 were called.

Software versions used

go.mod

``` module github.com/riton/cobra-traverse-and-exec-child-cmd go 1.19 require ( github.com/pkg/errors v0.9.1 github.com/spf13/cobra v1.6.1 ) require ( github.com/inconshreveable/mousetrap v1.0.1 // indirect github.com/spf13/pflag v1.0.5 // indirect ) ```

$ go version
go version go1.19.3 linux/amd64

What I've tried

Most of the code bellow was generated using cobra-cli.

All of the other commands code

cmd/root.go

```go package cmd import ( "fmt" "os" "github.com/spf13/cobra" ) // rootCmd represents the base command when called without any subcommands var rootCmd = &cobra.Command{ Use: "cobra-traverse-and-exec-child-cmd", PersistentPreRun: func(cmd *cobra.Command, args []string) { fmt.Println("[rootCmd] PersistentPreRun()") }, } // Execute adds all child commands to the root command and sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { err := rootCmd.Execute() if err != nil { os.Exit(1) } } ```

cmd/build.go

```go package cmd import ( "fmt" "github.com/spf13/cobra" ) // buildCmd represents the build command var buildCmd = &cobra.Command{ Use: "build", PersistentPreRun: func(cmd *cobra.Command, args []string) { rootCmd.PersistentPreRun(rootCmd, args) fmt.Println("[buildCmd] PersistentPreRun()") }, } func init() { rootCmd.AddCommand(buildCmd) } ```

cmd/build_tool1.go

```go package cmd import ( "fmt" "github.com/spf13/cobra" ) // buildTool1Cmd represents the buildTool1 command var buildTool1Cmd = &cobra.Command{ Use: "tool1", Run: func(cmd *cobra.Command, args []string) { fmt.Println("build_tool_1 called") }, } func init() { buildCmd.AddCommand(buildTool1Cmd) } ```

cmd/build_tool2.go

```go package cmd import ( "fmt" "github.com/spf13/cobra" ) // buildTool2cmd represents the buildTool2 command var buildTool2Cmd = &cobra.Command{ Use: "tool2", Run: func(cmd *cobra.Command, args []string) { fmt.Println("build_tool_2 called") }, } func init() { buildCmd.AddCommand(buildTool2Cmd) } ```

Attempt to discover and execute the sub commands

And here is what I've tried to use to discover and execute all of the buildTool1Cmd and buildTool2Cmd:

Filename: cmd/build_all_tools.go

package cmd

import (
    "fmt"

    "github.com/pkg/errors"
    "github.com/spf13/cobra"
)

// allCmd represents the all command
var allCmd = &cobra.Command{
    Use:   "all",
    Short: "Build all software",
    RunE:  buildAllRun,
}

func init() {
    buildCmd.AddCommand(allCmd)
}

// buildAllRun should discover all 'build' child commands
// other than 'all' and execute them.
func buildAllRun(cmd *cobra.Command, args []string) error {
    // Get all build commands, except the 'all' command to avoid infinite recursion
    var buildSoftwareCmds []*cobra.Command
    for _, builderCmd := range buildCmd.Commands() {
        if builderCmd != cmd {
            fmt.Printf("[buildAllRun] discovered cmd.Use = %q\n", builderCmd.Use)
            buildSoftwareCmds = append(buildSoftwareCmds, builderCmd)
        }
    }

    for _, builderCmd := range buildSoftwareCmds {
        fmt.Printf("[buildAllRun] executing command = %q\n", builderCmd.Use)
        //
        // Problem: This 'Execute()' call will cause infinite recursion :-(
        //
        if err := builderCmd.Execute(); err != nil {
            return errors.Wrapf(err, "running command %s", builderCmd.Use)
        }
    }

    return nil
}

Sub command discovery seem to work. The buildSoftwareCmds array in the code above successfully contains buildTool1Cmd and buildTool2Cmd command definitions.

As a side note: I'm not directly calling the Run() or RunE() functions on the subcommands here to honor the PersistenPreRun() logic that was positioned on the parents.

Problem

The problem is with my call to the Execute() function on any of those commands. It drops me in an infinite recursion...

Here is a sample of the execution output:

$ go run . build all > /tmp/build_all.log

=> kill the command using Ctrl-C

$ head -10 /tmp/build_all.log
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
[buildAllRun] discovered cmd.Use = "tool1"
[buildAllRun] discovered cmd.Use = "tool2"
[buildAllRun] executing command = "tool1"
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
[buildAllRun] discovered cmd.Use = "tool1"
[buildAllRun] discovered cmd.Use = "tool2"
[buildAllRun] executing command = "tool1"

Any help on how to solve this specific problem would be really appreciated :pray:

Thanks in advance

Regards

Rémi

riton commented 1 year ago

I've found a fix for my problem by calling rootCmd.SetArgs([]string{builderCmd.Parent().Use, builderCmd.Use}) just before builderCmd.Execute()

Here is my current solution:

func buildAllRun(cmd *cobra.Command, args []string) error {
    // Get all build commands, except the 'all' command to avoid infinite recursion
    [... previous code left unchanged ...]

    for _, builderCmd := range buildSoftwareCmds {
        [... previous code left unchanged ...]
        //
        // SetArgs() on the rootCmd to avoid infinite recursion
        //
        rootCmd.SetArgs([]string{builderCmd.Parent().Use, builderCmd.Use})
        if err := builderCmd.Execute(); err != nil {
            return errors.Wrapf(err, "running command %s", builderCmd.Use)
        }
    }

    return nil
}

Everything is now working as I expect:

$ go run . build all
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
[buildAllRun] discovered cmd.Use = "tool1"
[buildAllRun] discovered cmd.Use = "tool2"
[buildAllRun] executing command = "tool1"
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
buildTool1 called
[buildAllRun] executing command = "tool2"
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
buildTool2 called

Is this the recommended way to achieve such behavior ?

Thanks in advance, Regards

riton commented 1 year ago

Damn, I've just discovered that #1168 existed which was exactly what I'm trying to do.

Problem, even after reading this issue, I'm not really sure If my solution is the recommended way to achieve such behavior...

github-actions[bot] commented 1 year ago

The Cobra project currently lacks enough contributors to adequately respond to all issues. This bot triages issues and PRs according to the following rules:

After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the issue is closed. You can:
Make a comment to remove the stale label and show your support. The 60 days reset. - If an issue has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interested in reopening

AmadeusK525 commented 1 year ago

I'm thinking the problem is that you're calling builderCmd.Execute() instead of builderCmd.Run(), @riton. Take a look at these lines:

// Regardless of what command execute is called on, run on Root only
if c.HasParent() {
    return c.Root().ExecuteC()
}

These are present in the Execute function, which I believe should only be called on the root command, and it's probably the cause of the recursion. builderCmd.Run() should work fine

migueleliasweb commented 3 months ago

These are present in the Execute function, which I believe should only be called on the root command, and it's probably the cause of the recursion. builderCmd.Run() should work fine

It seems we ended up doing the same kind of investigation.

As mentioned here https://github.com/spf13/cobra/issues/379, the issue of calling Run() or RunE directly is that you skip all possible hooks that might be in use for the command, sush as PostRun() or Args().

The workaround solution, if you only really want to call the subcommand programatically and not via the CLI, is to remove them from the root (basically making them not a real CLI command). This allows you to call .Execute() safely but at a cost.

migueleliasweb commented 3 months ago

I've found a fix for my problem by calling rootCmd.SetArgs([]string{builderCmd.Parent().Use, builderCmd.Use}) just before builderCmd.Execute()

Here is my current solution:

func buildAllRun(cmd *cobra.Command, args []string) error {
  // Get all build commands, except the 'all' command to avoid infinite recursion
  [... previous code left unchanged ...]

  for _, builderCmd := range buildSoftwareCmds {
      [... previous code left unchanged ...]
      //
      // SetArgs() on the rootCmd to avoid infinite recursion
      //
      rootCmd.SetArgs([]string{builderCmd.Parent().Use, builderCmd.Use})
      if err := builderCmd.Execute(); err != nil {
          return errors.Wrapf(err, "running command %s", builderCmd.Use)
      }
  }

  return nil
}

Everything is now working as I expect:

$ go run . build all
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
[buildAllRun] discovered cmd.Use = "tool1"
[buildAllRun] discovered cmd.Use = "tool2"
[buildAllRun] executing command = "tool1"
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
buildTool1 called
[buildAllRun] executing command = "tool2"
[rootCmd] PersistentPreRun()
[buildCmd] PersistentPreRun()
buildTool2 called

Is this the recommended way to achieve such behavior ?

Thanks in advance, Regards

I think this might be the only way currently to achieve this. It doesn't look particularly great but it gets the job done and it means you have will access to the hooks from the command you're trying to call. Great work finding this!

spf13 / cobra