Making buildx optional - #3851
Conversation
The local `docker` task passed `--cache-from type=registry,ref=...`, which is BuildKit-only syntax. On a Docker CLI without the buildx plugin the legacy builder parses it as an image reference and aborts: failed to parse query parameter 'cachefrom': invalid repo "type=registry" That broke `./gradlew docker` and, with it, the whole local e2e chain (runDockerE2e -> tagDockerLocal -> docker) for anyone on a buildx-less setup such as Podman behind the docker CLI. The flag is now appended only when `docker buildx version` succeeds, so buildx users keep the registry cache and everyone else still gets an image. CI publishing is untouched: dockerPublish and the release, prerelease-alpha and preview workflows keep using buildx, so multi-arch (linux/arm64 and linux/amd64) images are still produced.
📝 WalkthroughWalkthroughThe local Docker task now checks for Docker Buildx once. It adds the registry cache option only when Buildx is available and logs a fallback message otherwise. ChangesDocker Buildx cache handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gradle/docker.gradle`:
- Around line 7-18: Update buildxAvailable and the Docker build flow to
determine the effective builder, not merely whether the Buildx CLI exists:
disable registry-cache arguments when DOCKER_BUILDKIT=0 or Windows-container
mode selects the legacy builder, or route those builds through docker buildx
build while preserving local image output. Add coverage for both configurations
and the resulting cache/build command behavior.
- Around line 10-16: Update the Buildx probe around proc and buildxProbeResult
to catch InterruptedException separately: destroy proc, restore the thread’s
interrupt status, and rethrow the interruption. Keep the existing generic
exception fallback as unavailable, but replace the “docker buildx not found”
message because nonzero exits and other failures do not establish that Buildx is
missing.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2229ab6-2d1b-46ed-9ed6-05103519c817
📒 Files selected for processing (1)
gradle/docker.gradle
| def buildxProbeResult = null | ||
| def buildxAvailable = { | ||
| if (buildxProbeResult == null) { | ||
| try { | ||
| def proc = ["docker", "buildx", "version"].execute() | ||
| proc.consumeProcessOutput(new StringBuilder(), new StringBuilder()) | ||
| buildxProbeResult = proc.waitFor() == 0 | ||
| } catch (Exception ignored) { | ||
| buildxProbeResult = false | ||
| } | ||
| } | ||
| return buildxProbeResult |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- gradle/docker.gradle ---'
cat -n gradle/docker.gradle
printf '%s\n' '--- related Docker/BuildKit configuration ---'
rg -n -S --hidden 'DOCKER_BUILDKIT|buildx|cache-from|cache-to|docker build|windows-container|BUILDKIT' . \
-g '!node_modules' -g '!build' -g '!dist' || trueRepository: tolgee/tolgee-platform
Length of output: 7296
🌐 Web query:
Docker CLI docker build --cache-from type=registry BuildKit legacy builder DOCKER_BUILDKIT=0 Windows container mode builder selection documentation
💡 Result:
The legacy builder and BuildKit have significant functional differences, particularly regarding how they handle build cache and Windows container support. Compatibility with --cache-from The --cache-from type=registry syntax is a feature of BuildKit [1][2]. The legacy builder (invoked by setting DOCKER_BUILDKIT=0) does not support this specific cache backend syntax [3][4]. The legacy builder uses a different, implicit caching mechanism based on existing local image layers and does not support importing or exporting cache manifests or specialized OCI-formatted cache images from a registry [5][4]. Builder Selection and Windows Containers By default, Docker uses the BuildKit engine for Linux containers [4]. However, the legacy builder is still the default for Windows container mode due to historical lack of feature parity in BuildKit [3][4]. - Windows Container Support: While BuildKit has introduced experimental support for Windows containers (starting with v0.13.0), it is not yet the default [4][6]. Users who require BuildKit features on Windows must manually configure a separate BuildKit instance and use the docker buildx command to point to it [6]. - DOCKER_BUILDKIT=0: This environment variable explicitly disables BuildKit and forces the use of the legacy builder for the docker build command [3][7]. Using this variable is deprecated and generally discouraged, as the legacy builder lacks modern features such as concurrent build graph execution, advanced caching backends, and efficient multi-stage build handling [5][4][8]. - Selection Logic: The docker build command defaults to the legacy builder when Windows container mode is active or when DOCKER_BUILDKIT=0 is set [3][4]. Conversely, docker buildx build is designed to use BuildKit by default, regardless of the platform, and is the recommended approach for modern build workflows, even when using custom builder instances [2][9]. For reliable and modern build performance—including support for --cache-from type=registry—it is recommended to use the docker buildx build command with a properly configured BuildKit backend [1][9]. Projects relying on DOCKER_BUILDKIT=0 may experience performance limitations and issues with cache handling, particularly in cross-architecture or complex build scenarios [8].
Citations:
- 1: https://github.com/docker/buildx/blob/master/docs/reference/buildx_build.md
- 2: https://docs.docker.com/reference/cli/docker/buildx/build/
- 3: https://docs.docker.com/reference/cli/docker/image/build/
- 4: https://docs.docker.com/build/buildkit
- 5: https://docs.docker.com/build/building/multi-stage/
- 6: https://www.docker.com/blog/experimental-windows-containers-support-for-buildkit-released-in-v0-13-0/
- 7: README should clarify DOCKER_BUILDKIT vs docker buildx vs BuildKit moby/buildkit#1214
- 8: Cache broken for cross-architecture builds with DOCKER_BUILDKIT=0 since CVE-2024-24557 moby/moby#49947
- 9: https://docs.docker.com/build/builders/
Gate the registry cache on the effective builder.
docker buildx version only confirms that the Buildx CLI is available. When DOCKER_BUILDKIT=0 is set or Windows-container mode is active, docker build can use the legacy builder, which does not support type=registry cache syntax. Detect the effective builder or invoke docker buildx build while preserving local image output. Add coverage for these configurations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gradle/docker.gradle` around lines 7 - 18, Update buildxAvailable and the
Docker build flow to determine the effective builder, not merely whether the
Buildx CLI exists: disable registry-cache arguments when DOCKER_BUILDKIT=0 or
Windows-container mode selects the legacy builder, or route those builds through
docker buildx build while preserving local image output. Add coverage for both
configurations and the resulting cache/build command behavior.
| try { | ||
| def proc = ["docker", "buildx", "version"].execute() | ||
| proc.consumeProcessOutput(new StringBuilder(), new StringBuilder()) | ||
| buildxProbeResult = proc.waitFor() == 0 | ||
| } catch (Exception ignored) { | ||
| buildxProbeResult = false | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- tracked candidates ---'
git ls-files | rg '(^|/)(gradle/docker\.gradle|.*docker.*gradle)$' || true
printf '%s\n' '--- target file outline ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline gradle/docker.gradle --lang groovy || true
fi
printf '%s\n' '--- target file ---'
cat -n gradle/docker.gradle
printf '%s\n' '--- related symbols and messages ---'
rg -n -C 4 'buildxProbeResult|buildxAvailable|docker buildx|registry cache|consumeProcessOutput|waitFor' . \
-g '!build' -g '!dist' -g '!node_modules'Repository: tolgee/tolgee-platform
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target metadata ---'
git ls-files --stage -- gradle/docker.gradle || true
wc -l gradle/docker.gradle
printf '%s\n' '--- target lines 1-70 ---'
sed -n '1,70p' gradle/docker.gradle
printf '%s\n' '--- exact related references ---'
rg -n -C 3 'buildxProbeResult|buildxAvailable|docker buildx|building without the registry cache' \
gradle/docker.gradle build.gradle settings.gradle gradle \
-g '*.gradle' -g '*.gradle.kts' 2>/dev/null || true
printf '%s\n' '--- repository/tooling context ---'
rg -n -C 2 'Gradle|Groovy|sourceCompatibility|toolchain|JavaVersion' \
gradle build.gradle settings.gradle gradle.properties 2>/dev/null | head -200 || trueRepository: tolgee/tolgee-platform
Length of output: 9345
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- available runtimes ---'
java -version 2>&1 || true
groovy --version 2>&1 || true
gradle --version 2>&1 | head -20 || true
printf '%s\n' '--- standalone Process interruption probe ---'
if command -v java >/dev/null 2>&1; then
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat >"$tmpdir/ProcessInterruptProbe.java" <<'JAVA'
public class ProcessInterruptProbe {
public static void main(String[] args) throws Exception {
Process process = new ProcessBuilder("sh", "-c", "sleep 30").start();
Thread waiter = new Thread(() -> {
try {
process.waitFor();
System.out.println("waiter=returned");
} catch (InterruptedException e) {
System.out.println("waiter=interrupted");
System.out.println("interruptStatusInsideCatch=" + Thread.currentThread().isInterrupted());
System.out.println("aliveAfterInterrupt=" + process.isAlive());
process.destroy();
Thread.currentThread().interrupt();
System.out.println("interruptStatusAfterRestore=" + Thread.currentThread().isInterrupted());
}
});
waiter.start();
Thread.sleep(100);
waiter.interrupt();
waiter.join(5000);
System.out.println("waiterAliveAfterJoin=" + waiter.isAlive());
System.out.println("processAliveAfterDestroy=" + process.isAlive());
process.destroyForcibly();
process.waitFor();
}
}
JAVA
javac "$tmpdir/ProcessInterruptProbe.java"
java -cp "$tmpdir" ProcessInterruptProbe
fi
printf '%s\n' '--- source-shape checks ---'
python3 - <<'PY'
from pathlib import Path
p = Path("gradle/docker.gradle")
s = p.read_text()
checks = {
"waitFor call": "proc.waitFor()" in s,
"broad Exception catch": "catch (Exception ignored)" in s,
"false fallback": "buildxProbeResult = false" in s,
"not found message": "docker buildx not found" in s,
"probe process destruction": ".destroy()" in s or ".destroyForcibly()" in s,
"interrupt restoration": "Thread.currentThread().interrupt()" in s,
}
for name, present in checks.items():
print(f"{name}: {present}")
PYRepository: tolgee/tolgee-platform
Length of output: 562
🌐 Web query:
Oracle Java Process.waitFor InterruptedException documentation Process.destroy Gradle consumeProcessOutput documentation
💡 Result:
In Java and Gradle process management, here is the documentation regarding the requested methods: Java Process.waitFor and InterruptedException The java.lang.Process.waitFor method causes the current thread to block until the process represented by the Process object has terminated [1][2][3]. Because it is a blocking operation, it throws an InterruptedException if the current thread is interrupted by another thread while it is waiting [1][4]. When this exception is thrown, the wait is immediately ended [1][2]. The same behavior applies to the overloaded waitFor(long timeout, TimeUnit unit) method, which also throws an InterruptedException if the thread is interrupted before the timeout elapses or the process exits [1][4]. Java Process.destroy The java.lang.Process.destroy method kills the process [5]. Whether this termination is graceful or forced is implementation-dependent, though it is generally intended to allow for normal termination if possible [5]. For cases requiring immediate termination, the destroyForcibly method (available since Java 1.8) is used to forcibly terminate the process regardless of its state [5]. Gradle consumeProcessOutput The method consumeProcessOutput(OutputStream output, OutputStream error) is a Groovy-specific enhancement provided by Gradle to simplify the capture of a process's output [6]. It is not a standard Java API method but is added to Process objects when executed within a Gradle environment (e.g., inside an exec task or block) [6]. This method redirects the standard output and standard error streams of the process to the specified OutputStreams, which are typically closed automatically after the process completes [7][6]. In modern Gradle, users are encouraged to use the ProviderFactory#exec API for more robust, lazy-configured handling of process output [8]. Top Results: [1], [5], [7], [8]
Citations:
- 1: https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Process.html
- 2: https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/lang/Process.html
- 3: https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/lang/Process.html
- 4: https://docs.oracle.com/javase/8/docs/api/java/lang/Process.html
- 5: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Process.html
- 6: https://github.com/spring-attic/spring-cloud-pipelines/blob/main/build.gradle
- 7: https://docs.gradle.org/5.1/javadoc/org/gradle/process/BaseExecSpec.html
- 8: https://stackoverflow.com/questions/11093223/how-to-use-exec-output-in-gradle
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
source = Path("gradle/docker.gradle").read_text()
start = source.index("def buildxAvailable")
end = source.index("\ntasks.register('dockerPrepare')", start)
probe = source[start:end]
checks = [
("waitFor is inside the probe", "proc.waitFor()" in probe),
("InterruptedException has a dedicated handler", "catch (InterruptedException" in probe),
("broad handler assigns false", "catch (Exception ignored)" in probe and "buildxProbeResult = false" in probe),
("probe cleanup exists", ".destroy()" in probe or ".destroyForcibly()" in probe),
("interrupt status is restored", "Thread.currentThread().interrupt()" in probe),
("fallback message claims not found", 'logger.lifecycle("docker buildx not found' in source),
]
for label, result in checks:
print(f"{label}: {'present' if result else 'absent'}")
assert "proc.waitFor()" in probe
assert "catch (InterruptedException" not in probe
assert "catch (Exception ignored)" in probe
assert "buildxProbeResult = false" in probe
assert ".destroy()" not in probe and ".destroyForcibly()" not in probe
assert "Thread.currentThread().interrupt()" not in probe
assert 'logger.lifecycle("docker buildx not found' in source
print("Conclusion: the current probe collapses interruption, nonzero exit, and other exceptions into false, without cleanup or interrupt restoration.")
PYRepository: tolgee/tolgee-platform
Length of output: 539
Preserve interruption during the Buildx probe.
Handle InterruptedException separately. Destroy proc, restore the interrupt status, and rethrow it. Keep generic failures as unavailable, but change the message from "docker buildx not found" because a nonzero exit or other exception does not prove that Buildx is missing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gradle/docker.gradle` around lines 10 - 16, Update the Buildx probe around
proc and buildxProbeResult to catch InterruptedException separately: destroy
proc, restore the thread’s interrupt status, and rethrow the interruption. Keep
the existing generic exception fallback as unavailable, but replace the “docker
buildx not found” message because nonzero exits and other failures do not
establish that Buildx is missing.
Problem
The local
dockerGradle task passed--cache-from type=registry,ref=tolgee/tolgee:latest. That is BuildKit-only syntax — a Docker CLI without the buildx plugin falls back to the legacy builder, which parses the value as an image reference and aborts:This makes
./gradlew dockerunusable on buildx-less setups (e.g. Podman behind the docker CLI), and it takes the whole local e2e chain with it, sincerunDockerE2e→tagDockerLocal→docker. There is no workaround short of editing the build file locally.Note the local
dockertask never invokedbuildx buildin the first place — the single flag was the entire blocker.Change
gradle/docker.gradleprobes for buildx once (docker buildx version) and appends--cache-fromonly when it is present. Without buildx the build proceeds and logs:Developers with buildx keep the registry cache exactly as before.
CI and multi-arch are unaffected
dockerPublishand therelease,prerelease-alphaandpreviewworkflows still use buildx, so published images remain multi-arch (linux/arm64,linux/amd64).setup-envkeepssetup-qemu-action/setup-buildx-action.dockerSlimalready used plaindocker build.Verification
Run against Docker CLI 29.7.2 talking to Podman 5.8.4 (legacy builder, no buildx):
./gradlew docker→BUILD SUCCESSFUL, image taggedtolgee/tolgee, fallback message logged.--cache-fromis still appended (the daemon then rejects it with the error above — proving the flag reaches the command).--dry-runoverdockerPublish dockerSlim tagDockerLocalstill configures cleanly.COPY --chmod=755used in both Dockerfiles, so no Dockerfile changes were needed.Summary by CodeRabbit