Merge #66

66: `cargo-marker` refactoring and review r=xFrednet a=xFrednet

I've refactored parts of cargo-marker and reviewed the implementation. I found several limitations which are now documented in #60. This PR already fixes a few of these.


CC: #60

---

Same as always: If I don't get a review, I'll merge this, when a depending PR is ready. :)


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

Author: bors[bot]

.github/workflows/rust.yml

@@ -66,6 +66,6 @@ jobs:
     - run: rustc -vV
 
     # Test
-    - run: cargo build --package cargo-marker
+    - run: cargo build --package cargo_marker
     - run: cargo build --package marker_api
     - run: cargo build --package marker_lints

.github/workflows/rust_bors.yml

@@ -55,6 +55,6 @@ jobs:
     - run: rustc -vV
 
     # Test
-    - run: cargo build --package cargo-marker
+    - run: cargo build --package cargo_marker
     - run: cargo build --package marker_api
     - run: cargo build --package marker_lints

Cargo.lock

@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2022-2022 Rust-linting
+Copyright (c) 2022-2022 Rust-marker
 
 Permission is hereby granted, free of charge, to any
 person obtaining a copy of this software and associated

LICENSE-MIT

@@ -12,9 +12,9 @@ The project is currently only available in this GitHub repo. For a quick test, c
 
 ## License
 
-Copyright (c) 2022-2022 Rust-linting
+Copyright (c) 2022-2022 Rust-marker
 
-Rust-linting is distributed under the terms of both the MIT license
+Rust-marker is distributed under the terms of both the MIT license
 and the Apache License (Version 2.0).
 
 See [LICENSE-APACHE](./LICENSE-APACHE), [LICENSE-MIT](./LICENSE-MIT).

README.md

@@ -1,15 +1,22 @@
 [package]
-name = "cargo-marker"
+name = "cargo_marker"
 version = "0.1.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 
+# 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
+# rename the create on crates.io we now have this hack... At least it works
+[[bin]]
+name = "cargo-marker"
+path = "src/main.rs"
+
 [dependencies]
-clap = "4.0.26"
+clap = {version = "4.0.26", features = ["string"]}
 once_cell = "1.16.0"
 
 [features]
 default = []
-# Indicates that development features like auto driver building etc should be enabled.
-# This option assumes that it's being executed at the project root.
+# This enables developer features used to automatically build the local version
+# assuming, that it's being executed at the root of the repo.
 dev-build = []

cargo-marker/Cargo.toml

@@ -0,0 +1,63 @@
+use clap::{builder::ValueParser, Arg, ArgAction, Command};
+
+use crate::VERSION;
+
+const AFTER_HELP_MSG: &str = r#"CARGO ARGS
+    All arguments after double dashes(`--`) will be passed to cargo.
+    These options are the same as for `cargo check`.
+
+EXAMPLES:
+    * `cargo marker -l ./marker_lints`
+"#;
+
+pub fn get_clap_config() -> Command {
+    Command::new(VERSION)
+        .arg(
+            Arg::new("version")
+                .short('V')
+                .long("version")
+                .action(ArgAction::SetTrue)
+                .help("Print version info and exit"),
+        )
+        .arg(
+            Arg::new("verbose")
+                .short('v')
+                .long("verbose")
+                .action(ArgAction::SetTrue)
+                .help("Print additional debug information to the console"),
+        )
+        .arg(
+            Arg::new("test-setup")
+                .long("test-setup")
+                .action(ArgAction::SetTrue)
+                .help("This flag will compile the lint crate and print all relevant environment values"),
+        )
+        .subcommand(setup_command())
+        .subcommand(check_command())
+        .args(check_command_args())
+        .after_help(AFTER_HELP_MSG)
+        .override_usage("cargo-marker [OPTIONS] -- <CARGO ARGS>")
+}
+
+fn setup_command() -> Command {
+    Command::new("setup")
+        .about("A collection of commands to setup marker")
+        .after_help("By default this will install the driver for rustc.")
+}
+
+fn check_command() -> Command {
+    Command::new("check")
+        .about("Run marker on a local package")
+        .args(check_command_args())
+}
+
+fn check_command_args() -> impl IntoIterator<Item = impl Into<Arg>> {
+    vec![
+        Arg::new("lints")
+            .short('l')
+            .long("lints")
+            .num_args(1..)
+            .value_parser(ValueParser::os_string())
+            .help("Defines a set of lints crates that should be used"),
+    ]
+}

cargo-marker/src/cli.rs

@@ -0,0 +1,186 @@
+//! This module hosts functions required to run rustc as a driver.
+//!
+//! The rustc driver depends on rustc, which interfaces is unstable. This means
+//! that each driver version is bound to a specific version of rustc. The same
+//! goes for Clippy. However, Clippy has the advantage, that it's distributes via
+//! rustup, which handles the version matching for it. We're not so lucky, at
+//! least not yet. Therefore, we're responsible that the driver is compiled and
+//! run with the correct toolchain.
+//!
+//! If no driver is installed, the user will be requested to run the setup command.
+//! That command will first ensure that the required toolchain is installed and then
+//! run `cargo install` for the driver with a specific toolchain. The version and
+//! toolchain are hardcoded in this crate.
+
+use std::{ffi::OsString, path::PathBuf, process::Command};
+
+use once_cell::sync::Lazy;
+
+use crate::ExitStatus;
+
+/// This is the driver version and toolchain, that is used by the setup command
+/// to install the driver.
+static DEFAULT_DRIVER_INFO: Lazy<RustcDriverInfo> = Lazy::new(|| RustcDriverInfo {
+    toolchain: "nightly-2022-11-03".to_string(),
+    version: "0.1.0".to_string(),
+    api_version: "0.1.0".to_string(),
+});
+
+struct RustcDriverInfo {
+    toolchain: String,
+    version: String,
+    #[allow(unused)]
+    api_version: String,
+}
+
+pub fn print_driver_version() {
+    println!(
+        "rustc driver version: {} (toolchain: {}, api: {})",
+        DEFAULT_DRIVER_INFO.version, DEFAULT_DRIVER_INFO.toolchain, DEFAULT_DRIVER_INFO.api_version
+    );
+}
+
+/// This tries to install the rustc driver specified in [`DEFAULT_DRIVER_INFO`].
+pub fn install_driver(verbose: bool) -> 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;
+    check_toolchain(toolchain)?;
+
+    build_driver(
+        toolchain,
+        &DEFAULT_DRIVER_INFO.version,
+        verbose,
+        cfg!(feature = "dev-build"),
+    )?;
+
+    // We don't want to advice the user, to install the driver again.
+    check_driver(verbose, false)
+}
+
+/// This function checks if the specified toolchain is installed. This requires
+/// rustup. A dependency we have to live with for now.
+fn check_toolchain(toolchain: &str) -> Result<(), ExitStatus> {
+    let mut cmd = Command::new("cargo");
+    cmd.args([&format!("+{toolchain}"), "-V"]);
+    if cmd.output().is_err() {
+        eprintln!("Error: The required toolchain `{toolchain}` can't be found");
+        eprintln!();
+        eprintln!("You can install the toolchain by running: rustup toolchain install {toolchain}");
+        Err(ExitStatus::InvalidToolchain)
+    } else {
+        Ok(())
+    }
+}
+
+/// This tries to compile the driver. If successful the driver binary will
+/// be places next to the executable of `cargo-linter`.
+fn build_driver(toolchain: &str, version: &str, verbose: bool, dev_build: bool) -> Result<(), ExitStatus> {
+    if dev_build {
+        println!("Compiling rustc driver");
+    } else {
+        println!("Compiling rustc driver v{version} with {toolchain}");
+    }
+
+    // Build driver
+    let mut cmd = Command::new("cargo");
+
+    if !dev_build {
+        cmd.arg(&format!("+{toolchain}"));
+    }
+
+    if verbose {
+        cmd.arg("--verbose");
+    }
+
+    if dev_build {
+        cmd.args(["build", "--bin", "marker_driver_rustc"]);
+    } else {
+        // FIXME: This currently installs the binary in Cargo's default location.
+        // Ideally this should install the driver in the toolchain folder for the
+        // installed nightly version. This would allow the user to have multiple
+        // drivers depending on the project toolchain.
+        //
+        // We can just reuse rustup to select the correct driver for a defined
+        // toolchain. This would also simulate how the driver would be delivered
+        // in a perfect world.
+        //
+        // See #60
+        cmd.args(["install", "marker_rustc_driver", "--version", version]);
+    }
+
+    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)
+    }
+}
+
+fn check_driver(verbose: bool, print_advice: bool) -> Result<(), ExitStatus> {
+    let path = get_driver_path();
+    if verbose {
+        println!("Searching for driver at: {}", path.display());
+    }
+
+    if !path.exists() || !path.is_file() {
+        if print_advice {
+            eprintln!("Error: The driver binary could not be found.");
+            eprintln!();
+            eprintln!("Try installing it via `cargo marker setup`");
+        }
+
+        Err(ExitStatus::MissingDriver)
+    } else {
+        Ok(())
+    }
+}
+
+pub fn run_driver(
+    env: Vec<(OsString, OsString)>,
+    cargo_args: impl Iterator<Item = String>,
+    verbose: bool,
+) -> Result<(), ExitStatus> {
+    check_driver(verbose, true)?;
+    println!();
+    println!("Start linting:");
+
+    let mut cmd = Command::new("cargo");
+    cmd.envs(env).arg("check").args(cargo_args);
+    if verbose {
+        cmd.arg("--verbose");
+    }
+
+    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 get_driver_path() -> PathBuf {
+    #[allow(unused_mut)]
+    let mut path = std::env::current_exe()
+        .expect("unable to retrieve the path of the current executable")
+        .with_file_name("marker_driver_rustc");
+
+    #[cfg(target_os = "windows")]
+    path.set_extension("exe");
+
+    path
+}

cargo-marker/src/driver.rs

@@ -0,0 +1,63 @@
+use std::{
+    ffi::OsStr,
+    path::{Path, PathBuf},
+    process::Command,
+};
+
+use crate::ExitStatus;
+
+/// This creates a debug build for a local crate. The path of the build library
+/// will be returned, if the operation was successful.
+pub fn build_local_lint_crate(crate_dir: &Path, target_dir: &Path, verbose: bool) -> Result<PathBuf, ExitStatus> {
+    if !crate_dir.exists() {
+        eprintln!("The given lint can't be found, searched at: `{}`", crate_dir.display());
+        return Err(ExitStatus::LintCrateNotFound);
+    }
+
+    // Compile the lint crate
+    let mut cmd = Command::new("cargo");
+    if verbose {
+        cmd.arg("--verbose");
+    }
+    let exit_status = cmd
+        .current_dir(std::fs::canonicalize(crate_dir).unwrap())
+        .args(["build", "--lib", "--target-dir"])
+        .arg(target_dir.as_os_str())
+        .env("RUSTFLAGS", "--cap-lints=allow")
+        .spawn()
+        .expect("could not run cargo")
+        .wait()
+        .expect("failed to wait for cargo?");
+
+    if !exit_status.success() {
+        return Err(ExitStatus::LintCrateBuildFail);
+    }
+
+    // Find the final binary and return the string
+    #[cfg(any(target_os = "linux", target_os = "macos"))]
+    let lib_file_prefix = "lib";
+    #[cfg(target_os = "windows")]
+    let lib_file_prefix = "";
+
+    // FIXME: currently this expect, that the lib name is the same as the crate dir.
+    // See marker#60
+    let file_name = format!(
+        "{lib_file_prefix}{}",
+        crate_dir.file_name().and_then(OsStr::to_str).unwrap_or_default()
+    );
+    // Here `debug` is attached as the crate is build with the `cargo build` command
+    let mut krate_path = target_dir.join("debug").join(file_name);
+
+    #[cfg(target_os = "linux")]
+    krate_path.set_extension("so");
+    #[cfg(target_os = "macos")]
+    krate_path.set_extension("dylib");
+    #[cfg(target_os = "windows")]
+    krate_path.set_extension("dll");
+
+    if !krate_path.exists() && !krate_path.is_file() {
+        Err(ExitStatus::LintCrateLibNotFound)
+    } else {
+        Ok(krate_path)
+    }
+}