Better method for managing clang-format binaries

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:

spotless/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java

Lines 70 to 83 in 608e128

    
           String howToInstall = "" + 
        
           		"You can download clang-format from https://releases.llvm.org and " + 
        
           		"then point Spotless to it with `pathToExe('/path/to/clang-format')` " + 
        
           		"or you can use your platform's package manager:" + 
        
           		"\n  win:   choco install llvm --version {version}  (try dropping version if it fails)" + 
        
           		"\n  mac:   brew install clang-format (TODO: how to specify version?)" + 
        
           		"\n  linux: apt install clang-format  (try clang-format-{version} with dropped minor versions)"; 
        
           String exeAbsPath = ForeignExe.nameAndVersion("clang-format", version) 
        
           		.pathToExe(pathToExe) 
        
           		.fixCantFind(howToInstall) 
        
           		.fixWrongVersion( 
        
           				"You can tell Spotless to use the version you already have with `clangFormat('{versionFound}')`" + 
        
           						"or you can download the currently specified version, {version}.\n" + howToInstall) 
        
           		.confirmVersionAndGetAbsolutePath();

It would be nice if we could handle this better, or at least have better instructions.

Link clang-format binary management to its tracking issue #673.

@akornilov

@akornilov feel free to ignore, but I see that you maintain the only llvm plugins in the gradle plugin portal. Any tips?

Hello, Can you please explain in details what kind of tips you expect from me?

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 😁

Hi Ned, Are you from the Gradle developers team? What kind of software do you develop at the moment (I mean software which is related with this topic)? Is it Gradle core or some plugin to have support for LLVM with Gradle? Looks like you need something like my already created plugin for Gradle which allows you to use LLVM libraries (shared or static linkage) in a simple way :) Do you know anything about my plugin? Please take a look here: https://sourceforge.net/p/gradle-cpp/wiki/cpp-llvm Integration with LLVM is very simple: plugins { id 'cpp-application' id 'loggersoft.cpp-llvm' version '1.9' } llvm { version = '10.0.0' linkage = Linkage.SHARED suppressWarningsMsvc = true } For the shared linkage version of LLVM library I use default Gradle format for C++ artifacts (some kind of Maven) cpp-library + maven-publish plugins. There you can find binaries which my plugin uses (LLVM 9 and 10 yet). https://sourceforge.net/projects/llvm-binaries/files/maven/org/llvm For static linkage version of LLVM I use prebuild archives with LLVM binaries from official LLVM site (e.g. https://releases.llvm.org/8.0.0/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-18.04.tar.xz) or unofficial builds prepared by myself and hosted on Source Forge: https://sourceforge.net/projects/llvm-binaries/files. During build my Gradle plugin downloads archive (if it didn't download before) with the required version and puts it in the Gradle folder in the current user home directory. After that extracts archive and add to compiler and linker all necessary options include paths to headers and libraries. Also plugin supports the list of particular LLVM components to choose only exactly required for a project, because LLVM has too many separate libraries for static linkage (about ~150 static libraries). The source of cpp-llvm plugin you can found here: cpp-llvm <https://sourceforge.net/p/gradle-cpp/code/ci/default/tree/plugins/cpp-llvm> Feel free to ask my if something is not clear or I didn't answer on you original question :)

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:

spotless/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java

Lines 78 to 81 in 1ff43ad

    
           String exeAbsPath = ForeignExe.nameAndVersion("clang-format", version) 
        
           		.pathToExe(pathToExe) 
        
           		.fixCantFind(howToInstall) 
        
           		.fixWrongVersion(

I might have the regex there wrong, the default is this:

spotless/lib/src/main/java/com/diffplug/spotless/ForeignExe.java

Line 37 in 1ff43ad

private Pattern versionRegex = Pattern.compile("version (\\S*)");

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.

spotless/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java

Lines 78 to 85 in 8797048

    
           String exeAbsPath = ForeignExe.nameAndVersion("clang-format", version) 
        
           		.pathToExe(pathToExe) 
        
           		.fixCantFind(howToInstall) 
        
           		.fixWrongVersion( 
        
           				"You can tell Spotless to use the version you already have with {@code clangFormat('{versionFound}')}" + 
        
           						"or you can download the currently specified version, {version}.\n" + howToInstall) 
        
           		.confirmVersionAndGetAbsolutePath(); 
        
           return new State(this, 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

    
           // used for executing 
        
           final transient List<String> args; 
        
           State(ClangFormatStep step, String exeAbsPath) { 
        
           	this.version = step.version; 
        
           	this.style = step.style; 
        
           	args = new ArrayList<>(2); 
        
           	args.add(exeAbsPath); 
        
           	if (style != null) { 
        
           		args.add("--style=" + style); 
        
           	}

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.

@bh-tt

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

@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.

Navigation Menu

Repository navigation

Better method for managing clang-format binaries #673

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	String howToInstall = "" +
	"You can download clang-format from https://releases.llvm.org and " +
	"then point Spotless to it with `pathToExe('/path/to/clang-format')` " +
	"or you can use your platform's package manager:" +
	"\n win: choco install llvm --version {version} (try dropping version if it fails)" +
	"\n mac: brew install clang-format (TODO: how to specify version?)" +
	"\n linux: apt install clang-format (try clang-format-{version} with dropped minor versions)";
	String exeAbsPath = ForeignExe.nameAndVersion("clang-format", version)
	.pathToExe(pathToExe)
	.fixCantFind(howToInstall)
	.fixWrongVersion(
	"You can tell Spotless to use the version you already have with `clangFormat('{versionFound}')`" +
	"or you can download the currently specified version, {version}.\n" + howToInstall)
	.confirmVersionAndGetAbsolutePath();

	// used for executing
	final transient List<String> args;

	State(ClangFormatStep step, String exeAbsPath) {
	this.version = step.version;
	this.style = step.style;
	args = new ArrayList<>(2);
	args.add(exeAbsPath);
	if (style != null) {
	args.add("--style=" + style);
	}

Better method for managing clang-format binaries #673

Description

Activity

nedtwigg commented on Aug 25, 2020

akornilov commented on Aug 26, 2020

nedtwigg commented on Aug 26, 2020

akornilov commented on Aug 28, 2020

nedtwigg commented on Aug 28, 2020

ronhe commented on Jun 17, 2022

nedtwigg commented on Jun 25, 2022

bh-tt commented on Jul 8, 2022

ronhe commented on Jul 8, 2022

nedtwigg commented on Jul 8, 2022

nedtwigg commented on Jul 28, 2022

pjonsson commented on Jun 3, 2023

pjonsson commented on Nov 18, 2023

nedtwigg commented on Nov 20, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions