-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
file not found for module #70314
Comments
Which version are you on? Your "CI" link requires a login. Probably a duplicate of #70185 and you need to wait for the nightly version to update. |
@jxs can you try out if it works for you using |
pre-triage: assigning |
Still an issue on rustc 1.44.0-nightly (02046a5 2020-03-24) |
Let's try to shrink the input into something more manageable for finding a fix / understanding the cause. @rustbot ping cleanup |
Hey Cleanup Crew ICE-breakers! This bug has been identified as a good cc @AminArria @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @jakevossen5 @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke |
For those who don't have a CircleCI account or don't want to create one, here are the steps of the build. The problem is that I can't really reproduce the steps because the first step cargo install --path ./refinery_cli --no-default-features --features=mysql Doesn't build. Indeed, the build depends on a I then tried to only run the last step of tests. It depends on a running MySQL instance. From the configuration here, I launched a MySQL instance first with (do not forget to wait a few seconds, MySQL needs to be initialized first or the tests will fail). docker run \
--name mysql \
--publish 127.0.0.1:3306:3306 \
--env MYSQL_USER=refinery \
--env MYSQL_PASSWORD=root \
--env MYSQL_DATABASE=refinery_test \
--env MYSQL_ROOT_PASSWORD=root \
--detach \
mysql:latest Here is the command on which the CI is failing. cd refinery && cargo test --features tokio,mysql_async --test mysql_async -- --test-threads 1 I was able to run the tests on a |
thank you all for the support! |
A few questions:
|
@nikomatsakis
i am now going to try and bisect to find at which nightly did this start, but locally with |
@nikomatsakis @jxs result of
|
yeah got the same results:
|
So almost certainly this regressed due to #69838, but that was a big change -- what we really need, I think, is a better way to reproduce the problem. Currently the error occurs on a large crate, can we remove irrelevant code / modules etc to simplify the problem to something manageable? |
I've been trying to remove irrelevant code but I'm probably not even halfway into producing a MVCE. Still, I removed about 5000 lines and have still about 350 lines of code so far but still in 3 crates. I'll probably continue some other day but if someone else wants to help, I'm sharing what I've done so far in this branch (and I'll continue to push on this branch my updates). |
@woshilapin great progress, thanks! |
I've shrink it down a bit more into 2 crates (one of it is a procmacro so I'm not sure how (if?) I can shrink down to only one crate). The procmacro is now 27 lines long and the crate with the test failing at compilation is about a dozen lines of code split in 3 files. I hope this will help. |
@woshilapin thanks! ❤️ |
@Centril It sounds like @woshilapin has got the test case down to something pretty reasonable, maybe small enough to start poking about and see if you can isolate the problem? |
@petrochenkov do you think you'd have time to investigate this regression? |
Looks like there's a minimized reproduction in #70314 (comment), so yes, I'll try to look at this during this or next weekend. |
I have independently reduced the test input into something pretty small; small enough for me to cut and paste into this comment (I hope): Click to see the files for pnkfelix's MCVE test input
[package]
name = "refinery-macros"
version = "0.2.0"
authors = []
edition = "2018"
[lib]
proc-macro = true
[dependencies]
quote = "1"
syn = { version = "1", features=["full"] }
proc-macro2 = "1"
#![recursion_limit = "128"]
extern crate proc_macro;
use proc_macro::TokenStream;
#[proc_macro]
pub fn include_v1(_input: TokenStream) -> TokenStream {
let ident = syn::Ident::new("V1__initial", proc_macro2::Span::call_site());
let result = quote::quote! { pub mod #ident; };
result.into()
}
[package]
name = "refinery"
version = "0.2.0"
authors = []
edition = "2018"
[dependencies]
refinery-macros= { version = "0.2.0", path = "../refinery_macros" }
pub fn migration() -> String { format!("V1") }
pub mod migrations { refinery::include_v1!(); }
mod mod_migrations;
#[test]
fn demo() {
assert_eq!(mod_migrations::migrations::V1__initial::migration(), "V1");
}
pub use refinery_macros::include_v1; |
My hypothesis is that something has changed in the metadata (spans/hygiene/etc) associated with pub mod migrations { refinery::include_v1!(); } In particular, in nightly-2020-03-19, it successfully resolves to the file stored at path refinery/tests/mod_migrations/V1__initial.rs; but in nightly-2020-03-20, it attempts to instead resolve to the path refinery/tests/mod_migrations/migrations/V1__initial.rs I.e., it is not searching relative to the source file where the macro invocation appears. Instead, it is searching relative to the directory it expects to exist based on where in the module hierarchy that the macro invocation appears. (This might actually be expected breakage; I'll need to review some of the discussions the language team was having around this team regarding the changes to expansion.) |
Well I don't know if this breakage was expected; there were some discussions that seem at least tangentially related on PR #69838, such as #69838 (comment) that imply to me that we were not intending to change behavior in cases like this. |
(also, I have confirmed via a local build that PR #69838 is indeed where this was injected. I have not yet dissected which aspect of that PR injected it though.) |
I removed the E-needs-mcve label because I figured my demo above is pretty minimal. But I do note there is still more reduction possible: Namely, finding ways to restructure the macro definition so that it doesn't rely on external crates like |
Do we have documentation somewhere of the semantics that we've settled on, and/or the change itself suitable for e.g. the release notes? |
I don't think the span-based directory inference was ever documented anywhere. The current behavior should match what is written in the reference (https://doc.rust-lang.org/stable/reference/items/modules.html), I'm not sure there's a more detailed description. (Perhaps in some of the PRs implementing 2018 edition module directories? I don't know.) |
thank you all for the support! ❤️ |
Hi,
I have a crate that CI fails for all the tests on the latest nightly with the following output:
the refered code is here and the macro used is defined here the same tests pass on stable channel.
If there is there is anything else that I can do to help, I am happy to contribute
thanks!
The text was updated successfully, but these errors were encountered: