Skip to content

Commit

Permalink
Xtask CI (bevyengine#1387)
Browse files Browse the repository at this point in the history
This PR is easiest to review commit by commit.

Followup on bevyengine#1309 (comment)

- [x] Switch from a bash script to an xtask rust workspace member.
  - Results in ~30s longer CI due to compilation of the xtask itself
  - Enables Bevy contributors on any platform to run `cargo ci` to run linting -- if the default available Rust is the same version as on CI, then the command should give an identical result.
- [x] Use the xtask from official CI so there's only one place to update.
- [x] Bonus: Run clippy on the _entire_ workspace (existing CI setup was missing the `--workspace` flag
  - [x] Clean up newly-exposed clippy errors 

~bevyengine#1388 builds on this to clean up newly discovered clippy errors -- I thought it might be nicer as a separate PR.~  Nope, merged it into this one so CI would pass.

Co-authored-by: Carter Anderson <[email protected]>
  • Loading branch information
CleanCut and cart committed Feb 22, 2021
1 parent c9f19d8 commit 13b602e
Show file tree
Hide file tree
Showing 18 changed files with 63 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .cargo/config_fast_builds
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Rename this file to `config.toml` to enable "fast build" configuration. Please read the notes below.
# Add the contents of this file to `config.toml` to enable "fast build" configuration. Please read the notes below.

# NOTE: For maximum performance, build using a nightly compiler
# If you are using rust stable, remove the "-Zshare-generics=y" below (as well as "-Csplit-debuginfo=unpacked" when building on macOS).
Expand Down
9 changes: 2 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,8 @@ jobs:
if: runner.os == 'linux'

- name: Check the format
run: cargo fmt --all -- --check
if: runner.os == 'linux' && matrix.toolchain == 'stable'

# -A clippy::type_complexity: type complexity must be ignored because we use huge templates for queries.
# -A clippy::manual-strip: strip_prefix support was added in 1.45. We want to support earlier rust versions.
- name: Clippy
run: cargo clippy --all-targets --all-features -- -D warnings -A clippy::type_complexity -A clippy::manual-strip
# See tools/ci/src/main.rs for the commands this runs
run: cargo run --package ci
if: runner.os == 'linux' && matrix.toolchain == 'stable'

- name: Build & run tests
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ repository = "https://github.com/bevyengine/bevy"

[workspace]
exclude = ["benches"]
members = ["crates/*", "examples/ios"]
members = ["crates/*", "examples/ios", "tools/ci"]

[features]
default = [
Expand Down
4 changes: 2 additions & 2 deletions benches/benches/bevy_ecs/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn spawn_static(b: &mut Bencher) {
struct Bundle {
pos: Position,
vel: Velocity,
};
}

let mut world = World::new();
b.iter(|| {
Expand All @@ -48,7 +48,7 @@ fn spawn_batch(b: &mut Bencher) {
struct Bundle {
pos: Position,
vel: Velocity,
};
}

let mut world = World::new();
b.iter(|| {
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_core/src/time/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub(crate) fn time_system(mut time: ResMut<Time>) {
}

#[cfg(test)]
#[allow(clippy::float_cmp)]
mod tests {
use super::Time;
use bevy_utils::{Duration, Instant};
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_core/src/time/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ impl Timer {
}

#[cfg(test)]
#[allow(clippy::float_cmp)]
mod tests {
use super::Timer;

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/core/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl<'de> Deserialize<'de> for Entity {
where
D: serde::Deserializer<'de>,
{
Ok(deserializer.deserialize_u32(EntityVisitor)?)
deserializer.deserialize_u32(EntityVisitor)
}
}

Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_ecs/src/core/world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ impl World {
/// Create an empty world
pub fn new() -> Self {
// `flush` assumes archetype 0 always exists, representing entities with no components.
let mut archetypes = Vec::new();
archetypes.push(Archetype::new(Vec::new()));
let archetypes = vec![Archetype::new(Vec::new())];
let mut index = HashMap::default();
index.insert(Vec::new(), 0);
Self {
Expand Down
14 changes: 7 additions & 7 deletions crates/bevy_ecs/src/resource/resources.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{system::SystemId, AtomicBorrow, TypeInfo};
use crate::{system::SystemId, AtomicBorrow};
use bevy_utils::HashMap;
use core::any::TypeId;
use downcast_rs::{impl_downcast, Downcast};
Expand Down Expand Up @@ -204,15 +204,14 @@ impl Resources {

fn insert_resource<T: Resource>(&mut self, resource: T, resource_index: ResourceIndex) {
let type_id = TypeId::of::<T>();
let data = self.resource_data.entry(type_id).or_insert_with(|| {
let mut types = Vec::new();
types.push(TypeInfo::of::<T>());
ResourceData {
let data = self
.resource_data
.entry(type_id)
.or_insert_with(|| ResourceData {
storage: Box::new(VecResourceStorage::<T>::default()),
default_index: None,
system_id_to_archetype_index: HashMap::default(),
}
});
});

let storage = data
.storage
Expand Down Expand Up @@ -500,6 +499,7 @@ mod tests {
use crate::system::SystemId;

#[test]
#[allow(clippy::float_cmp)]
fn resource() {
let mut resources = Resources::default();
assert!(resources.get::<i32>().is_none());
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_ecs/src/system/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ impl Commands {
}

#[cfg(test)]
#[allow(clippy::float_cmp, clippy::approx_constant)]
mod tests {
use crate::{resource::Resources, Commands, World};
use core::any::TypeId;
Expand Down Expand Up @@ -480,7 +481,7 @@ mod tests {
.map(|(a, b)| (*a, *b))
.collect::<Vec<_>>();
assert_eq!(results_after, vec![]);
let results_after_u64 = world.query::<&u64>().map(|a| *a).collect::<Vec<_>>();
let results_after_u64 = world.query::<&u64>().copied().collect::<Vec<_>>();
assert_eq!(results_after_u64, vec![]);
}

Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_reflect/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub use bevy_reflect_derive::*;
pub use erased_serde;

#[cfg(test)]
#[allow(clippy::blacklisted_name, clippy::approx_constant)]
mod tests {
use ::serde::de::DeserializeSeed;
use bevy_utils::HashMap;
Expand All @@ -67,6 +68,7 @@ mod tests {
use crate::serde::{ReflectDeserializer, ReflectSerializer};

use super::*;

#[test]
fn reflect_struct() {
#[derive(Reflect)]
Expand Down Expand Up @@ -145,6 +147,7 @@ mod tests {
}

#[test]
#[allow(clippy::blacklisted_name)]
fn reflect_unit_struct() {
#[derive(Reflect)]
struct Foo(u32, u64);
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_reflect/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ fn next_token<'a>(path: &'a str, index: &mut usize) -> Option<Token<'a>> {
}

#[cfg(test)]
#[allow(clippy::float_cmp, clippy::approx_constant)]
mod tests {
use super::GetPath;
use crate::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,12 @@ mod tests {
bindings.set("b", resource2.clone());

let mut different_bindings = RenderResourceBindings::default();
different_bindings.set("a", resource3.clone());
different_bindings.set("b", resource4.clone());
different_bindings.set("a", resource3);
different_bindings.set("b", resource4);

let mut equal_bindings = RenderResourceBindings::default();
equal_bindings.set("a", resource1.clone());
equal_bindings.set("b", resource2.clone());
equal_bindings.set("b", resource2);

let status = bindings.update_bind_group_status(&bind_group_descriptor);
let id = if let BindGroupStatus::Changed(id) = status {
Expand Down Expand Up @@ -368,7 +368,7 @@ mod tests {
};

let mut unmatched_bindings = RenderResourceBindings::default();
unmatched_bindings.set("a", resource1.clone());
unmatched_bindings.set("a", resource1);
let unmatched_bind_group_status =
unmatched_bindings.update_bind_group_status(&bind_group_descriptor);
assert_eq!(unmatched_bind_group_status, BindGroupStatus::NoMatch);
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_render/src/shader/shader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ pub enum ShaderError {
not(target_arch = "wasm32"),
not(all(target_arch = "aarch64", target_os = "macos"))
))]
impl Into<bevy_glsl_to_spirv::ShaderType> for ShaderStage {
fn into(self) -> bevy_glsl_to_spirv::ShaderType {
match self {
impl From<ShaderStage> for bevy_glsl_to_spirv::ShaderType {
fn from(s: ShaderStage) -> bevy_glsl_to_spirv::ShaderType {
match s {
ShaderStage::Vertex => bevy_glsl_to_spirv::ShaderType::Vertex,
ShaderStage::Fragment => bevy_glsl_to_spirv::ShaderType::Fragment,
ShaderStage::Compute => bevy_glsl_to_spirv::ShaderType::Compute,
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_tasks/src/task_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ impl<'scope, T: Send + 'scope> Scope<'scope, T> {
}

#[cfg(test)]
#[allow(clippy::blacklisted_name)]
mod tests {
use super::*;
use std::sync::{
Expand Down
20 changes: 0 additions & 20 deletions tools/ci

This file was deleted.

13 changes: 13 additions & 0 deletions tools/ci/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
name = "ci"
version = "0.1.0"
authors = [
"Bevy Contributors <[email protected]>",
"Nathan Stocks <[email protected]>"
]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
xshell = "0.1"
19 changes: 19 additions & 0 deletions tools/ci/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use xshell::cmd;

fn main() {
// When run locally, results may from actual CI runs triggered by .github/workflows/ci.yml
// - Official CI runs latest stable
// - Local runs use whatever the default Rust is locally

// See if any code needs to be formatted
cmd!("cargo fmt --all -- --check")
.run()
.expect("Please run 'cargo fmt --all' to format your code.");

// See if clippy has any complaints.
// - Type complexity must be ignored because we use huge templates for queries
// - `-A clippy::manual-strip` strip_prefix support was added in 1.45
cmd!("cargo clippy --workspace --all-targets --all-features -- -D warnings -A clippy::type_complexity -A clippy::manual-strip")
.run()
.expect("Please fix clippy errors in output above.");
}

0 comments on commit 13b602e

Please sign in to comment.