servoshell: Replace getopts with bpaf for argument parsing (#37194)

This includes some small refactoring and some small breaking changes as
listed below. Other than these I tried to keep the functionality exactly
the same but because in the old code the parsing and settings of
preferences was intermingled it was difficult to figure out.

Small Breaking:
- Size and resources-path were unused but appeared in the help.
- soft-fail and hard-fail: Soft-fail flag got removed because it is too
  difficult to keep both. The default is now soft-fail and hard-fail can
be enabled.
- The help strings are obviously formatted differently now.
- -V does not work anymore but -v and --version.

Ideally, we want to have the ServoShellPreferences and Preferences be
directly the Argument structure but that needs a bit more discussion
because it would break backwards compatibility with the commandline.

This increases the binary size by ~280kb.

Testing: The testcases are still working but they do not cover much.
I added a unit test for the -p flag because it is the most difficult to
parse in general.
Fixes: This will fix a small number of various parsing misshaps. It will
also show if we are removing an option via unused lint.

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
This commit is contained in:
Narfinger 2025-09-05 10:17:38 +02:00 committed by GitHub
parent b29eab0ffe
commit ed66e0b0ca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 542 additions and 494 deletions

31
Cargo.lock generated
View file

@ -996,6 +996,26 @@ dependencies = [
"hex 0.3.2",
]
[[package]]
name = "bpaf"
version = "0.9.20"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "473976d7a8620bb1e06dcdd184407c2363fe4fec8e983ee03ed9197222634a31"
dependencies = [
"bpaf_derive",
]
[[package]]
name = "bpaf_derive"
version = "0.5.17"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fefb4feeec9a091705938922f26081aad77c64cd2e76cd1c4a9ece8e42e1618a"
dependencies = [
"proc-macro2",
"quote",
"syn",
]
[[package]]
name = "brotli"
version = "8.0.2"
@ -3058,15 +3078,6 @@ dependencies = [
"windows-targets 0.52.6",
]
[[package]]
name = "getopts"
version = "0.2.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cfe4fbac503b8d1f88e6676011885f34b7174f46e59956bba534ba83abded4df"
dependencies = [
"unicode-width",
]
[[package]]
name = "getrandom"
version = "0.2.16"
@ -7970,6 +7981,7 @@ dependencies = [
"accesskit_winit",
"android_logger",
"backtrace",
"bpaf",
"cc",
"cfg-if",
"constellation_traits",
@ -7983,7 +7995,6 @@ dependencies = [
"embedder_traits",
"env_filter",
"euclid",
"getopts",
"gilrs",
"glow",
"headers 0.4.1",

View file

@ -64,7 +64,6 @@ fnv = "1.0"
fonts_traits = { path = "components/shared/fonts" }
freetype-sys = "0.20"
fxhash = "0.2"
getopts = "0.2.24"
gleam = "0.15"
glow = "0.16"
gstreamer = { version = "0.23", features = ["v1_18"] }

View file

@ -176,7 +176,7 @@ impl DebugOptions {
}
}
#[derive(Clone, Debug, Deserialize, Serialize)]
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
pub enum OutputOptions {
/// Database connection config (hostname, name, user, pass)
FileName(String),

View file

@ -55,13 +55,13 @@ vello = ["libservo/vello"]
vello_cpu = ["libservo/vello_cpu"]
[dependencies]
bpaf = { version = "0.9.20", features = ["derive"] }
cfg-if = { workspace = true }
constellation_traits = { workspace = true }
crossbeam-channel = { workspace = true }
dpi = { workspace = true }
embedder_traits = { path = "../../components/shared/embedder" }
euclid = { workspace = true }
getopts = { workspace = true }
hitrace = { workspace = true, optional = true }
image = { workspace = true }
ipc-channel = { workspace = true }

View file

@ -24,6 +24,12 @@ pub fn main() {
ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => {
(opts, preferences, servoshell_preferences)
},
ArgumentParsingResult::Exit => {
std::process::exit(0);
},
ArgumentParsingResult::ErrorParsing => {
std::process::exit(1);
},
};
crate::init_tracing(servoshell_preferences.tracing_filter.as_deref());

View file

@ -61,7 +61,7 @@ pub extern "C" fn Java_org_servo_servoview_JNIServo_version<'local>(
env: JNIEnv<'local>,
_class: JClass<'local>,
) -> JString<'local> {
let v = crate::servo_version();
let v = crate::VERSION;
env.new_string(&v)
.unwrap_or_else(|_str| JObject::null().into())
}

View file

@ -58,6 +58,10 @@ pub fn init(
ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => {
(opts, preferences, servoshell_preferences)
},
ArgumentParsingResult::Exit => {
std::process::exit(0);
},
ArgumentParsingResult::ErrorParsing => std::process::exit(1),
};
crate::init_tracing(servoshell_preferences.tracing_filter.as_deref());

View file

@ -153,6 +153,8 @@ pub fn init(
ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => {
(opts, preferences, servoshell_preferences)
},
ArgumentParsingResult::Exit => std::process::exit(0),
ArgumentParsingResult::ErrorParsing => std::process::exit(1),
};
if servoshell_preferences.log_to_file {

View file

@ -97,9 +97,7 @@ pub fn init_tracing(filter_directives: Option<&str>) {
}
}
pub fn servo_version() -> String {
format!("Servo {}-{}", env!("CARGO_PKG_VERSION"), env!("GIT_SHA"))
}
pub const VERSION: &str = concat!("Servo ", env!("CARGO_PKG_VERSION"), "-", env!("GIT_SHA"));
/// Plumbs tracing spans into HiTrace, with the following caveats:
///

File diff suppressed because it is too large Load diff