chore(deps): upgrade urfave/cli from v1 to v3#5184
chore(deps): upgrade urfave/cli from v1 to v3#5184lifubang wants to merge 2 commits intoopencontainers:mainfrom
Conversation
5421f08 to
4a05b5d
Compare
|
We should update this, but I'm a little cautious about this change. There were a few features (bugs?) we depend on in v1 that (after discussing this with upstream many years ago) I suspect were broken in later versions -- parsing of arguments for I don't think we have any real regression tests for that. 🤔 |
They later introduced a configuration option called |
4a05b5d to
6106733
Compare
|
Ah okay, I haven't looked at this in quite a while (as you can probably tell). Sounds great then. 😸 |
There was a problem hiding this comment.
Pull request overview
This PR migrates the codebase from urfave/cli v1 to urfave/cli v3, updating command/flag wiring and adopting v3-specific configuration needed to preserve existing CLI behavior.
Changes:
- Upgrade dependency to
github.com/urfave/cli/v3and refactor commands/actions to the v3*cli.Command+context.ContextAPI. - Add v3 migration settings (
StopOnNthArg,DisableSliceFlagSeparator) to preserve argument/flag parsing semantics. - Update build configuration to drop the legacy
urfave_cli_no_docsbuild tag and remove now-unneeded indirect deps.
Reviewed changes
Copilot reviewed 26 out of 125 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| utils_linux.go | Refactors helper functions to use *cli.Command accessors for args/flags. |
| utils.go | Updates arg-checking/help display for cli v3 and adds an intPtr helper used by v3 options. |
| update.go | Converts update command definition/flags/action to cli v3 patterns. |
| tests/cmd/remap-rootfs/remap-rootfs.go | Migrates the test helper CLI app to cli v3 Command.Run(ctx, args). |
| tests/cmd/recvtty/recvtty.go | Migrates the test helper CLI app and flags to cli v3. |
| tests/cmd/pidfd-kill/pidfd-kill.go | Migrates the test helper CLI app and flags to cli v3. |
| state.go | Converts state command to cli v3 and sets v3 parsing option(s). |
| start.go | Converts start command to cli v3 and sets v3 parsing option(s). |
| spec.go | Converts spec command to cli v3, updates flags (aliases), and sets slice parsing behavior. |
| run.go | Converts run command to cli v3, updates flags (aliases), and sets slice parsing behavior. |
| rootless_linux.go | Switches rootless cgroup manager option lookup to cli v3 accessors. |
| restore.go | Converts restore command to cli v3 and updates flag definitions/aliases. |
| ps.go | Converts ps command to cli v3 and adds StopOnNthArg to forward ps flags correctly. |
| pause.go | Converts pause/resume commands to cli v3 and sets slice parsing behavior. |
| notify_socket.go | Refactors notify socket helpers to use *cli.Command for root flag reading. |
| main.go | Reworks root command setup for cli v3, including Before signature and logging config. |
| list.go | Converts list command to cli v3 and updates flags (aliases) + output writer usage. |
| kill.go | Converts kill command to cli v3 and adds StopOnNthArg to prevent flag parsing of signal args. |
| go.mod | Upgrades urfave/cli to v3 and removes indirect markdown-man dependencies. |
| features.go | Converts features command to cli v3 and uses command writer for JSON output. |
| exec.go | Converts exec command to cli v3, sets StopOnNthArg, and updates slice flag defaults. |
| events.go | Converts events command to cli v3, adjusts flag definitions, and uses v3 duration accessors. |
| delete.go | Converts delete command to cli v3 and updates flag aliases + root access. |
| create.go | Converts create command to cli v3 and updates flag aliases. |
| checkpoint.go | Converts checkpoint command and CRIU option helpers to cli v3 APIs. |
| Makefile | Removes urfave_cli_no_docs build tag (no longer applicable with cli v3). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Would be interesting to see the runc binary size comparison (hoping the new one is smaller). This also reminds me I spent a few days trying to get rid of urfave/cli and gave up after a few days 🫨 |
Alas it's not [kir@kir-tp1 runc]$ size runc.before runc
text data bss dec hex filename
6491412 4613585 229128 11334125 acf1ed runc.before
6801036 4888809 229576 11919421 b5e03d runcCan't call it a huge difference but it's not small either (570K). This is with go1.25.7 |
efa9fd4 to
86219e6
Compare
|
@kolyshkin interesting to know what contributes the most to the size. Maybe it is some new dependency, or just bloated output templates? |
|
It seems that the biggest part of it (more than 50%) is re-introduction of regexp dependency (which was eliminated in #3460). |
Opened urfave/cli#2288. Local testing shows it saves ~427 KB |
|
If we are to switch to urfave v3, we may as well use it's shell completion (to replace |
For that, we need index 3ee1009a..6056d479 100644
--- a/main.go
+++ b/main.go
@@ -82,7 +82,9 @@ value for "bundle" is the current directory.`
)
func main() {
- app := &cli.Command{}
+ app := &cli.Command{
+ EnableShellCompletion: true,
+ }
app.Name = "runc"
app.Version = strings.TrimSpace(version) + extraVersion
app.Usage = usage(which doesn't increase binary size) and then something like ./runc completion bash > runc-bash-completion(and perhaps the same for |
|
Thanks for the suggestion. I agree that leveraging However, for this PR, I propose keeping the existing runc script and treating the shell completion migration as a follow-up task. Proposed Plan:
|
86219e6 to
d167e20
Compare
|
This LGTM once a release containing urfave/cli#2288 is created. |
Aaaand it's done: https://github.com/urfave/cli/releases/tag/v3.8.0 |
|
For the size; try installing https://github.com/Zxilly/go-size-analyzer its really nice, and has a TUI mode that allows you to step through all packages to see what size is used. |
Thanks! I played with it a bit and it kind of shows the same what I wrote about earlier. To summarize:
|
|
Reminds me I want to write a proposal for Go to make |
d167e20 to
72ae394
Compare
Migrate from urfave/cli v1 (maintenance mode) to v3 to benefit from active development, improved features, and long-term support. Signed-off-by: lifubang <lifubang@acmcoder.com>
After migrate from urfave/cli v1 (maintenance mode) to v3, we don't need this build tag anymore. Signed-off-by: lifubang <lifubang@acmcoder.com>
72ae394 to
68e5e3b
Compare
kolyshkin
left a comment
There was a problem hiding this comment.
lgtm (but let's not include it into 1.5.0 I think)
|
@kolyshkin can you post the |
Sure [kir@kir-tp1 runc]$ gsa --tui ./runc ./runc.cli380
...
┌────────────────────────────────────────────────────────────────────────────────────┐
│ Diff between runc and runc.cli380 │
├─────────┬──────────────────────────────────────────┬──────────┬──────────┬─────────┤
│ PERCENT │ NAME │ OLD SIZE │ NEW SIZE │ DIFF │
├─────────┼──────────────────────────────────────────┼──────────┼──────────┼─────────┤
│ +100% │ github.com/urfave/cli/v3 │ │ 394 kB │ +394 kB │
│ +100% │ embed │ │ 8.7 kB │ +8.7 kB │
│ +3.54% │ main │ 121 kB │ 125 kB │ +4.3 kB │
│ +6.20% │ io │ 34 kB │ 36 kB │ +2.1 kB │
│ +4.38% │ math │ 25 kB │ 26 kB │ +1.1 kB │
│ +0.03% │ google.golang.org/protobuf │ 1.4 MB │ 1.4 MB │ +353 B │
│ +0.42% │ github.com/checkpoint-restore/go-criu/v7 │ 84 kB │ 85 kB │ +350 B │
│ +0.02% │ github.com/opencontainers/cgroups │ 324 kB │ 324 kB │ +60 B │
│ +0.09% │ encoding/binary │ 51 kB │ 51 kB │ +46 B │
│ +0.04% │ golang.org/x/sys │ 82 kB │ 82 kB │ +34 B │
│ +0.03% │ internal/poll │ 95 kB │ 95 kB │ +32 B │
│ +0.02% │ syscall │ 104 kB │ 104 kB │ +23 B │
│ +0.03% │ sync │ 56 kB │ 56 kB │ +19 B │
│ +0.04% │ context │ 38 kB │ 38 kB │ +17 B │
│ +0.00% │ crypto │ 34 MB │ 34 MB │ +16 B │
│ +0.04% │ debug/elf │ 41 kB │ 41 kB │ +15 B │
│ +0.00% │ net │ 455 kB │ 455 kB │ +13 B │
│ +0.17% │ github.com/vishvananda/netns │ 6.0 kB │ 6.0 kB │ +10 B │
│ +0.01% │ github.com/cyphar/filepath-securejoin │ 94 kB │ 94 kB │ +9 B │
│ +0.00% │ text/template │ 252 kB │ 252 kB │ +6 B │
│ +0.06% │ internal/godebug │ 9.6 kB │ 9.6 kB │ +6 B │
│ +0.00% │ github.com/vishvananda/netlink │ 163 kB │ 163 kB │ +6 B │
│ +0.01% │ github.com/coreos/go-systemd/v22 │ 30 kB │ 30 kB │ +4 B │
│ +0.04% │ hash/crc32 │ 10 kB │ 10 kB │ +4 B │
│ +0.00% │ os │ 181 kB │ 181 kB │ +4 B │
│ +1.12% │ iter │ 268 B │ 271 B │ +3 B │
│ +0.00% │ github.com/sirupsen/logrus │ 86 kB │ 86 kB │ +2 B │
│ +0.01% │ compress/flate │ 33 kB │ 33 kB │ +2 B │
│ +0.00% │ github.com/containerd/console │ 25 kB │ 25 kB │ +1 B │
│ -0.00% │ runtime │ 1.2 MB │ 1.2 MB │ -2 B │
│ -0.02% │ bufio │ 16 kB │ 16 kB │ -4 B │
│ -0.07% │ errors │ 5.6 kB │ 5.6 kB │ -4 B │
│ -0.02% │ bytes │ 28 kB │ 28 kB │ -5 B │
│ -0.05% │ strconv │ 17 kB │ 17 kB │ -8 B │
│ -0.11% │ compress/gzip │ 7.3 kB │ 7.3 kB │ -8 B │
│ -0.27% │ hash/adler32 │ 3.3 kB │ 3.3 kB │ -9 B │
│ -0.01% │ encoding/json │ 159 kB │ 159 kB │ -10 B │
│ -0.07% │ unique │ 15 kB │ 15 kB │ -10 B │
│ -0.00% │ github.com/opencontainers/runc │ 663 kB │ 663 kB │ -13 B │
│ -0.01% │ fmt │ 101 kB │ 101 kB │ -15 B │
│ -0.20% │ unicode │ 11 kB │ 11 kB │ -23 B │
│ -0.00% │ github.com/cilium/ebpf │ 669 kB │ 669 kB │ -27 B │
│ -0.07% │ golang.org/x/net │ 39 kB │ 39 kB │ -29 B │
│ -0.01% │ github.com/godbus/dbus/v5 │ 405 kB │ 405 kB │ -29 B │
│ -0.02% │ time │ 155 kB │ 155 kB │ -33 B │
│ -0.08% │ github.com/seccomp/libseccomp-golang │ 76 kB │ 76 kB │ -63 B │
│ -0.05% │ reflect │ 325 kB │ 325 kB │ -167 B │
│ -2.27% │ <autogenerated> │ 80 kB │ 78 kB │ -1.8 kB │
│ -77.53% │ flag │ 42 kB │ 9.4 kB │ -33 kB │
│ -100% │ github.com/urfave/cli │ 184 kB │ │ -184 kB │
├─────────┼──────────────────────────────────────────┼──────────┼──────────┼─────────┤
│ +6.41% │ .strtab │ 718 kB │ 764 kB │ +46 kB │
│ +2.71% │ .rela.dyn │ 1.1 MB │ 1.2 MB │ +31 kB │
│ +54.70% │ .data │ 50 kB │ 77 kB │ +27 kB │
│ +2.24% │ .data.rel.ro │ 1.2 MB │ 1.2 MB │ +26 kB │
│ +1.04% │ .debug_info │ 1.4 MB │ 1.4 MB │ +14 kB │
│ +1.98% │ .symtab │ 412 kB │ 420 kB │ +8.2 kB │
│ +1.07% │ .debug_loclists │ 746 kB │ 754 kB │ +8.0 kB │
│ +0.42% │ .debug_line │ 748 kB │ 751 kB │ +3.2 kB │
│ +0.96% │ .debug_frame │ 156 kB │ 158 kB │ +1.5 kB │
│ +0.42% │ .debug_rnglists │ 294 kB │ 295 kB │ +1.2 kB │
│ +0.37% │ .rodata │ 109 kB │ 109 kB │ +407 B │
│ +1.55% │ .debug_addr │ 24 kB │ 25 kB │ +378 B │
│ -1.41% │ .debug_aranges │ 425 B │ 419 B │ -6 B │
│ -3.09% │ .gopclntab │ 106 kB │ 102 kB │ -3.3 kB │
├─────────┼──────────────────────────────────────────┼──────────┼──────────┼─────────┤
│ +2.24% │ runc │ 16 MB │ 16 MB │ +350 kB │
│ │ runc.cli380 │ │ │ │
└─────────┴──────────────────────────────────────────┴──────────┴──────────┴─────────┘
... |
|
@kolyshkin thanks. So pure v3 - v1 is I am trying to build JSON query to get function by function comparison from |
|
The "best" query so far to get a list of functions with their files. gsa /usr/bin/runc -f json | jq -r '.packages."github.com/urfave/cli".files[] |
"\(.file_path)", (.functions[] | " \(."type") \(.receiver).\(.name) \t\t\(.code_size)") |
I think those may are ELF info; I recall looking at those for another binary; a quick search shows Not sure if we can reduce much there, unless stripping ( |
|
For the |
|
I don't really like the idea of dynamic completion generation from the app itself. I'd rather have it separately (e.g. compile a program with a special tag and generate the completion scripts from it). This will result in both faster completion and smaller binary (the downside is more complicated build process but I guess we can live with it). All this us about urfave/cli of course, not runc (so probably this is a wrong place to whine). |
|
@abitrolly you might want to look into adding something like a |
Yeah, it depends a bit on how much "dynamic" information to include. For runc; probably doable to continue a fairly static script. For (e.g.) the docker CLI, the scripts were much more complicated as they provided much more dynamic info for completion; the bash completion script was well maintained but the other shells severely lagged behind, so the Cobra completion was a sensible choice there. |
We already have a huge binary (compared to e.g. crun) and any further increase is undesirable. |
It bugs me that the feature had gone during migration to |
Migrate from urfave/cli v1 (maintenance mode) to v3 to benefit from active development,
improved features, and long-term support.
There are two key configuration options helps this migration success: