From 48d418c49bbd5e49330d285b395eb595d83b0c26 Mon Sep 17 00:00:00 2001 From: Nikita Date: Wed, 2 Aug 2023 03:46:17 +0300 Subject: [PATCH] Fixed toolchain version drift, improved error handling, config loading and refactoring This commit contains quite a lot if changes. See them all in the PR description https://github.com/rust-marker/marker/pull/217 --- Cargo.lock | 20 ++-- cargo-marker/Cargo.toml | 6 +- cargo-marker/src/backend.rs | 6 +- cargo-marker/src/backend/cargo.rs | 80 ++++++++++++++ cargo-marker/src/backend/driver.rs | 6 +- cargo-marker/src/backend/lints/fetch.rs | 4 +- cargo-marker/src/backend/toolchain.rs | 132 +++++++----------------- cargo-marker/src/cli.rs | 5 +- cargo-marker/src/config.rs | 121 ++++++++++------------ cargo-marker/src/exit.rs | 55 +++++++--- cargo-marker/src/main.rs | 11 +- cargo-marker/src/utils.rs | 16 +-- marker_adapter/src/loader.rs | 6 +- marker_rustc_driver/src/context.rs | 68 ++++++------ 14 files changed, 280 insertions(+), 256 deletions(-) create mode 100644 cargo-marker/src/backend/cargo.rs diff --git a/Cargo.lock b/Cargo.lock index 5182d8b1..036f342e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -133,18 +133,18 @@ checksum = "a3e2c3daef883ecc1b5d58c15adae93470a91d425f3532ba1695849656af3fc1" [[package]] name = "camino" -version = "1.1.4" +version = "1.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c530edf18f37068ac2d977409ed5cd50d53d73bc653c7647b48eb78976ac9ae2" +checksum = "c59e92b5a388f549b863a7bea62612c09f24c8393560709a54558a9abdfb3b9c" dependencies = [ "serde", ] [[package]] name = "cargo-platform" -version = "0.1.2" +version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cbdb825da8a5df079a43676dbe042702f1707b1109f713a01420fbb4cc71fa27" +checksum = "2cfa25e60aea747ec7e1124f238816749faa93759c6ff5b31f1ccdda137f4479" dependencies = [ "serde", ] @@ -153,10 +153,12 @@ dependencies = [ name = "cargo_marker" version = "0.1.1" dependencies = [ + "camino", "cargo_metadata", "clap", "once_cell", "serde", + "serde_json", "toml", ] @@ -409,7 +411,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cb0889898416213fab133e1d33a0e5858a48177452750691bde3666d0fdbaf8b" dependencies = [ "hermit-abi", - "rustix 0.38.3", + "rustix 0.38.6", "windows-sys", ] @@ -665,9 +667,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.3" +version = "0.38.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac5ffa1efe7548069688cd7028f32591853cd7b5b756d41bcffd2353e4fc75b4" +checksum = "1ee020b1716f0a80e2ace9b03441a749e402e86712f15f16fe8a8f75afac732f" dependencies = [ "bitflags 2.3.3", "errno", @@ -713,9 +715,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.100" +version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f1e14e89be7aa4c4b78bdbdc9eb5bf8517829a600ae8eaa39a6e1d960b5185c" +checksum = "076066c5f1078eac5b722a31827a8832fe108bed65dfa75e233c89f8206e976c" dependencies = [ "itoa", "ryu", diff --git a/cargo-marker/Cargo.toml b/cargo-marker/Cargo.toml index 77f47b69..b9dfa775 100644 --- a/cargo-marker/Cargo.toml +++ b/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" diff --git a/cargo-marker/src/backend.rs b/cargo-marker/src/backend.rs index e5e13f3a..a3dcf1f1 100644 --- a/cargo-marker/src/backend.rs +++ b/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 { ("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())); } diff --git a/cargo-marker/src/backend/cargo.rs b/cargo-marker/src/backend/cargo.rs new file mode 100644 index 00000000..5df909b6 --- /dev/null +++ b/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, +} + +#[derive(Deserialize, Debug)] +struct ProjectLocation { + root: Utf8PathBuf, +} + +impl Cargo { + pub fn with_toolchain(toolchain: impl Into) -> 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 { + 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() + } +} diff --git a/cargo-marker/src/backend/driver.rs b/cargo-marker/src/backend/driver.rs index a34ba0b3..eab0a3e7 100644 --- a/cargo-marker/src/backend/driver.rs +++ b/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); diff --git a/cargo-marker/src/backend/lints/fetch.rs b/cargo-marker/src/backend/lints/fetch.rs index c9ed73b5..e1eb6515 100644 --- a/cargo-marker/src/backend/lints/fetch.rs +++ b/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 { - 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) diff --git a/cargo-marker/src/backend/toolchain.rs b/cargo-marker/src/backend/toolchain.rs index d45fcda6..0e2c977a 100644 --- a/cargo-marker/src/backend/toolchain.rs +++ b/cargo-marker/src/backend/toolchain.rs @@ -1,13 +1,13 @@ use std::{ path::{Path, PathBuf}, process::Command, + str::FromStr, }; -use cargo_metadata::MetadataCommand; - -use crate::{utils::to_os_str, ExitStatus}; +use crate::{utils::is_local_driver, ExitStatus}; use super::{ + cargo::Cargo, driver::{DEFAULT_DRIVER_INFO, MARKER_DRIVER_BIN_NAME}, Config, }; @@ -15,40 +15,15 @@ use super::{ #[derive(Debug)] pub struct Toolchain { pub(crate) driver_path: PathBuf, - /// The Cargo binary that should be used for all Cargo commands. Prefer this - /// over the `CARGO` environment value as this is the Cargo binary used for - /// the specified toolchain. - pub(crate) cargo_path: PathBuf, - /// The rustc toolchain this driver belongs to. This can be `None` during - /// custom builds where the driver was found but not the connected toolchain. - pub(crate) toolchain: Option, + /// A type containing toolchain to which the driver belongs. + /// May not have a toolchain during custom builds when + /// a driver was found but not the connected toolchain. + pub(crate) cargo: Cargo, } impl Toolchain { - /// This returns a command, calling the Cargo instance of the selected toolchain. - /// It may add additional flags for verbose output. - /// - /// See also [`Self::cargo_build_command`] if the commands is intended to build - /// a crate. - pub fn cargo_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("cargo"); - - cmd.env("RUSTUP_TOOLCHAIN", toolchain); - - cmd - } else { - Command::new(&self.cargo_path) - } - } - pub fn cargo_with_driver(&self) -> Command { - let mut cmd = self.cargo_command(); + let mut cmd = self.cargo.command(); cmd.env("RUSTC_WORKSPACE_WRAPPER", &self.driver_path); @@ -56,7 +31,7 @@ impl Toolchain { } pub fn cargo_build_command(&self, config: &Config, manifest: &Path) -> Command { - let mut cmd = self.cargo_command(); + let mut cmd = self.cargo.command(); cmd.arg("build"); // Manifest @@ -78,40 +53,28 @@ impl Toolchain { cmd } - pub fn cargo_metadata_command(&self) -> MetadataCommand { - let mut command = MetadataCommand::new(); - command.cargo_path(&self.cargo_path); - if let Some(toolchain) = self.toolchain.as_ref() { - command.env("RUSTUP_TOOLCHAIN", toolchain); - } - command - } - pub fn find_target_dir(&self) -> Result { // FIXME(xFrednet): Handle errors properly. - let metadata = self - .cargo_metadata_command() - .exec() - .map_err(|_| ExitStatus::NoTargetDir)?; + let metadata = self.cargo.metadata().exec().map_err(|_| ExitStatus::NoTargetDir)?; Ok(metadata.target_directory.into()) } pub fn try_find_toolchain(verbose: bool) -> Result { - if cfg!(debug_assertions) { + if is_local_driver() { Self::search_next_to_cargo_marker(verbose) } else { // First check if there is a rustc driver for the current toolchain. This - // allows the used to override the used toolchain with `+` or + // allows the user to override the used toolchain with `+` or // `rust-toolchain` if let Ok(toolchain) = std::env::var("RUSTUP_TOOLCHAIN") { - if let Ok(info) = Self::search_toolchain(&toolchain, verbose) { + if let Ok(info) = Self::search_driver(&toolchain, verbose) { return Ok(info); } } // Next we check, if we can find a driver for the linked marker toolchain. - if let Ok(info) = Self::search_toolchain(&DEFAULT_DRIVER_INFO.toolchain, verbose) { + if let Ok(info) = Self::search_driver(&DEFAULT_DRIVER_INFO.toolchain, verbose) { return Ok(info); } @@ -125,15 +88,12 @@ impl Toolchain { } } - fn search_toolchain(toolchain: &str, verbose: bool) -> Result { + fn search_driver(toolchain: &str, verbose: bool) -> Result { if let Ok(driver_path) = rustup_which(toolchain, "marker_rustc_driver", verbose) { - if let Ok(cargo_path) = rustup_which(toolchain, "cargo", verbose) { - return Ok(Toolchain { - driver_path, - cargo_path, - toolchain: Some(toolchain.to_string()), - }); - } + return Ok(Toolchain { + driver_path, + cargo: Cargo::with_toolchain(toolchain), + }); } Err(ExitStatus::MissingDriver) @@ -152,10 +112,7 @@ impl Toolchain { } return Ok(Toolchain { driver_path, - cargo_path: PathBuf::from( - std::env::var_os("CARGO").expect("expected environment value `CARGO` to be set"), - ), - toolchain: None, + cargo: Cargo::default(), }); } } @@ -179,40 +136,25 @@ pub(crate) fn rustup_which(toolchain: &str, tool: &str, verbose: bool) -> Result println!("Searching for `{tool}` with rustup for toolchain `{toolchain}`"); } - // Check if the toolchain is installed. We don't want to install it accidentally - if let Ok(output) = Command::new("rustup").args(["toolchain", "list"]).output() { - let text = to_os_str(output.stdout).expect("`Command` output should always be a valid `OsString`"); - if !text.to_string_lossy().contains(toolchain) { - return Err(ExitStatus::MissingDriver); - } - } else { - return Err(ExitStatus::ToolExecutionFailed); - } - - // Check if the driver is installed - if let Ok(output) = Command::new("rustup") - .env("RUSTUP_TOOLCHAIN", toolchain) - .args(["which", tool]) + let output = Command::new("rustup") + .args(["which", "--toolchain", toolchain, tool]) .output() - { - // rustup will error, if it can't find the binary file. Therefore, - // we know that it exists if this succeeds - if output.status.success() { - if let Some(path_str) = to_os_str(output.stdout) { - let path = PathBuf::from(path_str); + .map_err(|err| ExitStatus::fatal(err, "failed to execute rustup"))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(ExitStatus::fatal( + stderr.trim(), + format!("failed to execute `rustup which` for `{tool}` with `{toolchain}` toolchain"), + )); + } - // Remove the `\n` from the print output - let trimmed_name = path.file_name().unwrap().to_str().unwrap().trim(); - let path = path.with_file_name(trimmed_name); + let string_path = String::from_utf8(output.stdout).map_err(|err| ExitStatus::fatal(err, "incorrect bytes"))?; + let path = PathBuf::from_str(string_path.trim()) + .map_err(|err| ExitStatus::fatal(err, format!("failed to parse path for `{tool}`")))?; - if verbose { - println!("Found `{tool}` for `{toolchain}` at {}", path.to_string_lossy()); - } - - return Ok(path); - } - } - return Err(ExitStatus::MissingDriver); + if verbose { + println!("Found `{tool}` for `{toolchain}` at {}", path.to_string_lossy()); } - Err(ExitStatus::ToolExecutionFailed) + Ok(path) } diff --git a/cargo-marker/src/cli.rs b/cargo-marker/src/cli.rs index 9fd7004b..b440d04e 100644 --- a/cargo-marker/src/cli.rs +++ b/cargo-marker/src/cli.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; +use camino::Utf8Path; use clap::{Args, Parser, Subcommand}; /// Marker's CLI interface @@ -89,7 +90,9 @@ pub fn collect_lint_deps(args: &CheckArgs) -> Result, +} + +#[derive(Deserialize, Debug)] +struct Workspace { + metadata: Option, +} + +#[derive(Deserialize, Debug)] +struct WorkspaceMetadata { + marker: Option, +} /// Markers metadata section `workspace.metadata.marker` in `Cargo.toml` #[derive(Deserialize, Debug)] @@ -36,10 +46,10 @@ pub enum LintDependency { impl LintDependency { /// This function normalizes the struct, by making all paths absolute paths - fn normalize(&mut self) { + fn normalize(&mut self, workspace_path: &Utf8Path) -> Result<(), ConfigFetchError> { match self { - LintDependency::Full(full) => full.source.normalize(), - LintDependency::Simple(_) => {}, + LintDependency::Full(full) => full.source.normalize(workspace_path), + LintDependency::Simple(_) => Ok(()), } } @@ -87,14 +97,16 @@ pub enum Source { impl Source { /// This function normalizes the struct, by making all paths absolute paths - fn normalize(&mut self) { - if let Source::Path { ref mut path } = self { - if let Ok(absolute_path) = std::fs::canonicalize(&path) { - if let Some(absolute_path) = absolute_path.to_str() { - *path = absolute_path.to_string(); - } - } - } + fn normalize(&mut self, workspace_path: &Utf8Path) -> Result<(), ConfigFetchError> { + let Source::Path { path } = self else { + return Ok(()); + }; + *path = workspace_path + .join(&path) + .canonicalize_utf8() + .map_err(ConfigFetchError::IoError)? + .into_string(); + Ok(()) } } @@ -108,14 +120,10 @@ pub enum GitRef { #[derive(Debug)] pub enum ConfigFetchError { - /// `Cargo.toml` wasn't found - FileNotFound, /// Read failed IoError(io::Error), /// Couldn't parse `Cargo.toml` ParseError(toml::de::Error), - /// `workspace.metadata.marker` has invalid structure - InvalidStructure, /// `workspace.metadata.marker` doesn't exist SectionNotFound, } @@ -123,71 +131,48 @@ pub enum ConfigFetchError { impl ConfigFetchError { pub fn emit_and_convert(self) -> ExitStatus { match self { - ConfigFetchError::FileNotFound => eprintln!("`Cargo.toml` wasn't found"), ConfigFetchError::IoError(err) => eprintln!("IO error reading config: {err:?}"), - ConfigFetchError::ParseError(err) => eprintln!("Can't parse config: {err:?}"), + // Better to use Display than Debug for toml error because it + // will display the snippet of toml and highlight the error span + ConfigFetchError::ParseError(err) => eprintln!("Can't parse config: {err}"), ConfigFetchError::SectionNotFound => eprintln!("Marker config wasn't found"), - ConfigFetchError::InvalidStructure => { - eprintln!("`workspace.metadata.marker` has invalid structure"); - return ExitStatus::WrongStructure; - }, }; ExitStatus::BadConfiguration } } impl Config { - pub fn try_from_manifest() -> Result { - let config_str = Self::load_raw_manifest()?; + pub fn try_from_manifest(path: &Utf8Path) -> Result { + let config_str = fs::read_to_string(path).map_err(ConfigFetchError::IoError)?; - Self::try_from_str(&config_str) + Self::try_from_str(&config_str, path) } - pub fn try_from_str(config_str: &str) -> Result { - let cargo_config: toml::Value = match toml::from_str(config_str) { - Ok(v) => v, - Err(e) => return Err(ConfigFetchError::ParseError(e)), - }; + pub(crate) fn try_from_str(config_str: &str, path: &Utf8Path) -> Result { + let cargo_toml: CargoToml = toml::from_str(config_str).map_err(ConfigFetchError::ParseError)?; - let config_value = if let Some(value) = cargo_config - .get("workspace") - .and_then(|v| v.get("metadata")) - .and_then(|v| v.get("marker")) - { - value - } else { - return Err(ConfigFetchError::SectionNotFound); - }; + let mut config = cargo_toml + .workspace + .ok_or(ConfigFetchError::SectionNotFound)? + .metadata + .ok_or(ConfigFetchError::SectionNotFound)? + .marker + .ok_or(ConfigFetchError::SectionNotFound)?; - let mut config: Config = if let Ok(config) = config_value.clone().try_into() { - config - } else { - return Err(ConfigFetchError::InvalidStructure); - }; - - config.normalize(); + let workspace_path = path + .parent() + .expect("path must have a parent after reading the `Cargo.toml` file"); + config.normalize(workspace_path)?; Ok(config) } - fn load_raw_manifest() -> Result { - // FIXME(xFrednet): Use `cargo locate-project` to find the `Cargo.toml` file - let Ok(mut config_file) = File::open(CARGO_TOML) else { - return Err(ConfigFetchError::FileNotFound); - }; - - // FIXME(xFrednet): Maybe load `Cargo.toml` with `toml` that allows to display - // warnings with a span - let mut config_str = String::new(); - if let Err(e) = config_file.read_to_string(&mut config_str) { - return Err(ConfigFetchError::IoError(e)); - } - Ok(config_str) - } - /// This function normalizes the config, to be generally applicable. Currently, /// it normalizes all relative paths to be absolute paths instead. - fn normalize(&mut self) { - self.lints.iter_mut().for_each(|(_name, lint)| lint.normalize()); + fn normalize(&mut self, workspace_path: &Utf8Path) -> Result<(), ConfigFetchError> { + for lint in &mut self.lints.values_mut() { + lint.normalize(workspace_path)?; + } + Ok(()) } } diff --git a/cargo-marker/src/exit.rs b/cargo-marker/src/exit.rs index e210fc3e..90177b86 100644 --- a/cargo-marker/src/exit.rs +++ b/cargo-marker/src/exit.rs @@ -5,7 +5,7 @@ use std::fmt::Debug; use crate::backend::driver::DEFAULT_DRIVER_INFO; -const HELP_FOR_NO_LINTS: &str = r#"No lints where specified. +const HELP_FOR_NO_LINTS: &str = r#"No lints were specified. * Try specifying them in `Cargo.toml` under `[workspace.metadata.marker.lints]` Example: @@ -49,35 +49,54 @@ const HELP_INSTALL_DRIVER_FAILED: &str = r#"Installing the driver failed pub enum ExitStatus { /// The toolchain validation failed. This could happen, if rustup is not /// installed or the required toolchain is not installed. - InvalidToolchain = 100, + InvalidToolchain, /// The execution of a tool, like rustup or cargo, failed. - ToolExecutionFailed = 101, + ToolExecutionFailed, /// Unable to find the driver binary - MissingDriver = 200, + MissingDriver, /// Nothing we can really do, but good to know. The user will have to analyze /// the forwarded cargo output. - DriverInstallationFailed = 300, + DriverInstallationFailed, /// A general collection status, for failures originating from the driver - DriverFailed = 400, + DriverFailed, /// The lint crate build failed for some reason - LintCrateBuildFail = 500, + LintCrateBuildFail, /// Lint crate could not be found - LintCrateNotFound = 501, + LintCrateNotFound, /// The lint crate has been build, but the resulting binary could not be found. - LintCrateLibNotFound = 502, + LintCrateLibNotFound, /// Failed to fetch the lint crate - LintCrateFetchFailed = 550, - NoTargetDir = 551, + LintCrateFetchFailed, + NoTargetDir, /// General "bad config" error - BadConfiguration = 600, + BadConfiguration, /// No lint crates were specified -> nothing to do - NoLints = 601, + NoLints, /// Can't deserialise `workspace.metadata.marker.lints` properly - WrongStructure = 602, + WrongStructure, /// An invalid configuration value was specified - InvalidValue = 603, + InvalidValue, /// Check failed - MarkerCheckFailed = 1000, + MarkerCheckFailed, + /// Uncategorized error happened. + /// If some kind of error needs to be handled specially, we may consider + /// moving it from `Fatal` to a dedicated enum variant. + Fatal { + message: String, + source: Option>, + }, +} + +impl ExitStatus { + pub(crate) fn fatal( + source: impl Into>, + message: impl Into, + ) -> Self { + Self::Fatal { + message: message.into(), + source: Some(source.into()), + } + } } impl Debug for ExitStatus { @@ -102,6 +121,10 @@ impl Debug for ExitStatus { Self::WrongStructure => write!(f, "WrongStructure"), Self::InvalidValue => write!(f, "InvalidValue"), Self::MarkerCheckFailed => write!(f, "MarkerCheckFailed"), + Self::Fatal { message, source } => match source { + Some(source) => write!(f, "{message}\nCaused by: {source}"), + None => f.write_str(message), + }, } } } diff --git a/cargo-marker/src/main.rs b/cargo-marker/src/main.rs index f239e59e..de9f4ffb 100644 --- a/cargo-marker/src/main.rs +++ b/cargo-marker/src/main.rs @@ -23,7 +23,10 @@ use crate::backend::driver::DriverVersionInfo; fn main() -> Result<(), ExitStatus> { let cli = MarkerCli::parse_args(); - let config = match Config::try_from_manifest() { + let cargo = backend::cargo::Cargo::default(); + + let path = cargo.cargo_locate_project()?; + let config = match Config::try_from_manifest(&path) { Ok(v) => Some(v), Err(e) => match e { config::ConfigFetchError::SectionNotFound => None, @@ -76,15 +79,15 @@ fn run_check(args: &CheckArgs, config: Option, kind: CheckKind) -> Resul } // If this is a dev build, we want to rebuild the driver before checking - #[cfg(debug_assertions)] - backend::driver::install_driver(false, "")?; + if utils::is_local_driver() { + backend::driver::install_driver(false, "")?; + } // Configure backend // FIXME(xFrednet): Implement better logging and remove verbose boolean in // favor of debug logging. let toolchain = backend::toolchain::Toolchain::try_find_toolchain(false)?; let backend_conf = backend::Config { - dev_build: cfg!(debug_assertions), lints, ..backend::Config::try_base_from(toolchain)? }; diff --git a/cargo-marker/src/utils.rs b/cargo-marker/src/utils.rs index 5d0ce872..04a3db91 100644 --- a/cargo-marker/src/utils.rs +++ b/cargo-marker/src/utils.rs @@ -1,14 +1,4 @@ -use std::ffi::OsString; - -#[allow(clippy::unnecessary_wraps)] -pub fn to_os_str(bytes: Vec) -> Option { - #[cfg(unix)] - { - use std::os::unix::prelude::OsStringExt; - Some(OsString::from_vec(bytes)) - } - - // Windows paths are guaranteed to be valid UTF - #[cfg(windows)] - Some(OsString::from(String::from_utf8(bytes).ok()?)) +/// Use local dev build of driver nearby `cargo-marker` executable +pub fn is_local_driver() -> bool { + std::env::var("MARKER_NO_LOCAL_DRIVER").is_err() && cfg!(debug_assertions) } diff --git a/marker_adapter/src/loader.rs b/marker_adapter/src/loader.rs index a8e0c3b3..238be01b 100644 --- a/marker_adapter/src/loader.rs +++ b/marker_adapter/src/loader.rs @@ -66,11 +66,7 @@ impl LintCrateRegistry { } pub(crate) fn collect_lint_pass_info(&self) -> Vec { - let mut info = vec![]; - for pass in &self.passes { - info.push((pass.bindings.info)()); - } - info + self.passes.iter().map(|pass| (pass.bindings.info)()).collect() } } diff --git a/marker_rustc_driver/src/context.rs b/marker_rustc_driver/src/context.rs index 1b083f7d..79f3a942 100644 --- a/marker_rustc_driver/src/context.rs +++ b/marker_rustc_driver/src/context.rs @@ -248,45 +248,45 @@ fn select_children_with_name( name: rustc_span::Symbol, ) -> Vec> { let mut next_search = vec![]; - search - .iter() - .filter_map(rustc_hir::def::Res::mod_def_id) - .for_each(|id| { - if let Some(local_id) = id.as_local() { - let hir = tcx.hir(); - let root_mod; - let item = match hir.find_by_def_id(local_id) { - Some(hir::Node::Crate(r#mod)) => { - root_mod = hir::ItemKind::Mod(r#mod); - Some(&root_mod) - }, - Some(hir::Node::Item(item)) => Some(&item.kind), - _ => None, - }; + let mod_def_ids = search.iter().filter_map(rustc_hir::def::Res::mod_def_id); - if let Some(hir::ItemKind::Mod(module)) = item { - module - .item_ids - .iter() - .filter_map(|&item_id| { - if hir.item(item_id).ident.name == name { - let def_id = item_id.owner_id.to_def_id(); - Some(hir::def::Res::Def(tcx.def_kind(def_id), def_id)) - } else { - None - } - }) - .collect_into(&mut next_search); - } - } else if let hir::def::DefKind::Mod = tcx.def_kind(id) { - tcx.module_children(id) + for id in mod_def_ids { + if let Some(local_id) = id.as_local() { + let hir = tcx.hir(); + + let root_mod; + let item = match hir.find_by_def_id(local_id) { + Some(hir::Node::Crate(r#mod)) => { + root_mod = hir::ItemKind::Mod(r#mod); + Some(&root_mod) + }, + Some(hir::Node::Item(item)) => Some(&item.kind), + _ => None, + }; + + if let Some(hir::ItemKind::Mod(module)) = item { + module + .item_ids .iter() - .filter(|item| item.ident.name == name) - .map(|child| child.res.expect_non_local()) + .filter_map(|&item_id| { + if hir.item(item_id).ident.name == name { + let def_id = item_id.owner_id.to_def_id(); + Some(hir::def::Res::Def(tcx.def_kind(def_id), def_id)) + } else { + None + } + }) .collect_into(&mut next_search); } - }); + } else if let hir::def::DefKind::Mod = tcx.def_kind(id) { + tcx.module_children(id) + .iter() + .filter(|item| item.ident.name == name) + .map(|child| child.res.expect_non_local()) + .collect_into(&mut next_search); + } + } next_search }