Why am I getting a nil pointer error depending on where I call BindPFlag?

Question

I've just recently started working with Go, and I've run into some behavior working with Cobra and Viper that I'm not sure I understand.

This is a slightly modified version of the sample code you get by running cobra init. In main.go I have:

package main

import (
    "github.com/larsks/example/cmd"
    "github.com/spf13/cobra"
)

func main() {
    rootCmd := cmd.NewCmdRoot()
    cobra.CheckErr(rootCmd.Execute())
}

In cmd/root.go I have:

package cmd

import (
    "fmt"
    "os"

    "github.com/spf13/cobra"

    "github.com/spf13/viper"
)

var cfgFile string

func NewCmdRoot() *cobra.Command {
    config := viper.New()

    var cmd = &cobra.Command{
        Use:   "example",
        Short: "A brief description of your application",
        PersistentPreRun: func(cmd *cobra.Command, args []string) {
            initConfig(cmd, config)
        },
        Run: func(cmd *cobra.Command, args []string) {
            fmt.Printf("This is a test\n")
        },
    }

    cmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.example.yaml)")
    cmd.PersistentFlags().String("name", "", "a name")

  // *** If I move this to the top of initConfig
  // *** the code runs correctly.
    config.BindPFlag("name", cmd.Flags().Lookup("name"))

    return cmd
}

func initConfig(cmd *cobra.Command, config *viper.Viper) {
    if cfgFile != "" {
        // Use config file from the flag.
        config.SetConfigFile(cfgFile)
    } else {
        config.AddConfigPath(".")
        config.SetConfigName(".example")
    }

    config.AutomaticEnv() // read in environment variables that match

    // If a config file is found, read it in.
    if err := config.ReadInConfig(); err == nil {
        fmt.Fprintln(os.Stderr, "Using config file:", config.ConfigFileUsed())
    }

  // *** This line triggers a nil pointer reference.
    fmt.Printf("name is %s\n", config.GetString("name"))
}

This code will panic with a nil pointer reference at the final call to fmt.Printf:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x50 pc=0x6a90e5]

If I move the call to config.BindPFlag from the NewCmdRoot function to the top of the initConfig command, everything runs without a problem.

What's going on here? According to the Viper docs regarding the use of BindPFlags:

Like BindEnv, the value is not set when the binding method is called, but when it is accessed. This means you can bind as early as you want, even in an init() function.

That's almost exactly what I'm doing here. At the time I call config.BindPflag, config is non-nil, cmd is non-nil, and the name argument has been registered.

I assume there's something going on with my use of config in a closure in PersistentPreRun, but I don't know exactly why that is causing this failure.

Answer 1

I don't have any issue if I use cmd.PersistentFlags().Lookup("name").

    // *** If I move this to the top of initConfig
    // *** the code runs correctly.
    pflag := cmd.PersistentFlags().Lookup("name")
    config.BindPFlag("name", pflag)

