Auto merge of rust-lang#15868 - wasd96040501:fix/symlink2, r=Veykril

fix: failed to infer OUT_DIR when workspace root contains symlink

fix rust-lang#15867

Author: bors

crates/project-model/src/build_scripts.rs

@@ -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 @ ..]) => {
@@ -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<Vec<WorkspaceBuildScripts>> {
         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`.

crates/project-model/src/workspace.rs

@@ -413,6 +413,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()
@@ -427,11 +428,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()

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();
         });

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::<HoverRequest>(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() {

crates/rust-analyzer/tests/slow-tests/support.rs

@@ -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<'_> {
@@ -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 =

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())
-        })
+        });
     }
 }