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

Create cfg aliases for bevy_app #18170

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions crates/bevy_app/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//! Build script containing cfg aliases
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inclusion of a build.rs is definitely going to be the controversial component of this PR. Between security and build time concerns, I see this single-handedly being a show-stopper.

Personally, I'm not against it, since we have many dependencies that use cfg_aliases already (which is effectively what you're doing here by hand). But I think this will be very hard to land.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, cfg_aliases is already being used? i wanted to decrease the contentiousness by a little by not adding a new dependency, but if this is already present, maybe there won't be pushback on that point

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already brought in as a build-dependency by winit and wgpu, so I wouldn't say it's a controversial inclusion in of itself. It's purely the use of a build.rs that I see as being controversial.


fn main() {
// Collecting information that will be used to decide which cfg aliases
// will be exported.
// Using `cfg!(target_arch = "wasm32")` and such does not work because
// `build.rs` uses the host architecture and other information, not the target's.
// Cargo then defines environment variables for the information of the target
let windows = std::env::var("CARGO_CFG_WINDOWS").is_ok();
let unix = std::env::var("CARGO_CFG_UNIX").is_ok();

let is_wasm32 = target_architecture("wasm32");

let feature_bevy_tasks = feature_present("bevy_tasks");
let feature_std = feature_present("std");

// Prevent warnings on uses of `#[cfg]`
let cfgs = ["can_run_tasks", "std_windows_or_unix"];
println!("cargo::rustc-check-cfg=cfg({})", cfgs.join(","));

// Defining cfg aliases
if !is_wasm32 && feature_bevy_tasks {
println!("cargo::rustc-cfg=can_run_tasks");
}
if (windows || unix) && feature_std {
println!("cargo::rustc-cfg=std_windows_or_unix");
}
}

fn feature_present(feature: &str) -> bool {
// This is very naive and may fail if there is `=`, `\0`, or unicode characters.
let feature_name = format!("CARGO_FEATURE_{}", feature.to_uppercase().replace("-", "_"));
std::env::var(feature_name.as_str())
.ok()
.filter(|feature| feature == "1")
.is_some()
}

fn target_architecture(target: &str) -> bool {
std::env::var("CARGO_CFG_TARGET_ARCH")
.ok()
.filter(|arch| arch == target)
.is_some()
}
2 changes: 1 addition & 1 deletion crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,7 @@ type RunnerFn = Box<dyn FnOnce(App) -> AppExit>;

fn run_once(mut app: App) -> AppExit {
while app.plugins_state() == PluginsState::Adding {
#[cfg(all(not(target_arch = "wasm32"), feature = "bevy_tasks"))]
#[cfg(can_run_tasks)]
bevy_tasks::tick_global_task_pools_on_main_thread();
}
app.finish();
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ mod schedule_runner;
mod sub_app;
#[cfg(feature = "bevy_tasks")]
mod task_pool_plugin;
#[cfg(all(any(unix, windows), feature = "std"))]
#[cfg(std_windows_or_unix)]
mod terminal_ctrl_c_handler;

pub use app::*;
Expand All @@ -44,7 +44,7 @@ pub use schedule_runner::*;
pub use sub_app::*;
#[cfg(feature = "bevy_tasks")]
pub use task_pool_plugin::*;
#[cfg(all(any(unix, windows), feature = "std"))]
#[cfg(std_windows_or_unix)]
pub use terminal_ctrl_c_handler::*;

/// The app prelude.
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_app/src/schedule_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl Plugin for ScheduleRunnerPlugin {
let plugins_state = app.plugins_state();
if plugins_state != PluginsState::Cleaned {
while app.plugins_state() == PluginsState::Adding {
#[cfg(all(not(target_arch = "wasm32"), feature = "bevy_tasks"))]
#[cfg(can_run_tasks)]
bevy_tasks::tick_global_task_pools_on_main_thread();
}
app.finish();
Expand Down