Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

poetry export pre-commit hook #2457

Closed
2 tasks done
Diaoul opened this issue May 22, 2020 · 12 comments · Fixed by #2511
Closed
2 tasks done

poetry export pre-commit hook #2457

Diaoul opened this issue May 22, 2020 · 12 comments · Fixed by #2511
Labels
kind/feature Feature requests/implementations

Comments

@Diaoul
Copy link

Diaoul commented May 22, 2020

  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have searched the documentation and believe that my question is not covered.

Feature Request

Having both a requirements.txt file and pyproject.toml + poetry.lock file may seem like duplication but it is sometimes needed for example for installing dependencies in Docker without loosing the cache on version bump or other changes unrelated to dependencies.
Installing poetry on the docker image is also a bit of a pain, and slower than pip as mentioned on other issues.

For all those reasons I decided to maintain a requirements.txt in addition to poetry, I wanted this process to be smooth and never forget about it so I created this simple pre-commit-hook that takes care of running poetry export for me! 🍰

This issue is a feature request for having poetry-related pre-commit hooks like this one maintained here rather than on 3rd party repositories.

@Diaoul Diaoul added kind/feature Feature requests/implementations status/triage This issue needs to be triaged labels May 22, 2020
@finswimmer finswimmer added area/plugin-api Related to plugins/plugin API and removed status/triage This issue needs to be triaged labels May 24, 2020
@finswimmer
Copy link
Member

Hey @Diaoul,

sounds like this is something for the upcoming plugin system.

fin swimmer

@Diaoul
Copy link
Author

Diaoul commented May 24, 2020

My code is only rearranging arguments for improved pre-commit compatibility. I don't think a full blown plugin system is necessary for that to be delivered.
Pre-commit hooks could be officially supported with for example a precommit {hook_name}.

It's common for libraries with a CLI to ship pre-commit hooks officially (black and isort for example).
The only difference is that here the current CLI of poetry doesn't allow for that to work perfectly.

So it would be an officially supported plugin? Is that a thing?

@finswimmer
Copy link
Member

Hello @Diaoul,

I see now. I misunderstood your feature request. This is really nothing for the plugin system.

I cannot answer the question by my own, whether we will create and support such a pre-commit hook. So I let this request open for further discussion.

fin swimmer

@finswimmer finswimmer removed the area/plugin-api Related to plugins/plugin API label May 28, 2020
@Diaoul
Copy link
Author

Diaoul commented May 29, 2020

Thanks @finswimmer, I'll wait for others to weight in.
This might not be a frequent feature request but I think keeping automatically a requirements.txt in sync with poetry would make the switch to poetry easier for existing projects!

@SanketDG
Copy link
Contributor

Also relevant to add here https://pre-commit.com/hooks.html

@Cielquan
Copy link
Contributor

Cielquan commented Jun 6, 2020

I just also had the idea to add .pre-commit-hooks.yaml to poetry to enable usability with pre-commit.

In itself the setup is straightforward and pretty easy. You just need to add the file with the correct config and done. I did this on my fork to prepare a PR (#2511).

Then I went to test it and a catastrophic error occurred: check-egg-error (as it seems)

When I clone the poetry repo I cannot install it via pip install . like pre-commit will do. For installing it it wants to build a wheel for poetry but fails so because for building it it wants to use itself but its dependencies are not there because its not installed - catch-22.

.pre-commit-config.yaml to reproduce the output below:

repos:
  - repo: https://github.com/Cielquan/poetry
    rev: 81d92b53d742f874f260f14651f340c04592cfdf
    hooks:
      - id: poetry-check
      - id: poetry-lock
      - id: poetry-export
        args: ["-f", "requirements.txt", "-o", "requirements.txt"]

You can also just clone the repo and try to install it with pip install . into a venv.

Here the output from pre-commit. It looks almost the same when you do pip install . in the repo.

Click to expand output
$ pre-commit run --all-files
[INFO] Initializing environment for https://github.com/Cielquan/poetry.
[INFO] Installing environment for https://github.com/Cielquan/poetry.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/bin/python', '-mpip', 'install', '.')
return code: 1
expected return code: 0
stdout:
    Processing /home/user/.cache/pre-commit/reponzle4x2t
      Installing build dependencies: started
      Installing build dependencies: finished with status 'done'
      Getting requirements to build wheel: started
      Getting requirements to build wheel: finished with status 'error'
    
stderr:
      ERROR: Command errored out with exit status 1:
       command: /home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/bin/python /home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmp87f36717
           cwd: /tmp/pip-req-build-6rb54p9s
      Complete output (36 lines):
      Traceback (most recent call last):
        File "/home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 280, in <module>
          main()
        File "/home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 263, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "/home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 114, in get_requires_for_build_wheel
          return hook(config_settings)
        File "/tmp/pip-build-env-6o4jcxh7/overlay/lib/python3.8/site-packages/intreehooks.py", line 53, in get_requires_for_build_wheel
          return self._backend.get_requires_for_build_sdist(config_settings)
        File "/tmp/pip-build-env-6o4jcxh7/overlay/lib/python3.8/site-packages/intreehooks.py", line 38, in _backend
          obj = self._module_from_dir(modname)
        File "/tmp/pip-build-env-6o4jcxh7/overlay/lib/python3.8/site-packages/intreehooks.py", line 25, in _module_from_dir
          mod = importlib.import_module(modname)
        File "/usr/lib/python3.8/importlib/__init__.py", line 127, in import_module
          return _bootstrap._gcd_import(name[level:], package, level)
        File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
        File "<frozen importlib._bootstrap>", line 991, in _find_and_load
        File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
        File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
        File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
        File "<frozen importlib._bootstrap>", line 991, in _find_and_load
        File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
        File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
        File "<frozen importlib._bootstrap_external>", line 783, in exec_module
        File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
        File "/tmp/pip-req-build-6rb54p9s/poetry/masonry/__init__.py", line 10, in <module>
          from .builder import Builder
        File "/tmp/pip-req-build-6rb54p9s/poetry/masonry/builder.py", line 1, in <module>
          from .builders.complete import CompleteBuilder
        File "/tmp/pip-req-build-6rb54p9s/poetry/masonry/builders/__init__.py", line 1, in <module>
          from .complete import CompleteBuilder
        File "/tmp/pip-req-build-6rb54p9s/poetry/masonry/builders/complete.py", line 6, in <module>
          from poetry.factory import Factory
        File "/tmp/pip-req-build-6rb54p9s/poetry/factory.py", line 10, in <module>
          from clikit.api.io.io import IO
      ModuleNotFoundError: No module named 'clikit'
      ----------------------------------------
    ERROR: Command errored out with exit status 1: /home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/bin/python /home/user/.cache/pre-commit/reponzle4x2t/py_env-python3/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmp87f36717 Check the logs for full command output.
    
Check the log at /home/user/.cache/pre-commit/pre-commit.log
*** Exit Code: 1 ***

Solutions

So before we can proceed the chicken-egg-problem must be solved.

The solutions I came up with could be:

  1. Bootstrapping (I think its called): change from using itself to using poetry as a package (like we all do)

old:

[build-system]
requires = ["intreehooks"]
build-backend = "intreehooks:loader"
[tool.intreehooks]
build-backend = "poetry.masonry.api"

new:

[build-system]
requires = ["poetry>=0.12"]
build-backend = "poetry.masonry.api"
  1. add the install dependencies to the requires section in [build-system] in pyproject.toml
  2. include dependencies in poetry repo itself

I think option 2 and 3 are overhead and IMO option 1 is the solution.

If the solutions above all don't sound good to the devs and there are no other solutions
I can think of two workarounds:

  1. Change the hooks to language: system. Then they would depend on a global installation of poetry or poetry must be manually installed inside the virtual environment from which one calls pre-commit.
  2. Open a second repo like python-poetry/poetry-hooks or whatever. There we need a dummy python package to be installed and add additional_dependencies: ["poetry"] to the hooks to let pre-commit install poetry from PyPI.

I personally don't like both workarounds because the first one is too specific and the second is just a hack to pre-commit and no clean solution.

I implemented solution 1 in my draft PR and you can test it with the following .pre-commit-config.yaml:

repos:
  - repo: https://github.com/Cielquan/poetry
    rev: b9c381b6a8c5226e79cbec30de99c7bcda537089
    hooks:
      - id: poetry-check
      - id: poetry-lock
      - id: poetry-export

EDIT: Added .pre-commit-config.yaml
EDIT2: Added solution.
EDIT3: Updated solutions
EDIT4: Added working .pre-commit-config.yaml
EDIT5: Reorganized comment.
EDIT6: Hid Error output in spoiler
EDIT7: The example above does not work anymore because of rebases.

@Cielquan
Copy link
Contributor

Cielquan commented Jun 7, 2020

I took the check-egg-problem discussion into a new issue #2515 with PR #2516 .

EDIT: #2515 is solved with develop and release 1.1. I will change #2511 to go to develop and be released alongside (if merged).

@ludaavics
Copy link

Stumbled on this looking for something else.
FWIW, dephell already does this (and more) for you: https://dephell.readthedocs.io/use-git-hook.html
But agreed, having something like this directly with poetry would smooth transitions / learning curves

@redraw
Copy link

redraw commented Oct 20, 2020

I took a simpler approach I guess? Supposing poetry is installed,

- repo: local
  hooks:
    - id: export-requirements
      name: Export requeriments.txt
      language: system
      pass_filenames: false
      entry: poetry export --without-hashes -o requirements.txt
      files: ^(pyproject.toml|poetry.lock)$

@Cielquan
Copy link
Contributor

I took a simpler approach I guess? Supposing poetry is installed,

- repo: local
  hooks:
    - id: export-requirements
      name: Export requeriments.txt
      language: system
      pass_filenames: false
      entry: poetry export --without-hashes -o requirements.txt
      files: ^(pyproject.toml|poetry.lock)$

Well good point. I think for local development its fine, but when you want to run it in CI I think it gets eventually problematic.
I run pre-commit via tox (pre-commit/pre-commit#1309) so in my case it would be fine, but for others they would have to eventually change setups.

When #2511 gets merged the issue will be nonexistent anymore.

@Cielquan
Copy link
Contributor

Cielquan commented Dec 2, 2020

I created a mirror of poetry for use with pre-commit.

I plan to support it as of now till #2511 gets merged. The current version 1.1.4 is the only one supported at the moment. As poetry releases new versions they will be added also.

If you need a specific earlier version for whatever reason leave me an issue so I will add the corresponding tag.

Copy link

github-actions bot commented Mar 2, 2024

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Feature requests/implementations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants