Skip to content

Commit

Permalink
Cargo: Adjusting cargo invocation to make rustup load libs correctly
Browse files Browse the repository at this point in the history
Toolchains are pain, it would be so awesome if marker could just be a part of rustup
  • Loading branch information
xFrednet committed Jul 12, 2023
1 parent 5a88b4a commit af6d518
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 50 deletions.
2 changes: 1 addition & 1 deletion cargo-marker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ 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]
# A local crate as a path
marker_lints = { path = "./marker_lints" }
Expand Down
2 changes: 1 addition & 1 deletion cargo-marker/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub fn run_check(config: &Config, info: CheckInfo, additional_cargo_args: &[Stri
println!();
println!("Start linting:");

let mut cmd = config.toolchain.cargo_command();
let mut cmd = config.toolchain.cargo_with_driver();
cmd.arg("check");
cmd.args(additional_cargo_args);

Expand Down
25 changes: 11 additions & 14 deletions cargo-marker/src/backend/driver.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{process::Command, str::from_utf8};
use std::{path::Path, process::Command, str::from_utf8};

use once_cell::sync::Lazy;

Expand Down Expand Up @@ -27,8 +27,16 @@ pub struct DriverVersionInfo {
}

impl DriverVersionInfo {
fn try_from_toolchain(toolchain: &Toolchain) -> Result<DriverVersionInfo, ExitStatus> {
if let Ok(output) = Command::new(toolchain.driver_path.as_os_str())
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()
{
Expand Down Expand Up @@ -62,17 +70,6 @@ impl DriverVersionInfo {
}
}

pub fn print_driver_version(dev_build: bool) {
if let Ok(ts) = Toolchain::try_find_toolchain(dev_build, false) {
if let Ok(info) = DriverVersionInfo::try_from_toolchain(&ts) {
println!(
"rustc driver version: {} (toolchain: {}, api: {})",
info.version, info.toolchain, info.api_version
);
}
}
}

/// This tries to install the rustc driver specified in [`DEFAULT_DRIVER_INFO`].
pub fn install_driver(
auto_install_toolchain: bool,
Expand Down
33 changes: 25 additions & 8 deletions cargo-marker/src/backend/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,26 @@ impl Toolchain {
/// See also [`Self::cargo_build_command`] if the commands is intended to build
/// a crate.
pub fn cargo_command(&self) -> Command {
let mut cmd = Command::new(&self.cargo_path);
// 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");

// In theory, it's not necessary to set the toolchain, as the comment
// takes the absolute path to the cargo of the toolchain. It's still
// better to set it, to keep everything consistent.
self.toolchain
.as_ref()
.map(|toolchain| cmd.env("RUSTUP_TOOLCHAIN", toolchain));
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();

cmd.env("RUSTC_WORKSPACE_WRAPPER", &self.driver_path);

cmd
}
Expand Down Expand Up @@ -71,7 +83,7 @@ impl Toolchain {
let metadata = MetadataCommand::new()
.cargo_path(&self.cargo_path)
.exec()
.map_err(|_| ExitStatus::LintCrateFetchFailed)?;
.map_err(|_| ExitStatus::NoTargetDir)?;

Ok(metadata.target_directory.into())
}
Expand Down Expand Up @@ -168,6 +180,11 @@ pub(crate) fn rustup_which(toolchain: &str, tool: &str, verbose: bool) -> Result
if output.status.success() {
if let Some(path_str) = to_os_str(output.stdout) {
let path = PathBuf::from(path_str);

// 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);

if verbose {
println!("Found `{tool}` for `{toolchain}` at {}", path.to_string_lossy());
}
Expand Down
55 changes: 54 additions & 1 deletion cargo-marker/src/exit.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,37 @@
//! The error handling in this crate is really not great. This is a quick hack
//! anyone interested in fixing this would be highly appreciated!!! <3

#[derive(Debug)]
use std::fmt::Debug;

const HELP_FOR_NO_LINTS: &str = r#"No lints where specified.
* Try specifying them in `Cargo.toml` under `[workspace.metadata.marker.lints]`
Example:
```
[workspace.metadata.marker.lints]
# 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"
```
* Try specifying them with the `--lints` argument
Example:
```
cargo marker --lints 'marker_lints = "<version>"'
```
"#;

const HELP_MISSING_DRIVER: &str = r#"Driver not found
* Try installing the driver by running:
```
cargo marker setup --auto-install-toolchain
```
"#;

pub enum ExitStatus {
/// The toolchain validation failed. This could happen, if rustup is not
/// installed or the required toolchain is not installed.
Expand All @@ -23,6 +53,7 @@ pub enum ExitStatus {
LintCrateLibNotFound = 502,
/// Failed to fetch the lint crate
LintCrateFetchFailed = 550,
NoTargetDir = 551,
/// General "bad config" error
BadConfiguration = 600,
/// No lint crates were specified -> nothing to do
Expand All @@ -34,3 +65,25 @@ pub enum ExitStatus {
/// Check failed
MarkerCheckFailed = 1000,
}

impl Debug for ExitStatus {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::InvalidToolchain => write!(f, "InvalidToolchain"),
Self::ToolExecutionFailed => write!(f, "ToolExecutionFailed"),
Self::MissingDriver => write!(f, "{HELP_MISSING_DRIVER}"),
Self::DriverInstallationFailed => write!(f, "DriverInstallationFailed"),
Self::DriverFailed => write!(f, "DriverFailed"),
Self::LintCrateBuildFail => write!(f, "LintCrateBuildFail"),
Self::LintCrateNotFound => write!(f, "LintCrateNotFound"),
Self::LintCrateLibNotFound => write!(f, "LintCrateLibNotFound"),
Self::LintCrateFetchFailed => write!(f, "LintCrateFetchFailed"),
Self::NoTargetDir => write!(f, "NoTargetDir"),
Self::BadConfiguration => write!(f, "BadConfiguration"),
Self::NoLints => write!(f, "{HELP_FOR_NO_LINTS}"),
Self::WrongStructure => write!(f, "WrongStructure"),
Self::InvalidValue => write!(f, "InvalidValue"),
Self::MarkerCheckFailed => write!(f, "MarkerCheckFailed"),
}
}
}
27 changes: 15 additions & 12 deletions cargo-marker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,16 @@ mod utils;

use std::{collections::HashMap, ffi::OsString};

use backend::CheckInfo;
use cli::{get_clap_config, Flags};
use config::Config;

pub use exit::ExitStatus;

use crate::backend::driver::DriverVersionInfo;

const CARGO_ARGS_SEPARATOR: &str = "--";
const VERSION: &str = concat!("cargo-marker ", env!("CARGO_PKG_VERSION"));
const NO_LINTS_ERROR: &str = concat!(
"Please provide at least one valid lint crate, ",
"with the `--lints` argument, ",
"or `[workspace.metadata.marker.lints]` in `Cargo.toml`"
);

fn main() -> Result<(), ExitStatus> {
let matches = get_clap_config().get_matches_from(
Expand All @@ -35,7 +33,7 @@ fn main() -> Result<(), ExitStatus> {
let flags = Flags::from_args(&matches);

if matches.get_flag("version") {
print_version(&flags);
print_version();
return Ok(());
}

Expand Down Expand Up @@ -78,7 +76,6 @@ fn run_check(args: &clap::ArgMatches, config: Option<Config>, flags: &Flags) ->

// Validation
if lints.is_empty() {
eprintln!("{NO_LINTS_ERROR}");
return Err(ExitStatus::NoLints);
}

Expand All @@ -100,7 +97,7 @@ fn run_check(args: &clap::ArgMatches, config: Option<Config>, flags: &Flags) ->

// Run backend
if flags.test_build {
print_env(&info.env).unwrap();
print_test_info(&backend_conf, &info).unwrap();
Ok(())
} else {
let additional_cargo_args: Vec<_> = std::env::args()
Expand All @@ -111,12 +108,18 @@ fn run_check(args: &clap::ArgMatches, config: Option<Config>, flags: &Flags) ->
}
}

fn print_version(flags: &Flags) {
fn print_version() {
println!("cargo-marker version: {}", env!("CARGO_PKG_VERSION"));
}

if flags.verbose {
backend::driver::print_driver_version(flags.dev_build);
}
fn print_test_info(config: &backend::Config, check: &CheckInfo) -> Result<(), ExitStatus> {
print_env(&check.env).unwrap();

let info = DriverVersionInfo::try_from_toolchain(&config.toolchain, &config.marker_dir.join("Cargo.toml"))?;
println!("info:toolchain={}", info.toolchain);
println!("info:marker-api={}", info.api_version);

Ok(())
}

#[allow(clippy::unnecessary_wraps)]
Expand Down
24 changes: 11 additions & 13 deletions marker_rustc_driver/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ fn main() {
exit(0);
}

if orig_args.iter().any(|a| a == "--toolchain") {
println!("toolchain: {RUSTC_TOOLCHAIN_VERSION}");
println!("driver: {}", env!("CARGO_PKG_VERSION"));
println!("marker-api: {}", marker_api::MARKER_API_VERSION);

exit(0);
}

// Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument.
// We're invoking the compiler programmatically, so we'll ignore this.
let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref());
Expand All @@ -179,19 +187,9 @@ fn main() {
orig_args.remove(1);
}

if !wrapper_mode {
if orig_args.iter().any(|a| a == "--help" || a == "-h") {
display_help();
exit(0);
}

if orig_args.iter().any(|a| a == "--toolchain") {
println!("toolchain: {RUSTC_TOOLCHAIN_VERSION}");
println!("driver: {}", env!("CARGO_PKG_VERSION"));
println!("marker-api: {}", marker_api::MARKER_API_VERSION);

exit(0);
}
if !wrapper_mode && orig_args.iter().any(|a| a == "--help" || a == "-h") {
display_help();
exit(0);
}

// We enable Marker if one of the following conditions is met
Expand Down

0 comments on commit af6d518

Please sign in to comment.