diff --git a/crates/prek/src/config/hook.rs b/crates/prek/src/config/hook.rs index ac5990295..2183f85a6 100644 --- a/crates/prek/src/config/hook.rs +++ b/crates/prek/src/config/hook.rs @@ -487,7 +487,7 @@ pub(crate) struct HookOptions { pub additional_dependencies: Option>, /// Additional arguments to pass to the hook. pub args: Option>, - /// Environment variables to set for the hook. + /// Environment variables to set while preparing and running the hook. pub env: Option>, /// This hook will run even if there are no matching files. /// Default is false. diff --git a/crates/prek/src/config/mod.rs b/crates/prek/src/config/mod.rs index f43b782a9..98ba084ee 100644 --- a/crates/prek/src/config/mod.rs +++ b/crates/prek/src/config/mod.rs @@ -58,7 +58,7 @@ pub(crate) struct Config { /// A configuration-wide default for the stages property of hooks. /// Default to all stages. pub default_stages: Option, - /// Default runtime environment variables for hooks. + /// Default environment variables for preparing and running hooks. pub default_env: Option>, /// Global file include pattern. pub files: Option, diff --git a/crates/prek/src/languages/mod.rs b/crates/prek/src/languages/mod.rs index 935866650..08ea586c8 100644 --- a/crates/prek/src/languages/mod.rs +++ b/crates/prek/src/languages/mod.rs @@ -1,7 +1,6 @@ use std::ffi::{OsStr, OsString}; use std::future::Future; use std::path::Path; -use std::pin::Pin; use std::process::Stdio; use std::str::FromStr; use std::sync::Arc; @@ -19,7 +18,7 @@ use crate::git::GitCommandExt; use crate::hook::{Hook, InstallInfo, InstalledHook, Repo}; use crate::hook_entry::PreparedHookEntry; use crate::hooks::{self, HookOutput}; -use crate::process::Cmd; +use crate::process::{Cmd, with_command_env}; use crate::run::run_by_batch; use crate::store::{CacheBucket, Store, ToolBucket}; @@ -266,8 +265,6 @@ fn is_path_env(key: impl AsRef) -> bool { } } -type LanguageFuture<'a, T> = Pin> + 'a>>; - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ShellSupport { Supported, @@ -562,11 +559,20 @@ impl Language { hook: Arc, install_cwd: &'a Path, reporter: &'a HookInstallReporter, - ) -> LanguageFuture<'a, InstalledHook> { - self.backend().install(store, hook, install_cwd, reporter) + ) -> impl Future> + 'a { + let env = hook + .env + .iter() + .map(|(key, value)| (key.into(), value.into())) + .collect(); + let install = self.backend().install(store, hook, install_cwd, reporter); + with_command_env(env, install) } - pub(crate) fn check_health<'a>(&'a self, info: &'a InstallInfo) -> LanguageFuture<'a, ()> { + pub(crate) fn check_health<'a>( + &'a self, + info: &'a InstallInfo, + ) -> impl Future> + 'a { self.backend().check_health(info) } diff --git a/crates/prek/src/process.rs b/crates/prek/src/process.rs index bce94cca1..22a9b6daa 100644 --- a/crates/prek/src/process.rs +++ b/crates/prek/src/process.rs @@ -27,8 +27,9 @@ //! Adapts [axoprocess](https://docs.rs/axoprocess) command helpers to use //! [`tokio::process::Command`] instead of [`std::process::Command`]. -use std::ffi::OsStr; +use std::ffi::{OsStr, OsString}; use std::fmt::Display; +use std::future::Future; use std::io::PipeReader; use std::ops::Range; use std::path::Path; @@ -44,6 +45,21 @@ use crate::run::HookRunOutput; #[cfg(not(windows))] use crate::terminal::USE_COLOR; +tokio::task_local! { + static COMMAND_ENV: Vec<(OsString, OsString)>; +} + +/// Apply environment variables to commands created while `future` is running. +/// +/// The scope is task-local so concurrent installs remain isolated. A spawned task does not inherit +/// it, so callers must keep command creation in the scoped future. +pub(crate) fn with_command_env( + env: Vec<(OsString, OsString)>, + future: impl Future, +) -> impl Future { + COMMAND_ENV.scope(env, future) +} + /// An error from executing a command. #[derive(Debug, Error)] pub enum Error { @@ -178,7 +194,10 @@ impl AsyncPipeReader { impl Cmd { /// Create a new command. pub fn new(command: impl AsRef) -> Self { - let inner = tokio::process::Command::new(command); + let mut inner = tokio::process::Command::new(command); + let _ = COMMAND_ENV.try_with(|env| { + inner.envs(env.iter().map(|(key, value)| (key, value))); + }); Self { inner, hidden_arg_ranges: Vec::new(), diff --git a/crates/prek/tests/languages/python.rs b/crates/prek/tests/languages/python.rs index bdba10acd..30b26cca4 100644 --- a/crates/prek/tests/languages/python.rs +++ b/crates/prek/tests/languages/python.rs @@ -459,6 +459,48 @@ fn git_env_vars_not_leaked_to_pip_install() -> anyhow::Result<()> { Ok(()) } +#[test] +fn configured_env_is_applied_to_python_install() -> anyhow::Result<()> { + let context = TestEnv::new_git(); + context + .work_dir() + .child("setup.py") + .write_str(indoc::indoc! {r#" + import os, sys + from setuptools import setup + if os.environ.get("PREK_TEST_INSTALL_ENV") != "configured": + sys.exit("configured hook env was not applied during installation") + setup(name="test-install-env", version="0.1.0") + "#})?; + + let dependency = serde_json::to_string(&context.work_dir().path().to_string_lossy())?; + let context = context.with_config(indoc::formatdoc! {r#" + repos: + - repo: local + hooks: + - id: install-env + name: install-env + language: python + entry: python -c "print('ok')" + additional_dependencies: [{dependency}] + env: + PREK_TEST_INSTALL_ENV: configured + always_run: true + "#}); + context.git().add_all(); + + cmd_snapshot!(context, context.run(), @r" + success: true + exit_code: 0 + ----- stdout ----- + install-env..............................................................Passed + + ----- stderr ----- + "); + + Ok(()) +} + /// Regression test for . #[test] fn local_relative_additional_dependency_is_not_resolved_from_worktree() -> anyhow::Result<()> { diff --git a/docs/authoring-hooks.md b/docs/authoring-hooks.md index e28cab97f..2d0d2b492 100644 --- a/docs/authoring-hooks.md +++ b/docs/authoring-hooks.md @@ -47,7 +47,7 @@ each manifest hook: | `exclude_types` | No | No | list of strings | Exclude files with any listed file type tag. | | `additional_dependencies` | No | No | list of strings | Extra dependencies installed into managed hook environments. | | `args` | No | No | list of strings | Extra arguments appended to `entry` before filenames. | -| `env` | No | Yes | map of strings | Runtime environment variables for the hook process. | +| `env` | No | Yes | map of strings | Environment variables for hook environment creation and execution. | | `always_run` | No | No | boolean | Run even when no files match. | | `fail_fast` | No | No | boolean | Stop the run immediately if this hook fails. | | `pass_filenames` | No | No | boolean or positive integer | Control whether, or how many, matching filenames are passed. | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 0ae60e0a1..9b926fcf9 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -309,7 +309,7 @@ See [Supported Git Hook Stages](#supported-git-hook-stages) for what each value `default_env` is a `prek` extension and may not be recognized by upstream `pre-commit`. -Runtime environment variables to apply to every hook in this config. +Environment variables to apply when `prek` creates hook environments and runs hooks in this config. If a hook sets the same variable in [`env`](#prek-only-env), the hook-level value wins. - Type: map of string to string @@ -893,27 +893,33 @@ Example: `env` is a `prek` extension and may not be recognized by upstream `pre-commit`. -Extra runtime environment variables for the hook process. +Extra environment variables for hook environment creation and the hook process. - Type: map of string to string Values override the existing process environment (including variables such as `PATH`). -They are applied when the hook runs, not when `prek` installs or prepares the hook environment. +They are applied to commands that create the hook environment and when the hook runs. -!!! note "Runtime only" +!!! note "Creation time" - `env` cannot be used to control hook installation or environment preparation. - Keeping it runtime-only means the installed hook environment does not depend on - ambient variables from the invoking shell. If those variables affected - installation, changing one of them would imply that `prek` may need to - recreate the hook environment. + When a matching hook environment already exists, `prek` reuses it without applying + these variables to installation again. Changing `env` does not recreate or select a + different environment. The new values affect installation the next time the environment + is created for another reason. If multiple hooks share an environment, only the `env` of + the hook that creates it affects installation. + +Language backends may override or remove variables they manage to preserve hook environment +isolation. +This applies to subprocesses started by a backend. `env` is not treated as configuration for +download logic implemented directly inside `prek`. For remote hooks, `env` may also be set by the hook author in `.pre-commit-hooks.yaml`. Values from the project configuration are merged with manifest values and override duplicate keys. Values set directly on a hook override matching [`default_env`](#default_env) values. -For `docker` / `docker_image` hooks, these variables are passed into the container rather than being applied to the container runtime command. +When running `docker` / `docker_image` hooks, these variables are passed into the container rather +than being applied to the container runtime command. Example: diff --git a/prek-hooks.schema.json b/prek-hooks.schema.json index 3692e19c9..d4c117184 100644 --- a/prek-hooks.schema.json +++ b/prek-hooks.schema.json @@ -79,7 +79,7 @@ } }, "env": { - "description": "Environment variables to set for the hook.", + "description": "Environment variables to set while preparing and running the hook.", "type": "object", "additionalProperties": { "type": "string" diff --git a/prek.schema.json b/prek.schema.json index b315c1dae..d1b4f9674 100644 --- a/prek.schema.json +++ b/prek.schema.json @@ -124,7 +124,7 @@ "uniqueItems": true }, "default_env": { - "description": "Default runtime environment variables for hooks.", + "description": "Default environment variables for preparing and running hooks.", "type": "object", "additionalProperties": { "type": "string" @@ -343,7 +343,7 @@ } }, "env": { - "description": "Environment variables to set for the hook.", + "description": "Environment variables to set while preparing and running the hook.", "type": "object", "additionalProperties": { "type": "string" @@ -653,7 +653,7 @@ } }, "env": { - "description": "Environment variables to set for the hook.", + "description": "Environment variables to set while preparing and running the hook.", "type": "object", "additionalProperties": { "type": "string" @@ -822,7 +822,7 @@ } }, "env": { - "description": "Environment variables to set for the hook.", + "description": "Environment variables to set while preparing and running the hook.", "type": "object", "additionalProperties": { "type": "string" @@ -996,7 +996,7 @@ } }, "env": { - "description": "Environment variables to set for the hook.", + "description": "Environment variables to set while preparing and running the hook.", "type": "object", "additionalProperties": { "type": "string"