pre-commit doesn't keep current project/virtualenv in sys.path

[guykisel]

I'm trying to get Prospector ( https://github.com/landscapeio/prospector ) to run as a pre-commit hook. By default, Prospector will run pylint, including pylint's import checker.

I was getting confused, because I kept getting output that looks like:

test_mailing_lists (<projectname>/tests/services/platform/email/test_mailing_lists.py):
    L8:0 None: pylint - F0401
    Unable to import '<projectname>.services.platform.email.mailing_lists'

Even though directly running Prospector in the project (as opposed to via pre-commit) doesn't raise these errors.

When I print sys.path from inside a Python script run by pre-commit as a hook, I get:

['/Users/gkisel/.pre-commit/repodfiUn9/py_env/bin',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python27.zip',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/plat-darwin',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/plat-mac',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/plat-mac/lib-scriptpackages',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/Extras/lib/python',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/lib-tk',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/lib-old',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/lib-dynload',
 '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7',
 '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-darwin',
 '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/lib-tk',
 '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-mac',
 '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-mac/lib-scriptpackages',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/site-packages',
 '/Users/gkisel/.pre-commit/repodfiUn9/py_env/lib/python2.7/site-packages/astroid/brain']

Note that this doesn't appear to include my project directory or my virtualenv path. I think this is why pylint (within Prospector) is unable to successfully run those import statements.

Is this behavior intentional? Am I doing something incorrectly? Is there a way I can get pre-commit to preserve my sys.path when running Python hooks?

For reference, see https://github.com/guykisel/prospector-mirror

[asottile]

This is actually by design. The hooks run in their own isolated environments.

A workaround is to use the system type and call the thing by name. Though this generally goes against the isolation mentaility, but might be enough to get this working for you :)

[guykisel]

I'm sorry, I'm not sure I understand what you mean by

use the system type and call the thing by name

Can you explain this in a little more detail? I appreciate your help.

[asottile]

In hooks.yaml you can create a hook which is of the langauge system instead of python. This has the unfortunate side-effect of depending on what is installed at the system level and doesn't take advantage of any of the containerization, etc. But it'll execute using the environment (including pythonpath, etc.) of the shell at the time of git commit (instead of executing in an isolated virtualenvironment).

An example file might look like this:

# hooks.yaml
-   id: my-id
    name: My Hook name
    description: A description for my hook
    language: system
    entry: pylint

[guykisel]

Oh, I see. Yes, I think that could work. I'll give it a try, thanks!

[asottile]

If that doesn't work let me know! We unfortunately haven't had many usecases with those type of hooks so there's potential that they're completely broken for anything beyond the trivial case that is tested: https://github.com/pre-commit/pre-commit/blob/master/testing/resources/system_hook_with_spaces_repo/hooks.yaml

[guykisel]

Update on this: looks like prospector can only accept one file/directory at a time as input, so I set my hooks.yaml to:

-   id: prospector
    name: prospector
    entry: -n 1 prospector
    language: system
    files: \.py$

Without the -n 1 it just fails silently.

[carlio]

@guykisel Actually prospector only accepting one file/directory at a time is a bug in prospector (or rather, 'lack of feature'). It's something that comes as a consequences of using setoptconf that I haven't looked into yet. Perhaps a better fix for all this is fixing prospector's behaviour?

[carlio]

@asottile Does isolating the environment work in general? I have found with prospector that without the complete path, most tools fail at inferring types and a lot of checks are somewhat worthless. I'm mostly talking about pylint here though.

[asottile]

@carlio
For the most part it seems to work unless the tool attempts to do in-depth import / type analysis on the dependencies of the code. pylint is the first time we've run into issues like this and it seems a somewhat acceptable workaround to use the system type and allow that. The unfortunate thing is it then depends on system libraries.

For pylint in particular, using an isolated version is really only useful for the syntax checks it does and not the import / type checks. Which has some nonzero benefit, but yeah isn't really ideal :/

@guykisel I like the -n 1 hack, I should probably make a way to encorporate that as a supported option. Thoughts?

[guykisel]

@asottile you could add another field in hooks.yaml, something like run_per_file: true/false. Alternatively, create a new "language", like system_once_per_file, but then you're out of luck if you want to get the same behavior under python and such.

It could also be interesting to try to run xargs in parallel to speed up processing of lots of files.

[lokesh1729]

In hooks.yaml you can create a hook which is of the langauge system instead of python. This has the unfortunate side-effect of depending on what is installed at the system level and doesn't take advantage of any of the containerization, etc. But it'll execute using the environment (including pythonpath, etc.) of the shell at the time of git commit (instead of executing in an isolated virtualenvironment).

An example file might look like this:

# hooks.yaml
-   id: my-id
    name: My Hook name
    description: A description for my hook
    language: system
    entry: pylint

