Skip to content

Commit

Permalink
Add support for cargo's --config arg (#209)
Browse files Browse the repository at this point in the history
# Objective

Closes #201.

Allows users to quickly try out different configurations without
modifying `Cargo.toml`.

For us, it's most useful for finding the best default config for the web
profiles.

# Solution

Add support for `--config` argument for `bevy build` and `bevy run`,
analogue to the `cargo` counterpart.
This essentially allows you to modify the `Cargo.toml` without editing
the file, e.g. `--config "profile.web.debug=false"`.

We already exploit `cargo`'s `--config` arg to configure our default web
compilation profiles.
So we have to ensure that the user-provided args _overwrite_ our default
ones.

To do this, we change the default web profiles to be prepended to the
user-provided `--config` args, instead of converting them to `--config`
arguments directly.
Since `--config` is resolved left-to-right, the defaults will be
overwritten by the user.

# Testing

Try for example: 
```
bevy build --config "profile.web.debug=false" --config 'profile.web.inherits= "release"' web
```

Now instead of 
```
Finished `web` profile [unoptimized + debuginfo] target(s)
```

you should get
```
Finished `web` profile [optimized] target(s)
```

Debug info removed and optimizations enabled!
(Of course this can be done easier via `bevy build --release web`, but
like this it's easier to benchmark different compilation profiles to
choose the best default)
  • Loading branch information
TimJentzsch authored Jan 19, 2025
1 parent 12635b7 commit 10d0dbc
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ fn main() -> Result<()> {
bevy_cli::template::generate_template(&new.name, &new.template, &new.branch)?;
}
Subcommands::Lint { args } => bevy_cli::lint::lint(args)?,
Subcommands::Build(args) => bevy_cli::build::build(&args)?,
Subcommands::Build(mut args) => bevy_cli::build::build(&mut args)?,
Subcommands::Run(args) => bevy_cli::run::run(&args)?,
}

