bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.26k stars 3.58k forks source link

`missing-metadata` workflow fails and posts a misleading comment if `Cargo` itself breaks #15400

Open clarfonthey opened 1 month ago

clarfonthey commented 1 month ago

Example comment: https://github.com/bevyengine/bevy/pull/15344#issuecomment-2369401470

As of commit: 0ebd7fcdf4545c05dd6c1f0c4b6cf057d1f5f1a7

Example Cargo breakage:

diff --git a/crates/bevy_sprite/Cargo.toml b/crates/bevy_sprite/Cargo.toml
index 30602e07c..aed3e65b6 100644
--- a/crates/bevy_sprite/Cargo.toml
+++ b/crates/bevy_sprite/Cargo.toml
@@ -10,7 +10,7 @@ keywords = ["bevy"]

 [features]
 default = ["bevy_sprite_picking_backend"]
-bevy_sprite_picking_backend = ["bevy_picking", "bevy_window"]
+bevy_sprite_picking_backend = ["dep:bevy_picking", "dep:bevy_window"]
 serialize = ["dep:serde"]
 webgl = []
 webgpu = []

Output of breakage:

error: failed to select a version for `bevy_sprite`.
    ... required by package `bevy_internal v0.15.0-dev (/home/ltdk/code/fork/bevy/crates/bevy_internal)`
    ... which satisfies path dependency `bevy_internal` (locked to 0.15.0-dev) of package `bevy_dylib v0.15.0-dev (/home/ltdk/code/fork/bevy/crates/bevy_dylib)`
versions that meet the requirements `^0.15.0-dev` (locked to 0.15.0-dev) are: 0.15.0-dev

the package `bevy_internal` depends on `bevy_sprite`, with features: `bevy_picking` but `bevy_sprite` does not have these features.
 It has an optional dependency with that name, but that dependency uses the "dep:" syntax in the features table, so it does not have an implicit feature with that name.

failed to select a version for `bevy_sprite` which could resolve this conflict

This particular breakage was just a silly mistake of mine, thinking something was broken when it wasn't. But in general, the result is the same: if cargo breaks in general, it can fall back to displaying a comment to do some other random unrelated action.

A few recommendations:

  1. Have a fail-fast test for dependency errors so things like this don't happen. cargo metadata --format-version 1 seems to be an option here, but you probably want --all-features as well just to verify that nothing is broken? Probably?
  2. These comments should be updated to display the specific flow that failed, so that if there's any funny business, you have a quick link to the action logs to see the specific error.
BenjaminBrienen commented 1 month ago

having a metadata-check job would be a little useful to help the developer be in the right mindset while looking at what went wrong, and a quick green checkmark is a bit better than "see if it stays orange for a while"