Merge #168

168: Cargo: Remove `cargo_fetch` dependency and rework the architecture r=xFrednet a=xFrednet

This PR exploded a bit 😅. Removing the dependency required a few more changes than expected. The good news are, that marker now uses a stable way to fetch lint crates which also supports registries directly. No more `cargo` dependency, just command calls. I'm happy :D

It also addresses or at least touches several other issues. Generally speaking, a nice update IMO. (The only big negative thing is the error handling. I've never learned how to do that properly, and cargo is getting more messy with less information after each refactoring 😅. I'll create an issue for that :))

---

Closes: #167
Closes: #155
Closes: #87

CC: #60

Co-authored-by: xFrednet <[email protected]>

Author: bors[bot]

Cargo.lock

@@ -11,4 +11,5 @@ members = [
 resolver = "2"
 
 [workspace.metadata.marker.lints]
-marker_uitest = { path = "marker_uitest" }
+marker_lints = { path = "./marker_lints" }
+marker_uitest = { path = "./marker_uitest" }

Cargo.toml

@@ -16,11 +16,11 @@ name = "cargo-marker"
 path = "src/main.rs"
 
 [dependencies]
-clap = { version = "4.0.26", features = ["string"] }
+clap = { version = "4.0", features = ["string"] }
 serde = { version = "1.0", features = ["derive"] }
-toml = { version = "0.7.3" }
+toml = { version = "0.7" }
 once_cell = "1.17.0"
-cargo_fetch = "0.1.2"
+cargo_metadata = "0.15.4"
 
 [features]
 default = []

cargo-marker/Cargo.toml

@@ -46,16 +46,16 @@ cargo marker setup --auto-install-toolchain
 
 Marker requires lint crates to be specified. The best way, is to add them to the `Cargo.toml` file, like this:
 
-```sh
+```toml
 [workspace.metadata.marker.lints]
-# Add a local crate as a path
-local_lint_crate = { path = "path/to/lint_crate" }
-# Add an external crate via git
-git_lint_crate = { git = "https://github.com/rust-marker/marker" }
+# A local crate as a path
+marker_lints = { path = "./marker_lints" }
+# An external crate via git
+marker_lints = { git = "https://github.com/rust-marker/marker" }
+# An external crate from a registry
+marker_lints = "0.1.0"
 ```
 
-Lints from registries, like crates.io, are sadly not yet supported. See [rust-marker/marker#87](https://github.com/rust-marker/marker/issues/87).
-
 ### Running Marker
 
 Running Marker is as simple as running its sibling *[Clippy]*. Navigate to your Rust project directory and run the following command:

cargo-marker/README.md

@@ -0,0 +1,117 @@
+//! The backend is the brains of rust-marker, it's responsible for installing or
+//! finding the correct driver, building lints and start linting. The backend should
+//! be decoupled from the frontend. Most of the time the frontend will be the
+//! `cargo-marker` CLI. However, `cargo-marker` might also be used as a library for UI
+//! tests later down the line.
+
+use std::{
+    collections::HashMap,
+    ffi::{OsStr, OsString},
+    path::PathBuf,
+};
+
+use crate::{config::LintDependencyEntry, ExitStatus};
+
+use self::{lints::LintCrate, toolchain::Toolchain};
+
+pub mod driver;
+pub mod lints;
+pub mod toolchain;
+
+/// Markers configuration for any action that requires lint crates to be available.
+///
+/// It's assumed that all paths in this struct are absolute paths.
+#[derive(Debug)]
+pub struct Config {
+    /// The base directory used by Marker to fetch and compile lints.
+    /// This will default to something like `./target/marker`.
+    ///
+    /// This should generally be used as a base path for everything. Notable
+    /// exceptions can be the installation of a driver or the compilation of
+    /// a lint for uitests.
+    pub marker_dir: PathBuf,
+    /// The list of lints.
+    pub lints: HashMap<String, LintDependencyEntry>,
+    /// Additional flags, which should be passed to rustc during the compilation
+    /// of crates.
+    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,
+}
+
+impl Config {
+    pub fn try_base_from(toolchain: Toolchain) -> Result<Self, ExitStatus> {
+        Ok(Self {
+            marker_dir: toolchain.find_target_dir()?.join("marker"),
+            lints: HashMap::default(),
+            build_rustc_flags: String::new(),
+            debug_build: false,
+            dev_build: cfg!(feature = "dev-build"),
+            toolchain,
+        })
+    }
+
+    fn markers_target_dir(&self) -> PathBuf {
+        self.marker_dir.join("target")
+    }
+
+    fn lint_crate_dir(&self) -> PathBuf {
+        self.marker_dir.join("lints")
+    }
+}
+
+/// This struct contains all information to use rustc as a driver.
+pub struct CheckInfo {
+    pub env: Vec<(&'static str, OsString)>,
+}
+
+pub fn prepare_check(config: &Config) -> Result<CheckInfo, ExitStatus> {
+    println!();
+    println!("Compiling Lints:");
+    let lints = lints::build_lints(config)?;
+
+    #[rustfmt::skip]
+    let mut env = vec![
+        ("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 {
+        env.push(("RUSTUP_TOOLCHAIN", toolchain.into()));
+    }
+
+    Ok(CheckInfo { env })
+}
+
+pub fn run_check(config: &Config, info: CheckInfo, additional_cargo_args: &[String]) -> Result<(), ExitStatus> {
+    println!();
+    println!("Start linting:");
+
+    let mut cmd = config.toolchain.cargo_with_driver();
+    cmd.arg("check");
+    cmd.args(additional_cargo_args);
+
+    cmd.envs(info.env);
+
+    let exit_status = cmd
+        .spawn()
+        .expect("could not run cargo")
+        .wait()
+        .expect("failed to wait for cargo?");
+
+    if exit_status.success() {
+        Ok(())
+    } else {
+        Err(ExitStatus::MarkerCheckFailed)
+    }
+}
+
+pub fn to_marker_lint_crates_env(lints: &[LintCrate]) -> OsString {
+    let lint_paths: Vec<_> = lints
+        .iter()
+        .map(|krate| OsString::from(krate.file.as_os_str()))
+        .collect();
+    lint_paths.join(OsStr::new(";"))
+}

cargo-marker/src/backend.rs

@@ -0,0 +1,167 @@
+use std::{path::Path, process::Command, str::from_utf8};
+
+use once_cell::sync::Lazy;
+
+use crate::ExitStatus;
+
+use super::toolchain::{get_toolchain_folder, rustup_which, Toolchain};
+
+#[cfg(unix)]
+pub const MARKER_DRIVER_BIN_NAME: &str = "marker_rustc_driver";
+#[cfg(windows)]
+pub const MARKER_DRIVER_BIN_NAME: &str = "marker_rustc_driver.exe";
+
+/// This is the driver version and toolchain, that is used by the setup command
+/// to install the driver.
+pub static DEFAULT_DRIVER_INFO: Lazy<DriverVersionInfo> = Lazy::new(|| DriverVersionInfo {
+    toolchain: "nightly-2023-06-01".to_string(),
+    version: "0.1.0".to_string(),
+    api_version: "0.1.0".to_string(),
+});
+
+/// The version info of one specific driver
+pub struct DriverVersionInfo {
+    pub toolchain: String,
+    pub version: String,
+    pub api_version: String,
+}
+
+impl DriverVersionInfo {
+    pub fn try_from_toolchain(toolchain: &Toolchain, manifest: &Path) -> Result<DriverVersionInfo, ExitStatus> {
+        // The driver has to be invoked via cargo, to ensure that the libraries
+        // are correctly linked. Toolchains are truly fun...
+        if let Ok(output) = toolchain
+            .cargo_with_driver()
+            .arg("rustc")
+            .arg("--quiet")
+            .arg("--manifest-path")
+            .arg(manifest.as_os_str())
+            .arg("--")
+            .arg("--toolchain")
+            .output()
+        {
+            if !output.status.success() {
+                return Err(ExitStatus::DriverFailed);
+            }
+
+            if let Ok(info) = from_utf8(&output.stdout) {
+                let mut toolchain = Err(ExitStatus::InvalidValue);
+                let mut driver_version = Err(ExitStatus::InvalidValue);
+                let mut api_version = Err(ExitStatus::InvalidValue);
+                for line in info.lines() {
+                    if let Some(value) = line.strip_prefix("toolchain: ") {
+                        toolchain = Ok(value.trim().to_string());
+                    } else if let Some(value) = line.strip_prefix("driver: ") {
+                        driver_version = Ok(value.trim().to_string());
+                    } else if let Some(value) = line.strip_prefix("marker-api: ") {
+                        api_version = Ok(value.trim().to_string());
+                    }
+                }
+
+                return Ok(DriverVersionInfo {
+                    toolchain: toolchain?,
+                    version: driver_version?,
+                    api_version: api_version?,
+                });
+            }
+        }
+
+        Err(ExitStatus::DriverFailed)
+    }
+}
+
+/// This tries to install the rustc driver specified in [`DEFAULT_DRIVER_INFO`].
+pub fn install_driver(
+    auto_install_toolchain: bool,
+    dev_build: bool,
+    additional_rustc_flags: &str,
+) -> Result<(), ExitStatus> {
+    // The toolchain, driver version and api version should ideally be configurable.
+    // However, that will require more prototyping and has a low priority rn.
+    // See #60
+
+    // Prerequisites
+    let toolchain = &DEFAULT_DRIVER_INFO.toolchain;
+    if rustup_which(toolchain, "cargo", false).is_err() {
+        if auto_install_toolchain {
+            install_toolchain(toolchain)?;
+        } else {
+            eprintln!("Error: The required toolchain `{toolchain}` can't be found");
+            eprintln!();
+            eprintln!("You can install the toolchain by running: `rustup toolchain install {toolchain}`");
+            eprintln!("Or by adding the `--auto-install-toolchain` flag");
+            return Err(ExitStatus::InvalidToolchain);
+        }
+    }
+
+    build_driver(
+        toolchain,
+        &DEFAULT_DRIVER_INFO.version,
+        dev_build,
+        additional_rustc_flags,
+    )
+}
+
+fn install_toolchain(toolchain: &str) -> Result<(), ExitStatus> {
+    let mut cmd = Command::new("rustup");
+
+    cmd.args(["toolchain", "install", toolchain]);
+
+    let status = cmd
+        .spawn()
+        .expect("unable to start rustup to install the toolchain")
+        .wait()
+        .expect("unable to wait on rustup to install the toolchain");
+    if status.success() {
+        Ok(())
+    } else {
+        // The user can see rustup's output, as the command output was passed on
+        // to the user via the `.spawn()` call.
+        Err(ExitStatus::InvalidToolchain)
+    }
+}
+
+/// This tries to compile the driver.
+fn build_driver(
+    toolchain: &str,
+    version: &str,
+    dev_build: bool,
+    additional_rustc_flags: &str,
+) -> Result<(), ExitStatus> {
+    if dev_build {
+        println!("Compiling rustc driver");
+    } else {
+        println!("Compiling rustc driver v{version} with {toolchain}");
+    }
+
+    let mut rustc_flags = additional_rustc_flags.to_string();
+
+    // Build driver
+    let mut cmd = Command::new("cargo");
+    if dev_build {
+        cmd.args(["build", "--bin", "marker_rustc_driver"]);
+    } else {
+        cmd.env("RUSTUP_TOOLCHAIN", toolchain);
+        cmd.args(["install", "marker_rustc_driver", "--version", version]);
+        rustc_flags += " --cap-lints=allow";
+
+        let install_root = get_toolchain_folder(toolchain)?;
+        cmd.arg("--root");
+        cmd.arg(install_root.as_os_str());
+        cmd.arg("--no-track");
+    }
+    cmd.env("RUSTFLAGS", rustc_flags);
+
+    let status = cmd
+        .spawn()
+        .expect("unable to start cargo install for the driver")
+        .wait()
+        .expect("unable to wait on cargo install for the driver");
+    if status.success() {
+        Ok(())
+    } else {
+        // The user can see cargo's output, as the command output was passed on
+        // to the user via the `.spawn()` call.
+        Err(ExitStatus::DriverInstallationFailed)
+    }
+}

cargo-marker/src/backend/driver.rs

@@ -0,0 +1,38 @@
+use std::path::PathBuf;
+
+use crate::ExitStatus;
+
+use super::Config;
+
+mod build;
+mod fetch;
+
+/// This struct contains all information of a lint crate required to compile
+/// the crate. See the [fetch] module for how external crates are fetched and
+/// this info is retrieved.
+#[derive(Debug)]
+#[allow(unused)]
+pub struct LintCrateSource {
+    /// The name of the package, for now we can assume that this is the name
+    /// that will be used to construct the dynamic library.
+    name: String,
+    /// The absolute path to the manifest of this lint crate
+    manifest: PathBuf,
+}
+
+/// The information of a compiled lint crate.
+#[derive(Debug)]
+pub struct LintCrate {
+    /// The absolute path of the compiled crate, as a dynamic library.
+    pub file: PathBuf,
+}
+
+/// This function fetches and builds all lints specified in the given [`Config`]
+pub fn build_lints(config: &Config) -> Result<Vec<LintCrate>, ExitStatus> {
+    // FIXME(xFrednet): Potentially handle local crates compiled for UI tests
+    // differently. Like running the build command in the project root. This
+    // would allow cargo to cache the compilation better. Right now normal
+    // Cargo and cargo-marker might invalidate each others caches.
+    let sources = fetch::fetch_crates(config)?;
+    build::build_lints(&sources, config)
+}