Emit used rustc invocation in the save-analysis file #54356

Xanewok · 2018-09-19T13:05:50Z

Blocked on rust-dev-tools/rls-data#19. (I'm guessing it won't pass CI due to an out-of-tree git dependency)

This should allow RLS to recreate a Rust compilation build plan from the save-analysis files alone, which should be useful when fetching those from external build systems, most notably Buck now.

Also this includes some more potentially useful compilation-specific options (e.g. sysroot or the actual path to extern crates) but that's not required for the build plan bits.

cc @jsgf @alexcrichton

r? @nrc

rust-highfive · 2018-09-19T13:38:24Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

######################################################################    98.0%
#######################################################################   99.1%
######################################################################## 100.0%
[00:25:24] extracting /checkout/obj/build/cache/2018-09-11/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:25:24]     Updating git repository `https://github.com/Xanewok/rls-data`
[00:26:18]  Downloading filetime v0.2.1
[00:26:19]  Downloading cc v1.0.22
[00:26:19]  Downloading toml v0.4.6
[00:26:19]  Downloading lazy_static v0.2.11

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Xanewok · 2018-09-19T15:15:25Z

One thing I was wondering was the current directory - should we include it in the save-analysis for the invocation or maybe we can assume that everything is relative to the directory of the save analysis file itself and just hope for the user to set it properly using --remap-path-prefix?

Xanewok · 2018-09-19T16:32:12Z

Including crate_name from session options might not be a good idea, since it can be inferred and we include it in the CrateId anyways. What's interesting is that target_triple is inferred and filled in the session options, but crate_name or sysroot seem not to. At a first glance it seems inconsistent? But maybe there's more to it, I'll have to read further.

rust-highfive · 2018-09-22T21:37:12Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

#######################################                                   54.7%
###################################################                       71.9%
######################################################################## 100.0%
[00:02:01] extracting /checkout/obj/build/cache/2018-09-11/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:02:01]     Updating git repository `https://github.com/Xanewok/rls-data`
[00:02:36]  Downloading serde v1.0.75
[00:02:37]  Downloading num_cpus v1.8.0
[00:02:37]  Downloading libc v0.2.43
[00:02:37]  Downloading time v0.1.40
---
tidy check
[00:04:44] * 555 error codes
[00:04:44] * highest error code: E0712
[00:04:44] * 231 features
[00:04:45] invalid source: "git+https://github.com/Xanewok/rls-data?branch=compilation-options#0b9dbe5bf766a20009def9acac2f997194823373"
[00:04:45] some tidy checks failed
[00:04:45] 
[00:04:45] 
[00:04:45] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:45] 
[00:04:45] 
[00:04:45] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:45] Build completed unsuccessfully in 0:00:49
[00:04:45] Build completed unsuccessfully in 0:00:49
[00:04:45] Makefile:79: recipe for target 'tidy' failed
[00:04:45] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:11d44f9d
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:080b6960:start=1537651696283338024,finish=1537651696287764768,duration=4426744
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:06871760
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0f276a50
travis_time:start:0f276a50
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0795d281
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Xanewok · 2018-09-22T21:42:26Z

As discussed, I removed the extra info, such as test, and added the directory and output fields.

This now includes the associated output file for the compilation (e.g. libfoo-extra_filename.rmeta), which can be used together with CrateSource and rmeta/rlib fields.

However, I forgot that to make save-analysis data able to refer to each other, that's exactly why crate disambiguators were added to save-analysis in the first place - we can use this information to generate the dependency information, instead.

@nrc do you think we should hold off including the output field?

Xanewok · 2018-09-22T21:44:02Z

src/librustc_codegen_utils/link.rs

+pub fn filename_for_metadata(sess: &Session,
+                             crate_name: &str,
+                             outputs: &OutputFilenames) -> PathBuf {
+    let libname = format!("{}{}", crate_name, sess.opts.cg.extra_filename);


Changed the formatting while moving to mimick filename_for_input

Xanewok · 2018-09-24T15:41:02Z

src/librustc_save_analysis/dump_visitor.rs

+                .filter(|(i, _)| !remap_arg_indices.contains(i))
+                .map(|(_, arg)| {
+                    match input {
+                        Input::File(ref path) if path == Path::new(&arg) => {


I decided to propagate Input so that I can directly compare the argument to the non-mapped version (Input seems to be the only place where we can get input file before it's remapped, apart from scanning the cmd) to replace it with the mapped version.

I was also thinking about trying to remap every argument and see if it matches the already remapped local_crate_source_file but that seemed like it's considerably more mechanical work and more error-prone, so in the end I decided against it.

Xanewok · 2018-09-24T15:43:02Z

src/librustc_save_analysis/span_utils.rs

-            _ => path.to_string(),
+    pub fn make_filename_string(&self, file: &SourceFile) -> String {
+        match &file.name {
+            FileName::Real(path) if !file.name_was_remapped => {


In practice every file that's going through SourceMap will be remapped but here it seemed to be a missing case for not-yet-remapped absolute path so I included it for the sake of completeness.

Xanewok · 2018-09-24T15:44:20Z

src/librustc_codegen_llvm/back/link.rs

@@ -218,15 +219,6 @@ fn preserve_objects_for_their_debuginfo(sess: &Session) -> bool {
    false
 }

-fn filename_for_metadata(sess: &Session, crate_name: &str, outputs: &OutputFilenames) -> PathBuf {


Moved it so I can only depend on librustc_codegen_utils in the save analysis crate (see here)

Xanewok · 2018-09-24T15:48:00Z

@nrc hopefully this should be ready to review again (once rust-dev-tools/rls-data#19 lands).
With this new info I managed to successfully recreate the build plan from save-analysis and run it inside RLS in a Buck project 🎉 so hopefully this will be enough as-is for now.

rust-highfive · 2018-09-24T16:15:45Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

########################################                                  56.0%
######################################################################    97.7%
######################################################################## 100.0%
[00:03:24] extracting /checkout/obj/build/cache/2018-09-11/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:03:24]     Updating git repository `https://github.com/Xanewok/rls-data`
[00:03:50]  Downloading getopts v0.2.17
[00:03:50]  Downloading time v0.1.40
[00:03:50]  Downloading lazy_static v0.2.11
[00:03:51]  Downloading num_cpus v1.8.0
---
tidy check
[00:06:02] * 556 error codes
[00:06:02] * highest error code: E0713
[00:06:02] * 231 features
[00:06:03] invalid source: "git+https://github.com/Xanewok/rls-data?branch=compilation-options#5d2c63f02db25a7530ddb52fea9d05cac0401355"
[00:06:03] some tidy checks failed
[00:06:03] 
[00:06:03] 
[00:06:03] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:06:03] 
[00:06:03] 
[00:06:03] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:06:03] Build completed unsuccessfully in 0:00:49
[00:06:03] Build completed unsuccessfully in 0:00:49
[00:06:03] Makefile:79: recipe for target 'tidy' failed
[00:06:03] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:24ddf5ad
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:01ab35fc:start=1537805268067661134,finish=1537805268072267080,duration=4605946
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:12b7da6a
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0d6f1ba4
travis_time:start:0d6f1ba4
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:335b41e4
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

nrc · 2018-09-24T23:50:11Z

src/Cargo.toml

@@ -57,6 +57,7 @@ cargo = { path = "tools/cargo" }
 # that we're shipping as well (to ensure that the rustfmt in RLS and the
 # `rustfmt` executable are the same exact version).
 rustfmt-nightly = { path = "tools/rustfmt" }
+rls-data = { git = "https://github.com/Xanewok/rls-data", branch = "compilation-options" }


can use 0.18.1 now

nrc · 2018-09-25T06:31:26Z

One thing I was wondering was the current directory - should we include it in the save-analysis for the invocation or maybe we can assume

I would include it some how, the less we rely on users 'doing the right thing', the better

Xanewok · 2018-09-25T06:36:38Z

I would include it some how, the less we rely on users 'doing the right thing', the better

This is included now (here).

@nrc bumped rls-data to 0.18.1.

nrc

r+ with the output stuff removed (unless it is needed).

nrc · 2018-09-25T06:34:43Z

src/librustc_save_analysis/lib.rs

@@ -110,6 +112,24 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
        }
    }

+    // Returns path to the compilation output (e.g. libfoo-12345678.rmeta)
+    pub fn compilation_output(&self, crate_name: &str) -> PathBuf {


I thought you said you were removing the output stuff? (Which seems like a good idea to me).

So this doesn't use the file-based approach to figure out dependencies (crate disambiguators are used instead) but if we decide to somehow use save-analysis to lower this info further into low level file-based dep graph (potentially for Cargo build system integration? or maybe we shouldn't use save-analysis for that?) then we need some kind of related file input/output.

I removed CrateSource because one-of-dylib-rlib-rmeta-is-some format is clunky to work with but left this for the reasons explained above. Should I remove this as well?

Edit: CompilationOptions::output is now in rls-data 0.18.1 so we'd need to remove it from there as well.

rust-lang/rust#54356

nrc · 2018-09-27T22:10:13Z

@bors: r+

bors · 2018-09-27T22:10:14Z

📌 Commit e5e0d8cfdc452c30b273ab716ac14c9f33fde359 has been approved by nrc

bors · 2018-09-27T23:46:15Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout save-analysis-invocation (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self save-analysis-invocation --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

warning: Cannot merge binary files: src/Cargo.lock (HEAD vs. heads/homu-tmp)
Auto-merging src/librustc_save_analysis/Cargo.toml
Auto-merging src/librustc_driver/lib.rs
Auto-merging src/Cargo.lock
CONFLICT (content): Merge conflict in src/Cargo.lock
Automatic merge failed; fix conflicts and then commit the result.

This function isn't strictly tied to LLVM (it's more of a utility) and it's now near an analogous, almost identical `filename_for_input` (for rlibs and so forth). Also this means not depending on the backend when one wants to know the accurate .rmeta output filename.

This is `command`, `directory` and `output` file.

Xanewok · 2018-09-28T00:01:49Z

@nrc rebased and hopefully works now.

nrc · 2018-09-28T09:45:29Z

@bors: r+

bors · 2018-09-28T09:45:30Z

📌 Commit 58a8621 has been approved by nrc

bors · 2018-09-28T11:48:56Z

⌛ Testing commit 58a8621 with merge 80e6e3e...

@jsgf

Emit used rustc invocation in the save-analysis file Blocked on rust-dev-tools/rls-data#19. (I'm guessing it won't pass CI due to an out-of-tree git dependency) This should allow RLS to recreate a Rust compilation build plan from the save-analysis files alone, which should be useful when fetching those from external build systems, most notably Buck now. Also this includes some more potentially useful compilation-specific options (e.g. sysroot or the actual path to extern crates) but that's not required for the build plan bits. cc @jsgf @alexcrichton r? @nrc

bors · 2018-09-28T14:25:24Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nrc
Pushing 80e6e3e to master...

rust-highfive · 2018-09-28T14:26:46Z

📣 Toolstate changed by #54356!

Tested on commit 80e6e3e.
Direct link to PR: #54356

💔 rls on windows: test-pass → build-fail (cc @nrc, @rust-lang/infra).
💔 rls on linux: test-pass → build-fail (cc @nrc, @rust-lang/infra).

@nrc

Tested on commit rust-lang/rust@80e6e3e. Direct link to PR: <rust-lang/rust#54356> 💔 rls on windows: test-pass → build-fail (cc @nrc, @rust-lang/infra). 💔 rls on linux: test-pass → build-fail (cc @nrc, @rust-lang/infra).

@kennytm

Fix RLS toolstate after #54356 merge Fixes toolstate regression caused by #54356. The `save::process_crate` now needs to be passed an additional `&Input`, this change contains the RLS equivalent of [this](https://github.com/rust-lang/rust/pull/54356/files#diff-707a0eda6b2f1a0537abc3d23133748cR983). r? @kennytm (or @nrc if you're not away yet)

rust-highfive assigned nrc Sep 19, 2018

rust-highfive added the S-waiting-on-review label Sep 19, 2018

Xanewok commented Sep 22, 2018

View reviewed changes

Xanewok commented Sep 24, 2018

View reviewed changes

nrc reviewed Sep 24, 2018

View reviewed changes

nrc reviewed Sep 25, 2018

View reviewed changes

Xanewok added a commit to Xanewok/rls that referenced this pull request Sep 26, 2018

Prepare to use Rust with save-analysis invocation PR

ba11cf7

rust-lang/rust#54356

bors added S-waiting-on-bors and removed S-waiting-on-review labels Sep 27, 2018

bors added S-waiting-on-author and removed S-waiting-on-bors labels Sep 27, 2018

Xanewok added 5 commits September 28, 2018 01:53

Emit used rustc invocation in the save-analysis file

c01e4ce

Emit only necessary compilation options in save-analysis

d45f877

This is `command`, `directory` and `output` file.

Remap only source files in the command line

1e593be

Minimize final diff

24123dd

Xanewok added 2 commits September 28, 2018 01:54

Also remap absolute source names in save-analysis

3ceae8d

Use rls-data 0.18.1

58a8621

Xanewok force-pushed the save-analysis-invocation branch from e5e0d8c to 58a8621 Compare September 27, 2018 23:55

bors added S-waiting-on-bors and removed S-waiting-on-author labels Sep 28, 2018

Xanewok mentioned this pull request Sep 28, 2018

Implement external build plan rust-lang/rls#1070

Merged

bors merged commit 58a8621 into rust-lang:master Sep 28, 2018

Xanewok added a commit to Xanewok/rust that referenced this pull request Sep 28, 2018

Fix RLS toolstate after rust-lang#54356 merge

632f3b1

Xanewok mentioned this pull request Sep 28, 2018

Fix RLS toolstate after #54356 merge #54643

Merged

Xanewok deleted the save-analysis-invocation branch September 28, 2018 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emit used rustc invocation in the save-analysis file #54356

Emit used rustc invocation in the save-analysis file #54356

Xanewok commented Sep 19, 2018

rust-highfive commented Sep 19, 2018

Xanewok commented Sep 19, 2018

Xanewok commented Sep 19, 2018

rust-highfive commented Sep 22, 2018

Xanewok commented Sep 22, 2018

Xanewok Sep 22, 2018

Xanewok Sep 24, 2018

Xanewok Sep 24, 2018

Xanewok Sep 24, 2018

Xanewok commented Sep 24, 2018

rust-highfive commented Sep 24, 2018

nrc Sep 24, 2018

nrc commented Sep 25, 2018

Xanewok commented Sep 25, 2018

nrc left a comment

nrc Sep 25, 2018

Xanewok Sep 25, 2018 •

edited

Loading

nrc commented Sep 27, 2018

bors commented Sep 27, 2018

bors commented Sep 27, 2018

Xanewok commented Sep 28, 2018

nrc commented Sep 28, 2018

bors commented Sep 28, 2018

bors commented Sep 28, 2018

bors commented Sep 28, 2018

rust-highfive commented Sep 28, 2018

Emit used rustc invocation in the save-analysis file #54356

Emit used rustc invocation in the save-analysis file #54356

Conversation

Xanewok commented Sep 19, 2018

rust-highfive commented Sep 19, 2018

Xanewok commented Sep 19, 2018

Xanewok commented Sep 19, 2018

rust-highfive commented Sep 22, 2018

Xanewok commented Sep 22, 2018

Xanewok Sep 22, 2018

Choose a reason for hiding this comment

Xanewok Sep 24, 2018

Choose a reason for hiding this comment

Xanewok Sep 24, 2018

Choose a reason for hiding this comment

Xanewok Sep 24, 2018

Choose a reason for hiding this comment

Xanewok commented Sep 24, 2018

rust-highfive commented Sep 24, 2018

nrc Sep 24, 2018

Choose a reason for hiding this comment

nrc commented Sep 25, 2018

Xanewok commented Sep 25, 2018

nrc left a comment

Choose a reason for hiding this comment

nrc Sep 25, 2018

Choose a reason for hiding this comment

Xanewok Sep 25, 2018 • edited Loading

Choose a reason for hiding this comment

nrc commented Sep 27, 2018

bors commented Sep 27, 2018

bors commented Sep 27, 2018

Xanewok commented Sep 28, 2018

nrc commented Sep 28, 2018

bors commented Sep 28, 2018

bors commented Sep 28, 2018

bors commented Sep 28, 2018

rust-highfive commented Sep 28, 2018

Xanewok Sep 25, 2018 •

edited

Loading