Context.IsSet not working with values set from environment variables

[varun06]

I used IsSet() to check for some values. It works the string flag is passed from command line but not when the flag values are set with environment values.

[jszwedko]

Indeed, this appears to be true, thanks for raising the issue @varun06 !

[AaronO]

@jszwedko I just encountered this bug too. Any update on this ?

[jszwedko]

@AaronO unfortunately I haven't had the time to address this yet, but PRs are welcome!

[bweston92]

Also just come across this, I'm a Go newb so my PR wouldn't look good.

[meatballhat]

tl;dr - this is not straightforward, and a proper solution may require reimplementing a lot of stuff

My first adventures with this have me hitting some unpleasantness. A travelogue, all prefixed with a strong AFAICT :

cli.Context.IsSet accepts a string, which is currently used to look up a matching stdlib *flag.Flag via flag.FlagSet.Visit, which only calls the given func on flags that have been set via stdlib flag parsing (not env vars). There is not currently a mapping of stdlib *flag.Flag to cli.Flag available to a given context, so getting at the right env vars for a given flag when all we have is one of the names it uses isn't straightforward. The guts of altsrc/flag.go include a method for checking via both cli.Context.IsSet and directly looking at the env via os.Getenv, but that's because at that level it's accessing a full cli.Flag.

I started down the path of building a name ➡️ cli.Flag mapping inside *cli.Context, but there's issues there as well. For example, the cli.Context.Command is not set during cli.NewContext, but assigned during cli.Command.Run, which means that we'd have to update the index at assignment time, namespacing the keys of whatever index to distinguish between flags from cli.Context.Command and cli.Context.App. Then there's the issues of avoiding index staleness, handling potential Command or App reassignment (they're public, after all,) and any bugs lurking in the fact that throughout the library is a mix of struct pointers and struct copies.

[varun06]

Thanks for looking into it.

[jszwedko]

This is fixed by #502 (some additional, related, work is being done in #530 to address #517).

[sarahhodne]

This still/again appears to be broken, c.IsSet("foo") returns false, but c.String("foo") returns a value with the following flag:

&cli.StringFlag{
    Name: "foo",
    EnvVars: []string{"FOO"},
}

Using urfave/cli e485446 (v2).

[jszwedko]

@henrikhodne aha, yep, this wasn't ported correctly when I merged these changes into the v2 branch, thank you for bringing this up. Opened #597 to correct.