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

Update cli/urfave to v2 #1610

Open
moskyb opened this issue Apr 11, 2022 · 3 comments
Open

Update cli/urfave to v2 #1610

moskyb opened this issue Apr 11, 2022 · 3 comments
Labels
breaking Changes to existing behaviour users might rely on v4 Breaking changes that will be included in Agent v4

Comments

@moskyb
Copy link
Contributor

moskyb commented Apr 11, 2022

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.

@moskyb moskyb added breaking Changes to existing behaviour users might rely on v4 Breaking changes that will be included in Agent v4 labels Apr 11, 2022
@yob
Copy link
Contributor

yob commented Apr 11, 2022

We've been burnt by this before 😬 #1286

@yob
Copy link
Contributor

yob commented Nov 29, 2022

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

@triarius
Copy link
Contributor

triarius commented Jan 11, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Changes to existing behaviour users might rely on v4 Breaking changes that will be included in Agent v4
Projects
None yet
Development

No branches or pull requests

3 participants