Repository navigation
Description
Re: GoogleContainerTools/skaffold#4129, spf13/cobra#1047
In Skaffold we use cobra and pflags to define our CLI. We have the flags toggle values directly in an options object. We have two commands that take the same flags but with different defaults. The following snippet demonstrates the issue:
var blowup bool
cmd1 := cobra.Command{}
cmd1.Flags().BoolVar(&blowup, "blowup", false, "When set to false, blowup")
cmd2 := cobra.Command{}
cmd2.Flags().BoolVar(&blowup, "blowup", true, "When set to false, blowup")
cmd1.ParseFlags(os.Args[1:])
fmt.Printf("blowup = %v\n", blowup)So even though we have two FlagSets, because they reference the same value, the default value for the last flag defined "wins" unless the option is explicitly defined on the command-line.
The issue here is that pflags applies the default value immediately. BoolVar() just delegates BoolVarP(), and BoolVarP() uses newBoolValue():
Lines 47 to 57 in 81378bb
| // BoolVar defines a bool flag with specified name, default value, and usage string. | |
| // The argument p points to a bool variable in which to store the value of the flag. | |
| func (f *FlagSet) BoolVar(p *bool, name string, value bool, usage string) { | |
| f.BoolVarP(p, name, "", value, usage) | |
| } | |
| // BoolVarP is like BoolVar, but accepts a shorthand letter that can be used after a single dash. | |
| func (f *FlagSet) BoolVarP(p *bool, name, shorthand string, value bool, usage string) { | |
| flag := f.VarPF(newBoolValue(value, p), name, shorthand, usage) | |
| flag.NoOptDefVal = "true" | |
| } |
newBoolValue() allocates a container to map the provider pointer to the default value, but it immediately assigns the value:
Lines 15 to 18 in 81378bb
| func newBoolValue(val bool, p *bool) *boolValue { | |
| *p = val | |
| return (*boolValue)(p) | |
| } |
Activity
[-]Flag default assignments should be applied lazily[/-][+]Flag default assignments should be lazily applied[/+]Enable file-watching for debug, but default to disabled
Enable file-watching for `debug` (#4089)
yufeiminds commentedon Jun 29, 2020
I also need a lazily applied default value in my command-line tool.
Now I write default value as zero value and fill them at runtime, then write default to flag usage manually.
Is there any way to resolve it?
pasamio commentedon Jan 25, 2021
I ran into this as well re-using an variable with two commands setting different defaults for the same variable as a convenience for the end user. I ended up isolating the variables and doing a small refactoring to make it work but it caught me off guard that a default value from a flag on a different command would be used based on essentially which one is configured last.
cornfeedhobo commentedon Jan 26, 2021
I think this is user error. I would suggest using a lookup instead, and stop relying on passing a pointer around.
If someone can present some other use cases for lazy parsing, I'd maybe take a look at the issue on my fork.
pasamio commentedon Jan 27, 2021
It could be classed as user error, I'm coming to this via Cobra that provided examples using this and I kept following that pattern. I think for someone running the code in two different commands via Cobra this provides a hard to understand and debug behaviour. It doesn't do what I expected and provides a surprise because the challenge is that this can influence the behaviour of another part of the program which is additionally confusing.
I ran into this twice before really realising what was going on. Using lookup instead is an alternative but the library provides a feature, I don't think it's an unreasonable expectation that when paired with Cobra that defaults are lazily applied when their commands are actually invoked. The current behaviour is in a sense hard to decipher because it isn't apparent that the two commands are interacting to change the default value which isn't entirely deterministic to the programmer at the time of writing. For people who know better they won't hit it however this is also a trap that at least a few people seem to have run into and I personally feel that in terms of being user friendly it doesn't seem wrong to defer setting the default value whilst removing a pitfall for a developer new to the library.
cornfeedhobo commentedon Jan 27, 2021
@pasamio my point is that this is fundamental to golang, not an error with the package. You are passing around a pointer and expecting it to not be overwritten.
pasamio commentedon Jan 27, 2021
I'm passing a pointer to a variable expecting that it will be filled with the value of my default argument for the flag that is currently being evaluated rather than it being filled with a value from a flag that is not currently being evaluated that is in another part of my Cobra application. I don't think it's an unreasonable expectation that when I hand the library a pointer to my variable that when my flags are evaluated that the variable is either populated with the value the user provided on the command line to the application or the default value I set for the flag in that command.
I think the challenge is the expectation between what pflag does and how that is used in Cobra.
outofforest commentedon Jan 3, 2022
I digged into the source code of pflag to fix this but the implementation is so strange that solving this would require rewriting of entire library
outofforest commentedon Jan 3, 2022
Assumption that value of pointer is overwritten immediately when flag is defined, not when
Parseis called onFlagSet(here for example: https://github.com/spf13/pflag/blob/master/bool.go#L15) is really counter-intuitive. Pointer should be touched just before reading flags from CLI, not earlier.I started working on fixing this by storing default value separately and applying it in
ParseandParseAllbefore reading values from CLI. ButValueinterface (https://github.com/spf13/pflag/blob/master/flag.go#L187) has no method to copy current value from oneValueto another. I thought that doing sth likevalue.Set(defaultValue.String())may work but due to reaaaaaalllyyy bad design ofpflaglibrary it doesn't work this way for all the types (built-in and custom ones). It would require adding new methodCopyto all the types which would break compatibility.I have to say that: During my research I found that
pflagis soo poorly designed that it should be reimplemented completely. Take logic of this function as an example: https://github.com/spf13/pflag/blob/master/flag.go#L538tomasaschan commentedon Jul 16, 2025
This issue has not been updated in a long time. Is it still relevant?
(If I receive no answer before the end of the summer, I'll close it. Feel free to re-open if you come here after that happens and it is relevant to you - but if you do, please also comment and specify what outcome you're interested in!)
MikailBag commentedon Mar 31, 2026
I've just run into this behavior.
I agree with the issue author that it is unintuitive. I.e. I assumed that default value will only be written into the pointer when flag is actually parsed.