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

target_dir of cargo doesn't seem to be used. #1154

Closed
Stargateur opened this issue Nov 29, 2018 · 1 comment
Closed

target_dir of cargo doesn't seem to be used. #1154

Stargateur opened this issue Nov 29, 2018 · 1 comment

Comments

@Stargateur
Copy link

According to this pull request #793, RLS should use cargo target_dir config and join it with rls. However, this doesn't work, I have a .cargo/config in my project, cargo correctly use my custom target_dir, but RLS seem to ignore it and build into target/rls. Maybe this feature only work for cargo config that are more global ? (in the home for exemple).

Also, I found that the behavior of target-dir is ambigus, in one way we could suppose that it's only for the root directory target by default. But it's seem to be for the full path target/rls. That confuse with the pull request that claim use default from cargo.

Maybe, it would be better to split this into two variables (target-dir-cargo, target-dir), or maybe use a totally different directory target for cargo and just rls for RLS ? Currently, I think the behavior is a little odd, and also not documented.

I ask this cause this trouble me when using ide-rust rust-lang/atom-ide-rust#110.

@alexheretic
Copy link
Member

Yep the code seems to want to infer cargo-target-dir + "rls", but this didn't work when tested.

Code:

rls/src/config.rs

Lines 245 to 280 in f024cdb

pub fn infer_defaults(&mut self, project_dir: &Path) -> CargoResult<()> {
// Note that this may not be equal build_dir when inside a workspace member
let manifest_path = important_paths::find_root_manifest_for_wd(project_dir)?;
trace!("root manifest_path: {:?}", &manifest_path);
let shell = Shell::from_write(Box::new(sink()));
let cwd = env::current_dir().expect("failed to get cwd");
let config = CargoConfig::new(shell, cwd, homedir(project_dir).unwrap());
let ws = Workspace::new(&manifest_path, &config)?;
// Constructing a `Workspace` also probes the filesystem and detects where to place the
// build artifacts. We need to rely on Cargo's behaviour directly not to possibly place our
// own artifacts somewhere else (e.g. when analyzing only a single crate in a workspace)
match self.target_dir {
// We require an absolute path, so adjust a relative one if it's passed.
Inferrable::Specified(Some(ref mut path)) if path.is_relative() => {
*path = project_dir.join(&path);
}
_ => {}
}
if self.target_dir.as_ref().is_none() {
let target_dir = ws.target_dir().into_path_unlocked();
let target_dir = target_dir.join("rls");
self.target_dir.infer(Some(target_dir));
trace!(
"For project path {:?} Cargo told us to use this target/ dir: {:?}",
project_dir,
self.target_dir.as_ref().as_ref().unwrap(),
);
}
Ok(())
}
}

We should investigate and probably add a test for this.

@nrc nrc closed this as completed in #1167 Dec 4, 2018
Xanewok added a commit to Xanewok/rust that referenced this issue Jan 3, 2019
Beta backport of a fix that already was backported to stable,
see rust-lang#56726 and
rust-lang/rls#1170 for the underlying RLS issue.

Also includes the fix for rust-lang/rls#1154
(respecting target-dir specified in .cargo/config for RLS artifacts).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants