Skip to content

Commit c146b7a

Browse files
authored
Merge pull request #1711 from EliahKagan/run-ci/mode-it
Add internal-tools `check-mode`, replacing `check-mode.sh`
2 parents ee33221 + 7e8aedf commit c146b7a

File tree

9 files changed

+141
-61
lines changed

9 files changed

+141
-61
lines changed

.github/workflows/ci.yml

-9
Original file line numberDiff line numberDiff line change
@@ -359,14 +359,6 @@ jobs:
359359
- name: gix-pack with all features (including wasm)
360360
run: cd gix-pack && cargo build --all-features --target "$TARGET"
361361

362-
check-mode:
363-
runs-on: ubuntu-latest
364-
365-
steps:
366-
- uses: actions/checkout@v4
367-
- name: Find scripts with mode/shebang mismatch
368-
run: etc/check-mode.sh
369-
370362
check-packetline:
371363
strategy:
372364
fail-fast: false
@@ -449,7 +441,6 @@ jobs:
449441
- test-32bit-cross
450442
- lint
451443
- cargo-deny
452-
- check-mode
453444
- check-packetline
454445
- check-blocking
455446

Cargo.lock

+2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

etc/check-mode.sh

-47
This file was deleted.

justfile

+5-3
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ alias c := check
1010
alias nt := nextest
1111

1212
# run all tests, clippy, including journey tests, try building docs
13-
test: clippy check doc unit-tests journey-tests-pure journey-tests-small journey-tests-async journey-tests
13+
test: clippy check doc unit-tests journey-tests-pure journey-tests-small journey-tests-async journey-tests check-mode
1414

1515
# run all tests, without clippy, and try building docs
16-
ci-test: check doc unit-tests
16+
ci-test: check doc unit-tests check-mode
1717

1818
# run all journey tests - should be run in a fresh clone or after `cargo clean`
1919
ci-journey-tests: journey-tests-pure journey-tests-small journey-tests-async journey-tests
@@ -196,6 +196,7 @@ target_dir := `cargo metadata --format-version 1 | jq -r .target_directory`
196196
ein := target_dir / "debug/ein"
197197
gix := target_dir / "debug/gix"
198198
jtt := target_dir / "debug/jtt"
199+
it := target_dir / "debug/it"
199200

200201
# run journey tests (max)
201202
journey-tests:
@@ -257,7 +258,8 @@ find-yanked:
257258

258259
# Find shell scripts whose +x/-x bits and magic bytes (e.g. `#!`) disagree
259260
check-mode:
260-
./etc/check-mode.sh
261+
cargo build -p internal-tools
262+
"{{ it }}" check-mode
261263

262264
# Delete gix-packetline-blocking/src and regenerate from gix-packetline/src
263265
copy-packetline:

tests/it/Cargo.toml

+3-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ name = "it"
1414
path = "src/main.rs"
1515

1616
[dependencies]
17-
clap = { version = "4.5.16", features = ["derive"] }
1817
anyhow = "1.0.86"
19-
18+
clap = { version = "4.5.16", features = ["derive"] }
2019
gix = { version = "^0.68.0", path = "../../gix", default-features = false, features = ["attributes", "revision"] }
20+
once_cell = "1.20.2"
21+
regex = { version = "1.11.1", default-features = false, features = ["std"] }

tests/it/src/args.rs

+11
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,17 @@ pub enum Subcommands {
6262
#[clap(value_parser = AsPathSpec)]
6363
patterns: Vec<gix::pathspec::Pattern>,
6464
},
65+
/// Check for executable bits that disagree with shebangs.
66+
///
67+
/// This checks committed and staged files, but not anything unstaged, to find shell scripts
68+
/// that either begin with a `#!` but not `+x` permissions, or do not begin with `#!` but do
69+
/// have `+x` permissions. Such mismatches are reported but not automatically corrected. Some
70+
/// platforms (at least Windows) do not have such permissions, but Git still represents them.
71+
///
72+
/// This currently only checks files name with an `.sh` suffix, and only operates on the
73+
/// current repository. Its main use is checking that fixture scripts are have correct modes.
74+
#[clap(visible_alias = "cm")]
75+
CheckMode {},
6576
}
6677

6778
#[derive(Clone)]

tests/it/src/commands/check_mode.rs

+116
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
pub(super) mod function {
2+
use anyhow::{bail, Context};
3+
use gix::bstr::ByteSlice;
4+
use once_cell::sync::Lazy;
5+
use regex::bytes::Regex;
6+
use std::ffi::{OsStr, OsString};
7+
use std::io::{BufRead, BufReader, Read};
8+
use std::process::{Command, Stdio};
9+
10+
pub fn check_mode() -> anyhow::Result<()> {
11+
let root = find_root()?;
12+
let mut any_mismatch = false;
13+
14+
let mut child = git_on(&root)
15+
.args(["ls-files", "-sz", "--", "*.sh"])
16+
.stdout(Stdio::piped())
17+
.spawn()
18+
.context("Can't start `git` subprocess to list index")?;
19+
20+
let stdout = child.stdout.take().expect("should have captured stdout");
21+
for result in BufReader::new(stdout).split(0) {
22+
let record = result.context(r"Can't read '\0'-terminated record")?;
23+
any_mismatch |= check_for_mismatch(&root, &record)?;
24+
}
25+
26+
let status = child.wait().context("Failure running `git` subprocess to list index")?;
27+
if !status.success() {
28+
bail!("`git` subprocess to list index did not complete successfully");
29+
}
30+
if any_mismatch {
31+
bail!("Mismatch found - scan completed, finding at least one `#!` vs. `+x` mismatch");
32+
}
33+
Ok(())
34+
}
35+
36+
/// Find the top-level directory of the current repository working tree.
37+
fn find_root() -> anyhow::Result<OsString> {
38+
let output = Command::new(gix::path::env::exe_invocation())
39+
.args(["rev-parse", "--show-toplevel"])
40+
.output()
41+
.context("Can't run `git` to find worktree root")?;
42+
43+
if !output.status.success() {
44+
bail!("`git` failed to find worktree root");
45+
}
46+
47+
let root = output
48+
.stdout
49+
.strip_suffix(b"\n")
50+
.context("Can't parse worktree root")?
51+
.to_os_str()?
52+
.to_owned();
53+
Ok(root)
54+
}
55+
56+
/// Prepare a `git` command, passing `root` as an operand to `-C`.
57+
///
58+
/// This is suitable when `git` gave us the path `root`. Then it should already be in a form
59+
/// where `git -C` will be able to use it, without alteration, regardless of the platform.
60+
/// (Otherwise, it may be preferable to set `root` as the `cwd` of the `git` process instead.)
61+
fn git_on(root: &OsStr) -> Command {
62+
let mut cmd = Command::new(gix::path::env::exe_invocation());
63+
cmd.arg("-C").arg(root);
64+
cmd
65+
}
66+
67+
/// On mismatch, report it and return `Some(true)`.
68+
fn check_for_mismatch(root: &OsStr, record: &[u8]) -> anyhow::Result<bool> {
69+
static RECORD_REGEX: Lazy<Regex> = Lazy::new(|| {
70+
let pattern = r"(?-u)\A([0-7]+) ([[:xdigit:]]+) [[:digit:]]+\t(.+)\z";
71+
Regex::new(pattern).expect("regex should be valid")
72+
});
73+
74+
let fields = RECORD_REGEX.captures(record).context("Malformed record from `git`")?;
75+
let mode = fields.get(1).expect("match should get mode").as_bytes();
76+
let oid = fields
77+
.get(2)
78+
.expect("match should get oid")
79+
.as_bytes()
80+
.to_os_str()
81+
.expect("oid field verified as hex digits, should be valid OsStr");
82+
let path = fields.get(3).expect("match should get path").as_bytes().as_bstr();
83+
84+
match mode {
85+
b"100644" if blob_has_shebang(root, oid)? => {
86+
println!("mode -x but has shebang: {path:?}");
87+
Ok(true)
88+
}
89+
b"100755" if !blob_has_shebang(root, oid)? => {
90+
println!("mode +x but no shebang: {path:?}");
91+
Ok(true)
92+
}
93+
_ => Ok(false),
94+
}
95+
}
96+
97+
fn blob_has_shebang(root: &OsStr, oid: &OsStr) -> anyhow::Result<bool> {
98+
let mut buf = [0u8; 2];
99+
100+
let mut child = git_on(root)
101+
.args(["cat-file", "blob"])
102+
.arg(oid)
103+
.stdout(Stdio::piped())
104+
.spawn()
105+
.context("Can't start `git` subprocess to read blob")?;
106+
107+
let mut stdout = child.stdout.take().expect("should have captured stdout");
108+
let count = stdout.read(&mut buf).context("Error reading data from blob")?;
109+
drop(stdout); // Let the pipe break rather than waiting for the rest of the blob.
110+
111+
// TODO: Maybe check status? On Unix, it should be 0 or SIGPIPE. Not sure about Windows.
112+
_ = child.wait().context("Failure running `git` subprocess to read blob")?;
113+
114+
Ok(&buf[..count] == b"#!")
115+
}
116+
}

tests/it/src/commands/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,6 @@ pub use copy_royal::function::copy_royal;
33

44
pub mod git_to_sh;
55
pub use git_to_sh::function::git_to_sh;
6+
7+
pub mod check_mode;
8+
pub use check_mode::function::check_mode;

tests/it/src/main.rs

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ fn main() -> anyhow::Result<()> {
3131
destination_dir,
3232
patterns,
3333
} => commands::copy_royal(dry_run, &worktree_root, destination_dir, patterns),
34+
Subcommands::CheckMode {} => commands::check_mode(),
3435
}
3536
}
3637

0 commit comments

Comments
 (0)