Basic rust indexing support. #50

emilio · 2017-11-17T23:44:44Z

Don't run it yet by default, since I haven't tested it on mozilla-central.

bill-mccloskey

This looks really nice.

bill-mccloskey · 2017-11-20T21:48:40Z

tools/src/bin/rust-indexer.rs

@@ -0,0 +1,235 @@
+#[macro_use]
+extern crate clap;
+extern crate rls_analysis as analysis;


It's kind of confusing to do this since there's already an analysis module in tools. Can you leave it as rls_analysis?

Sure thing, done.

bill-mccloskey · 2017-11-20T21:51:08Z

tools/src/bin/rust-indexer.rs

+    if span.column_start == span.column_end {
+        return format!("{}:{}", span.line_start.0, span.column_start.0 - 1);
+    }
+    let len = span.column_end.0 - span.column_end.0;


This looks like an error.

Yup, fixed it.

bill-mccloskey · 2017-11-20T21:53:39Z

tools/src/bin/rust-indexer.rs

+        return format!("{}:{}", span.line_start.0, span.column_start.0 - 1);
+    }
+    let len = span.column_end.0 - span.column_end.0;
+    format!("{}:{}-{}", span.line_start.0, span.column_start.0 - 1, len)


In any case, the last argument should actually be span.column_end.0 - 1.

Right, that was the fix, thanks! :)

bill-mccloskey · 2017-11-20T21:55:44Z

tools/src/bin/rust-indexer.rs

+        .args_from_usage(
+            "<src>    'Points to the source root'
+             <input>  'Points to the deps/save-analysis directory'
+             <output> 'Points to the directory where searchfox metadata should'",


where searchfox metadata should go?

nice catch :)

emilio · 2017-11-22T14:56:07Z

I'm trying to run it on central... though it may take a bit :)

emilio · 2017-11-22T19:42:59Z

I added a couple more commits that make this work in mozilla-central. r? @bill-mccloskey

bill-mccloskey · 2017-11-25T03:44:29Z

I tried running this locally on mozilla-central, and I got a segfault while building some Rust code. This was the error.

