Fix reachable panic in CLI flag handling

[gesslerpd]

Summary

Previously it was possible to reach panic condition in CLI flag handling when opposing flags were passed at different subcommand levels.

For example uv self --offline version --no-offline panics with the following RUST_BACKTRACE.

thread 'main2' panicked at crates\uv-cli\src\options.rs:17:17:
internal error: entered unreachable code: Clap should make this impossible
stack backtrace:
   0:     0x7ff6ee2a2ffc - <unknown>
   1:     0x7ff6ede968da - <unknown>
   2:     0x7ff6ee2a2557 - <unknown>
   3:     0x7ff6ee2a2e54 - <unknown>
   4:     0x7ff6ee2a2195 - <unknown>
   5:     0x7ff6ee2c96d2 - <unknown>
   6:     0x7ff6ee2c965f - <unknown>
   7:     0x7ff6ee2ccd4e - <unknown>
   8:     0x7ff6f031b941 - <unknown>
   9:     0x7ff6ee4e434a - <unknown>
  10:     0x7ff6ee674184 - <unknown>
  11:     0x7ff6ee65ebde - <unknown>
  12:     0x7ff6ef495290 - <unknown>
  13:     0x7ff6edcc6622 - <unknown>
  14:     0x7ff6ee2c40bd - <unknown>
  15:     0x7ff852477374 - BaseThreadInitThunk
  16:     0x7ff8527dcc91 - RtlUserThreadStart

thread 'main' panicked at C:\a\uv\uv\crates\uv\src\lib.rs:2275:10:
Tokio executor failed, was there a panic?: Any { .. }
stack backtrace:
   0:     0x7ff6ee2a2ffc - <unknown>
   1:     0x7ff6ede968da - <unknown>
   2:     0x7ff6ee2a2557 - <unknown>
   3:     0x7ff6ee2a2e54 - <unknown>
   4:     0x7ff6ee2a2195 - <unknown>
   5:     0x7ff6ee2c9709 - <unknown>
   6:     0x7ff6ee2c965f - <unknown>
   7:     0x7ff6ee2ccd4e - <unknown>
   8:     0x7ff6f031b941 - <unknown>
   9:     0x7ff6f031bd10 - <unknown>
  10:     0x7ff6edda51a2 - <unknown>
  11:     0x7ff6ef494186 - <unknown>
  12:     0x7ff6edda5f92 - <unknown>
  13:     0x7ff6f02f544c - <unknown>
  14:     0x7ff852477374 - BaseThreadInitThunk
  15:     0x7ff8527dcc91 - RtlUserThreadStart

Test Plan

The panic condition is removed in favor of None so should be impossible to reach condition now.

Based on the unreachable message, not sure if this is some issue with Clap or uv's usage of it but seems reasonable they would cancel out to None.

[zanieb]

Hm, but this should be unreachable since these use overrides_with?

uv/crates/uv-cli/src/lib.rs

Lines 239 to 244 in 5b2c359

	/// When disabled, uv will only use locally cached data and locally available files.
	#[arg(global = true, long, overrides_with("no_offline"), env = EnvVars::UV_OFFLINE, value_parser = clap::builder::BoolishValueParser::new())]
	pub offline: bool,
	#[arg(global = true, long, overrides_with("offline"), hide = true)]
	pub no_offline: bool,

[zanieb]

I guess this is a Clap bug?

[gesslerpd]

Yeah it could be a Clap issue, seems reasonable for UV to make them cancel out for now. Purposely used a non-destructive example to show it but think it has to do with when opposing flags exist at different subcommand levels.

[zanieb]

Yeah that makes sense. I don't think they should cancel out though, the latter one should "win". If they cancel out, we'll use the default value instead so I think this is a correctness bug if we don't panic.

[gesslerpd]

I'm not familiar with Clap, since these options are shared across levels I think that following should be equivalent (using harmless repro command)

• uv --offline --no-offline self version
• uv --offline self version --no-offline

Possibly the top example is not currently a cancel out scenario though, when clap is doing its normal overrides_with behavior?

It's a whatever is last wins?

In that case, I guess then only way to workaround this would be fixing in Clap.

[gesslerpd]

Closing in favor of issue/pr in https://github.com/clap-rs/clap

[konstin]

Filed upstream: clap-rs/clap#6049