Skip to content

Commit

Permalink
Auto merge of #1005 - brson:rls, r=Diggsey
Browse files Browse the repository at this point in the history
RLS support

I think this is sufficient for basic support.

With this PR, rustup will create 'rls' proxies in CARGO_HOME/bin, and will do so lazily when that binary doesn't exist.

After this PR one can write

```
rustup component add rls
rustup component add rust-src
rustup component add rust-analysis
```

to get a working RLS, assuming the 'rls' package is deployed.

Next steps are to make `rustup component` accept multiple components, so RLS can be installed with just `rustup compnent add rls rust-src rust-analysis` (cc @durka).

I'd suggest that RLS have nice error handling for cases where rust-src and rust-analysis aren't available. It might suggest running the proper rustup commands.

rustup probably needs to emit a better error message when somebody tries to run `rls` without installing it. Since the proxy always exists, it will say something like "the binary 'rls' doesn't exist in toolchain X". It would be better for rustup to know which package contains 'rls' and say so.

cc @nrc @jonathandturner
  • Loading branch information
bors committed Mar 27, 2017
2 parents 2ae338d + 1c9f146 commit 4e0c2a4
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 5 deletions.
6 changes: 2 additions & 4 deletions src/rustup-cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ tools, but otherwise, install the C++ build tools before proceeding.
"#;

static TOOLS: &'static [&'static str]
= &["rustc", "rustdoc", "cargo", "rust-lldb", "rust-gdb"];
= &["rustc", "rustdoc", "cargo", "rust-lldb", "rust-gdb", "rls"];

static UPDATE_ROOT: &'static str
= "https://static.rust-lang.org/rustup";
Expand Down Expand Up @@ -594,9 +594,7 @@ fn install_bins() -> Result<()> {
// like Android, does not support hardlinks, so we fallback to symlinks.
for tool in TOOLS {
let ref tool_path = bin_path.join(&format!("{}{}", tool, EXE_SUFFIX));
if utils::hardlink_file(rustup_path, tool_path).is_err() {
try!(utils::symlink_file(rustup_path, tool_path))
}
try!(utils::hard_or_symlink_file(rustup_path, tool_path));
}

Ok(())
Expand Down
23 changes: 22 additions & 1 deletion src/rustup-mock/src/clitools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pub fn setup(s: Scenario, f: &Fn(&Config)) {
let setup_path = config.exedir.join(format!("rustup-init{}", EXE_SUFFIX));
let rustc_path = config.exedir.join(format!("rustc{}", EXE_SUFFIX));
let cargo_path = config.exedir.join(format!("cargo{}", EXE_SUFFIX));
let rls_path = config.exedir.join(format!("rls{}", EXE_SUFFIX));

// Don't copy an executable via `fs::copy` on Unix because that'll require
// opening up the destination for writing. If one thread in our process then
Expand All @@ -119,6 +120,7 @@ pub fn setup(s: Scenario, f: &Fn(&Config)) {
fs::hard_link(rustup_path, setup_path).unwrap();
fs::hard_link(rustup_path, rustc_path).unwrap();
fs::hard_link(rustup_path, cargo_path).unwrap();
fs::hard_link(rustup_path, rls_path).unwrap();

// Make sure the host triple matches the build triple. Otherwise testing a 32-bit build of
// rustup on a 64-bit machine will fail, because the tests do not have the host detection
Expand Down Expand Up @@ -290,7 +292,7 @@ pub fn run(config: &Config, name: &str, args: &[&str], env: &[(&str, &str)]) ->
for env in env {
cmd.env(env.0, env.1);
}
let out = cmd.output().unwrap();
let out = cmd.output().expect("failed to run test command");

SanitizedOutput {
ok: out.status.success(),
Expand Down Expand Up @@ -365,6 +367,7 @@ fn build_mock_channel(s: Scenario, channel: &str, date: &str,
let std = build_mock_std_installer(host_triple);
let rustc = build_mock_rustc_installer(host_triple, version, version_hash);
let cargo = build_mock_cargo_installer(version, version_hash);
let rls = build_mock_rls_installer(version, version_hash);
let rust_docs = build_mock_rust_doc_installer();
let rust = build_combined_installer(&[&std, &rustc, &cargo, &rust_docs]);
let cross_std1 = build_mock_cross_std_installer(CROSS_ARCH1, date);
Expand All @@ -378,6 +381,7 @@ fn build_mock_channel(s: Scenario, channel: &str, date: &str,
(cross_std2, CROSS_ARCH2.to_string())]),
("rustc", vec![(rustc, host_triple.clone())]),
("cargo", vec![(cargo, host_triple.clone())]),
("rls", vec![(rls, host_triple.clone())]),
("rust-docs", vec![(rust_docs, host_triple.clone())]),
("rust-src", vec![(rust_src, "*".to_string())]),
("rust", vec![(rust, host_triple.clone())])];
Expand All @@ -386,6 +390,7 @@ fn build_mock_channel(s: Scenario, channel: &str, date: &str,
let std = build_mock_std_installer(MULTI_ARCH1);
let rustc = build_mock_rustc_installer(MULTI_ARCH1, version, version_hash);
let cargo = build_mock_cargo_installer(version, version_hash);
let rls = build_mock_rls_installer(version, version_hash);
let rust_docs = build_mock_rust_doc_installer();
let rust = build_combined_installer(&[&std, &rustc, &cargo, &rust_docs]);
let cross_std1 = build_mock_cross_std_installer(CROSS_ARCH1, date);
Expand All @@ -398,6 +403,7 @@ fn build_mock_channel(s: Scenario, channel: &str, date: &str,
(cross_std2, CROSS_ARCH2.to_string())]),
("rustc", vec![(rustc, triple.clone())]),
("cargo", vec![(cargo, triple.clone())]),
("rls", vec![(rls, triple.clone())]),
("rust-docs", vec![(rust_docs, triple.clone())]),
("rust-src", vec![(rust_src, "*".to_string())]),
("rust", vec![(rust, triple.clone())])];
Expand Down Expand Up @@ -445,6 +451,10 @@ fn build_mock_channel(s: Scenario, channel: &str, date: &str,
name: "rust-docs".to_string(),
target: target.to_string()
});
target_pkg.extensions.push(MockComponent {
name: "rls".to_string(),
target: target.to_string()
});
target_pkg.extensions.push(MockComponent {
name: "rust-std".to_string(),
target: CROSS_ARCH1.to_string(),
Expand Down Expand Up @@ -544,6 +554,17 @@ fn build_mock_cargo_installer(version: &str, version_hash: &str) -> MockInstalle
}
}

fn build_mock_rls_installer(version: &str, version_hash: &str) -> MockInstallerBuilder {
let cargo = format!("bin/rls{}", EXE_SUFFIX);
MockInstallerBuilder {
components: vec![
("rls".to_string(),
vec![MockCommand::File(cargo.clone())],
vec![(cargo, mock_bin("rls", version, version_hash))])
]
}
}

fn build_mock_rust_doc_installer() -> MockInstallerBuilder {
MockInstallerBuilder {
components: vec![
Expand Down
7 changes: 7 additions & 0 deletions src/rustup-utils/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,13 @@ pub fn symlink_dir(src: &Path, dest: &Path, notify_handler: &Fn(Notification)) -
})
}

pub fn hard_or_symlink_file(src: &Path, dest: &Path) -> Result<()> {
if hardlink_file(src, dest).is_err() {
symlink_file(src, dest)?;
}
Ok(())
}

pub fn hardlink_file(src: &Path, dest: &Path) -> Result<()> {
raw::hardlink(src, dest).chain_err(|| {
ErrorKind::LinkingFile {
Expand Down
23 changes: 23 additions & 0 deletions tests/cli-misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use time::Duration;
use std::ops::Sub;
use std::ops::Add;
use std::time::Duration as StdDuration;
use std::env::consts::EXE_SUFFIX;

macro_rules! for_host { ($s: expr) => (&format!($s, this_host_triple())) }

Expand Down Expand Up @@ -405,3 +406,25 @@ fn telemetry_cleanup_removes_old_files() {
assert_eq!(count, 100);
});
}

#[test]
fn rls_exists_in_toolchain() {
setup(&|config| {
expect_ok(config, &["rustup", "default", "stable"]);
expect_ok(config, &["rustup", "component", "add", "rls"]);
assert!(config.exedir.join(format!("rls{}", EXE_SUFFIX)).exists());
expect_ok(config, &["rls", "--version"]);
});
}

#[test]
fn rls_does_not_exist_in_toolchain() {
setup(&|config| {
// FIXME: If rls exists in the toolchain, this should suggest a command
// to run to install it
expect_ok(config, &["rustup", "default", "stable"]);
expect_err(config, &["rls", "--version"],
&format!("toolchain 'stable-{}' does not have the binary `rls{}`",
this_host_triple(), EXE_SUFFIX));
});
}
23 changes: 23 additions & 0 deletions tests/cli-self-upd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1158,3 +1158,26 @@ fn uninstall_removes_legacy_home_symlink() {
assert!(!multirust_dir.exists());
});
}

#[test]
fn rls_proxy_set_up_after_install() {
setup(&|config| {
expect_ok(config, &["rustup-init", "-y"]);
expect_err(config, &["rls", "--version"],
&format!("toolchain 'stable-{}' does not have the binary `rls{}`",
this_host_triple(), EXE_SUFFIX));
expect_ok(config, &["rustup", "component", "add", "rls"]);
expect_ok(config, &["rls", "--version"]);
});
}

#[test]
fn rls_proxy_set_up_after_update() {
update_setup(&|config, _| {
let ref rls_path = config.cargodir.join(format!("bin/rls{}", EXE_SUFFIX));
expect_ok(config, &["rustup-init", "-y"]);
fs::remove_file(rls_path).unwrap();
expect_ok(config, &["rustup", "self", "update"]);
assert!(rls_path.exists());
});
}

0 comments on commit 4e0c2a4

Please sign in to comment.