14:11.94 error: failed to run custom build command for style v0.0.1 (file:///home/vagrant/mozilla-index/mozilla-central/gecko-dev/servo/components/style)
14:11.94 process didn't exit successfully: /home/vagrant/mozilla-index/mozilla-central/objdir/toolkit/library/debug/build/style-348820704fd2a9fe/build-script-build (signal: 11, SIGSEGV: invalid memory reference)
14:11.94 --- stdout
14:11.94 cargo:rerun-if-changed=build.rs
14:11.94 cargo:out_dir=/home/vagrant/mozilla-index/mozilla-central/objdir/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-1623c3e1c6bdb176/out
14:11.94 cargo:rerun-if-changed=properties/declaration_block.rs
14:11.94 cargo:rerun-if-changed=properties/properties.mako.rs
14:11.94 cargo:rerun-if-changed=properties/properties.html.mako

Everything else finished successfully, but I didn't get any indexing results for WebRender or Stylo. Some crates did have data, though (libcubeb and a few other things).

Did you see anything like this, Emilio?

bill-mccloskey · 2017-11-25T04:14:44Z

The same thing happens without the -Zsave-analysis option. I wonder if the current Rust nightly is just unable to build Firefox right now? Or maybe it's an issue running inside Vagrant?

bill-mccloskey · 2017-11-25T04:19:52Z

I did have one idea about how to address the issue of how to land this at the same time we want to index C++ code in TaskCluster. It looks like running mach cargo check gkrust will generate the save-analysis data. That means we could analyze C++ code in TaskCluster and just run mach cargo check gkrust during the Searchfox indexing run, to index only Rust code.

emilio · 2017-11-25T16:10:07Z

The issue you're seeing is rust-lang/rust#46239. I'm landing a workaround in mozilla-central right now.

emilio · 2017-11-25T16:18:08Z

I did have one idea about how to address the issue of how to land this at the same time we want to index C++ code in TaskCluster. It looks like running mach cargo check gkrust will generate the save-analysis data. That means we could analyze C++ code in TaskCluster and just run mach cargo check gkrust during the Searchfox indexing run, to index only Rust code.

That sounds ok to me, how's the taskcluster stuff happening? I'm happy to try to integrate it that way too.

amccreight · 2017-11-27T23:21:54Z

Do you actually want my review on anything here?

emilio · 2017-11-28T11:14:12Z

I guess billm's review is enough

This will be needed to use -Zsave-analysis.

Don't run it yet, since I haven't tested it on mozilla-central.

This commit implements the necessary translation from local crate IDs to global crate IDs, so that cross-references work properly across crates.

They don't work well right now, but hey, worth testing it.

This allows the script to run on central.

Unfortunate, but this happens when indexing the `future` crate. I haven't debugged it further, but it was in builtin definitions that don't have symbols anyway.

It now works on mozilla-central!

emilio · 2017-12-10T03:20:31Z

Rebased and verified it still works. The failure on central was fixed long ago, too.

r? @bill-mccloskey

staktrace · 2017-12-27T03:46:54Z

@emilio are there any other changes you want to make here? I looked over the changes (not in much detail though) and they look sane, at least to the point where I'm comfortable merging them and iterating further afterwards. I also created new AMIs with the nightly rust and ran an indexing run with these patches, so the instance up on dev.searchfox.org right now has these changes and they seem to work fine. So if you have no objections I'd like to merge these patches (and deploy the new AMIs as default).

emilio · 2017-12-28T20:08:01Z

That'd be great! IIRC @bill-mccloskey mentioned that this was blocked on some kind of infra issue regarding reprovisioning, but if you managed to get it working I think this is good to go.

staktrace · 2017-12-29T06:18:46Z

I've deployed the reprovisioned servers to indexer-16.04 and web-server-16.04 so I think merging these patches should work for tomorrow's indexing run. If something goes wrong I'll investigate.

emilio requested review from bill-mccloskey and amccreight November 18, 2017 06:15

emilio force-pushed the rust branch 2 times, most recently from b8a0575 to f9fa996 Compare November 18, 2017 08:14

emilio mentioned this pull request Nov 18, 2017

infra: Use Rust nightly. #37

Closed

emilio changed the title ~~scripts: Add a simple rust analyzer script.~~ Basic rust indexing support. Nov 18, 2017

bill-mccloskey suggested changes Nov 20, 2017

View reviewed changes

nrc mentioned this pull request Nov 22, 2017

Tools news rust-dev-tools/dev-tools-team#28

Closed

emilio removed the request for review from amccreight November 28, 2017 11:14

emilio force-pushed the rust branch from 8735536 to d1e059e Compare November 30, 2017 16:46

emilio added 11 commits December 10, 2017 03:51

Add a rust indexer.

4a392ac

This will be needed to use -Zsave-analysis.

Add a simple rust crate to the test build.

ec65b31

scripts: Add a simple rust analyzer script.

c34d5fe

Don't run it yet, since I haven't tested it on mozilla-central.

Add an slightly more complex rust dependency.

3b59b4d

rust-indexer: Get refs across crates working.

e682297

This commit implements the necessary translation from local crate IDs to global crate IDs, so that cross-references work properly across crates.

Ensure that definition qual names always start with the crate name.

fcd9dad

Also test traits.

15e64f7

They don't work well right now, but hey, worth testing it.

Fix multi-column spans.

627a611

rust-indexer: Don't rename rls_analysis.

b4a07cb

rust-indexer: Fix location formatting.

88afbf9

rust-indexer: Fix usage documentation message.

1a6286d

emilio added 6 commits December 10, 2017 03:51

Find the save-analysis in a better way.

939ec71

This allows the script to run on central.

rust-indexer: Don't panic when finding a dupe definition.

a2fd82b

Unfortunate, but this happens when indexing the `future` crate. I haven't debugged it further, but it was in builtin definitions that don't have symbols anyway.

Pass the files root, not the index root, to the rust indexer.

3f04a35

scripts: Enable rust indexing.

fc2e5a2

It now works on mozilla-central!

rust: Avoid failing when there's no rust analysis in the repo.

01f5ea7

rust-indexer: Be a bit more idiomatic when converting spans.

730e0fb

emilio force-pushed the rust branch from d1e059e to 730e0fb Compare December 10, 2017 03:19

staktrace merged commit 9201a6b into mozsearch:master Dec 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic rust indexing support. #50

Basic rust indexing support. #50

emilio commented Nov 17, 2017

bill-mccloskey left a comment

bill-mccloskey Nov 20, 2017

emilio Nov 22, 2017

bill-mccloskey Nov 20, 2017

emilio Nov 22, 2017

bill-mccloskey Nov 20, 2017

emilio Nov 22, 2017

bill-mccloskey Nov 20, 2017

emilio Nov 22, 2017

emilio commented Nov 22, 2017

emilio commented Nov 22, 2017

bill-mccloskey commented Nov 25, 2017

bill-mccloskey commented Nov 25, 2017

bill-mccloskey commented Nov 25, 2017

emilio commented Nov 25, 2017

emilio commented Nov 25, 2017

amccreight commented Nov 27, 2017

emilio commented Nov 28, 2017

emilio commented Dec 10, 2017

staktrace commented Dec 27, 2017 •

edited

Loading

emilio commented Dec 28, 2017

staktrace commented Dec 29, 2017

Basic rust indexing support. #50

Basic rust indexing support. #50

Conversation

emilio commented Nov 17, 2017

bill-mccloskey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilio commented Nov 22, 2017

emilio commented Nov 22, 2017

bill-mccloskey commented Nov 25, 2017

bill-mccloskey commented Nov 25, 2017

bill-mccloskey commented Nov 25, 2017

emilio commented Nov 25, 2017

emilio commented Nov 25, 2017

amccreight commented Nov 27, 2017

emilio commented Nov 28, 2017

emilio commented Dec 10, 2017

staktrace commented Dec 27, 2017 • edited Loading

emilio commented Dec 28, 2017

staktrace commented Dec 29, 2017

staktrace commented Dec 27, 2017 •

edited

Loading