search

buildkite / agent

The Buildkite Agent is an open-source toolkit written in Go for securely running build jobs on any device or network
https://buildkite.com/
MIT License
808 stars 296 forks source link

Update cli/urfave to v2 #1610

Open moskyb opened 2 years ago

moskyb commented 2 years ago

The library that we use to have a nice cli, urfave/cli has had a new major version, v2, for a little while now. For the sake of staying as current as possible with our dependencies, we should update this to the newest version.

Unfortunately, v2 of urfave/cli includes a breaking change where now, arguments to functions must come before flags - ie, where 

buildkite-agent pipeline upload /path/to/pipeline --replace

is valid in v1, in v2 only

buildkite-agent pipeline upload --replace /path/to/pipeline

is allowed.

This issue is here to track that process, and note that we might want to (but won't necessarily) upgrade to the latest version of cli in agent v4.

yob commented 2 years ago

We've been burnt by this before :grimacing: #1286

yob commented 1 year ago

I haven't looked into it, but this urfave/cli PR (https://github.com/urfave/cli/pull/1568) just merged and claims to provide a way to upgrade to v2 in a compatible way

triarius commented 1 year ago

@yob Looks like it should work to me too. We should be able to specify flags on a subcommand as "Persistent" and position them around arguments.

Unfortunately, looks like that feature was merged into main, but is not the branch that v2 releases are from. Looking at the code, it defines the flags using golang generics, which I believe is a change made in v3. It would not be straightforward to back port it to v2.

So we may have to upgrade to v3, whose milestone is 64% complete. Even then, just running

go get -u github.com/urfave/cli/v3

is not sufficient as (at the time of writing) as they have not tagged a release on v3 since merging that PR. Only after tracking the main branch with

go get -u go get -u github.com/urfave/cli/v3@main

did I get the Persistent field to became available to flags.

And then there are a bunch of compilation failures from other changes in v3.

# github.com/buildkite/agent/v3/clicommand
clicommand/agent_start.go:208:13: cannot use &cli.StringSlice{} (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal
clicommand/agent_start.go:219:13: cannot use &cli.StringSlice{} (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal
clicommand/agent_start.go:225:13: cannot use &cli.StringSlice{} (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal
clicommand/agent_start.go:236:13: cannot use &cli.StringSlice{} (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal
clicommand/agent_start.go:242:13: cannot use &cli.StringSlice{} (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal
clicommand/agent_start.go:402:13: cannot use cli.NewStringSlice("*_PASSWORD", "*_SECRET", "*_TOKEN") (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal
clicommand/agent_start.go:420:13: cannot use &cli.StringSlice{} (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal
clicommand/global.go:74:11: cannot use &cli.StringSlice{} (value of type *cli.SliceBase[string, cli.NoConfig, cli.stringValue]) as type []string in struct literal

This is not unmanageable, and is not a fatal impediment to upgrading to v3.

Nevertheless, I would be very hesitant to upgrade to an alpha v3.