You can already do:

    additional_dependencies: [--index-url, https://pypi.mycompany.com/simple]

then you don't need the environment variable -- does this work?

Yeah, I guess we can do this. People have been telling me it's too hacky to merge, but now that you've documented it, I guess it's not a hack anymore :)

I was sort of hoping for install_environ as a top-level key so that we could avoid a manifest like:

diff --git a/app/templates/.pre-commit-config.yaml b/app/templates/.pre-commit-config.yaml
index 69647de..be8de27 100644
--- a/app/templates/.pre-commit-config.yaml
+++ b/app/templates/.pre-commit-config.yaml
@@ -3,35 +3,37 @@ repos:
     sha: v1.1.1
     hooks:
     -   id: trailing-whitespace
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: end-of-file-fixer
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: autopep8-wrapper
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: check-json
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: check-yaml
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: debug-statements
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: name-tests-test
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: flake8
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: requirements-txt-fixer
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: check-added-large-files
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: check-byte-order-marker
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: fix-encoding-pragma
         args: [--remove]
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: check-executables-have-shebangs
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]
     -   id: detect-aws-credentials
         args: ['--allow-missing-credentials']
+        additional_dependencies: [-i, https://pypi.yelpcorp.com/simple]

...which seems like a lot to copy-paste around everywhere and sort of distracts from everything else in the manifest.

The hard part about install_environ is correctly invalidating caches based on it. It's probably also only useful for python and js (ruby can't take advantage of it, many of the other languages don't care about environ). Also feels like a slippery slope (if there's environ overriding, do we clear environ variables? do we allow environ substitution? I don't really want to get into the business of either of those).

If you truly want all requests to go to internal pypi, maybe /etc/environment is a good place?

What's the hard part about correctly invalidating caches? Can't you hash the provided install environment and include that as part of the cache key?

I don't think we want every pip invocation on the box to default to internal PyPI. At least for us right now, I think per-project is the right level to opt-in to that, and the pre-commit config seems like a pretty natural place to do it.

I guess we will probably take the additional_dependencies approach everywhere and just live with cluttered pre-commit configs. I'm not crazy about it, but it's been a pretty big issue for us, and pre-commit doesn't seem to have an elegant solution (despite imo being in a good position to provide one).

I guess it's not difficult, could just do the same thing that additional_dependencies is doing:

And then thread through the environ settings.

Maybe that isn't the worst?

But leave unspecified env variables alone?

I've got another use-case for env vars:
There's https://github.com/jgosmann/pylint-venv, which can allow pylint to run in venv where it's not installed. While this could be addressed in the hook itself (ref pre-commit/mirrors-pylint#9), I feel like this could be a generic helper for lots of other cases.

pylint-venv seems a bit of a flawed idea (and more of a hack than a solution), mostly in that activate_this does not work across different python versions. The added component that the virtualenv it depends on may or may not be set up (or set up properly) is going to make errors look like pre-commit's fault.

@asottile @webknjaz Apparently this bug is likely to become a showstopper for some users and I now I do have a valid use case for it, with no known workaround.

Apparently ansible-lint may need user to set ANSIBLE_LIBRARY variable to point to the local ansible modules before being called, or linting will fail as soon it will encounter the custom ansible module.

Tox has a great solution for this kind of problems which is the passenv and setenv, so to make the linter work correctly under tox, I only need to add something like:

setenv
   ANSIBLE_LIBRARY={toxinidir}/library
commands =
  python -m pre_commit run --all-files

This would fix execution from inside tox, even if my command would use pre-commit hook.

The bad part is that this will not fix the git hook execution, which runs without any additional environment variables.

For the moment I don't really care if we add this feature at global level or on each hook, what matters is to get it sorted. I am sure that in the future we will encounter other cases where we need to pass some extra environment variables to the hooks.

This issue is a ~little different than runtime environment variables -- though runtime and install time environment variables are likely to be strongly related.

(at least to me) it seems like ansible-lint should take a commandline argument to allow that instead of needing to set it in the environment.

That said, let's get cheeky / clever (this'll probably work, similar caveats as usual with unix tools and cross platform support):

    -    id: ansible-lint
         entry: env ANSIBLE_LIBRARY=./library ansible-lint

re: passenv / setenv: (I also am a maintainer of tox so I'm pretty familiar with these!) -- if this feature were to be implemented I definitely do not want to implement the full environ clearing thing. While I agree it is a better end situation to be in, I don't really want to deal with maintaining a global whitelist. I also remember the headache that the tox 2.0 rollout was (scurrying around to hundreds of repositories to add passenv = SSH_AUTH_SOCK, etc. when I really didn't care so much about passenv since my CI already ran in a consistent environment).

@asottile I faced another issue which is very close to your last comment. The issue is that in my case the needed ansible module is intalled by a python package (zuul), which I added to the list of dependencies.

Still I need to be able to reffer to the pre-commit virtualenv in order to define some vars like:

ANSIBLE_LIBRARY= {envsitepackagesdir}/zuul/ansible/base/library
ANSIBLE_ACTION_PLUGINS = {envsitepackagesdir}/zuul/ansible/base/actiongeneral

So, I clearly need a variable that would be expanded by pre-commit at runtime to make it work. As you can see that piece above is taken from tox.

So far I was not able to find any example of expending the venv path.

Again, you have free rein if there's a shell involved:

repos:
-   repo: local
    hooks:
    -   id: test
        name: test
        language: python
        entry: bash -c 'exec env ANSIBLE_LIBRARY="$(python -c "import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())")/astpretty.py" bash -c "env | grep ANSIBLE"'
        verbose: true
        always_run: true
        additional_dependencies: [astpretty]

$ pre-commit  run test --verbose
[test] test..............................................................Passed
hookid: test

ANSIBLE_LIBRARY=/home/asottile/.cache/pre-commit/repo_0456vqj/py_env-python3/lib/python3.6/site-packages/astpretty.py

@asottile Remind me to buy you a 🍺 when we meet! Thanks. Sometimes ugly bash lines are just doing a good job.

I think that this approach would work as long you do not use any excludes. But total I faced on case where I am forced to have some excludes.

Unless pre-commit has a placeholder string that can be used to expand the filename I think we are blocked. That is because appending the filename at the end of the command would not work, it needs to be done inside:

entry: bash -c 'exec env ANSIBLE_LIBRARY="$(python -c "import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())")/zuul/ansible/base/library" ansible-lint --force-color -v'

To have this working the filename being linter needs to be before the last ' and not after. This can be implemented only via expansions.

Thanks! Another case sorted in 2 minutes. I really need to write a blog post about this issue as using ansible-lint seems to involve a lots of weird workarounds, hopefully it will save time for many other users.

In case someone wonders, for normal lintes you can also read from /dev/stdin but this does not work for ansible-lint as it requires to know the filename being linted. I guess the stdin approach works only for linters that do not need to know the filename or path.

sounds good -- I'd be happy to help edit it if you need :)

@asottile I'm stuck on this one. I'm trying to use https://gitlab.com/smop/pre-commit-hooks/blob/v1.0.0/.pre-commit-hooks.yaml

- id: check-gitlab-ci
  name: GitLab CI/CD configuration check
  description: Validates .gitlab-ci.yml file
  entry: pre_commit_hooks/check-gitlab-ci
  language: script
  pass_filenames: false
  files: .gitlab-ci.yml

with a non-standard .gitlab-ci.yml naming. The script itself relies on the usage of environment variables to configure a new name, however I seem to be unable to pass in an environment variable.

language: script doesn't know what to do with env:

Executable /Users/xxxx/.cache/pre-commit/repotym58z1n/env not found

And if I use the absolute path, /usr/bin/env, it can't find the original script

env: pre_commit_hooks/check-gitlab-ci: No such file or directory

Using language: system gives me the same "No such file or directory" problem on the script. Any thoughts?

yeah env isn't going to help you for language: script -- I'd send them a PR to support args (commandline flags) instead of the impossible interface they've designed

Hi @asottile, I am trying to set an environment variable for a hook sourced from a repo, but it's not working.

With env / no bash:

  - repo: https://github.com/gruntwork-io/pre-commit
    rev: v0.1.4
    hooks:
      - id: terraform-validate
        entry: env AWS_DEFAULT_REGION=us-east-1 "$@"

% pre-commit run --all-files
Terraform validate.......................................................Failed
- hook id: terraform-validate
- exit code: 1

Executable `/Users/scott/.cache/pre-commit/repovgq9d_g3/env` not found

Same thing with bash:

  - repo: https://github.com/gruntwork-io/pre-commit
    rev: v0.1.4
    hooks:
      - id: terraform-validate
        entry: bash -c "exec env AWS_DEFAULT_REGION=us-east-1 $@"

% pre-commit run --all-files
Terraform validate.......................................................Failed
- hook id: terraform-validate
- exit code: 1

Executable `/Users/scott/.cache/pre-commit/repovgq9d_g3/bash` not found

the answer is the same as my answer from #758 (comment)

from language: script:

Script hooks provide a way to write simple scripts which validate files. The entry should be a path relative to the root of the hook repository.

so when you have entry: bash it will run path/to/repository/bash and since that's definitely not a thing you'll get the error you see

Ahhh, didn't catch that the hook was using script. Thanks!

@asottile Since language: script doesn't support passing in env variables with any workaround, would it be possible to add some setenv/passenv magic for language: script?

For something like terraform_validate there is no really good way to define args either, as there can be any number of ENV vars that might be required.

@deiga While I did not need that yet for a hook, I can easily see a wide range of env vars that could be needed for using some tools, good examples would be DOCKER_* with DOCKER_HOST in particular if your hook makes use of docker, all the http proxy variables when you need network access.

@deiga as I said above:

yeah env isn't going to help you for language: script -- I'd send them a PR to support args (commandline flags) instead of the impossible interface they've designed

@asottile I saw that, but in the case of Terraform you can have any number of providers which might take configuration via ENV and thus having some args solution is not really feasible.
Either the terraform_validate plugin would need to add all possible Terraform related ENV variables as args or then build something dynamic that can populate ENV based whatever is input in args which might be a security issue.

build something dynamic that can populate ENV based whatever is input in args which might be a security issue.

surely if that's a security issue then doing it in pre-commit would also be the same security issue? please don't try and push for changes with fear. (additionally, I don't believe it is a security issue)

fwiw, language: script and language: system are the escape hatch from normal execution in pre-commit -- essentially "I know what I'm doing, don't manage me" and by that nature are less supported.