Fixed toolchain version drift, improved error handling, config loadin…

…g and refactoring

This commit contains quite a lot if changes. See them all in the PR description #217

cargo-marker/Cargo.toml

@@ -10,15 +10,17 @@ license     = "MIT OR Apache-2.0"
 repository  = "https://github.com/rust-marker/marker"
 
 # Crate names in Rust are fun. I reserved `cargo_marker` as a crate name. However,
-# Cargo requires it's subcommands to use a dash like `cargo-marker`. Unable to rename
+# Cargo requires it's subcommands to use a dash like `cargo-marker`. Unable to
 # rename the create on crates.io we now have this hack... At least it works
 [[bin]]
 name = "cargo-marker"
 path = "src/main.rs"
 
 [dependencies]
+camino         = { version = "1.1", features = ["serde1"] }
 cargo_metadata = "0.15.4"
 clap           = { version = "4.0", features = ["string", "derive"] }
 once_cell      = "1.17.0"
 serde          = { version = "1.0", features = ["derive"] }
-toml           = { version = "0.7" }
+serde_json     = "1.0"
+toml           = "0.7"

cargo-marker/src/backend.rs

@@ -14,6 +14,7 @@ use crate::{config::LintDependencyEntry, ExitStatus};
 
 use self::{lints::LintCrate, toolchain::Toolchain};
 
+pub mod cargo;
 pub mod driver;
 pub mod lints;
 pub mod toolchain;
@@ -37,8 +38,6 @@ pub struct Config {
     pub build_rustc_flags: String,
     /// Indicates if this is a release or debug build.
     pub debug_build: bool,
-    /// Indicates if this is a development build.
-    pub dev_build: bool,
     pub toolchain: Toolchain,
 }
 
@@ -49,7 +48,6 @@ impl Config {
             lints: HashMap::default(),
             build_rustc_flags: String::new(),
             debug_build: false,
-            dev_build: cfg!(feature = "dev-build"),
             toolchain,
         })
     }
@@ -78,7 +76,7 @@ pub fn prepare_check(config: &Config) -> Result<CheckInfo, ExitStatus> {
         ("RUSTC_WORKSPACE_WRAPPER", config.toolchain.driver_path.as_os_str().to_os_string()),
         ("MARKER_LINT_CRATES", to_marker_lint_crates_env(&lints)),
     ];
