From 22cda959b334cc82b85745cd7af6e554d97efc9b Mon Sep 17 00:00:00 2001 From: YangzeLuo Date: Fri, 12 Jan 2024 13:35:53 +0800 Subject: [PATCH] fix: failed to infer OUT_DIR when workspace root contains symlink --- crates/project-model/src/build_scripts.rs | 19 ++++++-- crates/project-model/src/workspace.rs | 13 +++-- crates/rust-analyzer/src/reload.rs | 9 +++- crates/rust-analyzer/tests/slow-tests/main.rs | 47 ++++++++++++------- .../rust-analyzer/tests/slow-tests/support.rs | 16 ++++++- .../rust-analyzer/tests/slow-tests/testdir.rs | 37 ++++++++++++++- 6 files changed, 114 insertions(+), 27 deletions(-) diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs index 68cd40c040..c1670c2004 100644 --- a/crates/project-model/src/build_scripts.rs +++ b/crates/project-model/src/build_scripts.rs @@ -60,6 +60,7 @@ impl WorkspaceBuildScripts { fn build_command( config: &CargoConfig, allowed_features: &FxHashSet, + workspace_root: &AbsPathBuf, ) -> io::Result { let mut cmd = match config.run_build_script_command.as_deref() { Some([program, args @ ..]) => { @@ -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); } @@ -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, @@ -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); @@ -169,6 +181,7 @@ impl WorkspaceBuildScripts { config: &CargoConfig, workspaces: &[&CargoWorkspace], progress: &dyn Fn(String), + workspace_root: &AbsPathBuf, ) -> io::Result> { assert_eq!(config.invocation_strategy, InvocationStrategy::Once); @@ -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`. diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index c04eddc56f..e14090a972 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -407,6 +407,7 @@ impl ProjectWorkspace { workspaces: &[ProjectWorkspace], config: &CargoConfig, progress: &dyn Fn(String), + workspace_root: &AbsPathBuf, ) -> Vec> { if matches!(config.invocation_strategy, InvocationStrategy::PerWorkspace) || config.run_build_script_command.is_none() @@ -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() diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 8e3fa7fa4d..d90933b004 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -275,6 +275,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(); @@ -284,7 +286,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(); }); diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs index 78411e2d58..58a99cc447 100644 --- a/crates/rust-analyzer/tests/slow-tests/main.rs +++ b/crates/rust-analyzer/tests/slow-tests/main.rs @@ -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] @@ -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::(HoverParams { text_document_position_params: TextDocumentPositionParams::new( @@ -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(any(feature = "sysroot-abi", rust_analyzer))] fn resolve_proc_macro() { diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs index 106b99cb93..e16990eabd 100644 --- a/crates/rust-analyzer/tests/slow-tests/support.rs +++ b/crates/rust-analyzer/tests/slow-tests/support.rs @@ -23,6 +23,7 @@ pub(crate) struct Project<'a> { tmp_dir: Option, roots: Vec, config: serde_json::Value, + root_dir_contains_symlink: bool, } impl Project<'_> { @@ -45,6 +46,7 @@ impl Project<'_> { "enable": false, } }), + root_dir_contains_symlink: false, } } @@ -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) { @@ -74,7 +81,14 @@ 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(|| { + if self.root_dir_contains_symlink { + TestDir::new_symlink() + } else { + TestDir::new() + } + }); + static INIT: Once = Once::new(); INIT.call_once(|| { let filter: tracing_subscriber::filter::Targets = diff --git a/crates/rust-analyzer/tests/slow-tests/testdir.rs b/crates/rust-analyzer/tests/slow-tests/testdir.rs index f7fceb5888..45241ad63e 100644 --- a/crates/rust-analyzer/tests/slow-tests/testdir.rs +++ b/crates/rust-analyzer/tests/slow-tests/testdir.rs @@ -11,6 +11,14 @@ pub(crate) struct TestDir { impl TestDir { pub(crate) fn new() -> TestDir { + return TestDir::new_dir(false); + } + + pub(crate) fn new_symlink() -> TestDir { + return TestDir::new_dir(true); + } + + fn new_dir(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: @@ -33,10 +41,24 @@ 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 = base.join(format!("{pid}_{cnt}_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_dir(path, &symlink_path).unwrap(); + + return TestDir { path: symlink_path, keep: false }; + } + return TestDir { path, keep: false }; } panic!("Failed to create a temporary directory") } + #[allow(unused)] pub(crate) fn keep(mut self) -> TestDir { self.keep = true; @@ -52,9 +74,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()); + + 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() + ) + }) + } + remove_dir_all(&self.path).unwrap_or_else(|err| { panic!("failed to remove temporary directory {}: {err}", self.path.display()) - }) + }); } }