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

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

raldone01
Copy link
Contributor

@raldone01 raldone01 commented Jan 12, 2025

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

Objective

TODO

  • BevyManifest needs cleanup.
  • External crate dependency cargo-manifest-proc-macros needs discussion.
  • Interface of cargo-manifest-proc-macros needs discussion.
  • Cleanup remaining crate as.
  • Add proper integration tests to the ci.

Open Questions

  • What is the policy of adding an external dependency?
  • cargo-manifest-proc-macros is written by me and based/inspired by the old BevyManifest implementation and bkchr/proc-macro-crate.
  • Any suggestions for the api of the cargo-manifest-proc-macros crate?
  • Should I lower the edition of cargo-manifest-proc-macros from 2024 to 2021?
  • 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.

* Use the `extern crate self as` trick to make proc macros work inside and outside bevy behave the same way.
While this does not catch RA errors it should simulate bevy users better.
@raldone01 raldone01 force-pushed the better-bevy-manifest branch from 28422af to 8f4a421 Compare January 12, 2025 20:28
@raldone01 raldone01 changed the title Make BevyManifest name resolution smarter. Harden proc macro path resolution and add integration tests. Jan 12, 2025
@raldone01 raldone01 marked this pull request as ready for review January 12, 2025 20:46
@@ -4,6 +4,7 @@ crates/**/target
benches/**/target
tools/**/target
**/*.rs.bk
rustc-ice-*.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

😆

@BenjaminBrienen
Copy link
Contributor

Great idea, just need to fix the errors

@raldone01
Copy link
Contributor Author

I will make the ci pass coming Thursday or Friday.

@mdickopp
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rust-analyzer autocomplete is broken for some ECS types
3 participants