Considering you just registered persistent flags (flag will be available to the command it's assigned to as well as every command under that command), it is safer to call cmd.PersistentFlags().Lookup("name"), rather than cmd.Flags().Lookup("name").

The latter returns nil, since the PersistentPreRun is only called when rootCmd.Execute() is called, which is after cmd.NewCmdRoot().
At cmd.NewCmdRoot() levels, flags have not yet been initialized, even after some were declared "persistent".

Answer 2

I thought this was interesting so I did some digging and found your exact problem documented in an issue. The problematic line is this:

config.BindPFlag("name", cmd.Flags().Lookup("name"))
//                           ^^^^^^^

You created a persistent flag, but bound the flag to the Flags property. If you change your code to bind to PersistentFlags, everything will work as intended even with this line in NewCmdRoot:

config.BindPFlag("name", cmd.PersistentFlags().Lookup("name"))

Answer 3

This ends up being a little more complex than it might appear at first glance, so while the other answers here helped me resolve the problem I'd like to add a little detail.

There are some nuances in the documentation that aren't particularly clear if you're just starting to work with Cobra. Let's start with the documentation for the PersistentFlags method:

PersistentFlags returns the persistent FlagSet specifically set in the current command.

The key is in ...in the current command. In my NewCmdRoot root method, we can use cmd.PersistentFlags() because the root command is the current command. We can even use cmd.PersistentFlags() in the PersistentPreRun method, as long as we're not processing a subcommand.

If we were to re-write cmd/root.go from the example so that it includes a subcommand, like this...

package cmd

import (
    "fmt"
    "os"

    "github.com/spf13/cobra"
    "github.com/spf13/viper"
)

var cfgFile string

func NewCmdSubcommand() *cobra.Command {
    var cmd = &cobra.Command{
        Use:   "subcommand",
        Short: "An example subcommand",
        Run: func(cmd *cobra.Command, args []string) {
            fmt.Printf("This is an example subcommand\n")
        },
    }

    return cmd
}

func NewCmdRoot() *cobra.Command {
    config := viper.New()

    var cmd = &cobra.Command{
        Use:   "example",
        Short: "A brief description of your application",
        PersistentPreRun: func(cmd *cobra.Command, args []string) {
            initConfig(cmd, config)
        },
        Run: func(cmd *cobra.Command, args []string) {
            fmt.Printf("Hello, world\n")
        },
    }

    cmd.PersistentFlags().StringVar(
    &cfgFile, "config", "", "config file (default is $HOME/.example.yaml)")
    cmd.PersistentFlags().String("name", "", "a name")

    cmd.AddCommand(NewCmdSubcommand())

    err := config.BindPFlag("name", cmd.PersistentFlags().Lookup("name"))
    if err != nil {
        panic(err)
    }

    return cmd
}

func initConfig(cmd *cobra.Command, config *viper.Viper) {
    name, err := cmd.PersistentFlags().GetString("name")
    if err != nil {
        panic(err)
    }
    fmt.Printf("name = %s\n", name)

    if cfgFile != "" {
        // Use config file from the flag.
        config.SetConfigFile(cfgFile)
    } else {
        config.AddConfigPath(".")
        config.SetConfigName(".example")
    }

    config.AutomaticEnv() // read in environment variables that match

    // If a config file is found, read it in.
    if err := config.ReadInConfig(); err == nil {
        fmt.Fprintln(os.Stderr, "Using config file:", config.ConfigFileUsed())
    }

    // *** This line triggers a nil pointer reference.
    fmt.Printf("name is %s\n", config.GetString("name"))
}

...we would find that it works when executing the root command:

$ ./example
name =
name is
Hello, world

But it fails when we run the subcommand:

[lars@madhatter go]$ ./example subcommand
panic: flag accessed but not defined: name

goroutine 1 [running]:
example/cmd.initConfig(0xc000172000, 0xc0001227e0)
        /home/lars/tmp/go/cmd/root.go:55 +0x368
example/cmd.NewCmdRoot.func1(0xc000172000, 0x96eca0, 0x0, 0x0)
        /home/lars/tmp/go/cmd/root.go:32 +0x34
github.com/spf13/cobra.(*Command).execute(0xc000172000, 0x96eca0, 0x0, 0x0, 0xc000172000, 0x96eca0)
        /home/lars/go/pkg/mod/github.com/spf13/[email protected]/command.go:836 +0x231
github.com/spf13/cobra.(*Command).ExecuteC(0xc00011db80, 0x0, 0xffffffff, 0xc0000240b8)
        /home/lars/go/pkg/mod/github.com/spf13/[email protected]/command.go:960 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
        /home/lars/go/pkg/mod/github.com/spf13/[email protected]/command.go:897
main.main()
        /home/lars/tmp/go/main.go:11 +0x2a

This is because the subcommand inherits the PersistentPreRun command from the root (this is what the Persistent part means), but when this method runs, the cmd argument passwd to PersistentPreRun is no longer the root command; it's the subcommand command. When we try to call cmd.PersistentFlags(), it fails because the current command doesn't have any persistent flags associated with it.

In this case, we need to instead use the Flags method:

Flags returns the complete FlagSet that applies to this command (local and persistent declared here and by all parents).

This gives us access to persistent flags declared by parents.

An additional issue, that doesn't appear to be called out explicitly in the documentation, is that Flags() is only available after command processing has been run (that is, after you call cmd.Execute() on the command or a parent). That means we can use it in PersistentPreRun, but we can't use it in NewCmdRoot (because that method finishes before we process the command line).

---

TL;DR

• We have to use cmd.PersistentFlags() in NewCmdRoot because we're looking for persistent flags applied to the current command, and the value from Flags() won't be available yet.
• We need to use cmd.Flags() in PersistentPreRun (and other persistent commands methods) because when processing a subcommand, PersistentFlags will only look for persistent flags on the current command, but won't traverse parents. We need to use cmd.Flags() instead, which will roll up persistent flags declared by parents.