Merge pull request #20315 from Veykril/push-pvmslwwouzzx

internal: Fix lockfile temp dir usage and use it for build scripts as well

Author: Veykril

crates/base-db/src/input.rs

@@ -30,7 +30,8 @@ pub type ProcMacroPaths =
 pub enum ProcMacroLoadingError {
     Disabled,
     FailedToBuild,
-    MissingDylibPath,
+    ExpectedProcMacroArtifact,
+    MissingDylibPath(Box<[String]>),
     NotYetBuilt,
     NoProcMacros,
     ProcMacroSrvError(Box<str>),
@@ -39,8 +40,9 @@ impl ProcMacroLoadingError {
     pub fn is_hard_error(&self) -> bool {
         match self {
             ProcMacroLoadingError::Disabled | ProcMacroLoadingError::NotYetBuilt => false,
-            ProcMacroLoadingError::FailedToBuild
-            | ProcMacroLoadingError::MissingDylibPath
+            ProcMacroLoadingError::ExpectedProcMacroArtifact
+            | ProcMacroLoadingError::FailedToBuild
+            | ProcMacroLoadingError::MissingDylibPath(_)
             | ProcMacroLoadingError::NoProcMacros
             | ProcMacroLoadingError::ProcMacroSrvError(_) => true,
         }
@@ -51,10 +53,23 @@ impl Error for ProcMacroLoadingError {}
 impl fmt::Display for ProcMacroLoadingError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
+            ProcMacroLoadingError::ExpectedProcMacroArtifact => {
+                write!(f, "proc-macro crate did not build proc-macro artifact")
+            }
             ProcMacroLoadingError::Disabled => write!(f, "proc-macro expansion is disabled"),
             ProcMacroLoadingError::FailedToBuild => write!(f, "proc-macro failed to build"),
-            ProcMacroLoadingError::MissingDylibPath => {
-                write!(f, "proc-macro crate build data is missing a dylib path")
+            ProcMacroLoadingError::MissingDylibPath(candidates) if candidates.is_empty() => {
+                write!(
+                    f,
+                    "proc-macro crate built but the dylib path is missing, this indicates a problem with your build system."
+                )
+            }
+            ProcMacroLoadingError::MissingDylibPath(candidates) => {
+                write!(
+                    f,
+                    "proc-macro crate built but the dylib path is missing, this indicates a problem with your build system. Candidates not considered due to not having a dynamic library extension: {}",
+                    candidates.join(", ")
+                )
             }
             ProcMacroLoadingError::NotYetBuilt => write!(f, "proc-macro not yet built"),
             ProcMacroLoadingError::NoProcMacros => {

crates/project-model/src/build_dependencies.rs

@@ -16,11 +16,13 @@ use la_arena::ArenaMap;
 use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
 use rustc_hash::{FxHashMap, FxHashSet};
 use serde::Deserialize as _;
+use stdx::never;
 use toolchain::Tool;
 
 use crate::{
     CargoConfig, CargoFeatures, CargoWorkspace, InvocationStrategy, ManifestPath, Package, Sysroot,
-    TargetKind, utf8_stdout,
+    TargetKind, cargo_config_file::make_lockfile_copy,
+    cargo_workspace::MINIMUM_TOOLCHAIN_VERSION_SUPPORTING_LOCKFILE_PATH, utf8_stdout,
 };
 
 /// Output of the build script and proc-macro building steps for a workspace.
@@ -30,6 +32,15 @@ pub struct WorkspaceBuildScripts {
     error: Option<String>,
 }
 
+#[derive(Debug, Clone, Default, PartialEq, Eq)]
+pub enum ProcMacroDylibPath {
+    Path(AbsPathBuf),
+    DylibNotFound(Box<[Utf8PathBuf]>),
+    NotProcMacro,
+    #[default]
+    NotBuilt,
+}
+
 /// Output of the build script and proc-macro building step for a concrete package.
 #[derive(Debug, Clone, Default, PartialEq, Eq)]
 pub(crate) struct BuildScriptOutput {
@@ -43,15 +54,15 @@ pub(crate) struct BuildScriptOutput {
     /// Directory where a build script might place its output.
     pub(crate) out_dir: Option<AbsPathBuf>,
     /// Path to the proc-macro library file if this package exposes proc-macros.
-    pub(crate) proc_macro_dylib_path: Option<AbsPathBuf>,
+    pub(crate) proc_macro_dylib_path: ProcMacroDylibPath,
 }
 
 impl BuildScriptOutput {
     fn is_empty(&self) -> bool {
         self.cfgs.is_empty()
             && self.envs.is_empty()
             && self.out_dir.is_none()
-            && self.proc_macro_dylib_path.is_none()
+            && self.proc_macro_dylib_path == ProcMacroDylibPath::NotBuilt
     }
 }
 
@@ -67,7 +78,7 @@ impl WorkspaceBuildScripts {
         let current_dir = workspace.workspace_root();
 
         let allowed_features = workspace.workspace_features();
-        let cmd = Self::build_command(
+        let (_guard, cmd) = Self::build_command(
             config,
             &allowed_features,
             workspace.manifest_path(),
@@ -88,7 +99,7 @@ impl WorkspaceBuildScripts {
     ) -> io::Result<Vec<WorkspaceBuildScripts>> {
         assert_eq!(config.invocation_strategy, InvocationStrategy::Once);
 
-        let cmd = Self::build_command(
+        let (_guard, cmd) = Self::build_command(
             config,
             &Default::default(),
             // This is not gonna be used anyways, so just construct a dummy here
@@ -126,6 +137,8 @@ impl WorkspaceBuildScripts {
             |package, cb| {
                 if let Some(&(package, workspace)) = by_id.get(package) {
                     cb(&workspaces[workspace][package].name, &mut res[workspace].outputs[package]);
+                } else {
+                    never!("Received compiler message for unknown package: {}", package);
                 }
             },
             progress,
@@ -140,12 +153,9 @@ impl WorkspaceBuildScripts {
         if tracing::enabled!(tracing::Level::INFO) {
             for (idx, workspace) in workspaces.iter().enumerate() {
                 for package in workspace.packages() {
-                    let package_build_data = &mut res[idx].outputs[package];
+                    let package_build_data: &mut BuildScriptOutput = &mut res[idx].outputs[package];
                     if !package_build_data.is_empty() {
-                        tracing::info!(
-                            "{}: {package_build_data:?}",
-                            workspace[package].manifest.parent(),
-                        );
+                        tracing::info!("{}: {package_build_data:?}", workspace[package].manifest,);
                     }
                 }
             }
@@ -198,39 +208,58 @@ impl WorkspaceBuildScripts {
                         let path = dir_entry.path();
                         let extension = path.extension()?;
                         if extension == std::env::consts::DLL_EXTENSION {
-                            let name = path.file_stem()?.to_str()?.split_once('-')?.0.to_owned();
-                            let path = AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).ok()?)
-                                .ok()?;
-                            return Some((name, path));
+                            let name = path
+                                .file_stem()?
+                                .to_str()?
+                                .split_once('-')?
+                                .0
+                                .trim_start_matches("lib")
+                                .to_owned();
+                            let path = match Utf8PathBuf::from_path_buf(path) {
+                                Ok(path) => path,
+                                Err(path) => {
+                                    tracing::warn!(
+                                        "Proc-macro dylib path contains non-UTF8 characters: {:?}",
+                                        path.display()
+                                    );
+                                    return None;
+                                }
+                            };
+                            return match AbsPathBuf::try_from(path) {
+                                Ok(path) => Some((name, path)),
+                                Err(path) => {
+                                    tracing::error!(
+                                        "proc-macro dylib path is not absolute: {:?}",
+                                        path
+                                    );
+                                    None
+                                }
+                            };
                         }
                     }
                     None
                 })
                 .collect();
             for p in rustc.packages() {
                 let package = &rustc[p];
-                if package
-                    .targets
-                    .iter()
-                    .any(|&it| matches!(rustc[it].kind, TargetKind::Lib { is_proc_macro: true }))
-                {
-                    if let Some((_, path)) = proc_macro_dylibs
-                        .iter()
-                        .find(|(name, _)| *name.trim_start_matches("lib") == package.name)
-                    {
-                        bs.outputs[p].proc_macro_dylib_path = Some(path.clone());
+                bs.outputs[p].proc_macro_dylib_path =
+                    if package.targets.iter().any(|&it| {
+                        matches!(rustc[it].kind, TargetKind::Lib { is_proc_macro: true })
+                    }) {
+                        match proc_macro_dylibs.iter().find(|(name, _)| *name == package.name) {
+                            Some((_, path)) => ProcMacroDylibPath::Path(path.clone()),
+                            _ => ProcMacroDylibPath::DylibNotFound(Box::default()),
+                        }
+                    } else {
+                        ProcMacroDylibPath::NotProcMacro
                     }
-                }
             }
 
             if tracing::enabled!(tracing::Level::INFO) {
                 for package in rustc.packages() {
                     let package_build_data = &bs.outputs[package];
                     if !package_build_data.is_empty() {
-                        tracing::info!(
-                            "{}: {package_build_data:?}",
-                            rustc[package].manifest.parent(),
-                        );
+                        tracing::info!("{}: {package_build_data:?}", rustc[package].manifest,);
                     }
                 }
             }
@@ -263,6 +292,12 @@ impl WorkspaceBuildScripts {
             |package, cb| {
                 if let Some(&package) = by_id.get(package) {
                     cb(&workspace[package].name, &mut outputs[package]);
+                } else {
+                    never!(
+                        "Received compiler message for unknown package: {}\n {}",
+                        package,
+                        by_id.keys().join(", ")
+                    );
                 }
             },
             progress,
@@ -272,10 +307,7 @@ impl WorkspaceBuildScripts {
             for package in workspace.packages() {
                 let package_build_data = &outputs[package];
                 if !package_build_data.is_empty() {
-                    tracing::info!(
-                        "{}: {package_build_data:?}",
-                        workspace[package].manifest.parent(),
-                    );
+                    tracing::info!("{}: {package_build_data:?}", workspace[package].manifest,);
                 }
             }
         }
@@ -348,15 +380,21 @@ impl WorkspaceBuildScripts {
                             progress(format!(
                                 "building compile-time-deps: proc-macro {name} built"
                             ));
+                            if data.proc_macro_dylib_path == ProcMacroDylibPath::NotBuilt {
+                                data.proc_macro_dylib_path = ProcMacroDylibPath::NotProcMacro;
+                            }
                             if message.target.kind.contains(&cargo_metadata::TargetKind::ProcMacro)
                             {
-                                // Skip rmeta file
-                                if let Some(filename) =
-                                    message.filenames.iter().find(|file| is_dylib(file))
-                                {
-                                    let filename = AbsPath::assert(filename);
-                                    data.proc_macro_dylib_path = Some(filename.to_owned());
-                                }
+                                data.proc_macro_dylib_path =
+                                    match message.filenames.iter().find(|file| is_dylib(file)) {
+                                        Some(filename) => {
+                                            let filename = AbsPath::assert(filename);
+                                            ProcMacroDylibPath::Path(filename.to_owned())
+                                        }
+                                        None => ProcMacroDylibPath::DylibNotFound(
+                                            message.filenames.clone().into_boxed_slice(),
+                                        ),
+                                    };
                             }
                         });
                     }
@@ -393,14 +431,15 @@ impl WorkspaceBuildScripts {
         current_dir: &AbsPath,
         sysroot: &Sysroot,
         toolchain: Option<&semver::Version>,
-    ) -> io::Result<Command> {
+    ) -> io::Result<(Option<temp_dir::TempDir>, Command)> {
         match config.run_build_script_command.as_deref() {
             Some([program, args @ ..]) => {
                 let mut cmd = toolchain::command(program, current_dir, &config.extra_env);
                 cmd.args(args);
-                Ok(cmd)
+                Ok((None, cmd))
             }
             _ => {
+                let mut requires_unstable_options = false;
                 let mut cmd = sysroot.tool(Tool::Cargo, current_dir, &config.extra_env);
 
                 cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
@@ -416,7 +455,19 @@ impl WorkspaceBuildScripts {
                 if let Some(target) = &config.target {
                     cmd.args(["--target", target]);
                 }
-
+                let mut temp_dir_guard = None;
+                if toolchain
+                    .is_some_and(|v| *v >= MINIMUM_TOOLCHAIN_VERSION_SUPPORTING_LOCKFILE_PATH)
+                {
+                    let lockfile_path =
+                        <_ as AsRef<Utf8Path>>::as_ref(manifest_path).with_extension("lock");
+                    if let Some((temp_dir, target_lockfile)) = make_lockfile_copy(&lockfile_path) {
+                        temp_dir_guard = Some(temp_dir);
+                        cmd.arg("--lockfile-path");
+                        cmd.arg(target_lockfile.as_str());
+                        requires_unstable_options = true;
+                    }
+                }
                 match &config.features {
                     CargoFeatures::All => {
                         cmd.arg("--all-features");
@@ -438,6 +489,7 @@ impl WorkspaceBuildScripts {
                 }
 
                 if manifest_path.is_rust_manifest() {
+                    requires_unstable_options = true;
                     cmd.arg("-Zscript");
                 }
 
@@ -447,7 +499,7 @@ impl WorkspaceBuildScripts {
                 // available in current toolchain's cargo, use it to build compile time deps only.
                 const COMP_TIME_DEPS_MIN_TOOLCHAIN_VERSION: semver::Version = semver::Version {
                     major: 1,
-                    minor: 89,
+                    minor: 189,
                     patch: 0,
                     pre: semver::Prerelease::EMPTY,
                     build: semver::BuildMetadata::EMPTY,
@@ -457,8 +509,7 @@ impl WorkspaceBuildScripts {
                     toolchain.is_some_and(|v| *v >= COMP_TIME_DEPS_MIN_TOOLCHAIN_VERSION);
 
                 if cargo_comp_time_deps_available {
-                    cmd.env("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS", "nightly");
-                    cmd.arg("-Zunstable-options");
+                    requires_unstable_options = true;
                     cmd.arg("--compile-time-deps");
                     // we can pass this unconditionally, because we won't actually build the
                     // binaries, and as such, this will succeed even on targets without libtest
@@ -481,7 +532,11 @@ impl WorkspaceBuildScripts {
                         cmd.env("RA_RUSTC_WRAPPER", "1");
                     }
                 }
-                Ok(cmd)
+                if requires_unstable_options {
+                    cmd.env("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS", "nightly");
+                    cmd.arg("-Zunstable-options");
+                }
+                Ok((temp_dir_guard, cmd))
             }
         }
     }

crates/project-model/src/cargo_config_file.rs

@@ -1,4 +1,5 @@
 //! Read `.cargo/config.toml` as a JSON object
+use paths::{Utf8Path, Utf8PathBuf};
 use rustc_hash::FxHashMap;
 use toolchain::Tool;
 
@@ -32,3 +33,24 @@ pub(crate) fn read(
 
     Some(json)
 }
+
+pub(crate) fn make_lockfile_copy(
+    lockfile_path: &Utf8Path,
+) -> Option<(temp_dir::TempDir, Utf8PathBuf)> {
+    let temp_dir = temp_dir::TempDir::with_prefix("rust-analyzer").ok()?;
+    let target_lockfile = temp_dir.path().join("Cargo.lock").try_into().ok()?;
+    match std::fs::copy(lockfile_path, &target_lockfile) {
+        Ok(_) => {
+            tracing::debug!("Copied lock file from `{}` to `{}`", lockfile_path, target_lockfile);
+            Some((temp_dir, target_lockfile))
+        }
+        // lockfile does not yet exist, so we can just create a new one in the temp dir
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => Some((temp_dir, target_lockfile)),
+        Err(e) => {
+            tracing::warn!(
+                "Failed to copy lock file from `{lockfile_path}` to `{target_lockfile}`: {e}",
+            );
+            None
+        }
+    }
+}