Cache lintcheck binary in ci

This commit is contained in:
Alex Macleod 2024-06-22 12:33:51 +00:00
parent 0ce07f61db
commit 2194304b05
2 changed files with 77 additions and 96 deletions

View File

@ -12,13 +12,10 @@ concurrency:
cancel-in-progress: true cancel-in-progress: true
jobs: jobs:
# Generates `lintcheck-logs/base.json` and stores it in a cache # Runs lintcheck on the PR's target branch and stores the results as an artifact
base: base:
runs-on: ubuntu-latest runs-on: ubuntu-latest
outputs:
key: ${{ steps.key.outputs.key }}
steps: steps:
- name: Checkout - name: Checkout
uses: actions/checkout@v4 uses: actions/checkout@v4
@ -37,57 +34,67 @@ jobs:
rm -rf lintcheck rm -rf lintcheck
git checkout ${{ github.sha }} -- lintcheck git checkout ${{ github.sha }} -- lintcheck
- name: Cache lintcheck bin
id: cache-lintcheck-bin
uses: actions/cache@v4
with:
path: target/debug/lintcheck
key: lintcheck-bin-${{ hashfiles('lintcheck/**') }}
- name: Build lintcheck
if: steps.cache-lintcheck-bin.outputs.cache-hit != 'true'
run: cargo build --manifest-path=lintcheck/Cargo.toml
- name: Create cache key - name: Create cache key
id: key id: key
run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
- name: Cache results - name: Cache results JSON
id: cache id: cache-json
uses: actions/cache@v4 uses: actions/cache@v4
with: with:
path: lintcheck-logs/base.json path: lintcheck-logs/lintcheck_crates_logs.json
key: ${{ steps.key.outputs.key }} key: ${{ steps.key.outputs.key }}
- name: Run lintcheck - name: Run lintcheck
if: steps.cache.outputs.cache-hit != 'true' if: steps.cache-json.outputs.cache-hit != 'true'
run: cargo lintcheck --format json run: ./target/debug/lintcheck --format json
- name: Rename JSON file - name: Upload base JSON
if: steps.cache.outputs.cache-hit != 'true' uses: actions/upload-artifact@v4
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/base.json with:
name: base
path: lintcheck-logs/lintcheck_crates_logs.json
# Generates `lintcheck-logs/head.json` and stores it in a cache # Runs lintcheck on the PR and stores the results as an artifact
head: head:
runs-on: ubuntu-latest runs-on: ubuntu-latest
outputs:
key: ${{ steps.key.outputs.key }}
steps: steps:
- name: Checkout - name: Checkout
uses: actions/checkout@v4 uses: actions/checkout@v4
- name: Create cache key - name: Cache lintcheck bin
id: key id: cache-lintcheck-bin
run: echo "key=lintcheck-head-${{ github.sha }}" >> "$GITHUB_OUTPUT"
- name: Cache results
id: cache
uses: actions/cache@v4 uses: actions/cache@v4
with: with:
path: lintcheck-logs/head.json path: target/debug/lintcheck
key: ${{ steps.key.outputs.key }} key: lintcheck-bin-${{ hashfiles('lintcheck/**') }}
- name: Build lintcheck
if: steps.cache-lintcheck-bin.outputs.cache-hit != 'true'
run: cargo build --manifest-path=lintcheck/Cargo.toml
- name: Run lintcheck - name: Run lintcheck
if: steps.cache.outputs.cache-hit != 'true' run: ./target/debug/lintcheck --format json
run: cargo lintcheck --format json
- name: Rename JSON file - name: Upload head JSON
if: steps.cache.outputs.cache-hit != 'true' uses: actions/upload-artifact@v4
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/head.json with:
name: head
path: lintcheck-logs/lintcheck_crates_logs.json
# Retrieves `lintcheck-logs/base.json` and `lintcheck-logs/head.json` from the cache and prints # Retrieves the head and base JSON results and prints the diff to the GH actions step summary
# the diff to the GH actions step summary
diff: diff:
runs-on: ubuntu-latest runs-on: ubuntu-latest
@ -97,19 +104,15 @@ jobs:
- name: Checkout - name: Checkout
uses: actions/checkout@v4 uses: actions/checkout@v4
- name: Restore base JSON - name: Restore lintcheck bin
uses: actions/cache/restore@v4 uses: actions/cache/restore@v4
with: with:
key: ${{ needs.base.outputs.key }} path: target/debug/lintcheck
path: lintcheck-logs/base.json key: lintcheck-bin-${{ hashfiles('lintcheck/**') }}
fail-on-cache-miss: true fail-on-cache-miss: true
- name: Restore head JSON - name: Download JSON
uses: actions/cache/restore@v4 uses: actions/download-artifact@v4
with:
key: ${{ needs.head.outputs.key }}
path: lintcheck-logs/head.json
fail-on-cache-miss: true
- name: Diff results - name: Diff results
run: cargo lintcheck diff lintcheck-logs/base.json lintcheck-logs/head.json >> $GITHUB_STEP_SUMMARY run: ./target/debug/lintcheck diff {base,head}/lintcheck_crates_logs.json >> $GITHUB_STEP_SUMMARY

View File

@ -29,7 +29,7 @@
use std::hash::Hash; use std::hash::Hash;
use std::io::{self, ErrorKind}; use std::io::{self, ErrorKind};
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::process::{Command, ExitStatus}; use std::process::{Command, ExitStatus, Stdio};
use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::atomic::{AtomicUsize, Ordering};
use std::time::Duration; use std::time::Duration;
use std::{env, fs, thread}; use std::{env, fs, thread};
@ -348,7 +348,6 @@ impl Crate {
#[allow(clippy::too_many_arguments, clippy::too_many_lines)] #[allow(clippy::too_many_arguments, clippy::too_many_lines)]
fn run_clippy_lints( fn run_clippy_lints(
&self, &self,
cargo_clippy_path: &Path,
clippy_driver_path: &Path, clippy_driver_path: &Path,
target_dir_index: &AtomicUsize, target_dir_index: &AtomicUsize,
total_crates_to_lint: usize, total_crates_to_lint: usize,
@ -374,25 +373,17 @@ fn run_clippy_lints(
); );
} }
let cargo_clippy_path = fs::canonicalize(cargo_clippy_path).unwrap();
let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir"); let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir");
let mut cargo_clippy_args = if config.fix {
vec!["--quiet", "--fix", "--"]
} else {
vec!["--quiet", "--message-format=json", "--"]
};
let cargo_home = env!("CARGO_HOME"); let cargo_home = env!("CARGO_HOME");
// `src/lib.rs` -> `target/lintcheck/sources/crate-1.2.3/src/lib.rs` // `src/lib.rs` -> `target/lintcheck/sources/crate-1.2.3/src/lib.rs`
let remap_relative = format!("={}", self.path.display()); let remap_relative = format!("={}", self.path.display());
// Fallback for other sources, `~/.cargo/...` -> `$CARGO_HOME/...` // Fallback for other sources, `~/.cargo/...` -> `$CARGO_HOME/...`
let remap_cargo_home = format!("{cargo_home}=$CARGO_HOME"); let remap_cargo_home = format!("{cargo_home}=$CARGO_HOME");
// `~/.cargo/registry/src/github.com-1ecc6299db9ec823/crate-2.3.4/src/lib.rs` // `~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crate-2.3.4/src/lib.rs`
// -> `crate-2.3.4/src/lib.rs` // -> `crate-2.3.4/src/lib.rs`
let remap_crates_io = format!("{cargo_home}/registry/src/github.com-1ecc6299db9ec823/="); let remap_crates_io = format!("{cargo_home}/registry/src/index.crates.io-6f17d22bba15001f/=");
let mut clippy_args = vec![ let mut clippy_args = vec![
"--remap-path-prefix", "--remap-path-prefix",
@ -418,23 +409,23 @@ fn run_clippy_lints(
clippy_args.extend(lint_filter.iter().map(String::as_str)); clippy_args.extend(lint_filter.iter().map(String::as_str));
} }
if let Some(server) = server { let mut cmd = Command::new("cargo");
let target = shared_target_dir.join("recursive"); cmd.arg(if config.fix { "fix" } else { "check" })
.arg("--quiet")
.current_dir(&self.path)
.env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__"));
if let Some(server) = server {
// `cargo clippy` is a wrapper around `cargo check` that mainly sets `RUSTC_WORKSPACE_WRAPPER` to // `cargo clippy` is a wrapper around `cargo check` that mainly sets `RUSTC_WORKSPACE_WRAPPER` to
// `clippy-driver`. We do the same thing here with a couple changes: // `clippy-driver`. We do the same thing here with a couple changes:
// //
// `RUSTC_WRAPPER` is used instead of `RUSTC_WORKSPACE_WRAPPER` so that we can lint all crate // `RUSTC_WRAPPER` is used instead of `RUSTC_WORKSPACE_WRAPPER` so that we can lint all crate
// dependencies rather than only workspace members // dependencies rather than only workspace members
// //
// The wrapper is set to the `lintcheck` so we can force enable linting and ignore certain crates // The wrapper is set to `lintcheck` itself so we can force enable linting and ignore certain crates
// (see `crate::driver`) // (see `crate::driver`)
let status = Command::new(env::var("CARGO").unwrap_or("cargo".into())) let status = cmd
.arg("check") .env("CARGO_TARGET_DIR", shared_target_dir.join("recursive"))
.arg("--quiet")
.current_dir(&self.path)
.env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__"))
.env("CARGO_TARGET_DIR", target)
.env("RUSTC_WRAPPER", env::current_exe().unwrap()) .env("RUSTC_WRAPPER", env::current_exe().unwrap())
// Pass the absolute path so `crate::driver` can find `clippy-driver`, as it's executed in various // Pass the absolute path so `crate::driver` can find `clippy-driver`, as it's executed in various
// different working directories // different working directories
@ -446,23 +437,19 @@ fn run_clippy_lints(
assert_eq!(status.code(), Some(0)); assert_eq!(status.code(), Some(0));
return Vec::new(); return Vec::new();
};
if !config.fix {
cmd.arg("--message-format=json");
} }
cargo_clippy_args.extend(clippy_args); let all_output = cmd
let all_output = Command::new(&cargo_clippy_path)
// use the looping index to create individual target dirs // use the looping index to create individual target dirs
.env("CARGO_TARGET_DIR", shared_target_dir.join(format!("_{thread_index:?}"))) .env("CARGO_TARGET_DIR", shared_target_dir.join(format!("_{thread_index:?}")))
.args(&cargo_clippy_args) // Roughly equivalent to `cargo clippy`/`cargo clippy --fix`
.current_dir(&self.path) .env("RUSTC_WORKSPACE_WRAPPER", clippy_driver_path)
.output() .output()
.unwrap_or_else(|error| { .unwrap();
panic!(
"Encountered error:\n{error:?}\ncargo_clippy_path: {}\ncrate path:{}\n",
&cargo_clippy_path.display(),
&self.path.display()
);
});
let stdout = String::from_utf8_lossy(&all_output.stdout); let stdout = String::from_utf8_lossy(&all_output.stdout);
let stderr = String::from_utf8_lossy(&all_output.stderr); let stderr = String::from_utf8_lossy(&all_output.stderr);
let status = &all_output.status; let status = &all_output.status;
@ -509,15 +496,17 @@ fn run_clippy_lints(
} }
/// Builds clippy inside the repo to make sure we have a clippy executable we can use. /// Builds clippy inside the repo to make sure we have a clippy executable we can use.
fn build_clippy() { fn build_clippy() -> String {
let status = Command::new(env::var("CARGO").unwrap_or("cargo".into())) let output = Command::new("cargo")
.arg("build") .args(["run", "--bin=clippy-driver", "--", "--version"])
.status() .stderr(Stdio::inherit())
.expect("Failed to build clippy!"); .output()
if !status.success() { .unwrap();
if !output.status.success() {
eprintln!("Error: Failed to compile Clippy!"); eprintln!("Error: Failed to compile Clippy!");
std::process::exit(1); std::process::exit(1);
} }
String::from_utf8_lossy(&output.stdout).into_owned()
} }
/// Read a `lintcheck_crates.toml` file /// Read a `lintcheck_crates.toml` file
@ -633,26 +622,16 @@ fn main() {
#[allow(clippy::too_many_lines)] #[allow(clippy::too_many_lines)]
fn lintcheck(config: LintcheckConfig) { fn lintcheck(config: LintcheckConfig) {
println!("Compiling clippy..."); let clippy_ver = build_clippy();
build_clippy();
println!("Done compiling");
let cargo_clippy_path = fs::canonicalize(format!("target/debug/cargo-clippy{EXE_SUFFIX}")).unwrap();
let clippy_driver_path = fs::canonicalize(format!("target/debug/clippy-driver{EXE_SUFFIX}")).unwrap(); let clippy_driver_path = fs::canonicalize(format!("target/debug/clippy-driver{EXE_SUFFIX}")).unwrap();
// assert that clippy is found // assert that clippy is found
assert!( assert!(
cargo_clippy_path.is_file(), clippy_driver_path.is_file(),
"target/debug/cargo-clippy binary not found! {}", "target/debug/clippy-driver binary not found! {}",
cargo_clippy_path.display() clippy_driver_path.display()
); );
let clippy_ver = Command::new(&cargo_clippy_path)
.arg("--version")
.output()
.map(|o| String::from_utf8_lossy(&o.stdout).into_owned())
.expect("could not get clippy version!");
// download and extract the crates, then run clippy on them and collect clippy's warnings // download and extract the crates, then run clippy on them and collect clippy's warnings
// flatten into one big list of warnings // flatten into one big list of warnings
@ -715,7 +694,6 @@ fn lintcheck(config: LintcheckConfig) {
.par_iter() .par_iter()
.flat_map(|krate| { .flat_map(|krate| {
krate.run_clippy_lints( krate.run_clippy_lints(
&cargo_clippy_path,
&clippy_driver_path, &clippy_driver_path,
&counter, &counter,
crates.len(), crates.len(),
@ -914,7 +892,7 @@ fn lintcheck_test() {
"--crates-toml", "--crates-toml",
"lintcheck/test_sources.toml", "lintcheck/test_sources.toml",
]; ];
let status = Command::new(env::var("CARGO").unwrap_or("cargo".into())) let status = Command::new("cargo")
.args(args) .args(args)
.current_dir("..") // repo root .current_dir("..") // repo root
.status(); .status();