Skip to content

Commit

Permalink
pre-commit: fixing import bug for built-in hooks
Browse files Browse the repository at this point in the history
Pre-commit's "system" language is a sort of escape hach from the
encapsulation which pre-commit typically provides.  It just runs
commands without thinking too hard about setting up or tearing down
environments.

But it is used for two things: user-provided hooks which reference
things in calling environment, and pre-commit-provided hooks which
live in the pre-commit package (I'm calling these "meta" hooks).

The challenge becomes: which PATH and PYTHONPATH variables do we expose
to pre-commit?  If it's the calling environment, the meta hooks break,
and if it's the packaged environment, the external-facing system hooks
break.

In order to make both of these work at the same time, this commit
separates 'system' into two languages: 'system', and 'meta'. System
hooks get access to the calling environment, and meta hooks get access
to the packaged environment.
  • Loading branch information
MatrixManAtYrService committed Jul 7, 2024
1 parent 5cc7a0f commit 423b0dd
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 4 deletions.
22 changes: 18 additions & 4 deletions pkgs/tools/misc/pre-commit/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ buildPythonApplication rec {

patches = [
./languages-use-the-hardcoded-path-to-python-binaries.patch
./system-hooks-use-calling-environment-paths.patch
./hook-tmpl.patch
./pygrep-pythonpath.patch
];
Expand Down Expand Up @@ -104,12 +105,25 @@ buildPythonApplication rec {
deactivate
'';

# Propagating dependencies leaks them through $PYTHONPATH which causes issues
# when used in nix-shell.

postFixup = ''
# Propagating inputs causes them to show up in $PYTHONPATH in a nix shell,
# this is problematic because they might conflict with the user's dependencies,
# so we remove them.
rm $out/nix-support/propagated-build-inputs
# the wrapper script modifies PATH and PYTHONPATH to point into the nix store
# this prevents 'system' type hooks from referencing the calling environment
# save untainted copies of that environment for such cases
sed -i '2i\export "OUTER_PYTHONPATH=$PYTHONPATH"' $out/bin/pre-commit
sed -i '2i\export "OUTER_PATH=$PATH"' $out/bin/pre-commit
'';

# Pre-commit also has built-in hooks which it needs to import from the nix store
# we hid them from user devshells by removing propated-build-inputs, but now we
# need to restore them for pre-commit's use (but only inside of the wrapper).
makeWrapperArgs = ''--set PYTHONPATH $PYTHONPATH'';

disabledTests = [
# ERROR: The install method you used for conda--probably either `pip install conda`
# or `easy_install conda`--is not compatible with using conda as an application.
Expand Down Expand Up @@ -178,8 +192,8 @@ buildPythonApplication rec {
"pre_commit"
];

passthru.tests.version = testers.testVersion {
package = pre-commit;
passthru.tests = callPackage ./tests.nix {
inherit git pre-commit;
};

meta = with lib; {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
diff --git a/pre_commit/all_languages.py b/pre_commit/all_languages.py
index 476bad9..742172a 100644
--- a/pre_commit/all_languages.py
+++ b/pre_commit/all_languages.py
@@ -21,6 +21,7 @@ from pre_commit.languages import rust
from pre_commit.languages import script
from pre_commit.languages import swift
from pre_commit.languages import system
+from pre_commit.languages import meta


languages: dict[str, Language] = {
@@ -44,6 +45,7 @@ languages: dict[str, Language] = {
'script': script,
'swift': swift,
'system': system,
+ 'meta': meta,
# TODO: fully deprecate `python_venv`
'python_venv': python,
}
diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py
index d0651ca..008307b 100644
--- a/pre_commit/clientlib.py
+++ b/pre_commit/clientlib.py
@@ -265,8 +265,8 @@ META_HOOK_DICT = cfgv.Map(
'Hook', 'id',
cfgv.Required('id', cfgv.check_string),
cfgv.Required('id', cfgv.check_one_of(tuple(k for k, _ in _meta))),
- # language must be system
- cfgv.Optional('language', cfgv.check_one_of({'system'}), 'system'),
+ # language must be meta
+ cfgv.Optional('language', cfgv.check_one_of({'meta'}), 'meta'),
# entry cannot be overridden
NotAllowed('entry', cfgv.check_any),
*(
diff --git a/pre_commit/lang_base.py b/pre_commit/lang_base.py
index 4a993ea..eba8f3e 100644
--- a/pre_commit/lang_base.py
+++ b/pre_commit/lang_base.py
@@ -130,6 +130,27 @@ def no_install(
def no_env(prefix: Prefix, version: str) -> Generator[None, None, None]:
yield

+@contextlib.contextmanager
+def outer_env(prefix: Prefix, version: str) -> Generator[None, None, None]:
+
+
+ # access the non-nix-modified environment
+ # which was captured pre-modifications by the wrapper script
+ # which was configured by the postFixup hook here:
+ # nixpkgs/pkgs/tools/misc/pre-commit/default.nix
+ backups = {}
+ for var in ["PATH", "PYTHONPATH"]:
+ val = os.environ.get(var)
+ outer_val = os.environ.get("OUTER_" + var)
+ if outer_val:
+ backups[var] = val
+ os.environ[var] = outer_val
+
+ yield
+
+ # restore the nix-modified environment
+ for k, v in backups.items():
+ os.environ[k] = v

def target_concurrency() -> int:
if 'PRE_COMMIT_NO_CONCURRENCY' in os.environ:
diff --git a/pre_commit/languages/meta.py b/pre_commit/languages/meta.py
new file mode 100644
index 0000000..f6ad688
--- /dev/null
+++ b/pre_commit/languages/meta.py
@@ -0,0 +1,10 @@
+from __future__ import annotations
+
+from pre_commit import lang_base
+
+ENVIRONMENT_DIR = None
+get_default_version = lang_base.basic_get_default_version
+health_check = lang_base.basic_health_check
+install_environment = lang_base.no_install
+in_env = lang_base.no_env
+run_hook = lang_base.basic_run_hook
diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py
index f6ad688..d8237e7 100644
--- a/pre_commit/languages/system.py
+++ b/pre_commit/languages/system.py
@@ -6,5 +6,5 @@ ENVIRONMENT_DIR = None
get_default_version = lang_base.basic_get_default_version
health_check = lang_base.basic_health_check
install_environment = lang_base.no_install
-in_env = lang_base.no_env
+in_env = lang_base.outer_env
run_hook = lang_base.basic_run_hook
37 changes: 37 additions & 0 deletions pkgs/tools/misc/pre-commit/tests.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{ git
, pre-commit
, runCommand
, testers
}: {
check-meta-hooks = runCommand "check-meta-hooks" {
nativeBuildInputs = [ git pre-commit ];
} ''
cd "$(mktemp --directory)"
export HOME="$PWD"
cat << 'EOF' > .pre-commit-config.yaml
repos:
- repo: local
hooks:
- id: echo
name: echo
entry: echo
files: \.yaml$
language: system
- repo: meta
hooks:
- id: check-hooks-apply
- id: check-useless-excludes
- id: identity
EOF
git config --global user.email "[email protected]"
git config --global user.name "Your Name"
git init --initial-branch=main
git add .
pre-commit run --all-files
touch $out
'';

version = testers.testVersion {
package = pre-commit;
};
}

0 comments on commit 423b0dd

Please sign in to comment.