Better method for managing clang-format binaries
Opened this issue · 14 comments
We are able to call clang-format by shelling out to it on the system path, and we are able to cache its results by enforcing a version check on the binary. However, if it isn't on the path, or the version is wrong, we just show a helpful error message and let the user figure it out from there:
It would be nice if we could handle this better, or at least have better instructions.
@akornilov feel free to ignore, but I see that you maintain the only llvm plugins in the gradle plugin portal. Any tips?
When a user declares com.google.guava:guava:20, they don't have to go download it and put it on their path, gradle handles that for them.
We'd like to do something similar for clangFormat('10.0.0') or clangFormat('10.0.1'). We compiled instructions (top) for using the package managers on each platform, but it's not seamless, and it's hard to control which version you get.
Do you have any suggestions for how to grab and cache llvm binaries, or better yet just the clang-format part of the binaries?
Thanks, and "not interested" is a fine answer, don't mean to bother 😁
Thanks, this is very helpful!!
Are you from the Gradle developers team? ...
What kind of software do you develop at the moment
I'm not affiliated with Gradle. I work on tooling for embedded systems, and Spotless is a side project that does autoformatting. I don't use llvm to compile, but I do want to use clang-format to enforce formatting rules on some C code.
my already created plugin for Gradle which allows you to use LLVM libraries
Perfect, this is exactly what I was looking for, thanks very much! I found your plugin at https://plugins.gradle.org/, but I couldn't figure out if it did installation for you, or if the user still needed to do that themselves. I think where I got stuck is that I didn't think to click the "wiki" section, where you have very good documentation. What I like about GitHub is that the README shows up really big, so it's easy to tell new users the details of the project. The SourceForge landing page doesn't leave much space for documentation, so it can be hard to figure out where to get started. But now I know where to start!
Hi,
I recently started using Spotless (awesome project!), and found it very frustrating to use with clang-format. That's mainly due to the combination of Spotless' strict version check, and clang-format's os/distro specific versioning (i.e. 10.0.0-4ubuntu1).
If I understand correctly, the purpose of the strict version check is to allow safe caching. Then, why not instead of enforcing a specific version, just invalidating/ignoring the cache in case of a version mismatch? IMO this would make clang-format much easier to use.
Moreover, if the purpose of the strict version check is also to control the allowed version of clang-format, we could assert only the x.y.z part of the version.
the purpose of the strict version check is to allow safe caching
That's one aspect, but the other part is to make sure that the team and CI are all on the same page. You've got clang 1, I've got clang 2, and we keep formatting-over each other's code. That's bad. It's better to force us to install the same clang.
But it's definitely a problem if we are including a platform-specific identifier, that ought to get stripped off the check. The way to do that is to add .versionRegex("version (\\S*)(?:\\-)") to this hunk of code:
I might have the regex there wrong, the default is this:
We need to add a non-capturing group to grab the hyphen in the platform-specific part and keep it out of the version group.
Would it be possible to only fail the build if a clang-format call actually happens? The gradle plugin crashes the build during the configuration phase if it is not available or the wrong version, no matter which tasks are run.
Context: I have some CI scanning jobs on my projects and I cannot easily change the images used. I would love to use the spotless plugin but if I have to choose between vulnerability scanning and a nice format I will take vulnerability scanning.
We need to add a non-capturing group to grab the hyphen in the platform-specific part and keep it out of the version group.
I'm afraid that this is not good enough, because the minor/patch parts of the version might also differ across distros.
For example, for clang-format-9, on Ubuntu 20.04 I get 9.0.1-12 while on Ubuntu 18.04 I get 9.0.0-2~ubuntu18.04.2.
I think that Spotless should either assert only the major version part, or it should allow the user to customize the version check by supplying a custom regex pattern.
Would it be possible to only fail the build if a clang-format call actually happens?
Yes, this is possible, but a bit tricky. You can see here that State takes a String exeAbsPath.
This could instead be a Supplier<String>. This would also mean that the List<String> args field in State would have to be lazily constructed on first usage.
spotless/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java
Lines 94 to 104 in 8797048
I think your feature request for this is a good one, and it's probably worth somehow generalizing this lazy construction of args into the ForeignExe class. The other usage of it, BlackStep, would benefit from the same thing. I'd be happy to merge a PR for this (doesn't matter if the PR is just for Clang, includes ForeignExe, or if it includes Black too, if it solves your problem then I'd merge it).
grab the hyphen ... and keep it out of the version group ... is not good enough, because the minor/patch parts of the version might also differ across distros
I'm happy to merge a PR which supports this too, the tricky part is docs. When you first encountered this issue of different clang-format-9's, and you looked at our docs, what do you wish it had said? Figure that out, and then building to it would be easy.
Thanks to @bh-tt, starting with plugin-gradle 6.9.0 and plugin-maven 2.24.0, the check for foreign-exes is now lazy.
Still happy to merge a PR to allow new version regex. One thought which just occurred to me - you could make your build configuration platform-specific, e.g.:
clang().version(if (ubuntu20) "9.0.1-12" else if (ubuntu18) "9.0.0-2~ubuntu18.04.2")Not great, but a possible workaround until something better comes along...
I find the comment "version is optional" right after clangFormat('10.0.1') in plugin-gradle/README.md misleading, removing the version makes it default to a clang-format that is too old for Ubuntu 22.04 LTS (clang-format-11 to 15 are available).
For those who want to detect the clang-format version, here's a snippet for build.gradle:
def clangFormatBinary = (String) project.findProperty('clangFormat') ?: 'clang-format'
spotless {
java {
def clangOutput = new ByteArrayOutputStream()
exec {
commandLine clangFormatBinary, '--version'
standardOutput = clangOutput
}
def clangVersion = (String) (clangOutput.toString() =~ /\d+\.\d+\.\d+/)[0]
clangFormat(clangVersion).pathToExe(clangFormatBinary).style('file')
}
}Run with ./gradlew spotlessCheck -PclangFormat=clang-format-15 if the clang-format-binary is called something other than clang-format.
@nedtwigg what is the recommended way of handling clang-format for both macOS and Linux? The clang-format binary has a major version number appended at the end on Ubuntu (clang-format-15), but it can also just be called clang-format. Different LTS releases of Ubuntu has different versions of clang-format available as packages, so there isn't an obvious smallest common denominator that can be used.
The snippet in my previous comment does what I want ("I tell Gradle the clang-format binary to use, Gradle figures out what version it is and how to use it without bothering me"), except that the snippet causes failures for people without clang-format installed on their computer. I understand people cannot format the code without clang-format, but ./gradlew tasks and ./gradlew run also fails for them, and I do have an interest in letting people at least run (=start) HEAD without having to install a complete development environment.
You could set a myproj_enable_spotless=true in your ~/.gradle/gradle.properties.