Skip to content

Commit 2194304

Browse files
AlexendooAlexendoo
committed
Cache lintcheck binary in ci
1 parent 0ce07f6 commit 2194304

File tree

2 files changed

+77
-96
lines changed

2 files changed

+77
-96
lines changed

‎.github/workflows/lintcheck.yml

+44-41
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,10 @@ concurrency:
1212
cancel-in-progress: true
1313

1414
jobs:
15-
# Generates `lintcheck-logs/base.json` and stores it in a cache
15+
# Runs lintcheck on the PR's target branch and stores the results as an artifact
1616
base:
1717
runs-on: ubuntu-latest
1818

19-
outputs:
20-
key: ${{ steps.key.outputs.key }}
21-
2219
steps:
2320
- name: Checkout
2421
uses: actions/checkout@v4
@@ -37,57 +34,67 @@ jobs:
3734
rm -rf lintcheck
3835
git checkout ${{ github.sha }} -- lintcheck
3936
37+
- name: Cache lintcheck bin
38+
id: cache-lintcheck-bin
39+
uses: actions/cache@v4
40+
with:
41+
path: target/debug/lintcheck
42+
key: lintcheck-bin-${{ hashfiles('lintcheck/**') }}
43+
44+
- name: Build lintcheck
45+
if: steps.cache-lintcheck-bin.outputs.cache-hit != 'true'
46+
run: cargo build --manifest-path=lintcheck/Cargo.toml
47+
4048
- name: Create cache key
4149
id: key
4250
run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
4351

44-
- name: Cache results
45-
id: cache
52+
- name: Cache results JSON
53+
id: cache-json
4654
uses: actions/cache@v4
4755
with:
48-
path: lintcheck-logs/base.json
56+
path: lintcheck-logs/lintcheck_crates_logs.json
4957
key: ${{ steps.key.outputs.key }}
5058

5159
- name: Run lintcheck
52-
if: steps.cache.outputs.cache-hit != 'true'
53-
run: cargo lintcheck --format json
60+
if: steps.cache-json.outputs.cache-hit != 'true'
61+
run: ./target/debug/lintcheck --format json
5462

55-
- name: Rename JSON file
56-
if: steps.cache.outputs.cache-hit != 'true'
57-
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/base.json
63+
- name: Upload base JSON
64+
uses: actions/upload-artifact@v4
65+
with:
66+
name: base
67+
path: lintcheck-logs/lintcheck_crates_logs.json
5868

59-
# Generates `lintcheck-logs/head.json` and stores it in a cache
69+
# Runs lintcheck on the PR and stores the results as an artifact
6070
head:
6171
runs-on: ubuntu-latest
6272

63-
outputs:
64-
key: ${{ steps.key.outputs.key }}
65-
6673
steps:
6774
- name: Checkout
6875
uses: actions/checkout@v4
6976

70-
- name: Create cache key
71-
id: key
72-
run: echo "key=lintcheck-head-${{ github.sha }}" >> "$GITHUB_OUTPUT"
73-
74-
- name: Cache results
75-
id: cache
77+
- name: Cache lintcheck bin
78+
id: cache-lintcheck-bin
7679
uses: actions/cache@v4
7780
with:
78-
path: lintcheck-logs/head.json
79-
key: ${{ steps.key.outputs.key }}
81+
path: target/debug/lintcheck
82+
key: lintcheck-bin-${{ hashfiles('lintcheck/**') }}
83+
84+
- name: Build lintcheck
85+
if: steps.cache-lintcheck-bin.outputs.cache-hit != 'true'
86+
run: cargo build --manifest-path=lintcheck/Cargo.toml
8087

8188
- name: Run lintcheck
82-
if: steps.cache.outputs.cache-hit != 'true'
83-
run: cargo lintcheck --format json
89+
run: ./target/debug/lintcheck --format json
8490

85-
- name: Rename JSON file
86-
if: steps.cache.outputs.cache-hit != 'true'
87-
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/head.json
91+
- name: Upload head JSON
92+
uses: actions/upload-artifact@v4
93+
with:
94+
name: head
95+
path: lintcheck-logs/lintcheck_crates_logs.json
8896

89-
# Retrieves `lintcheck-logs/base.json` and `lintcheck-logs/head.json` from the cache and prints
90-
# the diff to the GH actions step summary
97+
# Retrieves the head and base JSON results and prints the diff to the GH actions step summary
9198
diff:
9299
runs-on: ubuntu-latest
93100

@@ -97,19 +104,15 @@ jobs:
97104
- name: Checkout
98105
uses: actions/checkout@v4
99106

100-
- name: Restore base JSON
107+
- name: Restore lintcheck bin
101108
uses: actions/cache/restore@v4
102109
with:
103-
key: ${{ needs.base.outputs.key }}
104-
path: lintcheck-logs/base.json
110+
path: target/debug/lintcheck
111+
key: lintcheck-bin-${{ hashfiles('lintcheck/**') }}
105112
fail-on-cache-miss: true
106113

107-
- name: Restore head JSON
108-
uses: actions/cache/restore@v4
109-
with:
110-
key: ${{ needs.head.outputs.key }}
111-
path: lintcheck-logs/head.json
112-
fail-on-cache-miss: true
114+
- name: Download JSON
115+
uses: actions/download-artifact@v4
113116

114117
- name: Diff results
115-
run: cargo lintcheck diff lintcheck-logs/base.json lintcheck-logs/head.json >> $GITHUB_STEP_SUMMARY
118+
run: ./target/debug/lintcheck diff {base,head}/lintcheck_crates_logs.json >> $GITHUB_STEP_SUMMARY

‎lintcheck/src/main.rs

+33-55
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use std::fmt::{self, Display, Write as _};
2929
use std::hash::Hash;
3030
use std::io::{self, ErrorKind};
3131
use std::path::{Path, PathBuf};
32-
use std::process::{Command, ExitStatus};
32+
use std::process::{Command, ExitStatus, Stdio};
3333
use std::sync::atomic::{AtomicUsize, Ordering};
3434
use std::time::Duration;
3535
use std::{env, fs, thread};
@@ -348,7 +348,6 @@ impl Crate {
348348
#[allow(clippy::too_many_arguments, clippy::too_many_lines)]
349349
fn run_clippy_lints(
350350
&self,
351-
cargo_clippy_path: &Path,
352351
clippy_driver_path: &Path,
353352
target_dir_index: &AtomicUsize,
354353
total_crates_to_lint: usize,
@@ -374,25 +373,17 @@ impl Crate {
374373
);
375374
}
376375

377-
let cargo_clippy_path = fs::canonicalize(cargo_clippy_path).unwrap();
378-
379376
let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir");
380377

381-
let mut cargo_clippy_args = if config.fix {
382-
vec!["--quiet", "--fix", "--"]
383-
} else {
384-
vec!["--quiet", "--message-format=json", "--"]
385-
};
386-
387378
let cargo_home = env!("CARGO_HOME");
388379

389380
// `src/lib.rs` -> `target/lintcheck/sources/crate-1.2.3/src/lib.rs`
390381
let remap_relative = format!("={}", self.path.display());
391382
// Fallback for other sources, `~/.cargo/...` -> `$CARGO_HOME/...`
392383
let remap_cargo_home = format!("{cargo_home}=$CARGO_HOME");
393-
// `~/.cargo/registry/src/github.com-1ecc6299db9ec823/crate-2.3.4/src/lib.rs`
384+
// `~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crate-2.3.4/src/lib.rs`
394385
// -> `crate-2.3.4/src/lib.rs`
395-
let remap_crates_io = format!("{cargo_home}/registry/src/github.com-1ecc6299db9ec823/=");
386+
let remap_crates_io = format!("{cargo_home}/registry/src/index.crates.io-6f17d22bba15001f/=");
396387

397388
let mut clippy_args = vec![
398389
"--remap-path-prefix",
@@ -418,23 +409,23 @@ impl Crate {
418409
clippy_args.extend(lint_filter.iter().map(String::as_str));
419410
}
420411

421-
if let Some(server) = server {
422-
let target = shared_target_dir.join("recursive");
412+
let mut cmd = Command::new("cargo");
413+
cmd.arg(if config.fix { "fix" } else { "check" })
414+
.arg("--quiet")
415+
.current_dir(&self.path)
416+
.env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__"));
423417

418+
if let Some(server) = server {
424419
// `cargo clippy` is a wrapper around `cargo check` that mainly sets `RUSTC_WORKSPACE_WRAPPER` to
425420
// `clippy-driver`. We do the same thing here with a couple changes:
426421
//
427422
// `RUSTC_WRAPPER` is used instead of `RUSTC_WORKSPACE_WRAPPER` so that we can lint all crate
428423
// dependencies rather than only workspace members
429424
//
430-
// The wrapper is set to the `lintcheck` so we can force enable linting and ignore certain crates
425+
// The wrapper is set to `lintcheck` itself so we can force enable linting and ignore certain crates
431426
// (see `crate::driver`)
432-
let status = Command::new(env::var("CARGO").unwrap_or("cargo".into()))
433-
.arg("check")
434-
.arg("--quiet")
435-
.current_dir(&self.path)
436-
.env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__"))
437-
.env("CARGO_TARGET_DIR", target)
427+
let status = cmd
428+
.env("CARGO_TARGET_DIR", shared_target_dir.join("recursive"))
438429
.env("RUSTC_WRAPPER", env::current_exe().unwrap())
439430
// Pass the absolute path so `crate::driver` can find `clippy-driver`, as it's executed in various
440431
// different working directories
@@ -446,23 +437,19 @@ impl Crate {
446437
assert_eq!(status.code(), Some(0));
447438

448439
return Vec::new();
449-
}
440+
};
450441

451-
cargo_clippy_args.extend(clippy_args);
442+
if !config.fix {
443+
cmd.arg("--message-format=json");
444+
}
452445

453-
let all_output = Command::new(&cargo_clippy_path)
446+
let all_output = cmd
454447
// use the looping index to create individual target dirs
455448
.env("CARGO_TARGET_DIR", shared_target_dir.join(format!("_{thread_index:?}")))
456-
.args(&cargo_clippy_args)
457-
.current_dir(&self.path)
449+
// Roughly equivalent to `cargo clippy`/`cargo clippy --fix`
450+
.env("RUSTC_WORKSPACE_WRAPPER", clippy_driver_path)
458451
.output()
459-
.unwrap_or_else(|error| {
460-
panic!(
461-
"Encountered error:\n{error:?}\ncargo_clippy_path: {}\ncrate path:{}\n",
462-
&cargo_clippy_path.display(),
463-
&self.path.display()
464-
);
465-
});
452+
.unwrap();
466453
let stdout = String::from_utf8_lossy(&all_output.stdout);
467454
let stderr = String::from_utf8_lossy(&all_output.stderr);
468455
let status = &all_output.status;
@@ -509,15 +496,17 @@ impl Crate {
509496
}
510497

511498
/// Builds clippy inside the repo to make sure we have a clippy executable we can use.
512-
fn build_clippy() {
513-
let status = Command::new(env::var("CARGO").unwrap_or("cargo".into()))
514-
.arg("build")
515-
.status()
516-
.expect("Failed to build clippy!");
517-
if !status.success() {
499+
fn build_clippy() -> String {
500+
let output = Command::new("cargo")
501+
.args(["run", "--bin=clippy-driver", "--", "--version"])
502+
.stderr(Stdio::inherit())
503+
.output()
504+
.unwrap();
505+
if !output.status.success() {
518506
eprintln!("Error: Failed to compile Clippy!");
519507
std::process::exit(1);
520508
}
509+
String::from_utf8_lossy(&output.stdout).into_owned()
521510
}
522511

523512
/// Read a `lintcheck_crates.toml` file
@@ -633,26 +622,16 @@ fn main() {
633622

634623
#[allow(clippy::too_many_lines)]
635624
fn lintcheck(config: LintcheckConfig) {
636-
println!("Compiling clippy...");
637-
build_clippy();
638-
println!("Done compiling");
639-
640-
let cargo_clippy_path = fs::canonicalize(format!("target/debug/cargo-clippy{EXE_SUFFIX}")).unwrap();
625+
let clippy_ver = build_clippy();
641626
let clippy_driver_path = fs::canonicalize(format!("target/debug/clippy-driver{EXE_SUFFIX}")).unwrap();
642627

643628
// assert that clippy is found
644629
assert!(
645-
cargo_clippy_path.is_file(),
646-
"target/debug/cargo-clippy binary not found! {}",
647-
cargo_clippy_path.display()
630+
clippy_driver_path.is_file(),
631+
"target/debug/clippy-driver binary not found! {}",
632+
clippy_driver_path.display()
648633
);
649634

650-
let clippy_ver = Command::new(&cargo_clippy_path)
651-
.arg("--version")
652-
.output()
653-
.map(|o| String::from_utf8_lossy(&o.stdout).into_owned())
654-
.expect("could not get clippy version!");
655-
656635
// download and extract the crates, then run clippy on them and collect clippy's warnings
657636
// flatten into one big list of warnings
658637

@@ -715,7 +694,6 @@ fn lintcheck(config: LintcheckConfig) {
715694
.par_iter()
716695
.flat_map(|krate| {
717696
krate.run_clippy_lints(
718-
&cargo_clippy_path,
719697
&clippy_driver_path,
720698
&counter,
721699
crates.len(),
@@ -914,7 +892,7 @@ fn lintcheck_test() {
914892
"--crates-toml",
915893
"lintcheck/test_sources.toml",
916894
];
917-
let status = Command::new(env::var("CARGO").unwrap_or("cargo".into()))
895+
let status = Command::new("cargo")
918896
.args(args)
919897
.current_dir("..") // repo root
920898
.status();

0 commit comments

Comments
 (0)