-    if let Some(toolchain) = &config.toolchain.toolchain {
+    if let Some(toolchain) = &config.toolchain.cargo.toolchain {
         env.push(("RUSTUP_TOOLCHAIN", toolchain.into()));
     }
 

cargo-marker/src/backend/cargo.rs

@@ -0,0 +1,80 @@
+use std::process::Command;
+
+use camino::Utf8PathBuf;
+use cargo_metadata::MetadataCommand;
+use serde::Deserialize;
+
+use crate::ExitStatus;
+
+#[derive(Debug, Default)]
+pub struct Cargo {
+    /// The rustc toolchain this driver belongs to. This can be `None` during
+    /// execution commands such as `cargo locate-project`
+    pub(crate) toolchain: Option<String>,
+}
+
+#[derive(Deserialize, Debug)]
+struct ProjectLocation {
+    root: Utf8PathBuf,
+}
+
+impl Cargo {
+    pub fn with_toolchain(toolchain: impl Into<String>) -> Self {
+        Self {
+            toolchain: Some(toolchain.into()),
+        }
+    }
+
+    /// This returns a command calling rustup, which acts as a proxy for the
+    /// Cargo of the selected toolchain.
+    /// It may add additional flags for verbose output.
+    ///
+    /// See also [`super::toolchain::Toolchain::cargo_build_command`] if the
+    /// commands is intended to build a crate.
+    pub fn command(&self) -> Command {
+        // Marker requires rustc's shared libraries to be available. These are
+        // added by rustup, when it acts like a proxy for cargo/rustc/etc.
+        // This also means that cargo needs to be invoked via rustup, to have
+        // the libraries available when Marker is invoked. This is such a mess...
+        // All of this would be so, so much simpler if marker was part of rustup :/
+        if let Some(toolchain) = &self.toolchain {
+            let mut cmd = Command::new("rustup");
+            cmd.args(["run", toolchain, "cargo"]);
+            cmd
+        } else {
+            // for cargo locate-project - this command can be run without
+            // specified toolchain
+            Command::new("cargo")
+        }
+    }
+
+    pub fn cargo_locate_project(&self) -> Result<Utf8PathBuf, ExitStatus> {
+        let mut cmd = self.command();
+
+        let output = cmd
+            .arg("locate-project")
+            .arg("--workspace")
+            .output()
+            .map_err(|err| ExitStatus::fatal(err, "error locating workspace manifest Cargo.toml"))?;
+
+        if !output.status.success() {
+            let stderr = String::from_utf8_lossy(&output.stderr);
+            return Err(ExitStatus::fatal(
+                stderr.trim(),
+                format!("cargo locate-project failed with {}", output.status),
+            ));
+        }
+
+        let manifest_location: ProjectLocation = serde_json::from_slice(&output.stdout)
+            .map_err(|err| ExitStatus::fatal(err, "failed to deserialize cargo locate-project output"))?;
+
+        Ok(manifest_location.root)
+    }
+
+    // Keep self for future changes. It's implemented in such way that clippy
+    // doesn't ask to write it as an associative function.
+    #[allow(clippy::unused_self)]
+    pub fn metadata(&self) -> MetadataCommand {
+        MetadataCommand::new()
+    }
+}

cargo-marker/src/backend/driver.rs

@@ -2,7 +2,7 @@ use std::{path::Path, process::Command, str::from_utf8};
 
 use once_cell::sync::Lazy;
 
-use crate::ExitStatus;
+use crate::{utils::is_local_driver, ExitStatus};
 
 use super::toolchain::{get_toolchain_folder, rustup_which, Toolchain};
 
@@ -125,7 +125,7 @@ fn install_toolchain(toolchain: &str) -> Result<(), ExitStatus> {
 
 /// This tries to compile the driver.
 fn build_driver(toolchain: &str, version: &str, additional_rustc_flags: &str) -> Result<(), ExitStatus> {
-    if cfg!(debug_assertions) {
+    if is_local_driver() {
         println!("Compiling rustc driver");
     } else {
         println!("Compiling rustc driver v{version} with {toolchain}");
@@ -135,7 +135,7 @@ fn build_driver(toolchain: &str, version: &str, additional_rustc_flags: &str) ->
 
     // Build driver
     let mut cmd = Command::new("cargo");
-    if cfg!(debug_assertions) {
+    if is_local_driver() {
         cmd.args(["build", "--bin", "marker_rustc_driver"]);
     } else {
         cmd.env("RUSTUP_TOOLCHAIN", toolchain);

cargo-marker/src/backend/lints/fetch.rs

@@ -114,7 +114,7 @@ const DUMMY_MAIN_CONTENT: &str = r#"
 "#;
 
 fn call_cargo_fetch(manifest: &Path, config: &Config) -> Result<(), ExitStatus> {
-    let mut cmd = config.toolchain.cargo_command();
+    let mut cmd = config.toolchain.cargo.command();
     cmd.arg("fetch");
     cmd.arg("--manifest-path");
     cmd.arg(manifest.as_os_str());
@@ -141,7 +141,7 @@ fn call_cargo_fetch(manifest: &Path, config: &Config) -> Result<(), ExitStatus>
 }
 
 fn call_cargo_metadata(manifest: &PathBuf, config: &Config) -> Result<Metadata, ExitStatus> {
-    let res = config.toolchain.cargo_metadata_command().manifest_path(manifest).exec();
+    let res = config.toolchain.cargo.metadata().manifest_path(manifest).exec();
 
     // FIXME(xFrednet): Handle errors properly.
     res.map_err(|_| ExitStatus::LintCrateFetchFailed)