Update OUT_DIR based on `cargo check` output #3582

kiljacken · 2020-03-13T15:48:24Z

This entire PR is basically annoying plumbing needed to pass through the correct OUT_DIR from a cargo check run to the right place in the crate graph.

It's in a semi working state: It correctly updates OUT_DIR in the crate graph, and get's to the point where macro's depending on env!("OUT_DIR") are re-evaluated, but I don't seem to get completion for the gl crate still. Not sure why, but decided to get the code public ASAP so we can discuss the right direction to take it.

Things that still need to be done/decided:

When/if to schedule a single cargo check run on startup to get the correct OUT_DIR values.
What's the story with features, targets and OUT_DIR?
All the other things that I've overlooked

Depends on rust-analyzer/ra_vfs#20 but is currently patched to use my fork so things can work.

edwin0cheng · 2020-03-13T16:45:01Z

but I don't seem to get completion for the gl crate still

I am investigating now, and it seem to be due to FileLoaderDelegate::resolve_extern_path return a wrong id. And it is because in rust-analyzer/ra_vfs#20 , we find the path by starts_with. However, usually the OUT_DIR lives in the main crate target diretory and thus it will return the main crate instead.

jyn514 · 2020-03-15T17:58:38Z

And it is because in rust-analyzer/ra_vfs#20 , we find the path by starts_with. However, usually the OUT_DIR lives in the main crate target diretory and thus it will return the main crate instead.

Will that handle a target directory that's not a subdirectory of the crate root? E.g. if you set CARGO_TARGET_DIR to an absolute path

kiljacken · 2020-03-15T18:00:49Z

@jyn514 It doesn't yet, but should once I get around to making it add a new vfs root for each out_dir.

kiljacken · 2020-03-16T12:48:40Z

Okay pushed a new take on the whole thing. This time it actually works!

So I've added a config option to run cargo check when loading a cargo workspace to get the right OUT_DIR paths. It did take some juggling to get the extern source stuff to a reasonable point.

It currently doesn't support setting OUT_DIR for sysroot crates, but I don't think any of them should be using build scripts with OUT_DIR?

I'll take a look at folding the custom path support into the project model config, so we can eliminate some duplication, and then add OUT_DIR support to the json workspaces.

kiljacken · 2020-03-16T13:20:22Z

Okay that wasn't too bad either.

@matklad Do you want me to hook up OUT_DIR support for CLI runs of rust-analyzer as well, while I'm at it?

matklad · 2020-03-16T13:22:53Z

Do you want me to hook up OUT_DIR support for CLI runs of rust-analyzer as well, while I'm at it?