@asottile
I have configured pylint as a system hook, but it wasn't taking the current shell's python virtualenv...

this is the config I have

-   repo: local
    hooks:
    - id: pylint
      name: pylint
      language: system
      types: [python]
      entry: "python -m pylint --rcfile=.pylintrc --load-plugins=pylint_django --load-plugins=pylint_django.checkers.db_performance"
-   repo: local
    hooks:
    - id: hey
      name: hey
      language: system
      entry: "which python"
-   repo: local
    hooks:
    - id: hello
      name: hello
      language: system
      entry: "which pylint"

when I run it, I see the path of python and pylint like this

hey......................................................................Failed
hookid: hey

/Users/slokesh/.pyenv/versions/3.6.5/bin/python

hello....................................................................Failed
hookid: hello

/Users/slokesh/.pyenv/versions/3.6.5/bin/pylint

But, when I see python and pylint paths, they are in virtual env...

[asottile]

Can you hash -r and then try your which commands again?

[lokesh1729]

@asottile yes... hash -r does the trick... but I am wondered what's the trick??? what is the cause of the issue??/ I actually have installed pre-commit hooks before and then clean them and then tried installing again... is that what it's causing... can you explain what's the logic behind ???

[asottile]

your which command was misleading you -- the shell implements a "cache" of binaries that it knows from PATH lookup -- whereas pre-commit computes the executable location from the "true" PATH

[lokesh1729]

@asottile the reason I added "which" is because pylint is failing to detect django in my current virtual environment... if pre-commit computes the location, then why does it fail to import django ??

[asottile]

right -- I was mostly explaining why in your screenshot above you see a different which result inside / outside of pre-commit. because the whichs outside are using your shell's hash cache despite not actually being available.

For language: system you should be able to run the same command in and outside of pre-commit and receive the same results -- if you can show that not happening then there's a bug (though a report that involves which or shell builtins should run hash -r before invoking those commands). Alternatively you can use python3 -c 'import shutil; print(shutil.which("python"))' etc.

---

an aside, one thing I noticed about the screenshot above is the which hooks show up as Failed -- however that seems impossible since which produced a result and exited zero -- was your configuration something different from that?

[lokesh1729]

@asottile keeping which aside, the main purpose to put them is to debug pylint failing to import django. my main concern is pylint not the which commands. so, when I did hash -r not only which command is showing the correct path but also previous pylint errors also gone, now it's able to import django from my virtual environment. can you tell me one thing, I am using pyenv to manage my python versions. since pyenv manipulates PATH variable by appending it's shims path in the front, does it affecting pylint path ???

[asottile]

pyenv itself shouldn't matter, but I imagine the hash cache (while in your shell) also was causing you to use the pylint executable you didn't expect as well?

[lokesh1729]

I didn't try with my shell before clearing the hash, so should I put a hash -r before the command?? is that needed for the system commands ???

[asottile]

the hash cache only affects the shell and has nothing to do with pre-commit here, I'd suggest reading the manual for your particular shell (for instance bash or zsh) describing the shell builtins and how they function -- the shell is a program like any other and the hash cache is stored in that program's memory. pre-commit does not have access to that.

[lokesh1729]

@asottile I have gone through hash. Agreed to your points... So, it is good to use hash -r command before invoking pylint?

[asottile]

there's no point to doing that in pre-commit's configuration -- I was only suggesting you try that in your shell to establish an amount of sanity / correct expectations for what pre-commit is going to do (while eliminating the cache invalidation problem of your shell)

[miry]

Just in case:

I had next hook:

  - repo: local
    hooks:
      - id: pytest
        name: pytest
        entry: poetry run pytest --
        language: python
        types: [python]
        files: ^tests\/.*test_.*\.py$
        pass_filenames: false  # or use with: require_serial

I use poetry. And on each changes of tests, it used different python.

Base on comment @asottile , I changed language: python to language: system and everything started works.

[scop]

Another tool that is affected by this problem is mypy, it really needs stuff from the project's venv to work properly unless one pretty much completely ignores errors resulting from imports.

I think the system workaround isn't really applicable for cases where for example a git GUI tool (such as GitHub Desktop) is launched and stuff committed to a pre-commit enabled working dir; there's likely no opportunity to activate the project venv there.

Maybe a script approach that would activate the venv would work, dunno. But a problem there is that the way people use venvs differ; some prefer plain python -m venvs, others pyenv virtualenvs, the dirs where the venvs are set up vary, so the script would not be able to activate the venv properly in all cases, and forcing people to use virtualenvs some specific way would be kind of meh.

So it would be great if pre-commit would offer some more assistance in coping with these scenarios. Don't know what exactly would that be. Maybe e.g. some option that would "record" the currently active virtualenv settings at hook install time and use them later to construct the same environment when running the installed hook, dunno.