Add env to set environment variables for hooks (#1279)#1285
Add env to set environment variables for hooks (#1279)#1285j178 merged 3 commits intoj178:masterfrom
env to set environment variables for hooks (#1279)#1285Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1285 +/- ##
==========================================
- Coverage 90.01% 89.94% -0.07%
==========================================
Files 80 80
Lines 15754 15808 +54
==========================================
+ Hits 14181 14219 +38
- Misses 1573 1589 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📦 Cargo Bloat ComparisonBinary size change: +0.00% (16.7 MiB → 16.7 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
|
I added a commit to point the script test at my fork of prek-test-repos/script-hooks; it will need to be removed before merging. |
There was a problem hiding this comment.
Pull request overview
This PR adds support for setting environment variables for hooks via a new env configuration option. This feature allows users to pass custom environment variables to hooks, which will override existing environment variables like $PATH.
Key changes:
- Added
envfield to hook configuration schema accepting key-value pairs of environment variable names and values - Implemented environment variable support across all language implementations (system, script, python, ruby, rust, node, golang, lua, pygrep, docker, docker_image)
- Updated command-line argument length calculation on Unix to account for environment variable space in
ARG_MAXlimit - Added documentation and tests demonstrating the new
envfeature
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| prek.schema.json | Added env field schema definition to both hook and manifest schemas, accepting object or null with string values |
| docs/configuration.md | Added comprehensive documentation for the env hook-level option with usage example showing RUSTDOCFLAGS |
| crates/prek/src/config.rs | Added env: Option<FxHashMap<String, String>> field to HookOptions with custom merge logic that extends rather than replaces |
| crates/prek/src/hook.rs | Added env: FxHashMap<String, String> field to Hook struct and initialized it in the builder |
| crates/prek/src/run.rs | Updated Unix ARG_MAX calculation to account for environment variable sizes, avoiding double-counting overridden variables |
| crates/prek/src/languages/system.rs | Added .envs(&hook.env) call to pass environment variables to system commands |
| crates/prek/src/languages/script.rs | Added .envs(&hook.env) call to pass environment variables to script commands |
| crates/prek/src/languages/python/python.rs | Added .envs(&hook.env) call after setting Python-specific environment variables |
| crates/prek/src/languages/ruby/ruby.rs | Added .envs(&hook.env) call after setting Ruby-specific environment variables |
| crates/prek/src/languages/rust/rust.rs | Added .envs(&hook.env) call after setting Rust-specific environment variables |
| crates/prek/src/languages/node/node.rs | Added .envs(&hook.env) call after setting Node-specific environment variables |
| crates/prek/src/languages/golang/golang.rs | Added .envs(&hook.env) call after setting Go-specific environment variables |
| crates/prek/src/languages/lua.rs | Added .envs(&hook.env) call after setting Lua-specific environment variables |
| crates/prek/src/languages/pygrep/pygrep.rs | Added .envs(&hook.env) call to pass environment variables to pygrep commands |
| crates/prek/src/languages/docker.rs | Implemented environment variable passing via -e flags on docker run command line |
| crates/prek/src/languages/docker_image.rs | Implemented environment variable passing via -e flags on docker run command line |
| crates/prek/tests/languages/script.rs | Updated tests to use versioned repo and added tests demonstrating env variable functionality |
| crates/prek/tests/languages/docker.rs | Updated test to demonstrate env variable functionality with docker hooks |
| crates/prek/src/snapshots/prek__config__tests__read_manifest.snap | Updated snapshot to include env: None in hook definitions |
| crates/prek/src/snapshots/prek__config__tests__read_config.snap | Updated snapshot to include env: None in hook configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you, great PR! |
This key is not supported by
pre-commit.This requires prek-test-repos/script-hooks to be updated (prek-test-repos/script-hooks#1) before tests will pass. Unfortunately, merging that PR will break tests on this repo until this PR is merged.
I only added a couple of tests here; would you like me to go through and add tests for other languages?
Closes #1279