Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Remove workspace mode, change some options #858

Merged
merged 3 commits into from
May 11, 2018
Merged

Remove workspace mode, change some options #858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bec14e8
Make build_lib, build_bin, and cfg_test unstable
nrc May 9, 2018
f0a3c7a
Remove non-workspace mode
nrc May 9, 2018
e96e9ed
Fixup tests
nrc May 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,6 @@ project.

Currently we accept the following options:

* `build_lib` (`bool`, defaults to `false`) checks the project as if you passed
the `--lib` argument to cargo. Mutually exclusive with, and preferred over,
`build_bin`.
* `build_bin` (`String`, defaults to `""`) checks the project as if you passed
`-- bin <build_bin>` argument to cargo. Mutually exclusive with `build_lib`.
* `cfg_test` (`bool`, defaults to `false`) checks the project as if you were
running `cargo test` rather than `cargo build`. I.e., compiles (but does not
run) test code.
* `unstable_features` (`bool`, defaults to `false`) enables unstable features.
Currently no option requires this flag.
* `sysroot` (`String`, defaults to `""`) if the given string is not empty, use
Expand All @@ -110,7 +102,7 @@ Currently we accept the following options:
the given target triple for all rustc invocations
* `wait_to_build` (`u64`, defaults to `1500`) time in milliseconds between
receiving a change notification and starting build
* `all_targets` (`bool`, defaults to `false`) checks the project as if you were
* `all_targets` (`bool`, defaults to `true`) checks the project as if you were
running `cargo check --all-targets`. I.e., check all targets and integration
tests too
* `use_crate_blacklist` (`bool`, defaults to `true`) if disabled, also indexes
Expand All @@ -129,6 +121,18 @@ Currently we accept the following options:
- `"opt-in"` Clippy lints are shown when crates specify `#![warn(clippy)]`.
- `"on"` Clippy lints enabled for all crates in workspace.

and the following unstable options:

* `build_lib` (`bool`, defaults to `false`) checks the project as if you passed
the `--lib` argument to cargo. Mutually exclusive with, and preferred over,
`build_bin`.
* `build_bin` (`String`, defaults to `""`) checks the project as if you passed
`-- bin <build_bin>` argument to cargo. Mutually exclusive with `build_lib`.
* `cfg_test` (`bool`, defaults to `false`) checks the project as if you were
running `cargo test` rather than `cargo build`. I.e., compiles (but does not
run) test code.


## Troubleshooting

For tips on debugging and troubleshooting, see [debugging.md](debugging.md).
Expand Down
169 changes: 34 additions & 135 deletions src/build/cargo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ use std::thread;

