Add the RLS as a submodule and build a package out of it #40584

nrc · 2017-03-16T20:55:06Z

r? @brson (and cc @alexcrichton) Please review closely, I am not at all convinced I've done the right things here. I did run x.py dist and it makes an rls package which looks right to my eyes, but I haven't tested on non-linux platforms nor am I really sure what it should look like.

This does not attempt to run tests for the RLS yet.

alexcrichton · 2017-03-17T15:20:01Z

A few thoughts:

this is touching the windows packaging code. That configuration hasn't been touched in what I believe is literally years. I'd highly recommend running locally to confirm the installers actually work (I'd rather not discover this after the nightly is published)
I thought we were gonna fold rust-analysis into the rls? (in terms of packages) That way we don't have to deal with the problem of encoding package dependencies.
I'd also recommend running the OSX installers to ensure they work
I think that this is going to install the rls by default with the combined installers, is that what we want? (I thought we wanted it optional)
This is likely also going to want to update build-manifest/src/main.rs I believe
@brson has indicated that rustup needs support for the RLS, so we may want to wait on that?

alexcrichton · 2017-03-17T15:20:14Z

src/bootstrap/lib.rs

@@ -1018,6 +1018,21 @@ impl Build {
        panic!("failed to find version in cargo's Cargo.toml")
    }

+    /// Returns the `a.b.c` version that the RLS is at.
+    fn rls_release_num(&self) -> String {


Could this be deduplicated with the cargo version of this function above?

alexcrichton · 2017-03-17T15:21:15Z

I think it'd also be helpful to generate some rls tarballs and poke around to make sure they look right (e.g. similar to Cargo tarballs and such)

bors · 2017-03-21T19:37:22Z

☔ The latest upstream changes (presumably #40693) made this pull request unmergeable. Please resolve the merge conflicts.

brson · 2017-03-23T07:04:55Z

I thought we were gonna fold rust-analysis into the rls? (in terms of packages) That way we don't have to deal with the problem of encoding package dependencies.

I think we should not do that after all, the reason being that the rls also depends on the rust-src package, and there are reasons not to merge rust-src into rls, so if we have to have 2 packages, why not 3.

I'll look closer tomorrow.

alexcrichton · 2017-03-23T18:37:57Z

ah ok yeah that makes sense, if we have rust-src + rls at least then it doesn't make much sense to worry about rust-analysis.

brson · 2017-03-23T21:48:01Z

src/bootstrap/dist.rs

@@ -598,6 +636,11 @@ pub fn extended(build: &Build, stage: u32, target: &str) {
    let cargo_installer = dist.join(format!("cargo-{}-{}.tar.gz",
                                            build.package_vers(&cargo_vers),
                                            target));
+    let rls_installer = dist.join(format!("{}.tar.gz",
+                                          pkgname(build, "rls")));


This needs to be "{}-{}.tar.gz", with a final argument of target. That will put the target triple in the tarball filename, which is needed for disambiguation.

brson · 2017-03-23T21:49:08Z

src/bootstrap/dist.rs

        t!(fs::create_dir_all(pkg.join("rust-docs")));
        t!(fs::create_dir_all(pkg.join("rust-std")));

        cp_r(&work.join(&format!("{}-{}", pkgname(build, "rustc"), target)),
             &pkg.join("rustc"));
        cp_r(&work.join(&format!("cargo-nightly-{}", target)),
             &pkg.join("cargo"));
+        cp_r(&work.join(pkgname(build, "rls")),
+             &pkg.join("rls"));


This also needs the target.

(but just delete it)

brson · 2017-03-23T21:51:58Z

src/bootstrap/dist.rs

@@ -729,6 +786,12 @@ pub fn extended(build: &Build, stage: u32, target: &str) {
        cp_r(&work.join(&format!("cargo-nightly-{}", target))
                  .join("cargo"),
             &exe.join("cargo"));
+        cp_r(&work.join(pkgname(build, "rls"))
+                  .join("rls"),
+             &exe.join("rls"));


brson · 2017-03-23T21:55:00Z

src/bootstrap/dist.rs

@@ -669,20 +714,28 @@ pub fn extended(build: &Build, stage: u32, target: &str) {
        let _ = fs::remove_dir_all(&pkg);
        t!(fs::create_dir_all(pkg.join("rustc")));
        t!(fs::create_dir_all(pkg.join("cargo")));
+        t!(fs::create_dir_all(pkg.join("rls")));
+        t!(fs::create_dir_all(pkg.join("rust-analysis")));


All of the changes in this file from here down only affect the platform-specific .msi and .pkg installers, and will not work correctly without further modifications of their definitions. I think you should remove them all for now (everything inside the target.contains("apple-darwin") and `target.contains("windows").

brson · 2017-03-23T21:55:42Z

src/bootstrap/dist.rs

+                        .arg("-t").arg(etc.join("msi/squash-components.xsl")));
+        build.run(Command::new(&heat)
+                        .current_dir(&exe)
+                        .arg("dir")


This is all .msi stuff and isn't going to work without defining RlsGroup.wxs etc. Just remove for now.

brson · 2017-03-23T22:00:26Z

As @alexcrichton said build-manifest needs to be taught about the rls in order for rustup to find it. Just copy the cargo stuff, except that the rls is an "extension", not a "component".

nrc · 2017-03-27T03:21:28Z

@alexcrichton

I think that this is going to install the rls by default with the combined installers, is that what we want? (I thought we wanted it optional)

Could you explain why please? My understanding was that by adding only to extended it becomes an optional component.

alexcrichton · 2017-03-27T15:14:16Z

@nrc oh that's for rustup, but the combined installers (e.g. foo.pkg, foo.msi, etc) I think have a different configuration for what's optional and what isn't.

nrc · 2017-03-27T19:04:40Z

oh that's for rustup, but the combined installers (e.g. foo.pkg, foo.msi, etc) I think have a different configuration for what's optional and what isn't.

So, I think I've removed all that stuff (at brson's request) and that it should be just optional now?

Ready for re-review @alexcrichton /@brson

brson · 2017-03-29T19:30:59Z

@bors r+

bors · 2017-03-29T19:30:59Z

📌 Commit 6f06143 has been approved by brson

bors · 2017-03-29T20:49:34Z

⌛ Testing commit 6f06143 with merge ed6580b...

bors · 2017-03-29T21:08:14Z

💔 Test failed - status-appveyor

nrc · 2017-03-30T02:37:04Z

@bors: retry

nrc · 2017-03-30T08:32:51Z

@bors: retry

bors · 2017-03-30T09:44:17Z

⌛ Testing commit 6f06143 with merge 899539b...

bors · 2017-03-30T11:12:17Z

💔 Test failed - status-appveyor

alexcrichton · 2017-03-30T21:07:29Z

@bors: r-

Oh I was hoping to do some more testing on this before landing, have we run the installers on Windows yet? I also am worried about the regression to cycle time this will bring because we are already a half hour too slow.

To test I think we should generate the windows and osx installers and make sure they look right. For cycle time we should split all the dist builders in two (they're all producing two targets right now).

Sorry on my phone so it's hard to link issues but I'll try to fill in soon. I'll also try to help out with this when I'm back from traveling, but that'll be a week or so

alexcrichton · 2017-04-10T20:32:51Z

@bors: r+

I'll open follow-up tracking issues for various issues

bors · 2017-04-10T20:32:52Z

📌 Commit 99656e9 has been approved by alexcrichton

bors · 2017-04-10T20:54:59Z

⌛ Testing commit 99656e9 with merge b36dfb0...

@brson

Add the RLS as a submodule and build a package out of it r? @brson (and cc @alexcrichton) Please review closely, I am not at all convinced I've done the right things here. I did run `x.py dist` and it makes an rls package which looks right to my eyes, but I haven't tested on non-linux platforms nor am I really sure what it should look like. This does not attempt to run tests for the RLS yet.

bors · 2017-04-10T21:56:10Z

💔 Test failed - status-travis

aidanhs · 2017-04-10T22:33:17Z

DEBUG:sccache::simples3::s3: GET http://rust-lang-ci-sccache.s3.amazonaws.com/2/6/1/26128c6d17efd88aaf5f6eec4f07d42d4a697959
WARN:sccache::cache::s3: Got AWS error: Error(BadHTTPStatus(NotFound), State { next_error: None })
DEBUG:sccache::compiler::compiler: [CheckSymbolExists.c.o]: Cache miss
DEBUG:sccache::server: [Error(ProcessError(Output { status: ExitStatus(ExitStatus(256)), stdout: "", stderr: "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c: In function \'main\':\n/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c:8:19: error: \'arc4random\' undeclared (first use in this function)\n   return ((int*)(&arc4random))[argc];\n                   ^\n/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c:8:19: note: each undeclared identifier is reported only once for each function it appears in\n" }), State { next_error: None })] compilation failed: "CheckSymbolExists.c.o"
ERROR:sccache::server: [Error(ProcessError(Output { status: ExitStatus(ExitStatus(256)), stdout: "", stderr: "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c: In function \'main\':\n/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c:8:19: error: \'arc4random\' undeclared (first use in this function)\n   return ((int*)(&arc4random))[argc];\n                   ^\n/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c:8:19: note: each undeclared identifier is reported only once for each function it appears in\n" }), State { next_error: None })] 	CheckSymbolExists.c.o

Sounds valid? (search for error: in the log, there are a bunch)

sophiajt · 2017-04-10T22:36:03Z

@aidanhs - not sure how this would be related to the RLS stuff, though perhaps it's related to other infra fixes?

alexcrichton · 2017-04-10T22:42:02Z

Yes this is a valid error, but not related to sccache:

[00:45:06]     Finished release [optimized] target(s) in 15.34 secs

[00:45:06] Copying stage2 test from stage2 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / asmjs-unknown-emscripten)

[00:45:06] Dist std stage2 (x86_64-unknown-linux-gnu -> asmjs-unknown-emscripten)

[00:45:06] gen-installer: looking for programs

[00:45:06] gen-installer: 

[00:45:06] gen-installer: found tar

[00:45:06] gen-installer: found cp

[00:45:06] gen-installer: found rm

[00:45:06] gen-installer: found mkdir

[00:45:06] gen-installer: found echo

[00:45:06] gen-installer: found tr

[00:45:06] gen-installer: found awk

[00:45:06] gen-installer: 

[00:45:06] gen-installer: processing arguments

[00:45:06] gen-installer: 

[00:45:06] gen-installer: CFG_PRODUCT_NAME     := Rust 

[00:45:06] gen-installer: CFG_COMPONENT_NAME   := rust-std-asmjs-unknown-emscripten 

[00:45:06] gen-installer: CFG_PACKAGE_NAME     := rust-std-nightly-asmjs-unknown-emsc ...

[00:45:06] gen-installer: CFG_REL_MANIFEST_DIR := rustlib 

[00:45:06] gen-installer: CFG_SUCCESS_MESSAGE  := std-is-standing-at-the-ready. 

[00:45:06] gen-installer: CFG_LEGACY_MANIFEST_DIRS := rustlib,cargo 

[00:45:06] gen-installer: CFG_NON_INSTALLED_OVERLAY :=  

[00:45:06] gen-installer: CFG_BULK_DIRS        :=  

[00:45:06] gen-installer: CFG_IMAGE_DIR        := /checkout/obj/build/tmp/dist/rust-s ...

[00:45:06] gen-installer: CFG_WORK_DIR         := /checkout/obj/build/tmp/dist 

[00:45:06] gen-installer: CFG_OUTPUT_DIR       := /checkout/obj/build/dist 

[00:45:06] gen-installer: 

[00:45:06] gen-installer: validating arguments

[00:45:06] gen-installer: 

[00:45:06] gen-install-script: looking for install programs

[00:45:06] gen-install-script: 

[00:45:06] gen-install-script: found sed

[00:45:06] gen-install-script: found chmod

[00:45:06] gen-install-script: found cat

[00:45:06] gen-install-script: 

[00:45:06] gen-install-script: processing arguments

[00:45:06] gen-install-script: 

[00:45:06] gen-install-script: CFG_PRODUCT_NAME     := Rust 

[00:45:06] gen-install-script: CFG_REL_MANIFEST_DIR := rustlib 

[00:45:06] gen-install-script: CFG_SUCCESS_MESSAGE  := std-is-standing-at-the-ready. 

[00:45:06] gen-install-script: CFG_OUTPUT_SCRIPT    := /checkout/obj/build/tmp/dist/rust-s ...

[00:45:06] gen-install-script: CFG_LEGACY_MANIFEST_DIRS := rustlib,cargo 

[00:45:06] gen-install-script: 

[00:45:06] gen-install-script: validating arguments

[00:45:06] gen-install-script: 

[00:45:08] thread 'main' panicked at 'assertion failed: build.config.extended', /checkout/src/bootstrap/dist.rs:320

[00:45:08] note: Run with `RUST_BACKTRACE=1` for a backtrace.

[00:45:08] Build completed unsuccessfully in 0:44:05

I'll try to fix soon.

* Use the right version when building combined installer * Update dependencies of rls as it depends on rustc and plugins * Fix build-manifest and the versions it uses for the rls

alexcrichton · 2017-04-11T17:48:02Z

@bors: r+

bors · 2017-04-11T17:48:03Z

📌 Commit 13d008d has been approved by alexcrichton

bors · 2017-04-12T03:58:51Z

⌛ Testing commit 13d008d with merge 7803c08...

@brson

Add the RLS as a submodule and build a package out of it r? @brson (and cc @alexcrichton) Please review closely, I am not at all convinced I've done the right things here. I did run `x.py dist` and it makes an rls package which looks right to my eyes, but I haven't tested on non-linux platforms nor am I really sure what it should look like. This does not attempt to run tests for the RLS yet.

@brson

Add the RLS as a submodule and build a package out of it r? @brson (and cc @alexcrichton) Please review closely, I am not at all convinced I've done the right things here. I did run `x.py dist` and it makes an rls package which looks right to my eyes, but I haven't tested on non-linux platforms nor am I really sure what it should look like. This does not attempt to run tests for the RLS yet.

frewsxcv · 2017-04-12T04:07:30Z

@bors retry

Prioritizing the rollup which includes this

bors · 2017-04-12T05:26:15Z

⌛ Testing commit 13d008d with merge 8c6e2ff...

@brson

Add the RLS as a submodule and build a package out of it r? @brson (and cc @alexcrichton) Please review closely, I am not at all convinced I've done the right things here. I did run `x.py dist` and it makes an rls package which looks right to my eyes, but I haven't tested on non-linux platforms nor am I really sure what it should look like. This does not attempt to run tests for the RLS yet.

bors · 2017-04-12T08:02:54Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 8c6e2ff to master...

rust-highfive assigned brson Mar 16, 2017

nrc force-pushed the rls-submod branch from 62e872c to ff99073 Compare March 16, 2017 23:59

alexcrichton reviewed Mar 17, 2017

View reviewed changes

brson reviewed Mar 23, 2017

View reviewed changes

alexcrichton mentioned this pull request Mar 24, 2017

Overall cycle time has regressed ~25% from 2hrs to 2.5hrs #40804

Closed

nrc force-pushed the rls-submod branch from ff99073 to 6f06143 Compare March 27, 2017 19:03

This was referenced Apr 10, 2017

RLS should use the in-tree copy of Cargo #41198

Closed

RLS tests are not being run #41199

Closed

alexcrichton mentioned this pull request Apr 10, 2017

RLS is not present in all combined installers #41200

Closed

Touch up rls integration

13d008d

* Use the right version when building combined installer * Update dependencies of rls as it depends on rustc and plugins * Fix build-manifest and the versions it uses for the rls

alexcrichton force-pushed the rls-submod branch from 99656e9 to 13d008d Compare April 11, 2017 17:47

frewsxcv mentioned this pull request Apr 12, 2017

Rollup of 6 pull requests #41241

Closed

frewsxcv mentioned this pull request Apr 12, 2017

Rollup of 6 pull requests #41242

Closed

bors merged commit 13d008d into rust-lang:master Apr 12, 2017

erickt mentioned this pull request Apr 13, 2017

error: missing key: 'url' rust-lang/rustup#1062

Closed

o01eg mentioned this pull request Apr 13, 2017

dev-lang/rust: Fix configuration after removing --enable-save-analysis flag. gentoo/gentoo-rust#252

Merged

o01eg mentioned this pull request Apr 25, 2017

Cann't build dev-lang/rust-9999 with tools enabled. gentoo/gentoo-rust#255

Closed

Add the RLS as a submodule and build a package out of it #40584

Add the RLS as a submodule and build a package out of it #40584

Conversation

nrc commented Mar 16, 2017

alexcrichton commented Mar 17, 2017

alexcrichton Mar 17, 2017

Choose a reason for hiding this comment

alexcrichton commented Mar 17, 2017

bors commented Mar 21, 2017

brson commented Mar 23, 2017 • edited Loading

alexcrichton commented Mar 23, 2017

brson Mar 23, 2017

Choose a reason for hiding this comment

brson Mar 23, 2017

Choose a reason for hiding this comment

brson Mar 23, 2017

Choose a reason for hiding this comment

brson Mar 23, 2017

Choose a reason for hiding this comment

brson Mar 23, 2017

Choose a reason for hiding this comment

brson Mar 23, 2017

Choose a reason for hiding this comment

brson commented Mar 23, 2017 • edited Loading

nrc commented Mar 27, 2017

alexcrichton commented Mar 27, 2017

nrc commented Mar 27, 2017

brson commented Mar 29, 2017

bors commented Mar 29, 2017

bors commented Mar 29, 2017

bors commented Mar 29, 2017

nrc commented Mar 30, 2017

nrc commented Mar 30, 2017

bors commented Mar 30, 2017

bors commented Mar 30, 2017

alexcrichton commented Mar 30, 2017

alexcrichton commented Apr 10, 2017

bors commented Apr 10, 2017

bors commented Apr 10, 2017

bors commented Apr 10, 2017

aidanhs commented Apr 10, 2017 • edited Loading

sophiajt commented Apr 10, 2017

alexcrichton commented Apr 10, 2017

alexcrichton commented Apr 11, 2017

bors commented Apr 11, 2017

bors commented Apr 12, 2017

frewsxcv commented Apr 12, 2017

bors commented Apr 12, 2017

bors commented Apr 12, 2017

brson commented Mar 23, 2017 •

edited

Loading

brson commented Mar 23, 2017 •

edited

Loading

aidanhs commented Apr 10, 2017 •

edited

Loading