Implement check-executables-have-shebangs as builtin-hook

[feliblo]

Description

Following the information provided in 880. This is an implementation of the check-executables-have-shebangs hook.

The check-executables-have-shebangs has 1k grep.app hits. It's the final unimplemented hook used by airflow as mentioned in this comment.

---

Notes

• Windows; I'm having trouble testing this on windows, since .sh files aren't seen as executable and setting them as executable using git add --chmod=+x {file_name}. See [bug] Executables not correctly identified on windows #928
• Windows32
  • Currently we do not check windows32. In this PR and the original pre-commit implementation, it takes a completely different route than UNIX based on the return value of git config core.fileMode (returns false only on win32`
  • 2 options:
    1. Add win32 to the ci/cd pipeline
    2. We need to be able to overwrite the output of this command in tests (like one would in python)

# git config core.fileMode is used to determine UNIX vs win32
let status = Command::new("git")
      .arg("config")
      .arg("core.fileMode")
      .current_dir(repo_path)
      .status()?;

• Bugfix
  • Fixed a bug in identify.rs. Where the first element of a shebangs was being accessed. However the parse_shebang function can still return an Ok with an empty vector, making the access fail.
  • This caused prek to panic when running against the ardupilot repository.
  • I added the check in identify.rs but the parse_shebang function still contains a Todo.
  • Also described in: [bug] parse_shebang can return an empty list #925

Memory changes

cargo clean
cargo build --release 

• Master branch:  8.49 MB
• Feature branch: 8.58MB
  +89,800 bytes (+1.06%) increase from master to feature branch

---

Performance

Ran these commands on the ardupilot repository:
simply because it was the top-hit on grap.app

Linux

# pre-commit
pre-commit run check-executables-have-shebangs --all-files --verbose
check that executables have shebangs.....................................Passed
- hook id: check-executables-have-shebangs
- duration: 0.09s

# Old prek implementation (python)
prek run check-executables-have-shebangs --all-files --verbose
thread '<unnamed>' panicked at src/identify.rs:767:55:
index out of bounds: the len is 0 but the index is 0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

# Old prek implementation bugfixed (python)
prek run check-executables-have-shebangs --all-files --verbose
check that executables have shebangs.....................................Passed
- hook id: check-executables-have-shebangs
- duration: 0.07s

# New implementation (rust)
rek run check-executables-have-shebangs --all-files --verbose
check that executables have shebangs.....................................Passed
- hook id: check-executables-have-shebangs
- duration: 0.01s

Linux output:

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.07%. Comparing base (8470e04) to head (4d658d8).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #924      +/-   ##
==========================================
+ Coverage   89.93%   90.07%   +0.13%     
==========================================
  Files          66       67       +1     
  Lines       12223    12391     +168     
==========================================
+ Hits        10993    11161     +168     
  Misses       1230     1230              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[feliblo]

I'm having some trouble setting .sh files as executable on windows, even when using git chmod. I will boot up my windows pc later and take a look.

It's almost certainly due to line 750 in indentify.rs:

    #[cfg(not(unix))]
    {
        executable = {
            let ext = path.extension().and_then(|ext| ext.to_str());
            ext.is_some_and(|ext| ext == "exe" || ext == "bat" || ext == "cmd")
        };
    }

edit: created this issue: #928

---

With the other bug mentioned above it could be too much for just 1 PR though.

[j178]

Thank you! I'll take a look soon.

[j178]

I'll try to resolve #928, then we can get this merge.

[feliblo]

I'll try to resolve #928, then we can get this merge.

I mentioned it in the other PR. But the way pre-commit resolves this in windows is, send all files to the hook and resolve for those marked as executable in the hook itself. Hence it always returning passed in the output above.

[github-actions]

📦 Cargo Bloat Comparison

Binary size change: +0.62% (16.1 MiB → 16.2 MiB)

Expand for cargo-bloat output

Head Branch Results

 File  .text     Size          Crate Name
 0.6%   1.3% 101.1KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.6%   1.3%  98.1KiB           prek prek::builtin::pre_commit_hooks::Implemented::run::{{closure}}
 0.5%   1.2%  88.0KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}
 0.4%   0.8%  63.8KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.7%  49.2KiB           prek prek::run::{{closure}}
 0.3%   0.6%  42.0KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  40.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  40.1KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.4%  32.9KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  32.8KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  32.3KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  31.8KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.4%  31.7KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.4%  27.6KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.3%  26.2KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::send_request::{{closure}}
 0.2%   0.3%  25.8KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::connect_to::{{closure}}::{{closure}}::{{closure}}
 0.2%   0.3%  25.0KiB        globset globset::GlobSetBuilder::build
 0.2%   0.3%  25.0KiB           prek prek::main
 0.1%   0.3%  24.2KiB           prek prek::languages::golang::installer::GoInstaller::install::{{closure}}
 0.1%   0.3%  23.7KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
39.8%  87.5%   6.4MiB                And 10221 smaller methods. Use -n N to show more.
45.4% 100.0%   7.3MiB                .text section size, the file size is 16.2MiB

Base Branch Results

 File  .text     Size          Crate Name
 0.6%   1.3% 101.1KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.5%   1.2%  88.0KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}
 0.5%   1.1%  85.8KiB           prek prek::builtin::pre_commit_hooks::Implemented::run::{{closure}}
 0.4%   0.9%  63.8KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.7%  49.2KiB           prek prek::run::{{closure}}
 0.3%   0.6%  42.0KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  40.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  40.1KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.4%  32.9KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  32.8KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  32.3KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  31.8KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.4%  31.7KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.4%  27.6KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.3%  26.2KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::send_request::{{closure}}
 0.2%   0.3%  25.8KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::connect_to::{{closure}}::{{closure}}::{{closure}}
 0.2%   0.3%  25.0KiB        globset globset::GlobSetBuilder::build
 0.2%   0.3%  25.0KiB           prek prek::main
 0.1%   0.3%  24.2KiB           prek prek::languages::golang::installer::GoInstaller::install::{{closure}}
 0.1%   0.3%  23.7KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
39.8%  87.7%   6.4MiB                And 10186 smaller methods. Use -n N to show more.
45.4% 100.0%   7.3MiB                .text section size, the file size is 16.1MiB

[j178]

Thank you!