Skip to content

Refactor unsafe usage and improve documentation in prek-pty#2078

Open
nikosavola wants to merge 4 commits into
j178:masterfrom
nikosavola:nikosavola/push-mmupqzpuvxzl
Open

Refactor unsafe usage and improve documentation in prek-pty#2078
nikosavola wants to merge 4 commits into
j178:masterfrom
nikosavola:nikosavola/push-mmupqzpuvxzl

Conversation

@nikosavola
Copy link
Copy Markdown
Contributor

@nikosavola nikosavola commented May 13, 2026

This pull request adds missing safety comments to usages of unsafe code throughout the codebase. Additionally, one unsafe section can be circumvented by using rustix, which is already a dependency.

Copilot AI review requested due to automatic review settings May 13, 2026 13:47
@nikosavola nikosavola requested a review from j178 as a code owner May 13, 2026 13:47
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.33%. Comparing base (99ac4fb) to head (63f1bee).

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #2078   +/-   ##
=======================================
  Coverage   92.33%   92.33%           
=======================================
  Files         119      119           
  Lines       24436    24436           
=======================================
+ Hits        22562    22563    +1     
+ Misses       1874     1873    -1

☔ 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.

Copilot AI reviewed May 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR documents existing unsafe usages across prek/prek-pty with added SAFETY comments, and replaces a small libc-based UID/GID query with rustix in the Docker language runner.

Changes:

  • Added SAFETY justifications around unsafe calls (env var mutation, sysconf, kill, PTY FD wrappers, ioctl usage).
  • Switched Docker UID/GID lookup from libc::{geteuid,getegid} to rustix::process::{geteuid,getegid}.
  • Expanded test-only environment-mutation safety notes in the Ruby installer tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/prek/tests/run.rs Adds SAFETY comment for libc::kill used in an interrupt/restore integration test.
crates/prek/src/run.rs Adds SAFETY comments for libc::sysconf calls used to compute CLI limits on Unix.
crates/prek/src/main.rs Adds SAFETY comment for unsafe std::env::set_var of GIT_WORK_TREE.
crates/prek/src/languages/ruby/installer.rs Adds SAFETY comments for unsafe env var mutations in tests guarded by a mutex/RAII.
crates/prek/src/languages/docker.rs Replaces libc UID/GID calls with rustix equivalents.
crates/prek/src/cli/self_update.rs Adds SAFETY comment for unsafe env::set_var used to enable verbose installer output.
crates/prek/src/cli/run/run.rs Adds SAFETY comment for unsafe env var mutations used to emulate pre-commit hook env.
crates/prek-pty/src/sys.rs Adds SAFETY comments for unsafe FD constructors and for borrow_raw passed to ioctl_tiocsctty.
crates/prek-pty/src/pty.rs Adds SAFETY comments for unsafe from_fd wrappers around PTY/PTS construction.

