Skip to content

Commit

Permalink
Simplify lint crates build to a single cargo build invocation
Browse files Browse the repository at this point in the history
`cargo build` can be specified to build the dependent crates only. It means we can use the build workspace to both fetch and build the lint crate dlls with a single `cargo build` command.

So instead of
```bash
cargo fetch
cargo metadata
cargo build --manifest-path /path/to/lint-crate/a/Cargo.toml
cargo build --manifest-path /path/to/lint-crate/b/Cargo.toml
```

We have just
```bash
cargo build -p a -p b
```

This also removes the usage of the unstable `--out-dir` parameter for `cargo build`.
  • Loading branch information
Veetaha committed Sep 19, 2023
1 parent d150950 commit 072e308
Show file tree
Hide file tree
Showing 8 changed files with 141 additions and 357 deletions.
14 changes: 3 additions & 11 deletions cargo-marker/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! `cargo-marker` CLI. However, `cargo-marker` might also be used as a library for UI
//! tests later down the line.
use self::{lints::LintCrate, toolchain::Toolchain};
use self::{lints::LintDll, toolchain::Toolchain};
use crate::config::LintDependencyEntry;
use crate::error::prelude::*;
use crate::observability::display::{self, print_stage};
Expand Down Expand Up @@ -50,14 +50,6 @@ impl Config {
toolchain,
})
}

fn markers_target_dir(&self) -> Utf8PathBuf {
self.marker_dir.join("target")
}

fn lint_crate_dir(&self) -> Utf8PathBuf {
self.marker_dir.join("lints")
}
}

/// This struct contains all information to use rustc as a driver.
Expand All @@ -68,9 +60,9 @@ pub struct CheckInfo {

pub fn prepare_check(config: &Config) -> Result<CheckInfo> {
print_stage("compiling lints");
let lints = lints::build_lints(config)?
let lints = lints::build(config)?
.iter()
.map(|LintCrate { name, file }| format!("{name}:{file}"))
.map(|LintDll { name, file }| format!("{name}:{file}"))
.join(";");

#[rustfmt::skip]
Expand Down
9 changes: 3 additions & 6 deletions cargo-marker/src/backend/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,9 @@ impl Cargo {
}
}

/// 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.
/// This returns a command calling rustup, which acts as a proxy for
/// `cargo` of the selected toolchain. If no toolchain is selected, it
/// will return a command calling the default `cargo` command.
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.
Expand Down
87 changes: 65 additions & 22 deletions cargo-marker/src/backend/lints.rs
Original file line number Diff line number Diff line change
@@ -1,37 +1,80 @@
use super::Config;
mod build_space;

use crate::backend::Config;
use crate::error::prelude::*;
use crate::observability::prelude::*;
use camino::Utf8PathBuf;

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)]
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: Utf8PathBuf,
}
use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};

/// The information of a compiled lint crate.
#[derive(Debug)]
pub struct LintCrate {
pub(crate) struct LintDll {
/// The name of the crate
pub name: String,
pub(crate) name: String,

/// The absolute path of the compiled crate, as a dynamic library.
pub file: Utf8PathBuf,
pub(crate) file: Utf8PathBuf,
}

/// This function fetches and builds all lints specified in the given [`Config`]
pub fn build_lints(config: &Config) -> Result<Vec<LintCrate>> {
pub(crate) fn build(config: &Config) -> Result<Vec<LintDll>> {
// Check that there are at least some lints that we don't attempt to
// run a `cargo build` with no packages.
if config.lints.is_empty() {
return Err(Error::root("No lints were specified"));
}

// 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)
build_space::init(config)?;

let packages = config.lints.keys().flat_map(|package| ["-p", package]);

let mut cmd = config.toolchain.cargo.command();
cmd.current_dir(&config.marker_dir);
cmd.arg("build");
cmd.args(packages);

// Potential "--release" flag
if !config.debug_build {
cmd.arg("--release");
}

// Environment
cmd.env("RUSTFLAGS", &config.build_rustc_flags);

let exit_status = cmd
.log()
.spawn()
.expect("could not run cargo")
.wait()
.expect("failed to wait for cargo?");

if !exit_status.success() {
return Err(Error::root("Failed to compile the lint crates"));
}

let profile = if config.debug_build { "debug" } else { "release" };

let dlls = config
.lints
.keys()
.map(|package| {
let dll_name = package.replace('-', "_");
let file = config
.marker_dir
.join("target")
.join(profile)
.join(format!("{DLL_PREFIX}{dll_name}{DLL_SUFFIX}"));

LintDll {
name: package.clone(),
file,
}
})
.collect();

Ok(dlls)
}
144 changes: 0 additions & 144 deletions cargo-marker/src/backend/lints/build.rs

This file was deleted.

64 changes: 64 additions & 0 deletions cargo-marker/src/backend/lints/build_space.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use crate::error::prelude::*;
use crate::{backend::Config, config::LintDependencyEntry};
use camino::Utf8Path;
use itertools::Itertools;
use std::collections::BTreeMap;

/// Initializes a cargo workspace for building the lint crates dynamic libraries.
/// Creates a manifest file where all lint crates are mentioned as dependencies.
pub(crate) fn init(config: &Config) -> Result<()> {
/// A small hack, to have the lints namespaced under the `[dependencies]` section
#[derive(serde::Serialize)]
struct DepNamespace<'a> {
dependencies: &'a BTreeMap<String, LintDependencyEntry>,
}

let lints_as_deps = toml::to_string(&DepNamespace {
dependencies: &config.lints,
})
.expect("DepNamespace can be represented as TOML");

let toml_comments = comments("# ");
let manifest_content = format!("{toml_comments}\n{MANIFEST_HEADER}{lints_as_deps}");
let manifest_path = config.marker_dir.join("Cargo.toml");

write_to_file(&manifest_path, &manifest_content)?;
write_to_file(&config.marker_dir.join("src").join("lib.rs"), &comments("//! "))
}

fn write_to_file(path: &Utf8Path, content: &str) -> Result {
let parent = path
.parent()
.unwrap_or_else(|| panic!("The file must have a parent directory. Path: {path}"));

std::fs::create_dir_all(parent).context(|| format!("Failed to create the directory structure for {parent}"))?;

std::fs::write(path, content).context(|| format!("Failed to write a file at {path}"))
}

fn comments(comment_prefix: &str) -> String {
COMMENTS
.trim()
.lines()
.format_with("\n", |line, f| f(&format_args!("{comment_prefix}{line}")))
.to_string()
}

const COMMENTS: &str = r#"
Welcome 👋! This is an internal cargo workspace used by Marker to build lint crates 📦.
We put them as dependencies of this cargo package, and build them all at once 🔨.
As a result we get dynamic libraries 📚 for each of the lint crates and load them in
a custom rustc driver (marker_rustc_driver) at a later stage.
"#;

const MANIFEST_HEADER: &str = r#"
[package]
name = "marker-lint-crates-build-space"
version = "0.1.0"
edition = "2021"
publish = false
# This prevents Cargo from searching the parent directories for a workspace.
[workspace]
"#;
Loading

0 comments on commit 072e308

Please sign in to comment.