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

typos pre-commit hook building from source rather than binary install #682

Closed
tekumara opened this issue Mar 13, 2023 · 10 comments · Fixed by #685 or #686
Closed

typos pre-commit hook building from source rather than binary install #682

tekumara opened this issue Mar 13, 2023 · 10 comments · Fixed by #685 or #686

Comments

@tekumara
Copy link
Contributor

In .pre-commit-hooks.yaml I have:

  - repo: https://github.com/crate-ci/typos
    rev: v1.13.20
    hooks:
      - id: typos

But the install takes 3 mins because it is building from source on x86_64 linux (Ubuntu 22.04).

@tekumara
Copy link
Contributor Author

Apparently pre-commit hooks are installed by cloning the repo and running pip install .

So I can simulate this using pip install -v git+https://github.com/crate-ci/typos.git@v1.13.20. When I do, I can see it's compiling all the crates, rather than downloading and using the binaries.

@tekumara
Copy link
Contributor Author

tekumara commented Mar 13, 2023

It looks like its using maturin to build the wheels:

pip install -v git+https://github.com/crate-ci/typos.git@v1.13.20
....
  Running command Building wheel for typos (pyproject.toml)
  Running `maturin pep517 build-wheel -i /home/runner/work/aec/aec/typos/.venv/bin/python --compatibility off`
  📦 Including license file "/tmp/pip-req-build-1h1ots63/LICENSE-APACHE"
  📦 Including license file "/tmp/pip-req-build-1h1ots63/LICENSE-MIT"
  🔗 Found bin bindings
  📡 Using build options bindings from pyproject.toml
     Compiling proc-macro2 v1.0.51
     Compiling unicode-ident v1.0.6

I wonder if #675, which switched to the maturin backend instead of setup.py, means setup.py from #289 is now being ignored?

@tekumara
Copy link
Contributor Author

If I use the version prior, it fetches the binaries and is a lot faster:

pip install -v git+https://github.com/crate-ci/typos.git@v1.13.18
....
running fetch_binaries
....

tekumara added a commit to seek-oss/aec that referenced this issue Mar 13, 2023
@calumy
Copy link
Contributor

calumy commented Mar 13, 2023

For reference, Ruff solves this by making a pre-commit mirror: https://github.com/charliermarsh/ruff-pre-commit.

@epage
Copy link
Collaborator

epage commented Mar 13, 2023

Huh, that was an unintended side effect of adding PyPI packages.

I've been avoiding specialized repos for pre-commit and Github Actions to keep the release process automated within existing tooling.

@calumy
Copy link
Contributor

calumy commented Mar 13, 2023

I created a mirror at https://github.com/calumy/typos-pre-commit.

It can be added to your pre-commit config with the following:

  - repo: https://github.com/calumy/typos-pre-commit
    rev: 'v1.13.20'
    hooks:
      - id: typos

I'm happy to transfer this to @create-ci if that would be preferred.

With regards to keeping the release process automated, if I have setup the mirror correctly, it should update automatically.

@calumy
Copy link
Contributor

calumy commented Mar 13, 2023

However, if #685 fixes this, there is no need for this mirror.

@epage
Copy link
Collaborator

epage commented Mar 13, 2023

Can someone confirm that that #685 fixed things?

@calumy
Copy link
Contributor

calumy commented Mar 13, 2023

I don't think it did, I'm afraid. For pre-commit work for a python package you have to be able to install it with pip install .. When I ran this command I got the following error:

  Preparing metadata (setup.py) ... error
  error: subprocess-exited-with-error
  
  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [14 lines of output]
      error: Multiple top-level packages discovered in a flat-layout: ['docker', 'crates', 'benchsuite'].
      
      To avoid accidental inclusion of unwanted files or directories,
      setuptools will not proceed with this build.
      
      If you are trying to create a single distribution with multiple packages
      on purpose, you should not rely on automatic discovery.
      Instead, consider the following options:
      
      1. set up custom discovery (`find` directive with `include` or `exclude`)
      2. use a `src-layout`
      3. explicitly set `py_modules` or `packages` with a list of names
      
      To find more information, look for "package discovery" on setuptools docs.
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

When running pre-commit with the following config it created a similar error.

  - repo: https://github.com/crate-ci/typos
    rev: 5f7454815c1e60353d4fdd872c86982dc0d3a99b
    hooks:
      - id: typos

@calumy
Copy link
Contributor

calumy commented Mar 13, 2023

Suggested a fix in #686.

phip1611 pushed a commit to phip1611/typos that referenced this issue Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants