Harden proc macro path resolution and add integration tests. #17330

raldone01 · 2025-01-12T18:31:24Z

This pr uses the extern crate self as trick to make proc macros behave the same way inside and outside bevy.

Objective

Removes noise introduced by crate as in the whole bevy repo.
Fixes rust-analyzer autocomplete is broken for some ECS types #17004.
Hardens proc macro path resolution.

TODO

BevyManifest needs cleanup.
Cleanup remaining crate as.
Add proper integration tests to the ci.

Notes

cargo-manifest-proc-macros is written by me and based/inspired by the old BevyManifest implementation and bkchr/proc-macro-crate.
What do you think about the new integration test machinery I added to the ci?
More and better integration tests can be added at a later stage.
The goal of these integration tests is to simulate an actual separate crate that uses bevy. Ideally they would lightly touch all bevy crates.

Testing

Needs RA test
Needs testing from other users
Others need to run at least cargo run -p ci integration-test and verify that they work.

BenjaminBrienen · 2025-01-13T10:50:36Z

.gitignore

@@ -4,6 +4,7 @@ crates/**/target
 benches/**/target
 tools/**/target
 **/*.rs.bk
+rustc-ice-*.txt


It happens from time to time when tickling rust nightly or deleting the target folder while rust is building.

I like to keep them around while I consider if they should be reported. In any case they should not be committed to bevy I think.

BenjaminBrienen · 2025-01-13T10:53:18Z

Great idea, just need to fix the errors

raldone01 · 2025-01-13T17:22:22Z

I will make the ci pass coming Thursday or Friday.

mdickopp · 2025-01-17T10:48:14Z

When I try this branch, rust-analyzer logs this:

2025-01-17T11:44:08.66709452+01:00 ERROR Flycheck failed to run the following command: CommandHandle { program: "/home/martin/.cargo/bin/cargo", arguments: ["check", "--workspace", "--message-format=json-diagnostic-rendered-ansi", "--manifest-path", "/home/martin/tmp/bevy-rs/Cargo.toml", "--keep-going", "--all-targets"], current_dir: Some("/home/martin/tmp/bevy-rs") }, error=Cargo watcher failed, the command produced no valid metadata (exit code: ExitStatus(unix_wait_status(25856))):
error: failed to download `cargo-manifest-proc-macros v0.3.1`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/home/martin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-manifest-proc-macros-0.3.1/Cargo.toml`

Caused by:
  feature `edition2024` is required

  The package requires the Cargo feature called `edition2024`, but that feature is not stabilized in this version of Cargo (1.84.0 (66221abde 2024-11-19)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2024 for more information about the status of this feature.

2025-01-17T11:44:30.350601426+01:00 ERROR Flycheck failed to run the following command: CommandHandle { program: "/home/martin/.cargo/bin/cargo", arguments: ["check", "-p", "bevy", "--example", "alien_cake_addict", "--message-format=json-diagnostic-rendered-ansi", "--manifest-path", "/home/martin/tmp/bevy-rs/Cargo.toml", "--keep-going", "--all-targets"], current_dir: Some("/home/martin/tmp/bevy-rs") }, error=Cargo watcher failed, the command produced no valid metadata (exit code: ExitStatus(unix_wait_status(25856))):
error: failed to download `cargo-manifest-proc-macros v0.3.1`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/home/martin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-manifest-proc-macros-0.3.1/Cargo.toml`

Caused by:
  feature `edition2024` is required

  The package requires the Cargo feature called `edition2024`, but that feature is not stabilized in this version of Cargo (1.84.0 (66221abde 2024-11-19)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2024 for more information about the status of this feature.

Is it expected to work with stable Rust / edition 2021?

mdickopp · 2025-01-17T10:50:20Z

The command cargo check does not work either with Rust 1.84.0:

error: failed to download `cargo-manifest-proc-macros v0.3.1`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/home/martin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-manifest-proc-macros-0.3.1/Cargo.toml`

Caused by:
  feature `edition2024` is required

  The package requires the Cargo feature called `edition2024`, but that feature is not stabilized in this version of Cargo (1.84.0 (66221abde 2024-11-19)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2024 for more information about the status of this feature.

raldone01 · 2025-01-17T15:23:43Z

I know. Please try it with nightly for now. Clean your project first and restart rust analyzer.

I need to modify cargo-manifest-proc-macros to support older rust versions.

raldone01 · 2025-01-18T10:24:33Z

The ci passes now.

I am now investigating why RA still fails.
The macro expansions looks alright.

#[derive(Resource)]
struct MyResource {
    value: f32,
}

Expands to the following according to RA:

// Recursive expansion of Resource macro
// ======================================

impl ::bevy::ecs::system::Resource for MyResource where Self: Send + Sync + 'static {}

The following function:

fn hello_world(query: Query<&MyComponent>, resource: Res<MyResource>) {
    let value_deref = resource.value; // suggestions don't work but ctrl+click works once it's written, inlay hints also work
    let value_direct = resource.into_inner().value; // suggestions work
    println!(
        "hello world! Value: {} {} {}",
        query.iter().next().unwrap().value,
        value_deref,
        value_direct
    );
}

RA only seems to fail to complete the deref for me.
This happens both with this PR and bevy=0.13.

raldone01 · 2025-01-18T11:22:52Z

For me this PR is working correctly even with RA. The deref autocompletion doesn't really interact with the macro as far as I can tell.

If I replace the derive macro with the expanded code manually, RA still fails the deref suggestion.
The autocomplete also fails for bevy=0.13.

This might possibly maybe be a RA issue? Or I am missing something.

mdickopp · 2025-01-18T13:43:50Z

Current version (raldone01/bevy-rs@96fbf0e) works for me as well.

raldone01 · 2025-01-19T21:28:32Z

@BenjaminBrienen Should be ready from my side now.

raldone01 · 2025-01-31T14:12:50Z

I cleaned up the git history. It should be easier to review now.

* Use the `extern crate self as` trick to make proc macros work inside and outside bevy behave the same way.

alice-i-cecile

A little noisy to do this all at once, but I really like the extern crate trick, and the introduction of dedicated integration tests is long-overdue.

This particular design feels a bit complicated to me relative to just slapping stuff in the root level tests folder, but on reflection I think it's required to properly test the derive macros as if we were an external user.

alice-i-cecile · 2025-02-09T19:27:09Z

I've fixed up the merge conflicts for you; merging now.

raldone01 · 2025-02-09T19:56:52Z

I could have split it up easily as requested. Feel free to change it how you see fit.

I just needed a way to test that everything is working as intended.

Thank you for your work on bevy!

pcwalton · 2025-02-10T03:43:32Z

This broke all my usages of bevy derive with messages like this:

error[E0433]: failed to resolve: use of undeclared crate or module `bevy_reflect`
   --> C:\Users\pcwal\Source\Rust\leafwing-input-manager\src\lib.rs:144:41
    |
144 | #[derive(Debug, Clone, Copy, PartialEq, Reflect, Serialize, Deserialize)]
    |                                         ^^^^^^^ use of undeclared crate or module `bevy_reflect`
    |
    = note: struct `crate::user_input::trait_reflection::tripleaxislike::TypeRegistration` exists but is inaccessible
    = note: this error originates in the derive macro `Reflect` (in Nightly builds, run with -Z macro-backtrace for more info)

and

error[E0277]: `user_input::gamepad::GamepadStick` can not provide dynamic type information through reflection
   --> C:\Users\pcwal\Source\Rust\leafwing-input-manager\src\user_input\gamepad.rs:548:23
    |
548 | impl DualAxislike for GamepadStick {
    |                       ^^^^^^^^^^^^ the trait `bevy::bevy_reflect::Typed` is not implemented for `user_input::gamepad::GamepadStick`

For reference, I'm using Cargo overrides to redirect Bevy 0.16.0 to the local repo.

raldone01 · 2025-02-10T05:03:13Z

I think I know what went wrong!
Sorry about that.

I will add an integration test for it and fix it soon. It should be a minimal fix.

The path to bevy reflect should start with ::. But it does not.

raldone01 · 2025-02-10T05:25:01Z

@pcwalton can you open an issue with a minimal reproducer?
Did you clean your project after updating bevy?

I just tried a few bevy reflect derives and they all worked for me.

Tag me in the issue.

villor · 2025-02-10T18:39:24Z

Hitting this in bevy_editor_prototypes for the crate bevy_editor_cam when trying to update bevy to latest main. Not sure why it happens for that crate only.
Tried cargo clean and rm Cargo.lock.

See: bevyengine/bevy_editor_prototypes#178

raldone01 · 2025-02-10T18:41:50Z

@villor I will use your pr to track down the issue and fix it.

alice-i-cecile · 2025-02-10T18:45:36Z

Much thanks @raldone01. I'm going to spin up an issue so we don't forget to fix this for 0.16.

… dependencies. (#17795) This is a follow up fix for #17330 and fixes #17780. There was a logic error in the ambiguity detection of `cargo-manifest-proc-macros`. `cargo-manifest-proc-macros` now has a test for this case to prevent the issue in the future. I also opted to hard fail if the `cargo-manifest-proc-macros` crate fails. That way the error is more obvious and easier to fix and diagnose. ## Testing - The reproducer: bevyengine/bevy_editor_prototypes#178 works for me using these fixes.

…ine#17330) This pr uses the `extern crate self as` trick to make proc macros behave the same way inside and outside bevy. - Removes noise introduced by `crate as` in the whole bevy repo. - Fixes bevyengine#17004. - Hardens proc macro path resolution. - [x] `BevyManifest` needs cleanup. - [x] Cleanup remaining `crate as`. - [x] Add proper integration tests to the ci. - `cargo-manifest-proc-macros` is written by me and based/inspired by the old `BevyManifest` implementation and [`bkchr/proc-macro-crate`](https://github.com/bkchr/proc-macro-crate). - What do you think about the new integration test machinery I added to the `ci`? More and better integration tests can be added at a later stage. The goal of these integration tests is to simulate an actual separate crate that uses bevy. Ideally they would lightly touch all bevy crates. - Needs RA test - Needs testing from other users - Others need to run at least `cargo run -p ci integration-test` and verify that they work. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

raldone01 force-pushed the better-bevy-manifest branch from 28422af to 8f4a421 Compare January 12, 2025 20:28

raldone01 changed the title ~~Make BevyManifest name resolution smarter.~~ Harden proc macro path resolution and add integration tests. Jan 12, 2025

raldone01 marked this pull request as ready for review January 12, 2025 20:46

BenjaminBrienen approved these changes Jan 13, 2025

View reviewed changes

mdickopp mentioned this pull request Jan 17, 2025

rust-analyzer autocomplete is broken for some ECS types #17004

Closed

raldone01 force-pushed the better-bevy-manifest branch from cfbd221 to 96fbf0e Compare January 18, 2025 10:45

mdickopp mentioned this pull request Jan 18, 2025

Use one BevyManifest instance per crate in proc macros #17310

Closed

raldone01 force-pushed the better-bevy-manifest branch from 96fbf0e to 0b82b2e Compare January 19, 2025 20:37

This was referenced Jan 19, 2025

Tracking issue for Res<_> deref rust-analyzer autocompletion failure #17438

Open

Fix rust-analyzer error in example bounding 2d #17420

Closed

raldone01 force-pushed the better-bevy-manifest branch from 0b82b2e to b3a2078 Compare January 19, 2025 21:05

raldone01 force-pushed the better-bevy-manifest branch from b3a2078 to a6ba23e Compare January 19, 2025 22:09

raldone01 force-pushed the better-bevy-manifest branch 2 times, most recently from 0ed2ea8 to c0538b9 Compare January 21, 2025 08:55

raldone01 added 3 commits January 31, 2025 15:19

* Make BevyManifest name resolution smarter.

7ef0464

* Use the `extern crate self as` trick to make proc macros work inside and outside bevy behave the same way.

Fix a spelling mistake in a ci error message.

d8965cd

Add integration tests to the ci pipeline to closer simulate a bevy user.

26f820d

raldone01 force-pushed the better-bevy-manifest branch from e90db6d to 1041912 Compare January 31, 2025 14:23

Cleanup all use crate as and use crate::{self as usages.

22802c9

raldone01 force-pushed the better-bevy-manifest branch from 1041912 to 22802c9 Compare January 31, 2025 14:26

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 9, 2025

alice-i-cecile added 2 commits February 9, 2025 11:15

Merge remote-tracking branch 'origin/main' into better-bevy-manifest

badbe59

Manual merge conflict cleanup

89cbba1

alice-i-cecile approved these changes Feb 9, 2025

View reviewed changes

alice-i-cecile enabled auto-merge February 9, 2025 19:27

alice-i-cecile added this pull request to the merge queue Feb 9, 2025

Merged via the queue into bevyengine:main with commit 1b7db89 Feb 9, 2025
32 checks passed

villor mentioned this pull request Feb 10, 2025

Migrate bevy to latest main bevyengine/bevy_editor_prototypes#178

Merged

3 tasks

alice-i-cecile mentioned this pull request Feb 10, 2025

Hardening proc macro path resolution broke cargo overrides #17780

Closed

raldone01 mentioned this pull request Feb 11, 2025

Fix failing proc macros when depending on bevy through dev and normal dependencies. #17795

Merged

BD103 mentioned this pull request Feb 11, 2025

Ensure assorted non-engine crates are formatted in CI #10768

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harden proc macro path resolution and add integration tests. #17330

Harden proc macro path resolution and add integration tests. #17330

raldone01 commented Jan 12, 2025 •

edited

Loading

BenjaminBrienen Jan 13, 2025

raldone01 Jan 18, 2025 •

edited

Loading

BenjaminBrienen commented Jan 13, 2025

raldone01 commented Jan 13, 2025

mdickopp commented Jan 17, 2025

mdickopp commented Jan 17, 2025

raldone01 commented Jan 17, 2025

raldone01 commented Jan 18, 2025 •

edited

Loading

raldone01 commented Jan 18, 2025 •

edited

Loading

mdickopp commented Jan 18, 2025

raldone01 commented Jan 19, 2025

raldone01 commented Jan 31, 2025

alice-i-cecile left a comment

alice-i-cecile commented Feb 9, 2025

raldone01 commented Feb 9, 2025

pcwalton commented Feb 10, 2025 •

edited

Loading

raldone01 commented Feb 10, 2025 •

edited

Loading

raldone01 commented Feb 10, 2025 •

edited

Loading

villor commented Feb 10, 2025

raldone01 commented Feb 10, 2025 •

edited

Loading

alice-i-cecile commented Feb 10, 2025

Harden proc macro path resolution and add integration tests. #17330

Harden proc macro path resolution and add integration tests. #17330

Conversation

raldone01 commented Jan 12, 2025 • edited Loading

Objective

TODO

Notes

Testing

BenjaminBrienen Jan 13, 2025

Choose a reason for hiding this comment

raldone01 Jan 18, 2025 • edited Loading

Choose a reason for hiding this comment

BenjaminBrienen commented Jan 13, 2025

raldone01 commented Jan 13, 2025

mdickopp commented Jan 17, 2025

mdickopp commented Jan 17, 2025

raldone01 commented Jan 17, 2025

raldone01 commented Jan 18, 2025 • edited Loading

raldone01 commented Jan 18, 2025 • edited Loading

mdickopp commented Jan 18, 2025

raldone01 commented Jan 19, 2025

raldone01 commented Jan 31, 2025

alice-i-cecile left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Feb 9, 2025

raldone01 commented Feb 9, 2025

pcwalton commented Feb 10, 2025 • edited Loading

raldone01 commented Feb 10, 2025 • edited Loading

raldone01 commented Feb 10, 2025 • edited Loading

villor commented Feb 10, 2025

raldone01 commented Feb 10, 2025 • edited Loading

alice-i-cecile commented Feb 10, 2025

raldone01 commented Jan 12, 2025 •

edited

Loading

raldone01 Jan 18, 2025 •

edited

Loading

raldone01 commented Jan 18, 2025 •

edited

Loading

raldone01 commented Jan 18, 2025 •

edited

Loading

pcwalton commented Feb 10, 2025 •

edited

Loading

raldone01 commented Feb 10, 2025 •

edited

Loading

raldone01 commented Feb 10, 2025 •

edited

Loading

raldone01 commented Feb 10, 2025 •

edited

Loading