Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small maintainance improvements in cargo and rustc #125

Merged
merged 3 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
[alias]
dogfood = "run --features dev-build --bin cargo-marker"
dogfood = "run --features dev-build --bin cargo-marker -- --forward-rust-flags"
uitest = "test --test compile_test"

[profile.dev]
split-debuginfo = "unpacked"
27 changes: 26 additions & 1 deletion cargo-marker/src/cli.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clap::{builder::ValueParser, Arg, ArgAction, Command};
use clap::{builder::ValueParser, Arg, ArgAction, ArgMatches, Command};

use crate::VERSION;

Expand All @@ -10,6 +10,25 @@ EXAMPLES:
* `cargo marker -l ./marker_lints`
"#;

#[allow(clippy::struct_excessive_bools)]
pub struct Flags {
pub verbose: bool,
pub test_build: bool,
pub dev_build: bool,
pub forward_rust_flags: bool,
}

impl Flags {
pub fn from_args(args: &ArgMatches) -> Self {
Self {
verbose: args.get_flag("verbose"),
test_build: args.get_flag("test-setup"),
dev_build: cfg!(feature = "dev-build"),
forward_rust_flags: args.get_flag("forward-rust-flags"),
}
}
}

pub fn get_clap_config() -> Command {
Command::new(VERSION)
.arg(
Expand All @@ -32,6 +51,12 @@ pub fn get_clap_config() -> Command {
.action(ArgAction::SetTrue)
.help("This flag will compile the lint crate and print all relevant environment values"),
)
.arg(
Arg::new("forward-rust-flags")
.long("forward-rust-flags")
.action(ArgAction::SetTrue)
.help("Forwards the current `RUSTFLAGS` value during driver and lint compilation"),
)
.subcommand(setup_command())
.subcommand(check_command())
.args(check_command_args())
Expand Down
27 changes: 18 additions & 9 deletions cargo-marker/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{ffi::OsString, path::PathBuf, process::Command};

use once_cell::sync::Lazy;

use crate::ExitStatus;
use crate::{cli::Flags, ExitStatus};

/// This is the driver version and toolchain, that is used by the setup command
/// to install the driver.
Expand All @@ -41,7 +41,7 @@ pub fn print_driver_version() {
}

/// This tries to install the rustc driver specified in [`DEFAULT_DRIVER_INFO`].
pub fn install_driver(verbose: bool, dev_build: bool) -> Result<(), ExitStatus> {
pub fn install_driver(flags: &Flags) -> 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
Expand All @@ -50,10 +50,10 @@ pub fn install_driver(verbose: bool, dev_build: bool) -> Result<(), ExitStatus>
let toolchain = &DEFAULT_DRIVER_INFO.toolchain;
check_toolchain(toolchain)?;

build_driver(toolchain, &DEFAULT_DRIVER_INFO.version, verbose, dev_build)?;
build_driver(toolchain, &DEFAULT_DRIVER_INFO.version, flags)?;

// We don't want to advice the user, to install the driver again.
check_driver(verbose, false)
check_driver(flags.verbose, false)
}

/// This function checks if the specified toolchain is installed. This requires
Expand All @@ -73,8 +73,8 @@ fn check_toolchain(toolchain: &str) -> Result<(), ExitStatus> {

/// 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 {
fn build_driver(toolchain: &str, version: &str, flags: &Flags) -> Result<(), ExitStatus> {
if flags.dev_build {
println!("Compiling rustc driver");
} else {
println!("Compiling rustc driver v{version} with {toolchain}");
Expand All @@ -83,15 +83,21 @@ fn build_driver(toolchain: &str, version: &str, verbose: bool, dev_build: bool)
// Build driver
let mut cmd = Command::new("cargo");

if !dev_build {
if !flags.dev_build {
cmd.arg(&format!("+{toolchain}"));
}

if verbose {
if flags.verbose {
cmd.arg("--verbose");
}

if dev_build {
let mut rustc_flags = if flags.forward_rust_flags {
std::env::var("RUSTFLAGS").unwrap_or_default()
} else {
String::new()
};

if flags.dev_build {
cmd.args(["build", "--bin", "marker_driver_rustc"]);
} else {
// FIXME: This currently installs the binary in Cargo's default location.
Expand All @@ -105,8 +111,11 @@ fn build_driver(toolchain: &str, version: &str, verbose: bool, dev_build: bool)
//
// See #60
cmd.args(["install", "marker_rustc_driver", "--version", version]);
rustc_flags += " --cap-lints=allow";
}
cmd.env("RUSTFLAGS", rustc_flags);

// `RUSTFLAGS` is never set for driver compilation, we might want to
let status = cmd
.spawn()
.expect("unable to start cargo install for the driver")
Expand Down
19 changes: 13 additions & 6 deletions cargo-marker/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
process::Command,
};

use crate::ExitStatus;
use crate::{cli::Flags, ExitStatus};

pub struct LintCrateSpec<'a> {
/// Optional package name (this is always UTF-8, as opposed to `dir`), exists if supplied from
Expand Down Expand Up @@ -42,23 +42,30 @@ impl<'a> LintCrateSpec<'a> {

/// Creates a debug build for this crate. The path of the build library
/// will be returned, if the operation was successful.
pub fn build(&self, target_dir: &Path, verbose: bool) -> Result<PathBuf, ExitStatus> {
build_local_lint_crate(self, target_dir, verbose)
pub fn build(&self, target_dir: &Path, flags: &Flags) -> Result<PathBuf, ExitStatus> {
build_local_lint_crate(self, target_dir, flags)
}
}

/// This creates a debug build for a local crate. The path of the build library
/// will be returned, if the operation was successful.
fn build_local_lint_crate(krate: &LintCrateSpec<'_>, target_dir: &Path, verbose: bool) -> Result<PathBuf, ExitStatus> {
fn build_local_lint_crate(krate: &LintCrateSpec<'_>, target_dir: &Path, flags: &Flags) -> Result<PathBuf, ExitStatus> {
if !krate.dir.exists() {
eprintln!("The given lint can't be found, searched at: `{}`", krate.dir.display());
return Err(ExitStatus::LintCrateNotFound);
}

let mut rustc_flags = if flags.forward_rust_flags {
std::env::var("RUSTFLAGS").unwrap_or_default()
} else {
String::new()
};
rustc_flags += " --cap-lints=allow";

// Compile the lint crate
let mut cmd = Command::new("cargo");
cmd.arg("build");
if verbose {
if flags.verbose {
cmd.arg("--verbose");
}
if let Some(name) = krate.package_name {
Expand All @@ -69,7 +76,7 @@ fn build_local_lint_crate(krate: &LintCrateSpec<'_>, target_dir: &Path, verbose:
.current_dir(std::fs::canonicalize(krate.dir).unwrap())
.args(["--lib", "--target-dir"])
.arg(target_dir.as_os_str())
.env("RUSTFLAGS", "--cap-lints=allow")
.env("RUSTFLAGS", rustc_flags)
.spawn()
.expect("could not run cargo")
.wait()
Expand Down
41 changes: 12 additions & 29 deletions cargo-marker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::{
process::exit,
};

use cli::get_clap_config;
use cli::{get_clap_config, Flags};
use config::Config;
use driver::{get_driver_path, run_driver};
use lints::LintCrateSpec;
Expand Down Expand Up @@ -109,12 +109,10 @@ fn main() -> Result<(), ExitStatus> {
.take_while(|s| s != CARGO_ARGS_SEPARATOR),
);

let verbose = matches.get_flag("verbose");
let test_build = matches.get_flag("test-setup");
let dev_build = cfg!(feature = "dev-build");
let flags = Flags::from_args(&matches);

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

Expand All @@ -127,32 +125,17 @@ fn main() -> Result<(), ExitStatus> {
};

match matches.subcommand() {
Some(("setup", _args)) => driver::install_driver(verbose, dev_build),
Some(("check", args)) => run_check(
&choose_lint_crates(args, config.as_ref())?,
verbose,
dev_build,
test_build,
),
None => run_check(
&choose_lint_crates(&matches, config.as_ref())?,
verbose,
dev_build,
test_build,
),
Some(("setup", _args)) => driver::install_driver(&flags),
Some(("check", args)) => run_check(&choose_lint_crates(args, config.as_ref())?, &flags),
None => run_check(&choose_lint_crates(&matches, config.as_ref())?, &flags),
_ => unreachable!(),
}
}

fn run_check(
crate_entries: &[LintCrateSpec],
verbose: bool,
dev_build: bool,
test_build: bool,
) -> Result<(), ExitStatus> {
fn run_check(crate_entries: &[LintCrateSpec], flags: &Flags) -> Result<(), ExitStatus> {
// If this is a dev build, we want to recompile the driver before checking
if dev_build {
driver::install_driver(verbose, dev_build)?;
if flags.dev_build {
driver::install_driver(flags)?;
}

if crate_entries.is_empty() {
Expand All @@ -171,7 +154,7 @@ fn run_check(
println!("Compiling Lints:");
let target_dir = Path::new(&*MARKER_LINT_DIR);
for krate in crate_entries {
let crate_file = krate.build(target_dir, verbose)?;
let crate_file = krate.build(target_dir, flags)?;
lint_crates.push(crate_file.as_os_str().to_os_string());
}

Expand All @@ -180,12 +163,12 @@ fn run_check(
(OsString::from("RUSTC_WORKSPACE_WRAPPER"), get_driver_path().as_os_str().to_os_string()),
(OsString::from("MARKER_LINT_CRATES"), lint_crates.join(OsStr::new(";")))
];
if test_build {
if flags.test_build {
print_env(env).unwrap();
Ok(())
} else {
let cargo_args = std::env::args().skip_while(|c| c != CARGO_ARGS_SEPARATOR).skip(1);
run_driver(env, cargo_args, verbose)
run_driver(env, cargo_args, flags.verbose)
}
}

Expand Down
30 changes: 0 additions & 30 deletions marker_driver_rustc/src/conversion/common.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,9 @@
#[repr(C)]
pub struct GenericIdLayout {
pub krate: u32,
pub index: u32,
}

#[repr(C)]
pub struct TyDefIdLayout {
pub krate: u32,
pub index: u32,
}

#[repr(C)]
pub struct DefIdLayout {
pub krate: u32,
pub index: u32,
}

#[repr(C)]
pub struct ItemIdLayout {
pub krate: u32,
pub index: u32,
}

#[repr(C)]
pub struct BodyIdLayout {
// Note: AFAIK rustc only loads bodies from the current crate, this allows
Expand All @@ -39,12 +21,6 @@ pub struct DefIdInfo {
pub krate: u32,
}

#[repr(C)]
pub struct ExprIdLayout {
pub owner: u32,
pub index: u32,
}

#[repr(C)]
pub struct HirIdLayout {
pub owner: u32,
Expand All @@ -57,12 +33,6 @@ pub struct SpanSourceInfo {
pub rustc_start_offset: usize,
}

#[repr(C)]
pub struct VarIdLayout {
pub owner: u32,
pub index: u32,
}

#[macro_export]
macro_rules! transmute_id {
($t1:ty as $t2:ty = $e:expr) => {
Expand Down
Loading