Comment thread crates/prek-pty/src/sys.rs
Comment on lines 150 to 158
let pts_fd = self.0.as_raw_fd();
move || {
rustix::process::setsid()?;
// SAFETY: pts_fd is a valid file descriptor because it's owned
// by Pts, which must remain valid for the duration of the
// session leader setup (typically in pre_exec).
rustix::process::ioctl_tiocsctty(unsafe {
std::os::fd::BorrowedFd::borrow_raw(pts_fd)
})?;
Comment thread crates/prek/src/main.rs
Comment on lines 212 to 215
debug!("Setting {} to `{}`", EnvVars::GIT_WORK_TREE, cwd.display());
// SAFETY: We are in the initialization phase and there are no other
// threads running that could be concurrently accessing the environment.
unsafe { std::env::set_var(EnvVars::GIT_WORK_TREE, cwd) }
Comment thread crates/prek/src/cli/run/run.rs
Comment on lines 219 to 224
// `pre-commit` sets these environment variables for other git hooks.
fn set_env_vars(from_ref: Option<&String>, to_ref: Option<&String>, args: &RunExtraArgs) {
// SAFETY: We are in the initialization phase and there are no other
// threads running that could be concurrently accessing the environment.
unsafe {
std::env::set_var("PRE_COMMIT", "1");
Comment thread crates/prek/src/cli/self_update.rs
Comment on lines 58 to 63
let mut updater = AxoUpdater::new_for("prek");
if enabled!(tracing::Level::DEBUG) {
// SAFETY: We are in the command execution phase and there are no other
// threads running that could be concurrently accessing the environment.
unsafe { env::set_var("INSTALLER_PRINT_VERBOSE", "1") };
updater.enable_installer_output();
@prek-ci-bot
Copy link
Copy Markdown

prek-ci-bot Bot commented May 13, 2026

📦 Cargo Bloat Comparison

Binary size change: +0.00% (26.1 MiB → 26.1 MiB)

Expand for cargo-bloat output

Head Branch Results

 File  .text     Size             Crate Name
 1.2%   2.6% 332.0KiB        aws_lc_sys aws_lc_0_39_1_aes_gcm_encrypt_avx512
 1.2%   2.6% 332.0KiB        aws_lc_sys aws_lc_0_39_1_aes_gcm_decrypt_avx512
 0.3%   0.7%  90.9KiB              prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.3%   0.7%  83.8KiB              prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.2%   0.5%  65.5KiB              prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  60.3KiB             prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.2%   0.4%  53.3KiB annotate_snippets annotate_snippets::renderer::render::render
 0.2%   0.4%  48.8KiB              prek prek::run::{{closure}}
 0.2%   0.3%  43.8KiB              prek prek::cli::run::run::run::{{closure}}
 0.1%   0.3%  32.5KiB             prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.1%   0.2%  32.1KiB             prek? <prek::config::_::<impl serde_core::de::Deserialize for prek::config::Config>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map
 0.1%   0.2%  28.4KiB               std core::ptr::drop_in_place<prek::languages::<impl prek::config::Language>::install::{{closure}}>
 0.1%   0.2%  28.0KiB        aws_lc_sys aws_lc_0_39_1_edwards25519_scalarmuldouble_alt
 0.1%   0.2%  27.6KiB      serde_saphyr saphyr_parser_bw::scanner::Scanner<T>::fetch_more_tokens
 0.1%   0.2%  27.5KiB        aws_lc_sys aws_lc_0_39_1_edwards25519_scalarmuldouble
 0.1%   0.2%  26.4KiB              prek prek::cli::try_repo::try_repo::{{closure}}
 0.1%   0.2%  24.5KiB              prek prek::hooks::meta_hooks::MetaHooks::run::{{closure}}
 0.1%   0.2%  23.5KiB             prek? <prek::cli::Command as clap_builder::derive::FromArgMatches>::from_arg_matches_mut
 0.1%   0.2%  22.5KiB      serde_saphyr saphyr_parser_bw::scanner::Scanner<T>::fetch_more_tokens
 0.1%   0.2%  22.3KiB         [Unknown] Lp384_montjscalarmul_alt_p384_montjadd
41.6%  86.3%  10.9MiB                   And 23907 smaller methods. Use -n N to show more.
48.1% 100.0%  12.6MiB                   .text section size, the file size is 26.1MiB

Base Branch Results

 File  .text     Size             Crate Name
 1.2%   2.6% 332.0KiB        aws_lc_sys aws_lc_0_39_1_aes_gcm_encrypt_avx512
 1.2%   2.6% 332.0KiB        aws_lc_sys aws_lc_0_39_1_aes_gcm_decrypt_avx512
 0.3%   0.7%  91.9KiB              prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.3%   0.7%  83.8KiB              prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.2%   0.5%  65.5KiB              prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  60.3KiB             prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.2%   0.4%  53.3KiB annotate_snippets annotate_snippets::renderer::render::render
 0.2%   0.4%  48.8KiB              prek prek::run::{{closure}}
 0.2%   0.3%  43.4KiB              prek prek::cli::run::run::run::{{closure}}
 0.1%   0.3%  32.5KiB             prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.1%   0.2%  32.1KiB             prek? <prek::config::_::<impl serde_core::de::Deserialize for prek::config::Config>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map
 0.1%   0.2%  28.4KiB               std core::ptr::drop_in_place<prek::languages::<impl prek::config::Language>::install::{{closure}}>
 0.1%   0.2%  28.0KiB        aws_lc_sys aws_lc_0_39_1_edwards25519_scalarmuldouble_alt
 0.1%   0.2%  27.6KiB      serde_saphyr saphyr_parser_bw::scanner::Scanner<T>::fetch_more_tokens
 0.1%   0.2%  27.5KiB        aws_lc_sys aws_lc_0_39_1_edwards25519_scalarmuldouble
 0.1%   0.2%  26.4KiB              prek prek::cli::try_repo::try_repo::{{closure}}
 0.1%   0.2%  24.5KiB              prek prek::hooks::meta_hooks::MetaHooks::run::{{closure}}
 0.1%   0.2%  23.5KiB             prek? <prek::cli::Command as clap_builder::derive::FromArgMatches>::from_arg_matches_mut
 0.1%   0.2%  22.5KiB      serde_saphyr saphyr_parser_bw::scanner::Scanner<T>::fetch_more_tokens
 0.1%   0.2%  22.3KiB         [Unknown] Lp384_montjscalarmul_alt_p384_montjadd
41.6%  86.3%  10.9MiB                   And 23905 smaller methods. Use -n N to show more.
48.1% 100.0%  12.6MiB                   .text section size, the file size is 26.1MiB

@prek-ci-bot
Copy link
Copy Markdown

prek-ci-bot Bot commented May 13, 2026

⚡️ Hyperfine Benchmarks

Summary: 4 regressions, 4 improvements above the 10% threshold.

Environment
  • OS: Linux 6.17.0-1010-azure
  • CPU: 4 cores
  • prek version: prek 0.3.13+21 (ad9a852 2026-05-13)
  • Rust version: rustc 1.95.0 (59807616e 2026-04-14)
  • Hyperfine version: hyperfine 1.20.0
CLI Commands

Benchmarking basic commands in the main repo:

prek --version

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base --version 1.9 ± 0.3 1.6 3.7 1.04 ± 0.31
prek-head --version 1.8 ± 0.4 1.6 5.5 1.00

prek list

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base list 8.5 ± 1.3 7.4 12.4 1.12 ± 0.19
prek-head list 7.6 ± 0.4 7.2 11.3 1.00

✅ Performance improvement for prek list: 10.5600% faster

prek validate-config .pre-commit-config.yaml

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base validate-config .pre-commit-config.yaml 2.5 ± 0.1 2.4 2.9 1.00 ± 0.05
prek-head validate-config .pre-commit-config.yaml 2.5 ± 0.1 2.4 2.7 1.00

prek sample-config

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base sample-config 2.0 ± 0.1 1.9 2.1 1.00
prek-head sample-config 2.0 ± 0.1 1.9 2.2 1.01 ± 0.04
Cold vs Warm Runs

Comparing first run (cold) vs subsequent runs (warm cache):

prek run --all-files (cold - no cache)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run --all-files 94.5 ± 2.5 91.2 97.9 1.00
prek-head run --all-files 105.8 ± 17.6 93.9 147.3 1.12 ± 0.19

⚠️ Warning: Performance regression for prek run --all-files (cold - no cache): 11.9400% slower

prek run --all-files (warm - with cache)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run --all-files 95.4 ± 3.1 91.5 103.5 1.00
prek-head run --all-files 100.0 ± 9.8 92.7 139.4 1.05 ± 0.11
Full Hook Suite

Running the builtin hook suite on the benchmark workspace:

prek run --all-files (full builtin hook suite)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run --all-files 98.6 ± 6.3 92.4 125.6 1.03 ± 0.07
prek-head run --all-files 96.0 ± 2.3 91.5 101.7 1.00
Individual Hook Performance

Benchmarking each hook individually on the test repo:

prek run trailing-whitespace --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run trailing-whitespace --all-files 14.4 ± 0.3 13.9 15.3 1.02 ± 0.03
prek-head run trailing-whitespace --all-files 14.1 ± 0.2 13.5 14.5 1.00

prek run end-of-file-fixer --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run end-of-file-fixer --all-files 19.2 ± 1.2 17.3 21.3 1.00
prek-head run end-of-file-fixer --all-files 19.8 ± 2.9 17.5 33.7 1.03 ± 0.16

prek run check-json --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run check-json --all-files 9.0 ± 0.6 8.3 11.2 1.04 ± 0.16
prek-head run check-json --all-files 8.6 ± 1.2 7.8 13.7 1.00

prek run check-yaml --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run check-yaml --all-files 8.3 ± 0.2 8.0 9.0 1.00 ± 0.04
prek-head run check-yaml --all-files 8.2 ± 0.2 7.9 8.9 1.00

prek run check-toml --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run check-toml --all-files 8.6 ± 0.8 7.9 11.1 1.00
prek-head run check-toml --all-files 8.7 ± 0.4 8.1 9.8 1.01 ± 0.10

prek run check-xml --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run check-xml --all-files 8.6 ± 0.3 7.9 9.5 1.00
prek-head run check-xml --all-files 9.8 ± 1.2 8.2 12.9 1.15 ± 0.15

⚠️ Warning: Performance regression for prek run check-xml --all-files: 14.5600% slower

prek run detect-private-key --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run detect-private-key --all-files 18.3 ± 3.6 13.1 28.3 1.16 ± 0.37
prek-head run detect-private-key --all-files 15.8 ± 4.0 11.9 29.9 1.00

✅ Performance improvement for prek run detect-private-key --all-files: 13.8500% faster

prek run fix-byte-order-marker --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run fix-byte-order-marker --all-files 16.5 ± 1.6 14.8 24.0 1.01 ± 0.12
prek-head run fix-byte-order-marker --all-files 16.3 ± 1.0 14.5 18.4 1.00
Installation Performance

Benchmarking hook installation (fast path hooks skip Python setup):

prek install-hooks (cold - no cache)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base install-hooks 3.6 ± 0.1 3.5 3.7 1.02 ± 0.03
prek-head install-hooks 3.5 ± 0.1 3.4 3.6 1.00

prek install-hooks (warm - with cache)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base install-hooks 3.6 ± 0.1 3.5 3.7 1.00
prek-head install-hooks 3.6 ± 0.1 3.6 3.8 1.02 ± 0.03
File Filtering/Scoping Performance

Testing different file selection modes:

prek run (staged files only)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run 44.1 ± 27.4 32.9 126.7 1.17 ± 0.74
prek-head run 37.8 ± 4.3 33.1 44.7 1.00

✅ Performance improvement for prek run (staged files only): 14.3100% faster

prek run --files '*.json' (specific file type)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run --files '*.json' 6.6 ± 0.9 5.7 9.2 1.00
prek-head run --files '*.json' 7.4 ± 0.8 6.5 9.8 1.12 ± 0.20

⚠️ Warning: Performance regression for prek run --files '*.json' (specific file type): 12.1100% slower

Workspace Discovery & Initialization

Benchmarking hook discovery and initialization overhead:

prek run --dry-run --all-files (measures init overhead)

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run --dry-run --all-files 8.6 ± 2.3 7.3 16.4 1.16 ± 0.31
prek-head run --dry-run --all-files 7.4 ± 0.1 7.3 7.8 1.00

✅ Performance improvement for prek run --dry-run --all-files (measures init overhead): 13.4600% faster

Meta Hooks Performance

Benchmarking meta hooks separately:

prek run check-hooks-apply --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run check-hooks-apply --all-files 10.0 ± 1.2 8.9 12.4 1.10 ± 0.13
prek-head run check-hooks-apply --all-files 9.1 ± 0.2 8.9 9.8 1.00

prek run check-useless-excludes --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run check-useless-excludes --all-files 9.1 ± 0.3 8.8 9.6 1.00
prek-head run check-useless-excludes --all-files 9.6 ± 0.6 9.2 11.4 1.06 ± 0.07

prek run identity --all-files

Command Mean [ms] Min [ms] Max [ms] Relative
prek-base run identity --all-files 7.9 ± 0.2 7.7 8.4 1.00
prek-head run identity --all-files 9.6 ± 1.7 7.8 12.7 1.22 ± 0.22

⚠️ Warning: Performance regression for prek run identity --all-files: 21.6700% slower

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@j178 j178 Awaiting requested review from j178 j178 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nikosavola