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

Any chance for a clang-tidy version of this repository? #32

Open
HappyCerberus opened this issue Apr 6, 2022 · 13 comments
Open

Any chance for a clang-tidy version of this repository? #32

HappyCerberus opened this issue Apr 6, 2022 · 13 comments

Comments

@HappyCerberus
Copy link

The clang-format package and the corresponding hook are great.

I also rely on clang-tidy to enforce code style specifically through the readability-identifier-naming check.
However, there doesn't seem to be any freestanding package for clang-tidy that could be used with pre-commit.

Unfortunately, I have only minimal Python background so I'm unable to make a fork and do it myself.

@dokempf
Copy link
Member

dokempf commented Apr 11, 2022

My guess is that this should be rather easy, as the LLVM build logic is already in place. I will have a closer look when I find the time, but I am currently on parental leave.

@mgeplf
Copy link

mgeplf commented Apr 11, 2022

I vaguely remember looking at this several years ago; the reason I didn't end up doing it was that the wheel size would be so much larger (at the time, it was pulling in all of libLLVM, libclang-cpp, etc), and most distributions included clang-tidy in their clang installations.

Would still be neat.

but I am currently on parental leave.

Congrats!

@dokempf
Copy link
Member

dokempf commented Apr 11, 2022

My first thought was to realize this as a separate Python package. With the executables being statically linked, there is no size synergy between the two anyway and the packages should install exactly the tool the name suggests.

@mgeplf
Copy link

mgeplf commented Apr 11, 2022

My first thought was to realize this as a separate Python package.

That was my plan too. The size thing was even building it on its own. However, that was many years ago, so worth trying it again.

@narpfel
Copy link

narpfel commented Apr 11, 2022

I made the changes necessary to build clang-tidy here. Basically just s/format/tidy/.

However, the build times are considerably slower, so building in QEMU will take longer than the 6 hour limit for a single GH action workflow.

@fgrie
Copy link
Contributor

fgrie commented May 2, 2022

However, the build times are considerably slower, so building in QEMU will take longer than the 6 hour limit for a single GH action workflow.

A clang-tidy package would be great for me too. What is your opinion on just skipping the build of QEMU platforms for now?

@dokempf
Copy link
Member

dokempf commented May 5, 2022

What is your opinion on just skipping the build of QEMU platforms for now?

I am hesitant. Usage on these platforms might be rare, but pip would fall back to a source installation in these cases which will take an excessive amount of time to complete, which is not really acceptable for a pre-commit hook. I will have a look into setting up custom runners (never did that before).

@dokempf
Copy link
Member

dokempf commented May 5, 2022

I built clang-tidy locally using @narpfel 's fork and realized another obstacle: The statically linked executable exceeds 100MB and will therefore not be uploadable to PyPI without special permit. Edit: It compresses better than I expected, so it might be possible to stay under the 60MB limit, but maybe not for all platforms.

@narpfel
Copy link

narpfel commented May 8, 2022

Would you be interested in one of these workarounds for the QEMU platforms?

  • Including a pure Python shim that either calls the system clang-tidy or errors with a message explaining why clang-tidy currently can’t be built.
  • Cross-compiling outside of cibuildwheel. This would probably require some work, especially if you want to maintain compatibility with an old manylinux standard.

Or would you say that setting up a self-hosted runner is easy enough to not justify the work on one of these workarounds?

Implementing cross-compilation directly in cibuildwheel unfortunately seems to have stalled: pypa/cibuildwheel#598 pypa/cibuildwheel#804

As for the wheel sizes, the largest wheel built here is 34 MB for i686, so there is quite a margin to the PyPI limit.

@renefritze
Copy link

I too had a need for the tidy version of this repo. So I went ahead and did the changes in my fork only to discover this issue afterwards 🙄

I'd be happy to transfer/share rights for the fork and pypi projects.

Or would you say that setting up a self-hosted runner is easy enough to not justify the work on one of these workarounds?

Setup is indeed very easy and I might even be able to provide VM to host a runner.

@dokempf
Copy link
Member

dokempf commented Jun 28, 2022

Hey @renefritze - it's a small world. I am currently out of the office, but I would just call you some time end of the week or beginning of next week to discuss this.

@renefritze
Copy link

Indeed it is 😄
Monday/Wednesday would be best.

@dokempf
Copy link
Member

dokempf commented Jul 4, 2022

So @renefritze and me have decided how to move this forward. There will be a repository similar to this one that is similarly maintained: https:/ssciwr/clang-tidy-wheel It will take us a few more days (or weeks) to get everything up and running (self-hosted runner). I will close this issue once we are done with the transition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants