Skip to content

Commit

Permalink
fix: failed to infer OUT_DIR when workspace root contains symlink
Browse files Browse the repository at this point in the history
  • Loading branch information
wasd96040501 committed Nov 11, 2023
1 parent 11a87c9 commit 57b808e
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 30 deletions.
19 changes: 16 additions & 3 deletions crates/project-model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl WorkspaceBuildScripts {
fn build_command(
config: &CargoConfig,
allowed_features: &FxHashSet<String>,
workspace_root: &AbsPathBuf,
) -> io::Result<Command> {
let mut cmd = match config.run_build_script_command.as_deref() {
Some([program, args @ ..]) => {
Expand All @@ -73,6 +74,9 @@ impl WorkspaceBuildScripts {
cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
cmd.args(&config.extra_args);

cmd.arg("--manifest-path");
cmd.arg(workspace_root.join("Cargo.toml").as_os_str());

if let Some(target_dir) = &config.target_dir {
cmd.arg("--target-dir").arg(target_dir);
}
Expand Down Expand Up @@ -143,7 +147,11 @@ impl WorkspaceBuildScripts {
let allowed_features = workspace.workspace_features();

match Self::run_per_ws(
Self::build_command(config, &allowed_features)?,
Self::build_command(
config,
&allowed_features,
&workspace.workspace_root().to_path_buf(),
)?,
workspace,
current_dir,
progress,
Expand All @@ -153,7 +161,11 @@ impl WorkspaceBuildScripts {
{
// building build scripts failed, attempt to build with --keep-going so
// that we potentially get more build data
let mut cmd = Self::build_command(config, &allowed_features)?;
let mut cmd = Self::build_command(
config,
&allowed_features,
&workspace.workspace_root().to_path_buf(),
)?;
cmd.args(["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
let mut res = Self::run_per_ws(cmd, workspace, current_dir, progress)?;
res.error = Some(error);
Expand All @@ -169,6 +181,7 @@ impl WorkspaceBuildScripts {
config: &CargoConfig,
workspaces: &[&CargoWorkspace],
progress: &dyn Fn(String),
workspace_root: &AbsPathBuf,
) -> io::Result<Vec<WorkspaceBuildScripts>> {
assert_eq!(config.invocation_strategy, InvocationStrategy::Once);

Expand All @@ -181,7 +194,7 @@ impl WorkspaceBuildScripts {
))
}
};
let cmd = Self::build_command(config, &Default::default())?;
let cmd = Self::build_command(config, &Default::default(), workspace_root)?;
// NB: Cargo.toml could have been modified between `cargo metadata` and
// `cargo check`. We shouldn't assume that package ids we see here are
// exactly those from `config`.
Expand Down
13 changes: 8 additions & 5 deletions crates/project-model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ impl ProjectWorkspace {
workspaces: &[ProjectWorkspace],
config: &CargoConfig,
progress: &dyn Fn(String),
workspace_root: &AbsPathBuf,
) -> Vec<anyhow::Result<WorkspaceBuildScripts>> {
if matches!(config.invocation_strategy, InvocationStrategy::PerWorkspace)
|| config.run_build_script_command.is_none()
Expand All @@ -421,11 +422,13 @@ impl ProjectWorkspace {
_ => None,
})
.collect();
let outputs = &mut match WorkspaceBuildScripts::run_once(config, &cargo_ws, progress) {
Ok(it) => Ok(it.into_iter()),
// io::Error is not Clone?
Err(e) => Err(sync::Arc::new(e)),
};
let outputs =
&mut match WorkspaceBuildScripts::run_once(config, &cargo_ws, progress, workspace_root)
{
Ok(it) => Ok(it.into_iter()),
// io::Error is not Clone?
Err(e) => Err(sync::Arc::new(e)),
};

workspaces
.iter()
Expand Down
9 changes: 8 additions & 1 deletion crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ impl GlobalState {
tracing::info!(%cause, "will fetch build data");
let workspaces = Arc::clone(&self.workspaces);
let config = self.config.cargo();
let root_path = self.config.root_path().clone();

self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, move |sender| {
sender.send(Task::FetchBuildData(BuildDataProgress::Begin)).unwrap();

Expand All @@ -273,7 +275,12 @@ impl GlobalState {
sender.send(Task::FetchBuildData(BuildDataProgress::Report(msg))).unwrap()
}
};
let res = ProjectWorkspace::run_all_build_scripts(&workspaces, &config, &progress);
let res = ProjectWorkspace::run_all_build_scripts(
&workspaces,
&config,
&progress,
&root_path,
);

sender.send(Task::FetchBuildData(BuildDataProgress::End((workspaces, res)))).unwrap();
});
Expand Down
51 changes: 33 additions & 18 deletions crates/rust-analyzer/tests/slow-tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ fn test_missing_module_code_action_in_json_project() {
return;
}

let tmp_dir = TestDir::new();
let tmp_dir = TestDir::new(false);

let path = tmp_dir.path();

Expand Down Expand Up @@ -682,13 +682,12 @@ version = \"0.0.0\"
);
}

#[test]
fn out_dirs_check() {
fn out_dirs_check_impl(root_contains_symlink: bool) {
if skip_slow_tests() {
return;
}

let server = Project::with_fixture(
let mut server = Project::with_fixture(
r###"
//- /Cargo.toml
[package]
Expand Down Expand Up @@ -745,20 +744,26 @@ fn main() {
let another_str = include_str!("main.rs");
}
"###,
)
.with_config(serde_json::json!({
"cargo": {
"buildScripts": {
"enable": true
},
"sysroot": null,
"extraEnv": {
"RUSTC_BOOTSTRAP": "1"
);

if root_contains_symlink {
server = server.with_root_dir_contains_symlink();
}

let server = server
.with_config(serde_json::json!({
"cargo": {
"buildScripts": {
"enable": true
},
"sysroot": null,
"extraEnv": {
"RUSTC_BOOTSTRAP": "1"
}
}
}
}))
.server()
.wait_until_workspace_is_loaded();
}))
.server()
.wait_until_workspace_is_loaded();

let res = server.send_request::<HoverRequest>(HoverParams {
text_document_position_params: TextDocumentPositionParams::new(
Expand Down Expand Up @@ -831,6 +836,16 @@ fn main() {
);
}

#[test]
fn out_dirs_check() {
out_dirs_check_impl(false);
}

#[test]
fn root_contains_symlink_out_dirs_check() {
out_dirs_check_impl(true);
}

#[test]
#[cfg(feature = "sysroot-abi")]
fn resolve_proc_macro() {
Expand Down Expand Up @@ -963,7 +978,7 @@ fn test_will_rename_files_same_level() {
return;
}

let tmp_dir = TestDir::new();
let tmp_dir = TestDir::new(false);
let tmp_dir_path = tmp_dir.path().to_owned();
let tmp_dir_str = tmp_dir_path.to_str().unwrap();
let base_path = PathBuf::from(format!("file://{tmp_dir_str}"));
Expand Down
9 changes: 8 additions & 1 deletion crates/rust-analyzer/tests/slow-tests/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub(crate) struct Project<'a> {
tmp_dir: Option<TestDir>,
roots: Vec<PathBuf>,
config: serde_json::Value,
root_dir_contains_symlink: bool,
}

impl Project<'_> {
Expand All @@ -45,6 +46,7 @@ impl Project<'_> {
"enable": false,
}
}),
root_dir_contains_symlink: false,
}
}

Expand All @@ -58,6 +60,11 @@ impl Project<'_> {
self
}

pub(crate) fn with_root_dir_contains_symlink(mut self) -> Self {
self.root_dir_contains_symlink = true;
self
}

pub(crate) fn with_config(mut self, config: serde_json::Value) -> Self {
fn merge(dst: &mut serde_json::Value, src: serde_json::Value) {
match (dst, src) {
Expand All @@ -74,7 +81,7 @@ impl Project<'_> {
}

pub(crate) fn server(self) -> Server {
let tmp_dir = self.tmp_dir.unwrap_or_else(TestDir::new);
let tmp_dir = self.tmp_dir.unwrap_or_else(|| TestDir::new(self.root_dir_contains_symlink));
static INIT: Once = Once::new();
INIT.call_once(|| {
let filter: tracing_subscriber::filter::Targets =
Expand Down
30 changes: 28 additions & 2 deletions crates/rust-analyzer/tests/slow-tests/testdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub(crate) struct TestDir {
}

impl TestDir {
pub(crate) fn new() -> TestDir {
pub(crate) fn new(symlink: bool) -> TestDir {
let temp_dir = std::env::temp_dir();
// On MacOS builders on GitHub actions, the temp dir is a symlink, and
// that causes problems down the line. Specifically:
Expand All @@ -33,6 +33,19 @@ impl TestDir {
continue;
}
fs::create_dir_all(&path).unwrap();

#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]
if symlink {
let symlink_path = path.join("_symlink");
#[cfg(any(target_os = "macos", target_os = "linux"))]
std::os::unix::fs::symlink(path, &symlink_path).unwrap();

#[cfg(target_os = "windows")]
std::os::windows::fs::symlink_file(path, &symlink_path).unwrap();

return TestDir { path: symlink_path, keep: false };
}

return TestDir { path, keep: false };
}
panic!("Failed to create a temporary directory")
Expand All @@ -52,9 +65,22 @@ impl Drop for TestDir {
if self.keep {
return;
}

let filetype = fs::symlink_metadata(&self.path).unwrap().file_type();
let actual_path = filetype.is_symlink().then(|| fs::read_link(&self.path).unwrap());

remove_dir_all(&self.path).unwrap_or_else(|err| {
panic!("failed to remove temporary directory {}: {err}", self.path.display())
})
});

if let Some(actual_path) = actual_path {
remove_dir_all(&actual_path).unwrap_or_else(|err| {
panic!(
"failed to remove temporary link to directory {}: {err}",
actual_path.display()
)
})
}
}
}

Expand Down

0 comments on commit 57b808e

Please sign in to comment.