Expand Down
14 changes: 9 additions & 5 deletions src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{

pub mod args;

pub fn build(args: &BuildArgs) -> anyhow::Result<()> {
pub fn build(args: &mut BuildArgs) -> anyhow::Result<()> {
if args.is_web() {
build_web(args)?;
} else {
Expand All @@ -32,7 +32,7 @@ pub fn build(args: &BuildArgs) -> anyhow::Result<()> {
/// - Optimizing the Wasm binary (in release mode)
/// - Creating JavaScript bindings
/// - Creating a bundled folder (if requested)
pub fn build_web(args: &BuildArgs) -> anyhow::Result<WebBundle> {
pub fn build_web(args: &mut BuildArgs) -> anyhow::Result<WebBundle> {
let Some(BuildSubcommands::Web(web_args)) = &args.subcommand else {
bail!("tried to build for the web without matching arguments");
};
Expand All @@ -49,9 +49,13 @@ pub fn build_web(args: &BuildArgs) -> anyhow::Result<WebBundle> {
args.profile(),
)?;

let cargo_args = args
.cargo_args_builder()
.append(configure_default_web_profiles(&metadata)?);
let mut profile_args = configure_default_web_profiles(&metadata)?;
// `--config` args are resolved from left to right,
// so the default configuration needs to come before the user args
profile_args.append(&mut args.cargo_args.common_args.config);
args.cargo_args.common_args.config = profile_args;

let cargo_args = args.cargo_args_builder();

println!("Compiling to WebAssembly...");
cargo::build::command().args(cargo_args).ensure_status()?;
Expand Down
56 changes: 55 additions & 1 deletion src/external_cli/arg_builder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/// A helper to make passing arguments to [`Command`](std::process::Command) more convenient.
#[derive(Debug, Clone)]
#[must_use]
pub struct ArgBuilder(Vec<String>);

impl ArgBuilder {
Expand Down Expand Up @@ -83,7 +84,7 @@ impl ArgBuilder {
}
}

/// Add an argument with multiple values.
/// Add an argument with multiple values, separated by commas.
///
/// # Example
///
Expand All @@ -107,6 +108,33 @@ impl ArgBuilder {
}
}

/// Add an argument with multiple values, reusing the same argument name.
///
/// # Example
///
/// ```
/// # use bevy_cli::external_cli::arg_builder::ArgBuilder;
/// let features = ["dev", "file_watcher"];
/// ArgBuilder::new().add_values_separately("--features", features);
/// ```
pub fn add_values_separately<N, V>(
mut self,
name: N,
value_list: impl IntoIterator<Item = V>,
) -> Self
where
N: Into<String>,
V: Into<String>,
{
let arg: String = name.into();

for value in value_list {
self = self.add_with_value(&arg, value);
}

self
}

/// Add all arguments from the other builder to this one.
pub fn append(mut self, mut other: ArgBuilder) -> Self {
self.0.append(&mut other.0);
Expand Down Expand Up @@ -211,6 +239,32 @@ mod tests {
);
}

#[test]
fn add_values_separately_adds_multiple_args() {
let args = ArgBuilder::new().add_values_separately(
"--config",
[r#"profile.web.inherits="dev""#, "profile.web.debug=false"],
);
assert_eq!(
args.into_iter().collect::<Vec<String>>(),
vec![
"--config",
r#"profile.web.inherits="dev""#,
"--config",
"profile.web.debug=false"
]
);
}

#[test]
fn add_values_separately_empty_no_changes() {
let args = ArgBuilder::new().add_values_separately("--config", Vec::<String>::new());
assert_eq!(
args.into_iter().collect::<Vec<String>>(),
Vec::<String>::new()
);
}

#[test]
fn append_adds_args_after_self() {
let args = ArgBuilder::new()
Expand Down
5 changes: 4 additions & 1 deletion src/external_cli/cargo/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clap::{ArgAction, Args};

use crate::external_cli::arg_builder::ArgBuilder;

use super::{program, CargoCompilationArgs, CargoFeatureArgs, CargoManifestArgs};
use super::{program, CargoCommonArgs, CargoCompilationArgs, CargoFeatureArgs, CargoManifestArgs};

/// Create a command to run `cargo build`.
pub(crate) fn command() -> Command {
Expand All @@ -15,6 +15,8 @@ pub(crate) fn command() -> Command {

#[derive(Debug, Args)]
pub struct CargoBuildArgs {
#[clap(flatten)]
pub common_args: CargoCommonArgs,
#[clap(flatten)]
pub package_args: CargoPackageBuildArgs,
#[clap(flatten)]
Expand All @@ -30,6 +32,7 @@ pub struct CargoBuildArgs {
impl CargoBuildArgs {
pub(crate) fn args_builder(&self, is_web: bool) -> ArgBuilder {
ArgBuilder::new()
.append(self.common_args.args_builder())
.append(self.package_args.args_builder())
.append(self.target_args.args_builder())
.append(self.feature_args.args_builder())
Expand Down
18 changes: 18 additions & 0 deletions src/external_cli/cargo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,21 @@ impl CargoManifestArgs {
.add_flag_if("--frozen", self.is_frozen)
}
}

/// Common options available for `cargo` commands.
#[derive(Debug, Args, Clone)]
pub struct CargoCommonArgs {
/// Override a configuration value.
///
/// The argument should be in TOML syntax of KEY=VALUE,
/// or provided as a path to an extra configuration file.
/// This flag may be specified multiple times.
#[clap(long = "config", value_name = "KEY=VALUE|PATH")]
pub config: Vec<String>,
}

impl CargoCommonArgs {
pub(crate) fn args_builder(&self) -> ArgBuilder {
ArgBuilder::new().add_values_separately("--config", self.config.iter())
}
}
5 changes: 4 additions & 1 deletion src/external_cli/cargo/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clap::Args;

use crate::external_cli::arg_builder::ArgBuilder;

use super::{program, CargoCompilationArgs, CargoFeatureArgs, CargoManifestArgs};
use super::{program, CargoCommonArgs, CargoCompilationArgs, CargoFeatureArgs, CargoManifestArgs};

/// Create a command to run `cargo run`.
pub(crate) fn command() -> Command {
Expand All @@ -15,6 +15,8 @@ pub(crate) fn command() -> Command {

#[derive(Debug, Args, Clone)]
pub struct CargoRunArgs {
#[clap(flatten)]
pub common_args: CargoCommonArgs,
#[clap(flatten)]
pub package_args: CargoPackageRunArgs,
#[clap(flatten)]
Expand All @@ -30,6 +32,7 @@ pub struct CargoRunArgs {
impl CargoRunArgs {
pub(crate) fn args_builder(&self, is_web: bool) -> ArgBuilder {
ArgBuilder::new()
.append(self.common_args.args_builder())
.append(self.package_args.args_builder())
.append(self.target_args.args_builder())
.append(self.feature_args.args_builder())
Expand Down
1 change: 1 addition & 0 deletions src/run/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl From<RunArgs> for BuildArgs {
BuildArgs {
skip_prompts: args.skip_prompts,
cargo_args: CargoBuildArgs {
common_args: args.cargo_args.common_args,
compilation_args: args.cargo_args.compilation_args,
feature_args: args.cargo_args.feature_args,
manifest_args: args.cargo_args.manifest_args,
Expand Down
4 changes: 2 additions & 2 deletions src/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ mod serve;

pub fn run(args: &RunArgs) -> anyhow::Result<()> {
if let Some(RunSubcommands::Web(web_args)) = &args.subcommand {
let build_args = args.clone().into();
let web_bundle = build_web(&build_args)?;
let mut build_args = args.clone().into();
let web_bundle = build_web(&mut build_args)?;

let port = web_args.port;
let url = format!("http://localhost:{port}");
Expand Down
34 changes: 17 additions & 17 deletions src/web/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,23 @@ use std::{collections::HashMap, fs};
use anyhow::Context as _;
use toml_edit::DocumentMut;

use crate::external_cli::{arg_builder::ArgBuilder, cargo::metadata::Metadata};
use crate::external_cli::cargo::metadata::Metadata;

/// Create `--config` args to configure the default profiles to use when compiling for the web.
pub(crate) fn configure_default_web_profiles(metadata: &Metadata) -> anyhow::Result<ArgBuilder> {
pub(crate) fn configure_default_web_profiles(metadata: &Metadata) -> anyhow::Result<Vec<String>> {
let manifest = fs::read_to_string(metadata.workspace_root.join("Cargo.toml"))
.context("failed to read workspace manifest")?
.parse::<DocumentMut>()
.context("failed to parse workspace manifest")?;

let mut args = ArgBuilder::new();
let mut args = Vec::new();

if !is_profile_defined_in_manifest(&manifest, "web") {
args = args.append(configure_web_profile());
configure_web_profile(&mut args);
}

if !is_profile_defined_in_manifest(&manifest, "web-release") {
args = args.append(configure_web_release_profile());
configure_web_release_profile(&mut args);
}

Ok(args)
Expand All @@ -34,21 +34,21 @@ fn is_profile_defined_in_manifest(manifest: &DocumentMut, profile: &str) -> bool
/// Configure the default profile for web debug builds.
///
/// It is optimized for fast iteration speeds.
fn configure_web_profile() -> ArgBuilder {
configure_profile("web", "dev", HashMap::new())
fn configure_web_profile(args: &mut Vec<String>) {
configure_profile("web", "dev", HashMap::new(), args);
}

/// Configure the default profile for web release builds.
///
/// It is optimized both for run time performance and loading times.
fn configure_web_release_profile() -> ArgBuilder {
fn configure_web_release_profile(args: &mut Vec<String>) {
let config = HashMap::from_iter([
// Optimize for size, greatly reducing loading times
("opt-level", "s"),
// Remove debug information, reducing file size further
("strip", "debuginfo"),
]);
configure_profile("web-release", "release", config)
configure_profile("web-release", "release", config, args);
}

/// Create `--config` args for `cargo` to configure a new compilation profile.
Expand All @@ -61,17 +61,17 @@ fn configure_web_release_profile() -> ArgBuilder {
/// # config
/// key = "value"
/// ```
fn configure_profile(profile: &str, inherits: &str, config: HashMap<&str, &str>) -> ArgBuilder {
let mut args = ArgBuilder::new().add_with_value(
"--config",
format!(r#"profile.{profile}.inherits="{inherits}""#),
);
fn configure_profile(
profile: &str,
inherits: &str,
config: HashMap<&str, &str>,
args: &mut Vec<String>,
) {
args.push(format!(r#"profile.{profile}.inherits="{inherits}""#));

for (key, value) in config {
args = args.add_with_value("--config", format!(r#"profile.{profile}.{key}="{value}""#));
args.push(format!(r#"profile.{profile}.{key}="{value}""#));
}

args
}

#[cfg(test)]
Expand Down

0 comments on commit 10d0dbc

Please sign in to comment.