Yeah, I guess that would be helpful! It should be behind a --get-output-dir flag (we don't want to cargo-check by default). And there should be a better verb then get. sniff?

kiljacken · 2020-03-16T13:24:22Z

--load-output-dir maybe? The name should probably imply up front that some work is being done.

kiljacken · 2020-03-16T13:52:37Z

And with that in place I think this is ready for some actual review.

~/testcase $ rust-analyzer analysis-stats --load-output-dirs .                            (master|…6) kiljacken@exheater
Database loaded, 20 roots, 109.269843ms
Crates in this dir: 1
Total modules found: 1
Total declarations: 2
Total functions: 2
Item Collection: 3.081167556s, 0b allocated 0b resident
Total expressions: 12    
Expressions of unknown type: 0 (0%)
Expressions of partially unknown type: 0 (0%)
Type mismatches: 0
Inference: 2.604975548s, 0b allocated 0b resident
Total: 5.686150434s, 0b allocated 0b resident
~/testcase $ rust-analyzer analysis-stats .                                               (master|…6) kiljacken@exheater
Database loaded, 17 roots, 92.107763ms
Crates in this dir: 1
Total modules found: 1
Total declarations: 2
Total functions: 2
Item Collection: 1.61754515s, 0b allocated 0b resident
Total expressions: 12    
Expressions of unknown type: 1 (8%)
Expressions of partially unknown type: 0 (0%)
Type mismatches: 0
Inference: 2.2312987s, 0b allocated 0b resident
Total: 3.84884987s, 0b allocated 0b resident

kiljacken · 2020-03-16T13:54:13Z

NPM audit check seems to be causing issues upstream, not sure if I can do anything about it.

kiljacken · 2020-03-16T14:31:07Z

Rebased to preempt bors hi-jinx due to CI changes

crates/ra_cargo_watch/src/lib.rs

matklad · 2020-03-17T10:55:49Z

crates/ra_project_model/Cargo.toml

 ra_arena = { path = "../ra_arena" }
 ra_db = { path = "../ra_db" }
 ra_cfg = { path = "../ra_cfg" }
+ra_cargo_watch = { path = "../ra_cargo_watch" }


Not sure what the right direction of the dependency should be. It would be cool if there were exactly one crate that invokes cargo/rust. Perhaps we need to introduce a ra_toolchain crate... But this is mostly unrelated to the PR.

Had the same exact consideration. With VFS changes, we can keep it contained to ra_cargo_watch, but we need a good way to add roots and extern source on-the-go.

The dream scenario would be cargo metadata providing the needed information, but in previous issues the cargo folks don't seem to keen on that.

crates/ra_project_model/src/cargo_workspace.rs

editors/code/package.json

…ture

kiljacken · 2020-03-17T13:57:57Z

Concerns should be resolved in new commits. Also re-based once again to make it easier on poor bors

matklad · 2020-03-17T14:37:24Z

bors r+

3582: Update OUT_DIR based on `cargo check` output r=matklad a=kiljacken This entire PR is basically annoying plumbing needed to pass through the correct OUT_DIR from a `cargo check` run to the right place in the crate graph. It's in a semi working state: It correctly updates OUT_DIR in the crate graph, and get's to the point where macro's depending on `env!("OUT_DIR")` are re-evaluated, but I don't seem to get completion for the `gl` crate still. Not sure why, but decided to get the code public ASAP so we can discuss the right direction to take it. Things that still need to be done/decided: * [ ] When/if to schedule a single `cargo check` run on startup to get the correct OUT_DIR values. * [ ] What's the story with features, targets and `OUT_DIR`? * [ ] All the other things that I've overlooked Depends on rust-analyzer/ra_vfs#20 but is currently patched to use my fork so things can work. Co-authored-by: Emil Lauridsen <mine809@gmail.com>

bors · 2020-03-17T14:38:54Z

Build failed

TypeScript

matklad · 2020-03-17T14:41:26Z

Hm, so it seems like npm audit fails, and we can't really fix it, because eslint plugin is not updated yet....

#3582 (comment)

rust-lang#3582 (comment)

kiljacken mentioned this pull request Mar 15, 2020

Add path2root rust-analyzer/ra_vfs#20

Merged

edwin0cheng mentioned this pull request Mar 16, 2020

Use WorkDoneProgress LSP API for initial load #3587

Merged

kiljacken marked this pull request as ready for review March 16, 2020 13:52

kiljacken changed the title ~~WIP: Update OUT_DIR based on cargo check output~~ Update OUT_DIR based on cargo check output Mar 16, 2020

matklad reviewed Mar 17, 2020

View reviewed changes

kiljacken added 8 commits March 17, 2020 14:47

Support loading OUT_DIR from cargo check at launch

33c6c7a

Change existing OUT_DIR override config to make use of new infrastruc…

f5a2fcf

…ture

Support specifying OUT_DIR in json project

4fb79f2

Remove unused config from VSCode ext

9e7dbb1

Support loading OUT_DIR for CLI runs

f84deff

Remove outDirOverrides

e154132

Slight readablity improvement

5af81b8

Use dyn-ref instead of impl to impact compile times the least

2dd887d

matklad merged commit ec1312e into rust-lang:master Mar 17, 2020

matklad added a commit that referenced this pull request Mar 17, 2020

Disable npm audit, as it blocks unrelated PR without a way to fix

46c618f

#3582 (comment)

edwin0cheng mentioned this pull request Mar 18, 2020

include!(concat!(env!("OUT_DIR"), "/bindings.rs")) should work #1964

Closed

JoshMcguigan pushed a commit to JoshMcguigan/rust-analyzer that referenced this pull request Apr 1, 2020

Disable npm audit, as it blocks unrelated PR without a way to fix

6069e07

rust-lang#3582 (comment)

Update OUT_DIR based on cargo check output #3582

Update OUT_DIR based on cargo check output #3582

Uh oh!

Conversation

kiljacken commented Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edwin0cheng commented Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyn514 commented Mar 15, 2020

Uh oh!

kiljacken commented Mar 15, 2020

Uh oh!

kiljacken commented Mar 16, 2020

Uh oh!

kiljacken commented Mar 16, 2020

Uh oh!

matklad commented Mar 16, 2020

Uh oh!

kiljacken commented Mar 16, 2020

Uh oh!

kiljacken commented Mar 16, 2020

Uh oh!

kiljacken commented Mar 16, 2020

Uh oh!

kiljacken commented Mar 16, 2020

Uh oh!

Uh oh!

matklad Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

kiljacken Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kiljacken commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matklad commented Mar 17, 2020

Uh oh!

bors bot commented Mar 17, 2020

Build failed

Uh oh!

matklad commented Mar 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Update OUT_DIR based on `cargo check` output #3582

Update OUT_DIR based on `cargo check` output #3582

kiljacken commented Mar 13, 2020 •

edited

Loading

edwin0cheng commented Mar 13, 2020 •

edited

Loading

kiljacken commented Mar 17, 2020 •

edited

Loading