Skip to content

Add support for requirements.txt in python additional_dependencies argument #730

Closed

Description

@jhg03a

Currently the additional_dependencies parameter for python requires duplicating the contents of an existing requirements.txt. Instead it should be able to detect that an argument it provided includes *.txt, then adjust the pip command to use the -r argument instead of '.'

Activity

  1. asottile commented on Mar 20, 2018

    @asottile
    Member

    The problem with requirements.txt is it could be infinitely expanded (what if that requirements.txt itself contains -r?). How would pre-commit know when to invalidate an environment?

    -r requirements.txt is also purely a convention. You could have -r foo.bar just as well -- I don't think it makes sense to special case "files ending in .txt".

    In general, requirements.txt is not supposed to be used for libraries anyway (there shouldn't be duplication in setup.py in that case).

    Do you have a specific usecase in mind? I'd like to see something concrete before even trying to think of how this would be implemented :)

  2. asottile commented on Mar 20, 2018

    @asottile
    Member

    Actually, I think you can already accomplish what you want with:

        additional_dependencies: ['-rrequirements.txt']

    The first caveat (about cacheability) still stands, though at least it'll be limited to the repository in question.

    To validate, I tried the following configuration:

    repos:
    -   repo: https://github.com/pre-commit/pre-commit-hooks
        rev: v1.2.3
        hooks:
        -   id: trailing-whitespace
            additional_dependencies: [-r, requirements-dev.txt]
            entry: pip freeze --all
            pass_filenames: false

    And runtime:

    $ pre-commit run --all-files --verbose
    [INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks:-r,requirements-dev.txt.
    [INFO] Installing environment for https://github.com/pre-commit/pre-commit-hooks.
    [INFO] Once installed this environment will be reused.
    [INFO] This may take a few minutes...
    [trailing-whitespace] Trim Trailing Whitespace...........................Passed
    hookid: trailing-whitespace
    
    aspy.yaml==1.0.0
    attrs==17.4.0
    autopep8==1.3.4
    cached-property==1.4.0
    cfgv==1.0.0
    coverage==4.5.1
    flake8==3.5.0
    identify==1.0.8
    mccabe==0.6.1
    mock==2.0.0
    nodeenv==1.2.0
    pbr==3.1.1
    pip==9.0.1
    pluggy==0.6.0
    pre-commit==1.8.2
    pre-commit-hooks==1.2.3
    py==1.5.2
    pycodestyle==2.3.1
    pyflakes==1.6.0
    pytest==3.4.2
    PyYAML==3.12
    setuptools==28.8.0
    six==1.11.0
    virtualenv==15.1.0
    wheel==0.29.0
    You are using pip version 9.0.1, however version 9.0.2 is available.
    You should consider upgrading via the 'pip install --upgrade pip' command.

    (Note here that this requirements file was installed)

  3. jhg03a commented on Mar 20, 2018

    @jhg03a
    Author

    I'm not sure what you have access to in the pip environment, but you could either use the incoming git changeset or a simple file hash comparison to know if the environment needs a refresh.

  4. asottile commented on Mar 20, 2018

    @asottile
    Member

    Actually nevermind the cacheability issue -- should work fine (the repository is already keyed by the same things the configuration is keyed on)! (as long as you're referring to requirements files inside the repo)

  5. jhg03a commented on Mar 20, 2018

    @jhg03a
    Author

    The use case is writing simple python scripts. I use requirements.txt to manage my package import versions presently. Some use regular pip package names, while others use a full git+URL format.

  6. asottile commented on Mar 20, 2018

    @asottile
    Member

    Does the solution suggested above work for you?

  7. jhg03a commented on Mar 20, 2018

    @jhg03a
    Author

    Unfortunately no, if I use your example with the mirrors-pylint repohook it's looking in the wrong place for the requirements.txt.

  8. jhg03a commented on Mar 20, 2018

    @jhg03a
    Author

    It would be relative to the pylint repo instead of the repo to which pre-commit is being applied.

  9. jhg03a commented on Mar 20, 2018

    @jhg03a
    Author

    With regard to pylint specifically, without setting the additional_dependencies it will fail to locate some of the script imports and fail the pre-commit run.

  10. asottile commented on Mar 20, 2018

    @asottile
    Member

    Ah ok, yeah sorry I'm not interested in supporting that -- the cacheability concern is something that (imo) isn't solvable in this situation (you'd have to rebuild every time you committed which isn't reasonable at all).

    That said, since pylint does dynamic analysis, the suggestion is to configure it as a local hook (taking advantage of your virtualenv). You can find an example of that here or from our docs here

  11. asottile commented on Mar 20, 2018

    @asottile
    Member

    Here's some additional reading on the subject (I've accumulated a bunch of links related to pylint + pre-commit here): pre-commit/pre-commit-hooks#157

  12. virad3 commented on Oct 15, 2019

    @virad3

    I'm not able to install the requirements.txt when i use local instead of git repo

    Directory Structure:

    root_dir
    - server
       - requirements
           production.txt
           development.txt
       - tests
           initiate_test.py
    

    .pre-commit-config.yaml as follows

    repos:
      - repo: local
        hooks:
          - id: unittests
            name: unittests
            entry: python -m server.tests.initiate_test.py
            language: python
            language_version: python3.6
            additional_dependencies: ['-r', '.\server\requirements\production.txt']

    Attaching the error message

    pre-commit run --all-files --verbose
    
    [INFO] Installing environment for local.
    [INFO] Once installed this environment will be reused.
    [INFO] This may take a few minutes...
    An unexpected error has occurred: CalledProcessError: Command: ('C:\\Users\\xxxxxx\\.cache\\pre-commit\\repo5oe3vtdg\\py_env-python3.6\\Scripts\\pip.exe', 'install', '.', '-r', '.\\server\\requirements\\production.txt', '--trusted-host', 'pypi.org', '--trusted-host', 'files.pythonhosted.org')
    Return code: 1
    Expected return code: 0
    Output: (none)
    Errors:
        ERROR: Could not open requirements file: [Errno 2] No such file or directory: '.\\server\\requirements\\production.txt'
        WARNING: You are using pip version 19.2.3, however version 19.3 is available.
        You should consider upgrading via the 'python -m pip install --upgrade pip' command.
    
    
    Check the log at C:\Users\xxxxx/.cache\pre-commit\pre-commit.log

    What may be the cause for this?

  13. asottile commented on Oct 15, 2019

    @asottile
    Member

    pre-commit does not install from anything other than .pre-commit-config.yaml when using local repositories (it's an intractable cache invalidation problem otherwise)

  14. virad3 commented on Oct 15, 2019

    @virad3

    How can I resolve the same? Can I have your suggestions please.

  15. asottile commented on Oct 15, 2019

    @asottile
    Member

    to install dependencies into the environments, use additional_dependencies

  16. virad3 commented on Oct 15, 2019

    @virad3

    But the problem is that, I have more than 30 modules to be installed in the environment. Thats why I tried with requirements.txt. Mentioning all additional dependencies in the .pre-commit-config.yaml is not a good practice. How can I solve this using my requirements.txt file?

  17. asottile commented on Oct 15, 2019

    @asottile
    Member

    you cannot, please see the discussion in this issue and the linked issues as to why it is not going to happen

  18. dpinol commented on Dec 18, 2019

    @dpinol

    @asottile So is there a way to "wrap" a pre-commit hook repo (eg. mirrors-mypy) adding a requirements.txt file?
    Maybe though a local repo?
    Any example on how to do it?

    thanks!

  19. asottile commented on Dec 18, 2019

    @asottile
    Member

    there is not such a thing, no, there's additional_dependencies or language: system

  20. dpinol commented on Dec 18, 2019

    @dpinol

    ok, thanks

  21. added a commit that references this issue on Feb 28, 2020
  22. Skylion007 commented on May 9, 2021

    @Skylion007

    @asottile Seems like a good way to handle this would be a hook that inserted the requirements from a third party requirements.txt and ran on requirements and pre-commit-config files, right? The requirement could be specified by a special key or comment in the yaml.

  23. asottile commented on May 9, 2021

    @asottile
    Member

    pre-commit does not install from the repository under test. period.

    doing so makes caching intractable

  24. locked as resolved and limited conversation to collaborators on May 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions