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

Verifying the version of binary tool too early #1762

Closed
andrewparmet opened this issue Jul 18, 2023 · 3 comments
Closed

Verifying the version of binary tool too early #1762

andrewparmet opened this issue Jul 18, 2023 · 3 comments
Labels

Comments

@andrewparmet
Copy link
Contributor

andrewparmet commented Jul 18, 2023

Trying to put #1208 into action and running into a dependency issue - it looks like Spotless is validating the provided version at configuration time, which is earlier than the value of toolVersion on BufExtension is resolvable to anything but its default.

An abridged version of my configuration:

configure<BufExtension> {
    toolVersion = "1.25.0"
}

configure<SpotlessExtension> {
    protobuf {
        buf("1.25.0").pathToExe(configurations.getByName(BUF_BINARY_CONFIGURATION_NAME).singleFile.absolutePath)
    }
}

This fails because the default value for toolVersion is 1.17.0 and resolving the Buf binary configuration is too eager.

Do you know if it's possible to:

  • postpone this verification until we can read the BufExtension correctly?
  • remove the verification?

Or if there's another approach that would be better?

Thanks!

@andrewparmet
Copy link
Contributor Author

It actually goes deeper - Buf configures the binary to be executable as well. Calling it at configuration time doesn't work great with resolving a binary from a configuration.

@nedtwigg nedtwigg added the bug label Jul 28, 2023
@nedtwigg
Copy link
Member

We need to resolve the State at configuration time, that is what makes up-to-date checks work

private State createState() throws IOException, InterruptedException {
String instructions = "https://docs.buf.build/installation";
String exeAbsPath = ForeignExe.nameAndVersion("buf", version)
.pathToExe(pathToExe)
.versionRegex(Pattern.compile("(\\S*)"))
.fixCantFind("Try following the instructions at " + instructions + ", or else tell Spotless where it is with {@code buf().pathToExe('path/to/executable')}")
.confirmVersionAndGetAbsolutePath();
return new State(this, exeAbsPath);
}

But we don't need to create the FormatterFunc until execution time

FormatterFunc.Closeable toFunc() {
ProcessRunner runner = new ProcessRunner();
return FormatterFunc.Closeable.of(runner, this::format);
}

So we could remove exeAbsPath from the state, and instead move the ForeignExe calculation into the toFunc() calculation, which doesn't happen until a file is actually being formatted.

@andrewparmet
Copy link
Contributor Author

Turns out this was a bug in the buf-gradle-plugin: bufbuild/buf-gradle-plugin#148

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

No branches or pull requests

2 participants