// Runs an in-process instance of Cargo.
pub(super) fn cargo(internals: &Internals, package_arg: PackageArg, progress_sender: Sender<ProgressUpdate>) -> BuildResult {
let workspace_mode = internals.config.lock().unwrap().workspace_mode;

let compilation_cx = internals.compilation_cx.clone();
let config = internals.config.clone();
let vfs = internals.vfs.clone();
Expand Down Expand Up @@ -73,7 +71,7 @@ pub(super) fn cargo(internals: &Internals, package_arg: PackageArg, progress_sen
.map_err(|_| failure::err_msg("thread panicked"))
.and_then(|res| res)
{
Ok(ref cwd) if workspace_mode => {
Ok(ref cwd) => {
let diagnostics = Arc::try_unwrap(diagnostics_clone)
.unwrap()
.into_inner()
Expand All @@ -84,7 +82,6 @@ pub(super) fn cargo(internals: &Internals, package_arg: PackageArg, progress_sen
.unwrap();
BuildResult::Success(cwd.clone(), diagnostics, analysis, true)
}
Ok(cwd) => BuildResult::Success(cwd, vec![], vec![], true),
Err(err) => {
// This message goes like this to the UI via showMessage. In VSCode
// this ends up on one single line, so it's important to keep it concise.
Expand Down Expand Up @@ -164,22 +161,9 @@ fn run_cargo(
let rustflags = prepare_cargo_rustflags(&rls_config);


if rls_config.workspace_mode {
for package in &packages {
if ws.members().find(|x| *x.name() == *package).is_none() {
warn!("cargo - couldn't find member package `{}` specified in `analyze_package` configuration", package);
}
}
} else {
// Warn about invalid specified bin target or package depending on current mode
// TODO: Return client notifications along with diagnostics to inform the user
let cur_pkg_targets = ws.current()?.targets();

if let Some(ref build_bin) = *rls_config.build_bin.as_ref() {
let mut bins = cur_pkg_targets.iter().filter(|x| x.is_bin());
if bins.find(|x| x.name() == build_bin).is_none() {
warn!("cargo - couldn't find binary `{}` specified in `build_bin` configuration", build_bin);
}
for package in &packages {
if ws.members().find(|x| *x.name() == *package).is_none() {
warn!("cargo - couldn't find member package `{}` specified in `analyze_package` configuration", package);
}
}

Expand Down Expand Up @@ -254,15 +238,13 @@ fn run_cargo(

struct RlsExecutor {
compilation_cx: Arc<Mutex<CompilationContext>>,
cur_package_id: Mutex<Option<PackageId>>,
config: Arc<Mutex<Config>>,
/// Because of the Cargo API design, we first acquire outer lock before creating the executor
/// and calling the compilation function. This, resulting, inner lock is used to synchronize
/// env var access during underlying `rustc()` calls during parallel `exec()` callback threads.
env_lock: environment::InnerLock,
vfs: Arc<Vfs>,
analysis: Arc<Mutex<Vec<Analysis>>>,
workspace_mode: bool,
/// Packages which are directly a member of the workspace, for which
/// analysis and diagnostics will be provided
member_packages: Mutex<HashSet<PackageId>>,
Expand All @@ -282,26 +264,14 @@ impl RlsExecutor {
analysis: Arc<Mutex<Vec<Analysis>>>,
progress_sender: Sender<ProgressUpdate>,
) -> RlsExecutor {
let workspace_mode = config.lock().unwrap().workspace_mode;
let (cur_package_id, member_packages) = if workspace_mode {
let member_packages = ws.members().map(|x| x.package_id().clone()).collect();
(None, member_packages)
} else {
let pkg_id = ws.current_opt()
.expect("No current package in Cargo")
.package_id()
.clone();
(Some(pkg_id), HashSet::new())
};
let member_packages = ws.members().map(|x| x.package_id().clone()).collect();

RlsExecutor {
compilation_cx,
cur_package_id: Mutex::new(cur_package_id),
config,
env_lock,
vfs,
analysis,
workspace_mode,
member_packages: Mutex::new(member_packages),
compiler_messages,
progress_sender: Mutex::new(progress_sender),
Expand All @@ -311,15 +281,7 @@ impl RlsExecutor {
/// Returns whether a given package is a primary one (every member of the
/// workspace is considered as such).
fn is_primary_crate(&self, id: &PackageId) -> bool {
if self.workspace_mode {
self.member_packages.lock().unwrap().contains(id)
} else {
let cur_package_id = self.cur_package_id.lock().unwrap();
id
== cur_package_id
.as_ref()
.expect("Executor has not been initialized")
}
self.member_packages.lock().unwrap().contains(id)
}
}

Expand All @@ -339,7 +301,7 @@ impl Executor for RlsExecutor {
}

fn force_rebuild(&self, unit: &Unit) -> bool {
// In workspace_mode we need to force rebuild every package in the
// We need to force rebuild every package in the
// workspace, even if it's not dirty at a time, to cache compiler
// invocations in the build plan.
// We only do a cargo build if we want to force rebuild the last
Expand Down Expand Up @@ -463,46 +425,13 @@ impl Executor for RlsExecutor {

{
let config = self.config.lock().unwrap();
let crate_type = parse_arg(cargo_args, "--crate-type");
// Because we only try to emulate `cargo test` using `cargo check`, so for now
// assume crate_type arg (i.e. in `cargo test` it isn't specified for --test targets)
// and build test harness only for final crate type
let crate_type = if config.all_targets || config.cfg_test {
// Crate type may be undefined when `all_targets` is true, for example for integration tests
crate_type.unwrap_or_else(|| "undefined".to_owned())
} else {
// Panic if crate type undefined for other cases
crate_type.expect("no crate-type in rustc command line")
};
let build_lib = *config.build_lib.as_ref();
let is_final_crate_type = crate_type == "bin" || (crate_type == "lib" && build_lib);

if config.sysroot.is_none() {
args.push("--sysroot".to_owned());
args.push(sysroot);
}

// We can't omit compilation here, because Cargo is going to expect to get
// dep-info for this crate, so we shell out to rustc to get that.
// This is not really ideal, because we are going to
// compute this info anyway when we run rustc ourselves, but we don't do
// that before we return to Cargo.
// FIXME Don't do this. Start our build here rather than on another thread
// so the dep-info is ready by the time we return from this callback.
// NB: In `workspace_mode` regular compilation is performed here (and we don't
// only calculate dep-info) so it should fix the problem mentioned above.
let modified = args.iter()
.map(|a| {
// Emitting only dep-info is possible only for final crate type, as
// as others may emit required metadata for dependent crate types
if a.starts_with("--emit") && is_final_crate_type && !self.workspace_mode {
"--emit=dep-info"
} else {
a
}
})
.collect::<Vec<_>>();
cmd.args_replace(&modified);
cmd.args_replace(&args);
}

// Cache executed command for the build plan
Expand All @@ -524,30 +453,26 @@ impl Executor for RlsExecutor {
compilation_cx.cwd = cargo_cmd.get_cwd().map(|p| p.to_path_buf());
}

if self.workspace_mode {
let build_dir = {
let cx = self.compilation_cx.lock().unwrap();
cx.build_dir.clone().unwrap()
};
let build_dir = {
let cx = self.compilation_cx.lock().unwrap();
cx.build_dir.clone().unwrap()
};

if let BuildResult::Success(_, mut messages, mut analysis, success) = super::rustc::rustc(
&self.vfs,
&args,
&envs,
cargo_cmd.get_cwd(),
&build_dir,
Arc::clone(&self.config),
&self.env_lock.as_facade(),
) {
self.compiler_messages.lock().unwrap().append(&mut messages);
self.analysis.lock().unwrap().append(&mut analysis);

if !success {
return Err(format_err!("Build error"));
}
if let BuildResult::Success(_, mut messages, mut analysis, success) = super::rustc::rustc(
&self.vfs,
&args,
&envs,
cargo_cmd.get_cwd(),
&build_dir,
Arc::clone(&self.config),
&self.env_lock.as_facade(),
) {
self.compiler_messages.lock().unwrap().append(&mut messages);
self.analysis.lock().unwrap().append(&mut analysis);

if !success {
return Err(format_err!("Build error"));
}
} else {
cmd.exec()?;
}

Ok(())
Expand Down Expand Up @@ -585,40 +510,14 @@ impl Default for CargoOptions {

impl CargoOptions {
fn new(config: &Config) -> CargoOptions {
if config.workspace_mode {
CargoOptions {
target: config.target.clone(),
features: config.features.clone(),
all_features: config.all_features,
no_default_features: config.no_default_features,
jobs: config.jobs,
all_targets: config.all_targets,
..CargoOptions::default()
}
} else {
// In single-crate mode we currently support only one crate target,
// and if lib is set, then we ignore bin target config
let (lib, bin) = if *config.build_lib.as_ref() {
(true, vec![])
} else {
let bin = match *config.build_bin.as_ref() {
Some(ref bin) => vec![bin.clone()],
None => vec![],
};
(false, bin)
};

CargoOptions {
lib,
bin,
target: config.target.clone(),
features: config.features.clone(),
all_features: config.all_features,
no_default_features: config.no_default_features,
jobs: config.jobs,
all_targets: config.all_targets,
..CargoOptions::default()
}
CargoOptions {
target: config.target.clone(),
features: config.features.clone(),
all_features: config.all_features,
no_default_features: config.no_default_features,
jobs: config.jobs,
all_targets: config.all_targets,
..CargoOptions::default()
}
}
}
Expand Down
65 changes: 20 additions & 45 deletions src/build/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,53 +534,28 @@ impl Internals {

// Don't hold this lock when we run Cargo.
let needs_to_run_cargo = self.compilation_cx.lock().unwrap().args.is_empty();
let workspace_mode = self.config.lock().unwrap().workspace_mode;

if workspace_mode {
// If the build plan has already been cached, use it, unless Cargo
// has to be specifically rerun (e.g. when build scripts changed)
let work = {
let modified: Vec<_> = self.dirty_files.lock().unwrap().keys().cloned().collect();
let mut cx = self.compilation_cx.lock().unwrap();
let manifest_path = important_paths::find_root_manifest_for_wd(cx.build_dir.as_ref().unwrap());
let manifest_path = match manifest_path {
Ok(mp) => mp,
Err(e) => {
let msg = format!("Error reading manifest path: {:?}", e);
return BuildResult::Err(msg, None);
}
};
cx.build_plan.prepare_work(&manifest_path, &modified, needs_to_run_cargo)
};
return match work {
// In workspace_mode, cargo performs the full build and returns
// appropriate diagnostics/analysis data
WorkStatus::NeedsCargo(package_arg) => cargo::cargo(self, package_arg, progress_sender),
WorkStatus::Execute(job_queue) => job_queue.execute(self, progress_sender),

// If the build plan has already been cached, use it, unless Cargo
// has to be specifically rerun (e.g. when build scripts changed)
let work = {
let modified: Vec<_> = self.dirty_files.lock().unwrap().keys().cloned().collect();
let mut cx = self.compilation_cx.lock().unwrap();
let manifest_path = important_paths::find_root_manifest_for_wd(cx.build_dir.as_ref().unwrap());
let manifest_path = match manifest_path {
Ok(mp) => mp,
Err(e) => {
let msg = format!("Error reading manifest path: {:?}", e);
return BuildResult::Err(msg, None);
}
};
// In single package mode Cargo needs to be run to cache args/envs for
// future rustc calls
} else if needs_to_run_cargo {
if let e @ BuildResult::Err(..) = cargo::cargo(self, PackageArg::Unknown, progress_sender) {
return e;
}
cx.build_plan.prepare_work(&manifest_path, &modified, needs_to_run_cargo)
};
match work {
// Cargo performs the full build and returns
// appropriate diagnostics/analysis data
WorkStatus::NeedsCargo(package_arg) => cargo::cargo(self, package_arg, progress_sender),
WorkStatus::Execute(job_queue) => job_queue.execute(self, progress_sender),
}

let compile_cx = self.compilation_cx.lock().unwrap();
let args = &compile_cx.args;
assert!(!args.is_empty());
let envs = &compile_cx.envs;
let build_dir = compile_cx.build_dir.as_ref().unwrap();
let env_lock = self.env_lock.as_facade();
rustc::rustc(
&self.vfs,
args,
envs,
compile_cx.cwd.as_ref().map(|x| &**x),
build_dir,
Arc::clone(&self.config),
&env_lock,
)
}
}

Expand Down
Loading