Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BoolFlag.Count increment once per alias #1737

Closed
3 tasks done
matthewhughes934 opened this issue May 25, 2023 · 1 comment
Closed
3 tasks done

BoolFlag.Count increment once per alias #1737

matthewhughes934 opened this issue May 25, 2023 · 1 comment
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/triage maintainers still need to look into this

Comments

@matthewhughes934
Copy link

My urfave/cli version is

v2.25.3

Checklist

  • Are you running the latest v2 release? The list of releases is here.
  • Did you check the manual for your release? The v2 manual is here
  • Did you perform a search about this problem? Here's the GitHub guide about searching.

Dependency Management

  • My project is using go modules.

Describe the bug

The value of .Count for a BoolFlag is impacted by the number of aliases defined for that flag.

To reproduce

Reproducible program:

package main

import (
	"fmt"
	"os"

	"github.com/urfave/cli/v2"
)

func main() {
	verbosity := 0
	
	app := &cli.App{
		Flags: []cli.Flag{
			&cli.BoolFlag{
				Name: "verbose",
				Aliases: []string{"v"},
				Count: &verbosity,
			},
		},
		Action: func(c *cli.Context) error {
			fmt.Printf("Running with verbosity: %d\n", verbosity)
			return nil
		},
	}

	app.Run(os.Args)
}

Observed behavior

$ go run main.go -v
Running with verbosity: 2

Expected behavior

I would expect the count to be incremented only once, i.e.:

$ go run main.go -v
Running with verbosity: 1

Additional context

Beyond the initial value, things are incremented as expected:

$ go run main.go -v -v
Running with verbosity: 3

Want to fix this yourself?

I'd be happy to, though I don't think I have enough context to implement a suitable solution. The trace is:

  • When we .Apply a BoolFlag we copy the count pointer for each alias
    value := newBoolValue(f.Value, dest, count)
  • And when we .Set the flag we increment this count
    *b.count = *b.count + 1

So when we run a command:

  • We parse the flags

    cli/command.go

    Line 158 in acbbbf2

    set, err := c.parseFlags(&a, cCtx)
  • Which requires normalizing

    cli/command.go

    Line 384 in acbbbf2

    if err := normalizeFlags(c.Flags, set); err != nil {
  • When normalizing we copy for each alias we didn't see

    cli/flag.go

    Line 229 in acbbbf2

    copyFlag(name, ff, set)
  • Which in turn sets the value

    cli/flag.go

    Line 199 in acbbbf2

    _ = set.Set(name, ff.Value.String())

Consequently the flag is .Set once per alias, so the count is incremented once per alias.

Maybe flag types could implement the copying logic themselves?

Run go version and paste its output here

$ go version
go version go1.20.4 linux/amd64

Run go env and paste its output here

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/.local/share/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/.local/share/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/user/src/cli/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3085526498=/tmp/go-build -gno-record-gcc-switches"
@matthewhughes934 matthewhughes934 added area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/triage maintainers still need to look into this labels May 25, 2023
@dearchap
Copy link
Contributor

@matthewhughes934 Try this "fix"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/triage maintainers still need to look into this
Projects
None yet
Development

No branches or pull requests

2 participants