Feature: Add poetry as pre-commit hook

[Cielquan]

Pull Request Check List

Resolves: #2457

• Added tests for changed code. (no code was changed)
• Updated documentation for changed code.

I created this draft PR for easier discussion in the issue #2457 .

[Cielquan]

Thanks for the review. I fixed all issues.

But before I make the draft final I want to get feedback on the topic with the chicken-egg-problem I mentioned in the comments of the connected issue.

I fixed the problem with the first solution (bootstrapping) in commit 1121d06 but I would like to hear others opinions first.

EDIT: Commit 1121d06 will be removed before finalization.
I placed the problem in issue #2515 with PR #2516 .

• Resolve chicken-egg-problem - install poetry with poetry #2515
• Merge added the minimal dependencies to build-system #2516
• Remove 1121d06
• Finalize the PR

[Cielquan]

#2515 is solved on develop and will be released with 1.1 so I will change this PR to use develop.

[Cielquan]

Rebased the branch onto develop and changed merge target to develop.

[Cielquan]

@sdispater Do you have any idea why the Tests / FreeBSD / PYTHON:python3.5 pipeline fails?

I found the same issue in your PR #2509 and the PR #2448 .

The similarity is that all 3 have develop as target. But there are other PRs with target develop which succeeded in between.

py run-test: commands[1] | poetry run pytest -q --junitxml=junit.xml tests tests/
Traceback (most recent call last):
  File "/tmp/cirrus-ci-build/.tox/py/bin/poetry", line 6, in <module>
    from pkg_resources import load_entry_point
  File "/tmp/cirrus-ci-build/.tox/py/lib/python3.5/site-packages/pkg_resources/__init__.py", line 3252, in <module>
    @_call_aside
  File "/tmp/cirrus-ci-build/.tox/py/lib/python3.5/site-packages/pkg_resources/__init__.py", line 3236, in _call_aside
    f(*args, **kwargs)
  File "/tmp/cirrus-ci-build/.tox/py/lib/python3.5/site-packages/pkg_resources/__init__.py", line 3265, in _initialize_master_working_set
    working_set = WorkingSet._build_master()
  File "/tmp/cirrus-ci-build/.tox/py/lib/python3.5/site-packages/pkg_resources/__init__.py", line 584, in _build_master
    ws.require(__requires__)
  File "/tmp/cirrus-ci-build/.tox/py/lib/python3.5/site-packages/pkg_resources/__init__.py", line 901, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/tmp/cirrus-ci-build/.tox/py/lib/python3.5/site-packages/pkg_resources/__init__.py", line 787, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'typing-extensions<4.0,>=3.6; python_version >= "3.5.0" and python_version < "3.5.4"' distribution was not found and is required by clikit
ERROR: InvocationError for command /tmp/cirrus-ci-build/.tox/py/bin/poetry run pytest -q --junitxml=junit.xml tests tests/ (exited with code 1)
___________________________________ summary ____________________________________
ERROR:   py: commands failed

[abn]

@Cielquan there is a bug in clikit that is being resolved. Should be fixed soon.

sdispater/clikit#32

[abn]

@Cielquan please rebase this to trigger CI with the fix.

[Cielquan]

@abn Thanks for sharing.

Well I updated my branch like this:

$ git remote add upstream [email protected]:python-poetry/poetry.git
$ git fetch upstream
$ git checkout develop
$ git rebase 
$ git checkout feature/pre-commit-hooks
$ git rebase develop 
$ git push -f

All changes were fast-forward with no complication.

But now the tests are totally not working 😢

The windows tests fail in the configure poetry step:

Run poetry config virtualenvs.in-project true
  poetry config virtualenvs.in-project true
  shell: C:\Program Files\Git\bin\bash.EXE --noprofile --norc -e -o pipefail {0}
  env:
    pythonLocation: C:\hostedtoolcache\windows\Python\2.7.18\x64
    PATH: /c/Users/runneradmin/.poetry/bin:/mingw64/bin:/usr/bin:/c/Users/runneradmin/bin:/c/Users/runneradmin/AppData/Roaming/Python/Python27/Scripts:/c/hostedtoolcache/windows/Python/2.7.18/x64/Scripts:/c/hostedtoolcache/windows/Python/2.7.18/x64:/c/Program Files/Mercurial:/c/ProgramData/kind:/c/vcpkg:/c/cf-cli:/c/Program Files (x86)/NSIS:/c/Program Files/Mercurial:/c/hostedtoolcache/windows/stack/2.3.1/x64:/c/ProgramData/chocolatey/lib/ghc.8.10.1/tools/ghc-8.10.1/bin:/c/Program Files/dotnet:/c/mysql-5.7.21-winx64/bin:/c/Program Files/Java/zulu-8-azure-jdk_8.40.0.25-8.0.222-win_x64/bin:/c/SeleniumWebDrivers/GeckoDriver:/c/Program Files (x86)/sbt/bin:/c/Rust/.cargo/bin:/c/hostedtoolcache/windows/go/1.14.3/x64/bin:/c/Program Files (x86)/GitHub CLI:/bin:/c/hostedtoolcache/windows/Python/3.7.7/x64/Scripts:/c/hostedtoolcache/windows/Python/3.7.7/x64:/c/hostedtoolcache/windows/Ruby/2.5.8/x64/bin:/c/npm/prefix:/c/Program Files (x86)/Microsoft SDKs/Azure/CLI2/wbin:/c/windows/system32:/c/windows:/c/windows/System32/Wbem:/c/windows/System32/WindowsPowerShell/v1.0:/c/windows/System32/OpenSSH:/c/ProgramData/Chocolatey/bin:/c/Program Files/Docker:/c/Program Files/PowerShell/7:/c/Program Files/dotnet:/c/Program Files/Microsoft SQL Server/130/Tools/Binn:/c/Program Files/Microsoft SQL Server/Client SDK/ODBC/170/Tools/Binn:/c/Program Files (x86)/Microsoft SQL Server/110/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/130/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/140/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/150/DTS/Binn:/c/Program Files (x86)/Windows Kits/10/Windows Performance Toolkit:/c/Program Files/Microsoft Service Fabric/bin/Fabric/Fabric.Code:/c/Program Files/Microsoft SDKs/Service Fabric/Tools/ServiceFabricLocalClusterManager:/c/Program Files/nodejs:/c/Program Files/OpenSSL/bin:/c/Strawberry/c/bin:/c/Strawberry/perl/site/bin:/c/Strawberry/perl/bin:/cmd:/mingw64/bin:/usr/bin:/c/tools/php:/c/Program Files (x86)/sbt/bin:/c/Program Files (x86)/Subversion/bin:/c/SeleniumWebDrivers/ChromeDriver:/c/SeleniumWebDrivers/EdgeDriver:/c/ProgramData/chocolatey/lib/maven/apache-maven-3.6.3/bin:/c/Program Files/CMake/bin:/c/Program Files/Amazon/AWSCLIV2:/c/Program Files/Amazon/AWSSAMCLI/bin:/c/Users/runneradmin/.dotnet/tools:/c/Users/runneradmin/AppData/Local/Microsoft/WindowsApps
/c/Users/runneradmin/.poetry/bin/poetry: line 2: import: command not found
/c/Users/runneradmin/.poetry/bin/poetry: line 3: import: command not found
/c/Users/runneradmin/.poetry/bin/poetry: line 4: import: command not found
/c/Users/runneradmin/.poetry/bin/poetry: line 6: syntax error near unexpected token `('
/c/Users/runneradmin/.poetry/bin/poetry: line 6: `lib = os.path.normpath(os.path.join(os.path.realpath(__file__), "../..", "lib"))'
##[error]Process completed with exit code 2.

[Cielquan]

Problems solved themselves \o/ ? 🎆

[Diaoul]

You cannot have additional arguments like --extras or --without-hashes without overriding the whole args.
So it's not exactly the same feature-set as suggested in my implementation at #2457 where I had to wrap the current CLI in a more pre-commit friendly script, take a look at the README.

I don't remember why I didn't use the pass_filenames: false approach, maybe it had something to do with how pre-commit reports an error if you don't specify it.

I guess this is fine to leave out some complexity for the user to handle for the sake of a simpler implementation in the code.

[Diaoul]

I don't remember why I didn't use the pass_filenames: false approach, maybe it had something to do with how pre-commit reports an error if you don't specify it.

I just tested this, errors are reported the same way, so I think the reason probably was to use files instead of using args to specify the file. So you could just do that without messing with the whole args.

[Cielquan]

You cannot have additional arguments like --extras or --without-hashes without overriding the whole args.

I am fully aware of this. But I did not clarify that in the docs.

• I will update them.

So it's not exactly the same feature-set as suggested in my implementation at #2457 where I had to wrap the current CLI in a more pre-commit friendly script, take a look at the README.

I don't remember why I didn't use the pass_filenames: false approach, maybe it had something to do with how pre-commit reports an error if you don't specify it.

I guess this is fine to leave out some complexity for the user to handle for the sake of a simpler implementation in the code.

Well I use my branch with pre-commit like so:

  - repo: https://github.com/Cielquan/poetry
    rev: 4c3338d18d319fb7edea02d653d2c65622d84ff2
    hooks:
      #: Check config file
      - id: poetry-check
      #: Update lock file
      - id: poetry-lock

Those two hooks did work till now without a problem, but they also don't take args.

When I wrote the branch I tested it manually and did not see any errors.

---

I don't remember why I didn't use the pass_filenames: false approach, maybe it had something to do with how pre-commit reports an error if you don't specify it.

I just tested this, errors are reported the same way, so I think the reason probably was to use files instead of using args to specify the file. So you could just do that without messing with the whole args.

Well I understand your reasoning but normally you write your .pre-commit-config.yaml once and don't update it every commit. So IMO its fine if the user has to specify the default args again if he/she wants to change the behavior. I also think its more clean for the enduser this way. With the updated docs this should be fine.

EDIT: fix quote

[Cielquan]

@Diaoul

Cielquan dismissed Diaoul’s stale review via 43b0fd8 13 minutes ago

While you suggested a change I was changing the docs and overwrote it without noticing.

[Diaoul]

Did you run into issues installing pre-commit with poetry? On my computer it's pulling weird dependencies, I need to reproduce on a clean environment to see if I can confirm that behavior.

[Diaoul]

Weird, on my computer it tries to install a bunch of stuff and fails on mysql-connector-python-rf.
pip doesn't even install that.

pre-commit 2.5.1 A framework for managing and maintaining multi-language pre-commit hooks.
├── cfgv >=2.0.0
├── identify >=1.0.0
├── importlib-metadata *
│   └── zipp >=0.5 
├── importlib-resources *
│   ├── importlib-metadata * 
│   │   └── zipp >=0.5 
│   └── zipp >=0.4 (circular dependency aborted here)
├── nodeenv >=0.11.1
├── pyyaml >=5.1
├── toml *
└── virtualenv >=20.0.8
    ├── dnspython * 
    ├── mysnow * 
    │   ├── mysql-connector-python-rf * 
    │   ├── requests * 
    │   │   ├── certifi >=2017.4.17 
    │   │   ├── chardet >=3.0.2,<4 
    │   │   ├── idna >=2.5,<3 
    │   │   └── urllib3 >=1.21.1,<1.25.0 || >1.25.0,<1.25.1 || >1.25.1,<1.26 
    │   └── wheel * 
    ├── mysql-connector-python-rf * (circular dependency aborted here)
    ├── paramiko * 
    │   ├── bcrypt >=3.1.3 
    │   │   ├── cffi >=1.1 
    │   │   │   └── pycparser * 
    │   │   └── six >=1.4.1 
    │   ├── cryptography >=2.5 
    │   │   ├── cffi >=1.8,<1.11.3 || >1.11.3 (circular dependency aborted here)
    │   │   └── six >=1.4.1 (circular dependency aborted here)
    │   └── pynacl >=1.0.1 
    │       ├── cffi >=1.4.1 (circular dependency aborted here)
    │       └── six * (circular dependency aborted here)
    ├── requests * (circular dependency aborted here)
    ├── setuptools * 
    └── wheel * (circular dependency aborted here)

EDIT: Here is the pip output:

$ pip install pre-commit
Requirement already satisfied: pre-commit in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (2.5.1)
Requirement already satisfied: toml in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from pre-commit) (0.10.1)
Requirement already satisfied: pyyaml>=5.1 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from pre-commit) (5.3.1)
Requirement already satisfied: identify>=1.0.0 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from pre-commit) (1.4.19)
Requirement already satisfied: nodeenv>=0.11.1 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from pre-commit) (1.4.0)
Requirement already satisfied: importlib-metadata; python_version < "3.8" in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from pre-commit) (1.6.1)
Requirement already satisfied: virtualenv>=20.0.8 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from pre-commit) (20.0.23)
Requirement already satisfied: cfgv>=2.0.0 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from pre-commit) (3.1.0)
Requirement already satisfied: zipp>=0.5 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from importlib-metadata; python_version < "3.8"->pre-commit) (3.1.0)
Requirement already satisfied: appdirs<2,>=1.4.3 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from virtualenv>=20.0.8->pre-commit) (1.4.4)
Requirement already satisfied: filelock<4,>=3.0.0 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from virtualenv>=20.0.8->pre-commit) (3.0.12)
Requirement already satisfied: six<2,>=1.9.0 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from virtualenv>=20.0.8->pre-commit) (1.15.0)
Requirement already satisfied: distlib<1,>=0.3.0 in /home/user/.cache/pypoetry/virtualenvs/project-KjAM8iof-py3.7/lib/python3.7/site-packages (from virtualenv>=20.0.8->pre-commit) (0.3.0)

Unrelated to this PR but I think if this is an actual issue it's pretty embarrassing 😉

[neersighted]

Minor doc changes requested. I'd also love people to give the docs a close look (though I don't necessarily feel a need to hold up merging this) -- bad docs for pre-commit hooks are super common and super annoying in the ecosystem.

[Cielquan]

Minor doc changes requested. I'd also love people to give the docs a close look (though I don't necessarily feel a need to hold up merging this) -- bad docs for pre-commit hooks are super common and super annoying in the ecosystem.

Thanks for the review and suggestions. Your changed phrasing is easier to read than mine. 👍🏾
Now only the issue with the docs for groups is missing above.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[Cielquan]

Thanks for merging 😃

[MaxWinterstein]

Would love to use this at my repo, but it seems there is a problem at pre-commit.ci side I guess?

I can't find an already cached version, and the installation always fails on me, see e.g.:

https://results.pre-commit.ci/run/github/327882738/1639672185.PASU-kRYTIufuGkEgXVAAA

[graingert]

Would love to use this at my repo, but it seems there is a problem at pre-commit.ci side I guess?

I can't find an already cached version, and the installation always fails on me, see e.g.:

https://results.pre-commit.ci/run/github/327882738/1639672185.PASU-kRYTIufuGkEgXVAAA

You need a paid pre-commit.ci account to access the network during the running of hooks

[MaxWinterstein]

You need a paid pre-commit.ci account to access the network during the running of hooks

Oh, I see, found some notes about this here: pre-commit-ci/issues#55 (comment)

So, at GitHub level, using pre-commit.ci, this hook is unusable.

This might be worth mentioning in the documents?

[floatingpurr]

I'm trying the poetry-export hook but it doesn't work. I need to override the files value this way:

  # export python requirements
  - repo: https://github.com/python-poetry/poetry
    rev: 2fa6c17 # put here the next release with the hooks
    hooks:
      - id: poetry-export
        files: '^.*poetry\.lock$'

with the default value (i.e., '^.*/poetry\.lock$') it didn't work. I'm afraid that the same holds for poetry-check. If necessary, I can open up a PR about it.

[vwxyzjn]

For those of you who stumbled across this post like I do, you can do:

  - repo: https://github.com/python-poetry/poetry
    rev: 1.2.0b1
    hooks:
      - id: poetry-export
        args: ["-f", "requirements.txt", "-o", "requirements.txt"]

[Cielquan]

For those of you who stumbled across this post like I do, you can do:

  - repo: https://github.com/python-poetry/poetry
    rev: 1.2.0b1
    hooks:
      - id: poetry-export
        args: ["-f", "requirements.txt", "-o", "requirements.txt"]

You should also be able to shorten that snippet by the last line, because this is the default config:
https://github.com/python-poetry/poetry/blob/1.2.0b1/.pre-commit-hooks.yaml#L26

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.