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

feat: avoid checking the whole project during initial loading #8476

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

matklad
Copy link
Member

@matklad matklad commented Apr 12, 2021

bors r+
🤖

bors bot added a commit that referenced this pull request Apr 12, 2021
8476: feat: avoid checking the whole project during initial loading r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@matklad
Copy link
Member Author

matklad commented Apr 12, 2021

bors r-

@bors
Copy link
Contributor

bors bot commented Apr 12, 2021

Canceled.

@matklad
Copy link
Member Author

matklad commented Apr 12, 2021

bors r+

bors bot added a commit that referenced this pull request Apr 12, 2021
8476: feat: avoid checking the whole project during initial loading r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@bors
Copy link
Contributor

bors bot commented Apr 12, 2021

Build failed:

@matklad
Copy link
Member Author

matklad commented Apr 12, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 12, 2021

Canceled.

@matklad
Copy link
Member Author

matklad commented Apr 12, 2021

Fun fact -- took me a while to debug this, because cargo caches rustc invocations. So it cached the first, buggy implementation, and happy echoed buggy behavior at me and leaving me wondering why my bug fix doesn't work and why my logs don't show up. Guess who added this caching to cargo in the first place?

@matklad
Copy link
Member Author

matklad commented Apr 12, 2021

Turns out that the cache proprely accounts for rustc's own mtime (pat on the back, @matklad!) but misses the wrappers specifically: rust-lang/cargo#9348

@rishadbaniya
Copy link

Just installed a binary of rust analyzer for neovim, made "main.rs" file empty and then opened it, it showed "cargo check failed" and tells me to press enter and then loops ...............

@matklad
Copy link
Member Author

matklad commented Apr 12, 2021

@rishadbaniya please open a separate issuer with specific steps for reproduction, or ask more open-ended support questions in the forum: https://users.rust-lang.org/c/ide/14

@matklad matklad force-pushed the wrapper branch 4 times, most recently from 68193d8 to 5a5a609 Compare April 12, 2021 12:28
@matklad
Copy link
Member Author

matklad commented Apr 12, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 12, 2021

@bors bors bot merged commit a526d0a into rust-lang:master Apr 12, 2021
@matklad matklad deleted the wrapper branch April 12, 2021 12:53
@@ -527,7 +527,7 @@ version = \"0.0.0\"
#[test]
fn out_dirs_check() {
if skip_slow_tests() {
return;
// return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you left this in by accident

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, fixed #8584.

I wonder if there's a better way to convenently run a single slow test from the editor that uncommenting the thing...

Copy link
Member

Choose a reason for hiding this comment

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

Maybe an env var that RA can set similar to what we do for updating expect tests via command?

Copy link
Member

Choose a reason for hiding this comment

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

The way I did :

fn out_dirs_check() {
    if skip_slow_tests() {
        // TODO:
        // return;
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, didn't thought about that!

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.

5 participants