spotless icon indicating copy to clipboard operation
spotless copied to clipboard

Protobuf/Buf Formatter Fails for Files Larger than 64KiB

Open yzhaoa opened this issue 8 months ago • 3 comments

Test case:

  1. Define a .proto files larger than 64 KiB (or your platform's pipe buffer limit). Padding with whitespace should also work.
  2. Configure spotless to format said file.
  3. Run spotlessApply (gradle example).

Expected behavior:

Formatting runs and passes (if valid proto file).

Actual behavior:

org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':treatment:api:spotlessProtobufApply'.
   ...
Caused by: org.gradle.api.GradleException: There were 1 lint error(s), they must be fixed or suppressed.
src/...:LINE_UNDEFINED buf(java.io.IOException) Broken pipe (...)
Resolve these lints or suppress with `suppressLintsFor`
        at com.diffplug.gradle.spotless.SpotlessApply.performAction(SpotlessApply.java:60)

cat-ing /build/spotless-lints/... shows:

Broken pipe
	at java.base/java.io.FileOutputStream.writeBytes(Native Method)
	at java.base/java.io.FileOutputStream.write(FileOutputStream.java:367)
	at java.base/java.io.BufferedOutputStream.implWrite(BufferedOutputStream.java:217)
	at java.base/java.io.BufferedOutputStream.write(BufferedOutputStream.java:206)
	at java.base/java.io.FilterOutputStream.write(FilterOutputStream.java:110)
	at com.diffplug.spotless.ProcessRunner.start(ProcessRunner.java:165)
	at com.diffplug.spotless.ProcessRunner.start(ProcessRunner.java:131)
	at com.diffplug.spotless.ProcessRunner.exec(ProcessRunner.java:114)
	at com.diffplug.spotless.ProcessRunner.exec(ProcessRunner.java:109)
	at com.diffplug.spotless.protobuf.BufStep$State.format(BufStep.java:109)
	at com.diffplug.spotless.FormatterFunc$Closeable$3.apply(FormatterFunc.java:140)
	at com.diffplug.spotless.FormatterStepEqualityOnStateSerialization.format(FormatterStepEqualityOnStateSerialization.java:49)
	at com.diffplug.spotless.LintState.of(LintState.java:177)
	at com.diffplug.spotless.LintState.of(LintState.java:134)
	at com.diffplug.gradle.spotless.SpotlessTaskImpl.processInputFile(SpotlessTaskImpl.java:134)
	at com.diffplug.gradle.spotless.SpotlessTaskImpl.performAction(SpotlessTaskImpl.java:117)
...

yzhaoa avatar May 17 '25 00:05 yzhaoa

Cause:

According to https://github.com/bufbuild/buf/issues/1035 and https://github.com/diffplug/spotless/pull/1208#discussion_r1264439669, buf format does not read from stdin, and reads from the file name in command line arguments.

https://github.com/diffplug/spotless/blob/6f1919f2175b3a3dea785a1da69883244b567a2f/lib/src/main/java/com/diffplug/spotless/protobuf/BufStep.java#L109 shows that the BufStep passes the file-to-be-formatted's source code to buf format's stdin.

Thus, the entirely proto file gets buffered against buf format. Files smaller than 64KiB seems to not trigger EPIPE normally because the OS buffers this incorrect input. Files larger than 64KiB cause blocking while attempting to write to the stdin pipe, and once buf exits, successfully or not, the pipe write fails with EPIPE.

Fix:

Replace input.getBytes(StandardCharsets.UTF_8) with new byte[0]

yzhaoa avatar May 17 '25 01:05 yzhaoa

Workaround:

You can work-around this issue by creating a wrapper around buf that discards stdin, e.g. (untested)

#!/bin/bash
# Discard input
cat >/dev/null
exec buf "$@"

Caution! It seems that modifying buf(...).pathToExe(...) does not invalidate the gradle task's inputs. Since spotless caches errors under the build directory, it may seem that the wrapper script doesn't work, and is not called when you set it up the first time. To avoid this, run gradle clean first, and disable build cache for the first run after changing pathToExe.

yzhaoa avatar May 17 '25 01:05 yzhaoa

Another easier workaround with a custom patched formatter:

  protobuf {
    class State(private val pathToExe: String) : Serializable {
      fun toFunc(): FormatterFunc.Closeable {
        return FormatterFunc.Closeable.of(ProcessRunner()) { runner, _, file ->
          runner
            .exec(byteArrayOf(), listOf(pathToExe, "format", file.absolutePath))
            .assertExitZero(UTF_8)
        }
      }
    }
    addStep(
      FormatterStep.createLazy(
        "buf",
        { State(/* path to buf */ ...) },
        State::toFunc,
      )
    )
  }

yzhaoa avatar May 20 '25 19:05 yzhaoa