From 49a94d1db00a33239a908726cb7c3774752b089a Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 09:10:12 -0300 Subject: [PATCH 01/10] fix: cache SDK sdist across image builds Build the SDK sdist once per batch and reuse it through the SDK's native build() path by intercepting only the internal uv sdist subprocess call. This removes the per-image sdist overhead while preserving tag, cache, and build-arg behavior for current and future SDK versions. Also switch build-related modules to stdlib logging so worker forks do not inherit SDK Rich console state. Co-authored-by: openhands --- benchmarks/swebench/build_images.py | 4 +- benchmarks/swtbench/build_eval_env_images.py | 4 +- benchmarks/swtbench/image_utils.py | 3 +- benchmarks/utils/build_utils.py | 338 +++++++++++++------ benchmarks/utils/buildx_utils.py | 5 +- benchmarks/utils/image_utils.py | 5 +- tests/test_image_utils.py | 44 +++ 7 files changed, 286 insertions(+), 117 deletions(-) diff --git a/benchmarks/swebench/build_images.py b/benchmarks/swebench/build_images.py index 5ace5419b..d822442fb 100644 --- a/benchmarks/swebench/build_images.py +++ b/benchmarks/swebench/build_images.py @@ -9,6 +9,7 @@ --image ghcr.io/openhands/eval-agent-server --target source-minimal """ +import logging import sys from pathlib import Path @@ -23,10 +24,9 @@ ) from benchmarks.utils.dataset import get_dataset from benchmarks.utils.image_utils import remote_image_exists -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) WRAPPER_DOCKERFILE = Path(__file__).with_name("Dockerfile.swebench-deps") diff --git a/benchmarks/swtbench/build_eval_env_images.py b/benchmarks/swtbench/build_eval_env_images.py index 2f0ea9862..0b42a50b4 100644 --- a/benchmarks/swtbench/build_eval_env_images.py +++ b/benchmarks/swtbench/build_eval_env_images.py @@ -2,6 +2,7 @@ import argparse import json +import logging import os import sys from pathlib import Path @@ -13,10 +14,9 @@ from benchmarks.swtbench.image_utils import ensure_swt_bench_repo from benchmarks.utils.dataset import get_dataset from benchmarks.utils.image_utils import remote_image_exists -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) def select_instance_ids( diff --git a/benchmarks/swtbench/image_utils.py b/benchmarks/swtbench/image_utils.py index 0aa3ec6f5..708a7d8b5 100644 --- a/benchmarks/swtbench/image_utils.py +++ b/benchmarks/swtbench/image_utils.py @@ -9,10 +9,9 @@ from typing import Iterable from benchmarks.swtbench.config import EVAL_DEFAULTS -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) def ensure_swt_bench_repo(cache_dir: Path | None = None) -> Path: diff --git a/benchmarks/utils/build_utils.py b/benchmarks/utils/build_utils.py index b4e56b9ad..48ced628c 100644 --- a/benchmarks/utils/build_utils.py +++ b/benchmarks/utils/build_utils.py @@ -6,9 +6,12 @@ import argparse import contextlib import io +import logging import os +import shutil import subprocess import sys +import tempfile import time import tomllib from concurrent.futures import ProcessPoolExecutor, as_completed @@ -29,10 +32,9 @@ ) from benchmarks.utils.constants import EVAL_AGENT_SERVER_IMAGE from benchmarks.utils.image_utils import local_image_exists, remote_image_exists -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) class BuildOutput(BaseModel): @@ -198,6 +200,116 @@ def _get_sdk_submodule_info() -> tuple[str, str, str]: return git_ref, git_sha, sdk_version +def _pre_build_sdist() -> Path: + """ + Build the SDK sdist once and reuse it across all image builds in a batch. + + The caller must clean up the parent directory of the returned tarball. + """ + benchmarks_root = Path(__file__).resolve().parent.parent.parent + sdk_path = benchmarks_root / "vendor" / "software-agent-sdk" + sdist_dir = Path(tempfile.mkdtemp(prefix="shared-sdist-")).resolve() + + logger.info("Pre-building SDK sdist from %s", sdk_path) + start = time.time() + proc = subprocess.run( + ["uv", "build", "--sdist", "--out-dir", str(sdist_dir)], + cwd=str(sdk_path), + capture_output=True, + text=True, + ) + if proc.returncode != 0: + shutil.rmtree(sdist_dir, ignore_errors=True) + raise RuntimeError(f"Failed to build SDK sdist: {proc.stderr}") + + sdists = sorted(sdist_dir.glob("*.tar.gz")) + if len(sdists) != 1: + shutil.rmtree(sdist_dir, ignore_errors=True) + raise RuntimeError(f"Expected 1 SDK sdist, got {len(sdists)}") + + logger.info("Pre-built SDK sdist in %.1fs: %s", time.time() - start, sdists[0]) + return sdists[0] + + +def _is_sdist_build_command(cmd: list[str]) -> bool: + return ( + len(cmd) >= 4 + and cmd[0] == "uv" + and cmd[1] == "build" + and "--sdist" in cmd + and "--out-dir" in cmd + ) + + +def _reuse_cached_sdist( + cmd: list[str], + cached_sdist: Path, +) -> subprocess.CompletedProcess[str]: + out_dir = Path(cmd[cmd.index("--out-dir") + 1]).resolve() + out_dir.mkdir(parents=True, exist_ok=True) + target = out_dir / cached_sdist.name + shutil.copy2(cached_sdist, target) + logger.info("Reusing cached SDK sdist %s -> %s", cached_sdist, target) + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + +@contextlib.contextmanager +def _patch_sdk_sdist_build( + sdk_build_module, + cached_sdist: Path | None, +): + """ + Reuse a pre-built sdist while still calling the SDK's own build() logic. + + This keeps tag generation, build args, cache settings, and future SDK build + behavior inside the SDK. Benchmarks only short-circuits the expensive + repeated `uv build --sdist` step. + """ + if cached_sdist is None or not cached_sdist.is_file(): + yield + return + + original_run = getattr(sdk_build_module, "_run", None) + if original_run is None: + logger.warning( + "SDK build module has no _run helper; falling back to native build()" + ) + yield + return + + def patched_run( + cmd: list[str], + cwd: str | None = None, + ) -> subprocess.CompletedProcess[str]: + if _is_sdist_build_command(cmd): + return _reuse_cached_sdist(cmd, cached_sdist) + return original_run(cmd, cwd=cwd) + + sdk_build_module._run = patched_run + try: + yield + finally: + sdk_build_module._run = original_run + + +@contextlib.contextmanager +def _sdk_sdist_cache_env(): + cached_sdist_path: Path | None = None + try: + try: + cached_sdist_path = _pre_build_sdist() + os.environ["OPENHANDS_CACHED_SDIST"] = str(cached_sdist_path) + except Exception as e: + logger.warning( + "Failed to pre-build SDK sdist; each image will build its own: %s", e + ) + yield + finally: + if cached_sdist_path: + os.environ.pop("OPENHANDS_CACHED_SDIST", None) + shutil.rmtree(cached_sdist_path.parent, ignore_errors=True) + + @contextlib.contextmanager def capture_output(base_name: str, out_dir: Path): """ @@ -284,7 +396,10 @@ def build_image( ) -> BuildOutput: # Importing here because openhands.agent_server.docker.build runs git checks # which fails when installed as a package outside the git repo - from openhands.agent_server.docker.build import BuildOptions, build + from openhands.agent_server.docker import build as sdk_build_module + + BuildOptions = sdk_build_module.BuildOptions + build = sdk_build_module.build # Get SDK info from submodule to ensure tags use the correct SDK SHA git_ref, git_sha, sdk_version = _get_sdk_submodule_info() @@ -307,7 +422,12 @@ def build_image( if remote_image_exists(t): logger.info("Image %s already exists. Skipping build.", t) return BuildOutput(base_image=base_image, tags=[t], error=None) - tags = build(opts) + + cached_sdist = os.environ.get("OPENHANDS_CACHED_SDIST") + cached_sdist_path = Path(cached_sdist) if cached_sdist else None + with _patch_sdk_sdist_build(sdk_build_module, cached_sdist_path): + tags = build(opts) + return BuildOutput(base_image=base_image, tags=tags, error=None) @@ -511,121 +631,129 @@ def _chunks(seq: list[str], size: int): batches = list(_chunks(base_images, batch_size or len(base_images))) total_batches = len(batches) - with ( - manifest_file.open("w") as writer, - tqdm( - total=len(base_images), desc="Building agent-server images", leave=True - ) as pbar, - ): - _update_pbar(pbar, successes, failures, 0, None, "Queueing") - - for batch_idx, batch in enumerate(batches, start=1): - if not batch: - continue + with _sdk_sdist_cache_env(): + with ( + manifest_file.open("w") as writer, + tqdm( + total=len(base_images), desc="Building agent-server images", leave=True + ) as pbar, + ): + _update_pbar(pbar, successes, failures, 0, None, "Queueing") + + for batch_idx, batch in enumerate(batches, start=1): + if not batch: + continue - logger.info( - "Starting batch %d/%d (%d images)", batch_idx, total_batches, len(batch) - ) - in_progress: set[str] = set() - - with ProcessPoolExecutor(max_workers=max_workers) as ex: - futures = {} - for base in batch: - in_progress.add(base) - resolved_tag = ( - base_image_to_custom_tag_fn(base) - if base_image_to_custom_tag_fn - else "" - ) - fut = ex.submit( - _build_with_logging, - log_dir=build_log_dir, - base_image=base, - target_image=image, - custom_tag=resolved_tag, - target=target, - push=push, - max_retries=max_retries, - post_build_fn=post_build_fn, - ) - futures[fut] = base - - _update_pbar( - pbar, - successes, - failures, - len(in_progress), - next(iter(in_progress), None), - f"Batch {batch_idx}/{total_batches} running", + logger.info( + "Starting batch %d/%d (%d images)", + batch_idx, + total_batches, + len(batch), ) + in_progress: set[str] = set() + + with ProcessPoolExecutor(max_workers=max_workers) as ex: + futures = {} + for base in batch: + in_progress.add(base) + resolved_tag = ( + base_image_to_custom_tag_fn(base) + if base_image_to_custom_tag_fn + else "" + ) + fut = ex.submit( + _build_with_logging, + log_dir=build_log_dir, + base_image=base, + target_image=image, + custom_tag=resolved_tag, + target=target, + push=push, + max_retries=max_retries, + post_build_fn=post_build_fn, + ) + futures[fut] = base - for fut in as_completed(futures): - base = futures[fut] - status = None - try: - result: BuildOutput = fut.result() - except Exception as e: - logger.error("Build failed for %s: %r", base, e) - result = BuildOutput(base_image=base, tags=[], error=repr(e)) - - writer.write(result.model_dump_json() + "\n") - writer.flush() - - with mu: - if result.error or not result.tags: - failures += 1 - status = "❌ Failed" - else: - successes += 1 - status = "✅ Done" - - in_progress.discard(base) - pbar.update(1) _update_pbar( pbar, successes, failures, len(in_progress), next(iter(in_progress), None), - status, + f"Batch {batch_idx}/{total_batches} running", ) - used, total = buildkit_disk_usage() - if total > 0: - logger.info( - "BuildKit usage after batch %d/%d: %.2f%% (%0.2f GiB / %0.2f GiB)", - batch_idx, - total_batches, - (used / total) * 100, - used / (1 << 30), - total / (1 << 30), - ) + for fut in as_completed(futures): + base = futures[fut] + status = None + try: + result: BuildOutput = fut.result() + except Exception as e: + logger.error("Build failed for %s: %r", base, e) + result = BuildOutput( + base_image=base, + tags=[], + error=repr(e), + ) + + writer.write(result.model_dump_json() + "\n") + writer.flush() + + with mu: + if result.error or not result.tags: + failures += 1 + status = "❌ Failed" + else: + successes += 1 + status = "✅ Done" + + in_progress.discard(base) + pbar.update(1) + _update_pbar( + pbar, + successes, + failures, + len(in_progress), + next(iter(in_progress), None), + status, + ) - if prune_keep_storage_gb and prune_keep_storage_gb > 0: - pruned = maybe_prune_buildkit_cache( - keep_storage_gb=prune_keep_storage_gb, - threshold_pct=prune_threshold_pct, - filters=prune_filters, - ) - if pruned: + used, total = buildkit_disk_usage() + if total > 0: logger.info( - "Pruned BuildKit cache after batch %d/%d (keep=%d GiB, threshold=%.1f%%)", + "BuildKit usage after batch %d/%d: %.2f%% (%0.2f GiB / %0.2f GiB)", batch_idx, total_batches, - prune_keep_storage_gb, - prune_threshold_pct, + (used / total) * 100, + used / (1 << 30), + total / (1 << 30), ) - else: - logger.info( - "No prune needed after batch %d/%d (threshold %.1f%%)", - batch_idx, - total_batches, - prune_threshold_pct, + + if prune_keep_storage_gb and prune_keep_storage_gb > 0: + pruned = maybe_prune_buildkit_cache( + keep_storage_gb=prune_keep_storage_gb, + threshold_pct=prune_threshold_pct, + filters=prune_filters, ) - logger.info( - "Done. Built=%d Failed=%d Manifest=%s", - successes, - failures, - str(manifest_file), - ) - return 1 if failures else 0 + if pruned: + logger.info( + "Pruned BuildKit cache after batch %d/%d (keep=%d GiB, threshold=%.1f%%)", + batch_idx, + total_batches, + prune_keep_storage_gb, + prune_threshold_pct, + ) + else: + logger.info( + "No prune needed after batch %d/%d (threshold %.1f%%)", + batch_idx, + total_batches, + prune_threshold_pct, + ) + logger.info( + "Done. Built=%d Failed=%d Manifest=%s", + successes, + failures, + str(manifest_file), + ) + return 1 if failures else 0 diff --git a/benchmarks/utils/buildx_utils.py b/benchmarks/utils/buildx_utils.py index 648bb7d25..a54877cf9 100644 --- a/benchmarks/utils/buildx_utils.py +++ b/benchmarks/utils/buildx_utils.py @@ -4,6 +4,7 @@ """ import json +import logging import os import re import shutil @@ -11,10 +12,8 @@ import time from pathlib import Path -from openhands.sdk import get_logger - -logger = get_logger(__name__) +logger = logging.getLogger(__name__) def _read_reset_state(path: Path) -> dict[str, float]: diff --git a/benchmarks/utils/image_utils.py b/benchmarks/utils/image_utils.py index 467074cb9..3afe7384e 100644 --- a/benchmarks/utils/image_utils.py +++ b/benchmarks/utils/image_utils.py @@ -2,6 +2,7 @@ from __future__ import annotations import base64 +import logging import os import subprocess import sys @@ -14,10 +15,8 @@ import requests -from openhands.sdk import get_logger - -logger = get_logger(__name__) +logger = logging.getLogger(__name__) ACCEPT = ",".join( diff --git a/tests/test_image_utils.py b/tests/test_image_utils.py index c46830cb6..d98e7f0d4 100644 --- a/tests/test_image_utils.py +++ b/tests/test_image_utils.py @@ -6,6 +6,8 @@ import os import subprocess +from pathlib import Path +from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest @@ -255,3 +257,45 @@ def test_passes_target_to_build_image(self, mock_build, _mock_exists): _, kwargs = mock_build.call_args assert kwargs["target"] == "binary" assert kwargs["push"] is False + + +class TestCachedSdistReuse: + def test_is_sdist_build_command_matches_expected_shape(self): + from benchmarks.utils.build_utils import _is_sdist_build_command + + assert _is_sdist_build_command( + ["uv", "build", "--sdist", "--out-dir", "/tmp/out"] + ) + assert not _is_sdist_build_command(["uv", "build", "--wheel"]) + assert not _is_sdist_build_command(["docker", "buildx", "build"]) + + def test_patch_sdk_sdist_build_reuses_cached_sdist_only_for_sdist_commands( + self, + tmp_path: Path, + ): + from benchmarks.utils.build_utils import _patch_sdk_sdist_build + + cached_sdist = tmp_path / "openhands-sdk.tar.gz" + cached_sdist.write_text("cached", encoding="utf-8") + forwarded_calls: list[tuple[list[str], str | None]] = [] + + def original_run(cmd: list[str], cwd: str | None = None): + forwarded_calls.append((cmd, cwd)) + return subprocess.CompletedProcess(cmd, 0, stdout="forwarded", stderr="") + + sdk_build_module = SimpleNamespace(_run=original_run) + sdist_cmd = ["uv", "build", "--sdist", "--out-dir", str(tmp_path / "out")] + other_cmd = ["docker", "buildx", "build"] + + with _patch_sdk_sdist_build(sdk_build_module, cached_sdist): + result = sdk_build_module._run(sdist_cmd, cwd="/sdk") + assert result.returncode == 0 + assert (tmp_path / "out" / cached_sdist.name).read_text( + encoding="utf-8" + ) == "cached" + + forwarded = sdk_build_module._run(other_cmd, cwd="/sdk") + assert forwarded.stdout == "forwarded" + + assert sdk_build_module._run is original_run + assert forwarded_calls == [(other_cmd, "/sdk")] From c614f8324c894c89bc7539f78a26a0394ec8f109 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 10:45:14 -0300 Subject: [PATCH 02/10] ci: harden SDK image build workflows --- .github/workflows/build-swebench-images.yml | 21 +++++++++ .github/workflows/build-swtbench-images.yml | 51 +++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/.github/workflows/build-swebench-images.yml b/.github/workflows/build-swebench-images.yml index 6053db373..586c6b8af 100644 --- a/.github/workflows/build-swebench-images.yml +++ b/.github/workflows/build-swebench-images.yml @@ -78,6 +78,7 @@ jobs: runs-on: labels: blacksmith-32vcpu-ubuntu-2204 + timeout-minutes: 180 # Allow pushing to GHCR and commenting on issues permissions: @@ -86,6 +87,11 @@ jobs: issues: write steps: + - name: Record build start time + run: | + echo "BUILD_START=$(date +%s)" >> "$GITHUB_ENV" + echo "Build started at $(date -u)" + - name: Determine checkout ref id: checkout-ref run: | @@ -249,6 +255,21 @@ jobs: BUILDKIT_PROGRESS: plain BUILDKIT_RESET_ON_FAILURE: 1 + - name: Post-build disk and timing report + if: always() + run: | + set -euo pipefail + BUILD_END=$(date +%s) + ELAPSED=$(( BUILD_END - ${BUILD_START:-$BUILD_END} )) + echo "## Build Timing" >> "$GITHUB_STEP_SUMMARY" + echo "**Elapsed:** $((ELAPSED / 60))m $((ELAPSED % 60))s" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + + echo "## Disk Usage After Build" >> "$GITHUB_STEP_SUMMARY" + df -h / /var/lib/buildkit 2>/dev/null | tee -a "$GITHUB_STEP_SUMMARY" || true + echo "" >> "$GITHUB_STEP_SUMMARY" + docker buildx du --verbose 2>/dev/null | head -40 | tee -a "$GITHUB_STEP_SUMMARY" || true + - name: Archive build logs if: always() run: | diff --git a/.github/workflows/build-swtbench-images.yml b/.github/workflows/build-swtbench-images.yml index fe9d07be6..5901ab914 100644 --- a/.github/workflows/build-swtbench-images.yml +++ b/.github/workflows/build-swtbench-images.yml @@ -72,6 +72,7 @@ jobs: runs-on: labels: blacksmith-32vcpu-ubuntu-2204 + timeout-minutes: 180 permissions: contents: read @@ -88,6 +89,11 @@ jobs: SELECT_FILE: '' steps: + - name: Record build start time + run: | + echo "BUILD_START=$(date +%s)" >> "$GITHUB_ENV" + echo "Build started at $(date -u)" + - name: Determine checkout ref id: checkout-ref run: | @@ -134,6 +140,35 @@ jobs: run: | make build + - name: "Preflight: prune cache and verify BuildKit disk" + run: | + set -euo pipefail + KEEP_GB=60 + echo "Pruning BuildKit cache (target max-storage ${KEEP_GB} GiB, no filters)..." + if ! docker buildx prune --all --force --max-storage ${KEEP_GB}g; then + docker buildx prune --all --force --keep-storage ${KEEP_GB}g || true + fi + + if df -B1 /var/lib/buildkit > /tmp/buildkit_df 2>/dev/null; then + LINE=$(tail -n1 /tmp/buildkit_df) + TOTAL=$(echo "$LINE" | awk '{print $2}') + USED=$(echo "$LINE" | awk '{print $3}') + FREE=$(echo "$LINE" | awk '{print $4}') + if [ -n "$TOTAL" ] && [ -n "$FREE" ]; then + PCT=$(( 100 * USED / TOTAL )) + echo "BuildKit disk: used ${USED} / ${TOTAL} bytes (${PCT}%); free ${FREE} bytes" + MIN=$((75 * 1024 * 1024 * 1024)) + if [ "$FREE" -lt "$MIN" ]; then + echo "::error::Not enough free space on /var/lib/buildkit (${FREE} bytes free, need >= ${MIN})" + exit 1 + fi + else + echo "Warning: unable to parse df output for /var/lib/buildkit" + fi + else + echo "Warning: /var/lib/buildkit not found; skipping disk check" + fi + - name: Build and push SWT-Bench images run: | set -euo pipefail @@ -181,6 +216,7 @@ jobs: env: DOCKER_BUILDKIT: 1 BUILDKIT_PROGRESS: plain + BUILDKIT_RESET_ON_FAILURE: 1 - name: Build prebaked eval env images if: ${{ inputs.build-eval-env == 'true' }} @@ -240,6 +276,21 @@ jobs: docker ps -a || true docker system df || true + - name: Post-build disk and timing report + if: always() + run: | + set -euo pipefail + BUILD_END=$(date +%s) + ELAPSED=$(( BUILD_END - ${BUILD_START:-$BUILD_END} )) + echo "## Build Timing" >> "$GITHUB_STEP_SUMMARY" + echo "**Elapsed:** $((ELAPSED / 60))m $((ELAPSED % 60))s" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + + echo "## Disk Usage After Build" >> "$GITHUB_STEP_SUMMARY" + df -h / /var/lib/buildkit 2>/dev/null | tee -a "$GITHUB_STEP_SUMMARY" || true + echo "" >> "$GITHUB_STEP_SUMMARY" + docker buildx du --verbose 2>/dev/null | head -40 | tee -a "$GITHUB_STEP_SUMMARY" || true + - name: Archive build logs if: always() run: | From 65331d67482816d68b1d97659d3afa531b584181 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 11:04:35 -0300 Subject: [PATCH 03/10] refactor: simplify cached sdist plumbing --- benchmarks/utils/build_utils.py | 277 ++++++++++++++++++-------------- tests/test_image_utils.py | 77 ++++++++- 2 files changed, 232 insertions(+), 122 deletions(-) diff --git a/benchmarks/utils/build_utils.py b/benchmarks/utils/build_utils.py index 48ced628c..1ab7a0cec 100644 --- a/benchmarks/utils/build_utils.py +++ b/benchmarks/utils/build_utils.py @@ -200,14 +200,18 @@ def _get_sdk_submodule_info() -> tuple[str, str, str]: return git_ref, git_sha, sdk_version +def _sdk_root() -> Path: + benchmarks_root = Path(__file__).resolve().parent.parent.parent + return benchmarks_root / "vendor" / "software-agent-sdk" + + def _pre_build_sdist() -> Path: """ Build the SDK sdist once and reuse it across all image builds in a batch. The caller must clean up the parent directory of the returned tarball. """ - benchmarks_root = Path(__file__).resolve().parent.parent.parent - sdk_path = benchmarks_root / "vendor" / "software-agent-sdk" + sdk_path = _sdk_root() sdist_dir = Path(tempfile.mkdtemp(prefix="shared-sdist-")).resolve() logger.info("Pre-building SDK sdist from %s", sdk_path) @@ -241,6 +245,14 @@ def _is_sdist_build_command(cmd: list[str]) -> bool: ) +def _is_sdk_sdist_build_command(cmd: list[str], cwd: str | None) -> bool: + return ( + _is_sdist_build_command(cmd) + and cwd is not None + and Path(cwd).resolve() == _sdk_root().resolve() + ) + + def _reuse_cached_sdist( cmd: list[str], cached_sdist: Path, @@ -281,7 +293,7 @@ def patched_run( cmd: list[str], cwd: str | None = None, ) -> subprocess.CompletedProcess[str]: - if _is_sdist_build_command(cmd): + if _is_sdk_sdist_build_command(cmd, cwd): return _reuse_cached_sdist(cmd, cached_sdist) return original_run(cmd, cwd=cwd) @@ -293,20 +305,18 @@ def patched_run( @contextlib.contextmanager -def _sdk_sdist_cache_env(): +def _prepare_cached_sdist(): cached_sdist_path: Path | None = None try: try: cached_sdist_path = _pre_build_sdist() - os.environ["OPENHANDS_CACHED_SDIST"] = str(cached_sdist_path) except Exception as e: logger.warning( "Failed to pre-build SDK sdist; each image will build its own: %s", e ) - yield + yield cached_sdist_path finally: if cached_sdist_path: - os.environ.pop("OPENHANDS_CACHED_SDIST", None) shutil.rmtree(cached_sdist_path.parent, ignore_errors=True) @@ -393,6 +403,7 @@ def build_image( custom_tag: str, target: TargetType = "source-minimal", push: bool = False, + cached_sdist: Path | None = None, ) -> BuildOutput: # Importing here because openhands.agent_server.docker.build runs git checks # which fails when installed as a package outside the git repo @@ -423,9 +434,7 @@ def build_image( logger.info("Image %s already exists. Skipping build.", t) return BuildOutput(base_image=base_image, tags=[t], error=None) - cached_sdist = os.environ.get("OPENHANDS_CACHED_SDIST") - cached_sdist_path = Path(cached_sdist) if cached_sdist else None - with _patch_sdk_sdist_build(sdk_build_module, cached_sdist_path): + with _patch_sdk_sdist_build(sdk_build_module, cached_sdist): tags = build(opts) return BuildOutput(base_image=base_image, tags=tags, error=None) @@ -478,6 +487,7 @@ def _build_with_logging( push: bool = False, max_retries: int = 3, post_build_fn: Callable[[BuildOutput, bool], BuildOutput] | None = None, + cached_sdist: Path | None = None, ) -> BuildOutput: """ Module-level function for building a single image with output capture. @@ -499,7 +509,14 @@ def _build_with_logging( ) time.sleep(2 + attempt * 2) try: - result = build_image(base_image, target_image, custom_tag, target, push) + result = build_image( + base_image, + target_image, + custom_tag, + target, + push, + cached_sdist, + ) except Exception as e: result = BuildOutput( base_image=base_image, @@ -537,6 +554,51 @@ def _build_with_logging( raise RuntimeError("Unreachable code reached in _build_with_logging") +def _iter_batch_results( + batch: list[str], + log_dir: Path, + image: str, + target: TargetType, + push: bool, + base_image_to_custom_tag_fn: Callable[[str], str] | None, + max_workers: int, + max_retries: int, + post_build_fn: Callable[[BuildOutput, bool], BuildOutput] | None, + cached_sdist: Path | None, +): + with ProcessPoolExecutor(max_workers=max_workers) as ex: + futures = {} + for base in batch: + resolved_tag = ( + base_image_to_custom_tag_fn(base) if base_image_to_custom_tag_fn else "" + ) + fut = ex.submit( + _build_with_logging, + log_dir=log_dir, + base_image=base, + target_image=image, + custom_tag=resolved_tag, + target=target, + push=push, + max_retries=max_retries, + post_build_fn=post_build_fn, + cached_sdist=cached_sdist, + ) + futures[fut] = base + + for fut in as_completed(futures): + base = futures[fut] + try: + yield fut.result() + except Exception as e: + logger.error("Build failed for %s: %r", base, e) + yield BuildOutput( + base_image=base, + tags=[], + error=repr(e), + ) + + def _update_pbar( pbar: tqdm, successes: int, @@ -631,125 +693,102 @@ def _chunks(seq: list[str], size: int): batches = list(_chunks(base_images, batch_size or len(base_images))) total_batches = len(batches) - with _sdk_sdist_cache_env(): - with ( - manifest_file.open("w") as writer, - tqdm( - total=len(base_images), desc="Building agent-server images", leave=True - ) as pbar, - ): - _update_pbar(pbar, successes, failures, 0, None, "Queueing") - - for batch_idx, batch in enumerate(batches, start=1): - if not batch: - continue + with ( + _prepare_cached_sdist() as cached_sdist, + manifest_file.open("w") as writer, + tqdm( + total=len(base_images), desc="Building agent-server images", leave=True + ) as pbar, + ): + _update_pbar(pbar, successes, failures, 0, None, "Queueing") + + for batch_idx, batch in enumerate(batches, start=1): + if not batch: + continue + + logger.info( + "Starting batch %d/%d (%d images)", + batch_idx, + total_batches, + len(batch), + ) + in_progress: set[str] = set(batch) + _update_pbar( + pbar, + successes, + failures, + len(in_progress), + next(iter(in_progress), None), + f"Batch {batch_idx}/{total_batches} running", + ) + + for result in _iter_batch_results( + batch=batch, + log_dir=build_log_dir, + image=image, + target=target, + push=push, + base_image_to_custom_tag_fn=base_image_to_custom_tag_fn, + max_workers=max_workers, + max_retries=max_retries, + post_build_fn=post_build_fn, + cached_sdist=cached_sdist, + ): + status = None + writer.write(result.model_dump_json() + "\n") + writer.flush() + + with mu: + if result.error or not result.tags: + failures += 1 + status = "❌ Failed" + else: + successes += 1 + status = "✅ Done" + + in_progress.discard(result.base_image) + pbar.update(1) + _update_pbar( + pbar, + successes, + failures, + len(in_progress), + next(iter(in_progress), None), + status, + ) + used, total = buildkit_disk_usage() + if total > 0: logger.info( - "Starting batch %d/%d (%d images)", + "BuildKit usage after batch %d/%d: %.2f%% (%0.2f GiB / %0.2f GiB)", batch_idx, total_batches, - len(batch), + (used / total) * 100, + used / (1 << 30), + total / (1 << 30), ) - in_progress: set[str] = set() - - with ProcessPoolExecutor(max_workers=max_workers) as ex: - futures = {} - for base in batch: - in_progress.add(base) - resolved_tag = ( - base_image_to_custom_tag_fn(base) - if base_image_to_custom_tag_fn - else "" - ) - fut = ex.submit( - _build_with_logging, - log_dir=build_log_dir, - base_image=base, - target_image=image, - custom_tag=resolved_tag, - target=target, - push=push, - max_retries=max_retries, - post_build_fn=post_build_fn, - ) - futures[fut] = base - - _update_pbar( - pbar, - successes, - failures, - len(in_progress), - next(iter(in_progress), None), - f"Batch {batch_idx}/{total_batches} running", - ) - - for fut in as_completed(futures): - base = futures[fut] - status = None - try: - result: BuildOutput = fut.result() - except Exception as e: - logger.error("Build failed for %s: %r", base, e) - result = BuildOutput( - base_image=base, - tags=[], - error=repr(e), - ) - - writer.write(result.model_dump_json() + "\n") - writer.flush() - - with mu: - if result.error or not result.tags: - failures += 1 - status = "❌ Failed" - else: - successes += 1 - status = "✅ Done" - - in_progress.discard(base) - pbar.update(1) - _update_pbar( - pbar, - successes, - failures, - len(in_progress), - next(iter(in_progress), None), - status, - ) - used, total = buildkit_disk_usage() - if total > 0: + if prune_keep_storage_gb and prune_keep_storage_gb > 0: + pruned = maybe_prune_buildkit_cache( + keep_storage_gb=prune_keep_storage_gb, + threshold_pct=prune_threshold_pct, + filters=prune_filters, + ) + if pruned: logger.info( - "BuildKit usage after batch %d/%d: %.2f%% (%0.2f GiB / %0.2f GiB)", + "Pruned BuildKit cache after batch %d/%d (keep=%d GiB, threshold=%.1f%%)", batch_idx, total_batches, - (used / total) * 100, - used / (1 << 30), - total / (1 << 30), + prune_keep_storage_gb, + prune_threshold_pct, ) - - if prune_keep_storage_gb and prune_keep_storage_gb > 0: - pruned = maybe_prune_buildkit_cache( - keep_storage_gb=prune_keep_storage_gb, - threshold_pct=prune_threshold_pct, - filters=prune_filters, + else: + logger.info( + "No prune needed after batch %d/%d (threshold %.1f%%)", + batch_idx, + total_batches, + prune_threshold_pct, ) - if pruned: - logger.info( - "Pruned BuildKit cache after batch %d/%d (keep=%d GiB, threshold=%.1f%%)", - batch_idx, - total_batches, - prune_keep_storage_gb, - prune_threshold_pct, - ) - else: - logger.info( - "No prune needed after batch %d/%d (threshold %.1f%%)", - batch_idx, - total_batches, - prune_threshold_pct, - ) logger.info( "Done. Built=%d Failed=%d Manifest=%s", successes, diff --git a/tests/test_image_utils.py b/tests/test_image_utils.py index d98e7f0d4..07ec6c05f 100644 --- a/tests/test_image_utils.py +++ b/tests/test_image_utils.py @@ -269,11 +269,24 @@ def test_is_sdist_build_command_matches_expected_shape(self): assert not _is_sdist_build_command(["uv", "build", "--wheel"]) assert not _is_sdist_build_command(["docker", "buildx", "build"]) - def test_patch_sdk_sdist_build_reuses_cached_sdist_only_for_sdist_commands( + def test_is_sdk_sdist_build_command_requires_sdk_cwd(self): + from benchmarks.utils.build_utils import ( + _is_sdk_sdist_build_command, + _sdk_root, + ) + + cmd = ["uv", "build", "--sdist", "--out-dir", "/tmp/out"] + assert _is_sdk_sdist_build_command(cmd, str(_sdk_root())) + assert not _is_sdk_sdist_build_command(cmd, "/tmp") + assert not _is_sdk_sdist_build_command( + ["docker", "buildx", "build"], str(_sdk_root()) + ) + + def test_patch_sdk_sdist_build_reuses_cached_sdist_only_for_sdk_sdist_commands( self, tmp_path: Path, ): - from benchmarks.utils.build_utils import _patch_sdk_sdist_build + from benchmarks.utils.build_utils import _patch_sdk_sdist_build, _sdk_root cached_sdist = tmp_path / "openhands-sdk.tar.gz" cached_sdist.write_text("cached", encoding="utf-8") @@ -288,7 +301,7 @@ def original_run(cmd: list[str], cwd: str | None = None): other_cmd = ["docker", "buildx", "build"] with _patch_sdk_sdist_build(sdk_build_module, cached_sdist): - result = sdk_build_module._run(sdist_cmd, cwd="/sdk") + result = sdk_build_module._run(sdist_cmd, cwd=str(_sdk_root())) assert result.returncode == 0 assert (tmp_path / "out" / cached_sdist.name).read_text( encoding="utf-8" @@ -299,3 +312,61 @@ def original_run(cmd: list[str], cwd: str | None = None): assert sdk_build_module._run is original_run assert forwarded_calls == [(other_cmd, "/sdk")] + + def test_build_image_reuses_cached_sdist_with_real_sdk_build_module( + self, + tmp_path: Path, + ): + from benchmarks.utils.build_utils import _sdk_root, build_image + from openhands.agent_server.docker import build as sdk_build_module + + cached_sdist = tmp_path / "openhands-sdk.tar.gz" + cached_sdist.write_text("cached", encoding="utf-8") + sdk_out_dir = tmp_path / "sdk-out" + nonsdk_out_dir = tmp_path / "nonsdk-out" + forwarded_calls: list[tuple[list[str], str | None]] = [] + + def original_run(cmd: list[str], cwd: str | None = None): + forwarded_calls.append((cmd, cwd)) + return subprocess.CompletedProcess(cmd, 0, stdout="forwarded", stderr="") + + def fake_build(opts): + sdk_cmd = ["uv", "build", "--sdist", "--out-dir", str(sdk_out_dir)] + nonsdk_cmd = ["uv", "build", "--sdist", "--out-dir", str(nonsdk_out_dir)] + + reused = sdk_build_module._run(sdk_cmd, cwd=str(_sdk_root())) + assert reused.returncode == 0 + assert (sdk_out_dir / cached_sdist.name).read_text( + encoding="utf-8" + ) == "cached" + + forwarded = sdk_build_module._run(nonsdk_cmd, cwd=str(tmp_path)) + assert forwarded.stdout == "forwarded" + return ["integration:test"] + + with ( + patch( + "benchmarks.utils.build_utils.remote_image_exists", return_value=False + ), + patch( + "benchmarks.utils.build_utils._get_sdk_submodule_info", + return_value=("main", "abcdef0", "1.0.0"), + ), + patch.object(sdk_build_module, "_run", side_effect=original_run), + patch.object(sdk_build_module, "build", side_effect=fake_build), + ): + result = build_image( + base_image="base:latest", + target_image="ghcr.io/openhands/eval-agent-server", + custom_tag="mytag", + cached_sdist=cached_sdist, + ) + + assert result.error is None + assert result.tags == ["integration:test"] + assert forwarded_calls == [ + ( + ["uv", "build", "--sdist", "--out-dir", str(nonsdk_out_dir)], + str(tmp_path), + ) + ] From 68bf988ec21feec50bbc7ca26018156bc5ba96e6 Mon Sep 17 00:00:00 2001 From: openhands Date: Thu, 12 Mar 2026 14:24:10 +0000 Subject: [PATCH 04/10] docs: add comments explaining stdlib logging for fork-safety Ported from PR #505 - explains why openhands.sdk.get_logger is not used in build-related modules to avoid Rich console state deadlocks when ProcessPoolExecutor forks worker processes. Co-authored-by: openhands --- benchmarks/swebench/build_images.py | 2 ++ benchmarks/swtbench/build_eval_env_images.py | 2 ++ benchmarks/swtbench/image_utils.py | 2 ++ benchmarks/utils/build_utils.py | 2 ++ benchmarks/utils/buildx_utils.py | 2 ++ benchmarks/utils/image_utils.py | 2 ++ 6 files changed, 12 insertions(+) diff --git a/benchmarks/swebench/build_images.py b/benchmarks/swebench/build_images.py index d822442fb..5a892a720 100644 --- a/benchmarks/swebench/build_images.py +++ b/benchmarks/swebench/build_images.py @@ -9,6 +9,8 @@ --image ghcr.io/openhands/eval-agent-server --target source-minimal """ +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import logging import sys from pathlib import Path diff --git a/benchmarks/swtbench/build_eval_env_images.py b/benchmarks/swtbench/build_eval_env_images.py index 0b42a50b4..c64df2883 100644 --- a/benchmarks/swtbench/build_eval_env_images.py +++ b/benchmarks/swtbench/build_eval_env_images.py @@ -1,5 +1,7 @@ from __future__ import annotations +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import argparse import json import logging diff --git a/benchmarks/swtbench/image_utils.py b/benchmarks/swtbench/image_utils.py index 708a7d8b5..387a94085 100644 --- a/benchmarks/swtbench/image_utils.py +++ b/benchmarks/swtbench/image_utils.py @@ -1,5 +1,7 @@ from __future__ import annotations +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import json import logging import os diff --git a/benchmarks/utils/build_utils.py b/benchmarks/utils/build_utils.py index 1ab7a0cec..e4178efa5 100644 --- a/benchmarks/utils/build_utils.py +++ b/benchmarks/utils/build_utils.py @@ -3,6 +3,8 @@ Shared utilities for batch building agent-server images. """ +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import argparse import contextlib import io diff --git a/benchmarks/utils/buildx_utils.py b/benchmarks/utils/buildx_utils.py index a54877cf9..0a2aa2037 100644 --- a/benchmarks/utils/buildx_utils.py +++ b/benchmarks/utils/buildx_utils.py @@ -3,6 +3,8 @@ Buildx/BuildKit utilities for image build resets and pruning. """ +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import json import logging import os diff --git a/benchmarks/utils/image_utils.py b/benchmarks/utils/image_utils.py index 3afe7384e..45c6f770f 100644 --- a/benchmarks/utils/image_utils.py +++ b/benchmarks/utils/image_utils.py @@ -1,6 +1,8 @@ #!/usr/bin/env python3 from __future__ import annotations +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import base64 import logging import os From 355829edb28601064d71175e972cd58f7adfc692 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 12:03:48 -0300 Subject: [PATCH 05/10] fix: use SDK prebuilt sdist API --- benchmarks/utils/build_utils.py | 73 +--------------------------- tests/test_image_utils.py | 86 ++------------------------------- vendor/software-agent-sdk | 2 +- 3 files changed, 8 insertions(+), 153 deletions(-) diff --git a/benchmarks/utils/build_utils.py b/benchmarks/utils/build_utils.py index e4178efa5..01094f560 100644 --- a/benchmarks/utils/build_utils.py +++ b/benchmarks/utils/build_utils.py @@ -237,75 +237,6 @@ def _pre_build_sdist() -> Path: return sdists[0] -def _is_sdist_build_command(cmd: list[str]) -> bool: - return ( - len(cmd) >= 4 - and cmd[0] == "uv" - and cmd[1] == "build" - and "--sdist" in cmd - and "--out-dir" in cmd - ) - - -def _is_sdk_sdist_build_command(cmd: list[str], cwd: str | None) -> bool: - return ( - _is_sdist_build_command(cmd) - and cwd is not None - and Path(cwd).resolve() == _sdk_root().resolve() - ) - - -def _reuse_cached_sdist( - cmd: list[str], - cached_sdist: Path, -) -> subprocess.CompletedProcess[str]: - out_dir = Path(cmd[cmd.index("--out-dir") + 1]).resolve() - out_dir.mkdir(parents=True, exist_ok=True) - target = out_dir / cached_sdist.name - shutil.copy2(cached_sdist, target) - logger.info("Reusing cached SDK sdist %s -> %s", cached_sdist, target) - return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") - - -@contextlib.contextmanager -def _patch_sdk_sdist_build( - sdk_build_module, - cached_sdist: Path | None, -): - """ - Reuse a pre-built sdist while still calling the SDK's own build() logic. - - This keeps tag generation, build args, cache settings, and future SDK build - behavior inside the SDK. Benchmarks only short-circuits the expensive - repeated `uv build --sdist` step. - """ - if cached_sdist is None or not cached_sdist.is_file(): - yield - return - - original_run = getattr(sdk_build_module, "_run", None) - if original_run is None: - logger.warning( - "SDK build module has no _run helper; falling back to native build()" - ) - yield - return - - def patched_run( - cmd: list[str], - cwd: str | None = None, - ) -> subprocess.CompletedProcess[str]: - if _is_sdk_sdist_build_command(cmd, cwd): - return _reuse_cached_sdist(cmd, cached_sdist) - return original_run(cmd, cwd=cwd) - - sdk_build_module._run = patched_run - try: - yield - finally: - sdk_build_module._run = original_run - - @contextlib.contextmanager def _prepare_cached_sdist(): cached_sdist_path: Path | None = None @@ -429,6 +360,7 @@ def build_image( git_ref=git_ref, git_sha=git_sha, sdk_version=sdk_version, + prebuilt_sdist=cached_sdist, ) for t in opts.all_tags: # Check if image exists or not @@ -436,8 +368,7 @@ def build_image( logger.info("Image %s already exists. Skipping build.", t) return BuildOutput(base_image=base_image, tags=[t], error=None) - with _patch_sdk_sdist_build(sdk_build_module, cached_sdist): - tags = build(opts) + tags = build(opts) return BuildOutput(base_image=base_image, tags=tags, error=None) diff --git a/tests/test_image_utils.py b/tests/test_image_utils.py index 07ec6c05f..4b3548b04 100644 --- a/tests/test_image_utils.py +++ b/tests/test_image_utils.py @@ -7,7 +7,6 @@ import os import subprocess from pathlib import Path -from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest @@ -260,88 +259,19 @@ def test_passes_target_to_build_image(self, mock_build, _mock_exists): class TestCachedSdistReuse: - def test_is_sdist_build_command_matches_expected_shape(self): - from benchmarks.utils.build_utils import _is_sdist_build_command - - assert _is_sdist_build_command( - ["uv", "build", "--sdist", "--out-dir", "/tmp/out"] - ) - assert not _is_sdist_build_command(["uv", "build", "--wheel"]) - assert not _is_sdist_build_command(["docker", "buildx", "build"]) - - def test_is_sdk_sdist_build_command_requires_sdk_cwd(self): - from benchmarks.utils.build_utils import ( - _is_sdk_sdist_build_command, - _sdk_root, - ) - - cmd = ["uv", "build", "--sdist", "--out-dir", "/tmp/out"] - assert _is_sdk_sdist_build_command(cmd, str(_sdk_root())) - assert not _is_sdk_sdist_build_command(cmd, "/tmp") - assert not _is_sdk_sdist_build_command( - ["docker", "buildx", "build"], str(_sdk_root()) - ) - - def test_patch_sdk_sdist_build_reuses_cached_sdist_only_for_sdk_sdist_commands( + def test_build_image_passes_cached_sdist_to_sdk_build_module( self, tmp_path: Path, ): - from benchmarks.utils.build_utils import _patch_sdk_sdist_build, _sdk_root - - cached_sdist = tmp_path / "openhands-sdk.tar.gz" - cached_sdist.write_text("cached", encoding="utf-8") - forwarded_calls: list[tuple[list[str], str | None]] = [] - - def original_run(cmd: list[str], cwd: str | None = None): - forwarded_calls.append((cmd, cwd)) - return subprocess.CompletedProcess(cmd, 0, stdout="forwarded", stderr="") - - sdk_build_module = SimpleNamespace(_run=original_run) - sdist_cmd = ["uv", "build", "--sdist", "--out-dir", str(tmp_path / "out")] - other_cmd = ["docker", "buildx", "build"] - - with _patch_sdk_sdist_build(sdk_build_module, cached_sdist): - result = sdk_build_module._run(sdist_cmd, cwd=str(_sdk_root())) - assert result.returncode == 0 - assert (tmp_path / "out" / cached_sdist.name).read_text( - encoding="utf-8" - ) == "cached" - - forwarded = sdk_build_module._run(other_cmd, cwd="/sdk") - assert forwarded.stdout == "forwarded" - - assert sdk_build_module._run is original_run - assert forwarded_calls == [(other_cmd, "/sdk")] - - def test_build_image_reuses_cached_sdist_with_real_sdk_build_module( - self, - tmp_path: Path, - ): - from benchmarks.utils.build_utils import _sdk_root, build_image + from benchmarks.utils.build_utils import build_image from openhands.agent_server.docker import build as sdk_build_module cached_sdist = tmp_path / "openhands-sdk.tar.gz" cached_sdist.write_text("cached", encoding="utf-8") - sdk_out_dir = tmp_path / "sdk-out" - nonsdk_out_dir = tmp_path / "nonsdk-out" - forwarded_calls: list[tuple[list[str], str | None]] = [] - - def original_run(cmd: list[str], cwd: str | None = None): - forwarded_calls.append((cmd, cwd)) - return subprocess.CompletedProcess(cmd, 0, stdout="forwarded", stderr="") + captured = {} def fake_build(opts): - sdk_cmd = ["uv", "build", "--sdist", "--out-dir", str(sdk_out_dir)] - nonsdk_cmd = ["uv", "build", "--sdist", "--out-dir", str(nonsdk_out_dir)] - - reused = sdk_build_module._run(sdk_cmd, cwd=str(_sdk_root())) - assert reused.returncode == 0 - assert (sdk_out_dir / cached_sdist.name).read_text( - encoding="utf-8" - ) == "cached" - - forwarded = sdk_build_module._run(nonsdk_cmd, cwd=str(tmp_path)) - assert forwarded.stdout == "forwarded" + captured["prebuilt_sdist"] = opts.prebuilt_sdist return ["integration:test"] with ( @@ -352,7 +282,6 @@ def fake_build(opts): "benchmarks.utils.build_utils._get_sdk_submodule_info", return_value=("main", "abcdef0", "1.0.0"), ), - patch.object(sdk_build_module, "_run", side_effect=original_run), patch.object(sdk_build_module, "build", side_effect=fake_build), ): result = build_image( @@ -364,9 +293,4 @@ def fake_build(opts): assert result.error is None assert result.tags == ["integration:test"] - assert forwarded_calls == [ - ( - ["uv", "build", "--sdist", "--out-dir", str(nonsdk_out_dir)], - str(tmp_path), - ) - ] + assert captured["prebuilt_sdist"] == cached_sdist diff --git a/vendor/software-agent-sdk b/vendor/software-agent-sdk index bde715c12..d0c1a3927 160000 --- a/vendor/software-agent-sdk +++ b/vendor/software-agent-sdk @@ -1 +1 @@ -Subproject commit bde715c12bce8fb112980529d5ad162f6b81a7f1 +Subproject commit d0c1a392716811cb81c396b3223ca1eafb1f669c From 3d93862f394ba608f2547062f16287a96efcd741 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 15:06:01 -0300 Subject: [PATCH 06/10] build: expose parallelism knobs and shared SDK cache --- .github/workflows/build-swebench-images.yml | 13 ++++- .github/workflows/build-swtbench-images.yml | 14 ++++- benchmarks/commit0/build_images.py | 1 + benchmarks/gaia/build_images.py | 1 + benchmarks/multiswebench/build_images.py | 1 + benchmarks/swebench/build_images.py | 1 + benchmarks/swebenchmultimodal/build_images.py | 1 + benchmarks/swegym/build_images.py | 1 + benchmarks/swesmith/build_images.py | 1 + benchmarks/swtbench/build_images.py | 1 + benchmarks/utils/build_utils.py | 18 +++++- tests/test_image_utils.py | 57 +++++++++++++++++++ vendor/software-agent-sdk | 2 +- 13 files changed, 104 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-swebench-images.yml b/.github/workflows/build-swebench-images.yml index 586c6b8af..f06f9c3e0 100644 --- a/.github/workflows/build-swebench-images.yml +++ b/.github/workflows/build-swebench-images.yml @@ -22,7 +22,12 @@ on: max-workers: description: 'Number of concurrent builds' required: false - default: '12' + default: '32' + type: string + build-batch-size: + description: 'Number of images to submit per batch' + required: false + default: '50' type: string max-retries: description: 'Retries per image build' @@ -54,12 +59,12 @@ on: env: DATASET: princeton-nlp/SWE-bench_Verified SPLIT: test - MAX_WORKERS: '12' + MAX_WORKERS: '32' MAX_RETRIES: '5' N_LIMIT: '500' INSTANCE_IDS: '' SELECT_FILE: '' - BUILD_BATCH_SIZE: '15' + BUILD_BATCH_SIZE: '50' BUILDKIT_PRUNE_KEEP_GB: '60' BUILDKIT_PRUNE_THRESHOLD_PCT: '60' @@ -120,6 +125,7 @@ jobs: if [ -n "${{ inputs.dataset }}" ]; then echo "DATASET=${{ inputs.dataset }}" >> "$GITHUB_ENV"; fi if [ -n "${{ inputs.split }}" ]; then echo "SPLIT=${{ inputs.split }}" >> "$GITHUB_ENV"; fi if [ -n "${{ inputs.max-workers }}" ]; then echo "MAX_WORKERS=${{ inputs.max-workers }}" >> "$GITHUB_ENV"; fi + if [ -n "${{ inputs.build-batch-size }}" ]; then echo "BUILD_BATCH_SIZE=${{ inputs.build-batch-size }}" >> "$GITHUB_ENV"; fi if [ -n "${{ inputs.max-retries }}" ]; then echo "MAX_RETRIES=${{ inputs.max-retries }}" >> "$GITHUB_ENV"; fi # Empty string means "no limit" if [ -n "${{ inputs.n-limit }}" ]; then echo "N_LIMIT=${{ inputs.n-limit }}" >> "$GITHUB_ENV"; else echo "N_LIMIT=" >> "$GITHUB_ENV"; fi @@ -238,6 +244,7 @@ jobs: --image ghcr.io/openhands/eval-agent-server \ --push \ --max-workers '${MAX_WORKERS}' \ + --build-batch-size '${BUILD_BATCH_SIZE}' \ --max-retries '${MAX_RETRIES}'" # Only include --n-limit if provided (non-empty) diff --git a/.github/workflows/build-swtbench-images.yml b/.github/workflows/build-swtbench-images.yml index 5901ab914..49ef4fb4b 100644 --- a/.github/workflows/build-swtbench-images.yml +++ b/.github/workflows/build-swtbench-images.yml @@ -22,7 +22,12 @@ on: max-workers: description: 'Maximum number of parallel workers' required: false - default: '4' + default: '16' + type: string + agent-build-batch-size: + description: 'Number of agent-server images to submit per batch' + required: false + default: '50' type: string n-limit: description: 'Limit number of images to build (0 for all)' @@ -83,7 +88,8 @@ jobs: env: DATASET: eth-sri/SWT-bench_Verified_bm25_27k_zsp SPLIT: test - MAX_WORKERS: '4' + MAX_WORKERS: '16' + BUILD_BATCH_SIZE: '50' N_LIMIT: '0' INSTANCE_IDS: '' SELECT_FILE: '' @@ -176,7 +182,8 @@ jobs: # Get inputs with defaults DATASET="${{ inputs.dataset || 'eth-sri/SWT-bench_Verified_bm25_27k_zsp' }}" SPLIT="${{ inputs.split || 'test' }}" - MAX_WORKERS="${{ inputs.max-workers || '4' }}" + MAX_WORKERS="${{ inputs.max-workers || '16' }}" + BUILD_BATCH_SIZE="${{ inputs.agent-build-batch-size || '50' }}" N_LIMIT="${{ inputs.n-limit || '0' }}" INSTANCE_IDS="${{ inputs.instance-ids }}" @@ -201,6 +208,7 @@ jobs: --image ghcr.io/openhands/eval-agent-server \ --target ${TARGET} \ --max-workers ${MAX_WORKERS} \ + --build-batch-size ${BUILD_BATCH_SIZE} \ --push" # Add n-limit if specified diff --git a/benchmarks/commit0/build_images.py b/benchmarks/commit0/build_images.py index 3f24567ec..fce111401 100644 --- a/benchmarks/commit0/build_images.py +++ b/benchmarks/commit0/build_images.py @@ -124,6 +124,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/gaia/build_images.py b/benchmarks/gaia/build_images.py index 45aee7550..73ca42ad1 100644 --- a/benchmarks/gaia/build_images.py +++ b/benchmarks/gaia/build_images.py @@ -94,6 +94,7 @@ def tag_fn(_base: str) -> str: image=args.image, push=args.push, max_workers=1, # Only building one image + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=tag_fn, diff --git a/benchmarks/multiswebench/build_images.py b/benchmarks/multiswebench/build_images.py index 6c3ef3b9f..c967e8506 100644 --- a/benchmarks/multiswebench/build_images.py +++ b/benchmarks/multiswebench/build_images.py @@ -118,6 +118,7 @@ def main(): ), base_image_to_custom_tag_fn=extract_custom_tag, max_workers=args.num_workers, + build_batch_size=args.build_batch_size, dry_run=False, ) diff --git a/benchmarks/swebench/build_images.py b/benchmarks/swebench/build_images.py index 5a892a720..1e7cb8ff8 100644 --- a/benchmarks/swebench/build_images.py +++ b/benchmarks/swebench/build_images.py @@ -179,6 +179,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swebenchmultimodal/build_images.py b/benchmarks/swebenchmultimodal/build_images.py index 987cf7bda..32628bd78 100644 --- a/benchmarks/swebenchmultimodal/build_images.py +++ b/benchmarks/swebenchmultimodal/build_images.py @@ -83,6 +83,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swegym/build_images.py b/benchmarks/swegym/build_images.py index 6c116a020..009bb0dab 100644 --- a/benchmarks/swegym/build_images.py +++ b/benchmarks/swegym/build_images.py @@ -87,6 +87,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swesmith/build_images.py b/benchmarks/swesmith/build_images.py index 51fda90ae..7a1ee7469 100644 --- a/benchmarks/swesmith/build_images.py +++ b/benchmarks/swesmith/build_images.py @@ -84,6 +84,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swtbench/build_images.py b/benchmarks/swtbench/build_images.py index 3fcd2d8de..621a3385b 100644 --- a/benchmarks/swtbench/build_images.py +++ b/benchmarks/swtbench/build_images.py @@ -44,6 +44,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/utils/build_utils.py b/benchmarks/utils/build_utils.py index 01094f560..d21c6ed89 100644 --- a/benchmarks/utils/build_utils.py +++ b/benchmarks/utils/build_utils.py @@ -324,6 +324,15 @@ def get_build_parser() -> argparse.ArgumentParser: parser.add_argument( "--max-workers", type=int, default=1, help="Concurrent builds (be cautious)" ) + parser.add_argument( + "--build-batch-size", + type=int, + default=None, + help=( + "Number of images to submit per batch. Defaults to BUILD_BATCH_SIZE " + "when unset." + ), + ) parser.add_argument( "--dry-run", action="store_true", help="List base images only, don’t build" ) @@ -568,6 +577,7 @@ def build_all_images( push: bool = False, base_image_to_custom_tag_fn: Callable[[str], str] | None = None, max_workers: int = 1, + build_batch_size: int | None = None, dry_run: bool = False, max_retries: int = 3, post_build_fn: Callable[[BuildOutput, bool], BuildOutput] | None = None, @@ -585,6 +595,8 @@ def build_all_images( base_image_to_custom_tag_fn: Function to extract a custom tag from a base image. Evaluated before scheduling builds so it can safely be a closure. max_workers: Number of concurrent builds. + build_batch_size: Number of images to submit per batch. If None, use the + BUILD_BATCH_SIZE environment variable. dry_run: If True, only list base images without building. max_retries: Number of times to retry each failed build (default: 3). post_build_fn: Optional callback called after each successful build. @@ -609,7 +621,11 @@ def build_all_images( # Batch/prune settings (tunable via env to control disk usage on sticky runners) # Default to smaller batches and more aggressive pruning on shared runners. - batch_size = int(os.getenv("BUILD_BATCH_SIZE", "15")) + batch_size = ( + build_batch_size + if build_batch_size is not None + else int(os.getenv("BUILD_BATCH_SIZE", "15")) + ) prune_keep_storage_gb = int(os.getenv("BUILDKIT_PRUNE_KEEP_GB", "60")) prune_threshold_pct = float(os.getenv("BUILDKIT_PRUNE_THRESHOLD_PCT", "60")) # Prune aggressively by default; filters like "unused-for=12h" prevented GC from diff --git a/tests/test_image_utils.py b/tests/test_image_utils.py index 4b3548b04..82ade18a6 100644 --- a/tests/test_image_utils.py +++ b/tests/test_image_utils.py @@ -4,6 +4,7 @@ which centralize Docker image detection and build logic across all benchmarks. """ +import contextlib import os import subprocess from pathlib import Path @@ -294,3 +295,59 @@ def fake_build(opts): assert result.error is None assert result.tags == ["integration:test"] assert captured["prebuilt_sdist"] == cached_sdist + + +class TestBuildBatchSizeConfig: + def test_build_parser_accepts_build_batch_size(self): + from benchmarks.utils.build_utils import get_build_parser + + args = get_build_parser().parse_args(["--build-batch-size", "50"]) + + assert args.build_batch_size == 50 + + @patch.dict(os.environ, {"BUILD_BATCH_SIZE": "99"}) + def test_build_all_images_prefers_explicit_batch_size_over_env( + self, + tmp_path: Path, + ): + from benchmarks.utils import build_utils + + seen_batches: list[list[str]] = [] + + @contextlib.contextmanager + def fake_prepare_cached_sdist(): + yield None + + def fake_iter_batch_results(**kwargs): + batch = kwargs["batch"] + seen_batches.append(list(batch)) + for base in batch: + yield BuildOutput( + base_image=base, + tags=[f"tag:{base}"], + error=None, + ) + + with ( + patch.object( + build_utils, + "_prepare_cached_sdist", + side_effect=fake_prepare_cached_sdist, + ), + patch.object( + build_utils, + "_iter_batch_results", + side_effect=fake_iter_batch_results, + ), + patch.object(build_utils, "buildkit_disk_usage", return_value=(0, 0)), + patch.object(build_utils, "maybe_prune_buildkit_cache", return_value=False), + ): + exit_code = build_utils.build_all_images( + base_images=["base-1", "base-2", "base-3"], + target="source-minimal", + build_dir=tmp_path, + build_batch_size=2, + ) + + assert exit_code == 0 + assert seen_batches == [["base-1", "base-2"], ["base-3"]] diff --git a/vendor/software-agent-sdk b/vendor/software-agent-sdk index d0c1a3927..fc962c4c8 160000 --- a/vendor/software-agent-sdk +++ b/vendor/software-agent-sdk @@ -1 +1 @@ -Subproject commit d0c1a392716811cb81c396b3223ca1eafb1f669c +Subproject commit fc962c4c8d7fd6eb0e91db06a5ec9911b29589ac From 22ea7b51cdfff717925c66c9efbad411c3fd0ad4 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 20:13:03 -0300 Subject: [PATCH 07/10] build: point SWT experiments at shared-cache export fix Update the software-agent-sdk submodule from fc962c4 to 447aa91 so the issue-504 SWT branch uses the SDK build path that keeps shared cache reads but disables shared cache writes by default. This preserves the prebuilt-sdist work while removing the parallel registry export contention identified in issue #510. Co-authored-by: openhands --- vendor/software-agent-sdk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/software-agent-sdk b/vendor/software-agent-sdk index fc962c4c8..447aa918b 160000 --- a/vendor/software-agent-sdk +++ b/vendor/software-agent-sdk @@ -1 +1 @@ -Subproject commit fc962c4c8d7fd6eb0e91db06a5ec9911b29589ac +Subproject commit 447aa918b77ffaa1120b67c0cc9b55907eea7f82 From c1423f7d3fe261650f9211440a1d054d8031aaaa Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 23:42:26 -0300 Subject: [PATCH 08/10] build: disable cache exports for SWT image workflow --- .github/workflows/build-swtbench-images.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build-swtbench-images.yml b/.github/workflows/build-swtbench-images.yml index 49ef4fb4b..eca402d57 100644 --- a/.github/workflows/build-swtbench-images.yml +++ b/.github/workflows/build-swtbench-images.yml @@ -225,6 +225,7 @@ jobs: DOCKER_BUILDKIT: 1 BUILDKIT_PROGRESS: plain BUILDKIT_RESET_ON_FAILURE: 1 + OPENHANDS_BUILDKIT_CACHE_MODE: off - name: Build prebaked eval env images if: ${{ inputs.build-eval-env == 'true' }} From 277cb51be339c3f7fda1705be90cfa05840b3d81 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 12 Mar 2026 23:43:17 -0300 Subject: [PATCH 09/10] build: point SWT workflow at cache-export fix --- vendor/software-agent-sdk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/software-agent-sdk b/vendor/software-agent-sdk index 447aa918b..8e8223b24 160000 --- a/vendor/software-agent-sdk +++ b/vendor/software-agent-sdk @@ -1 +1 @@ -Subproject commit 447aa918b77ffaa1120b67c0cc9b55907eea7f82 +Subproject commit 8e8223b24dfd041875fe845b13326b0a1238b793 From 039aebd2fa315bcbf738cbbf5c92ad8a90bb4c75 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 13 Mar 2026 06:09:34 -0300 Subject: [PATCH 10/10] build: remove image workflow timeouts --- .github/workflows/build-swebench-images.yml | 2 -- .github/workflows/build-swtbench-images.yml | 2 -- 2 files changed, 4 deletions(-) diff --git a/.github/workflows/build-swebench-images.yml b/.github/workflows/build-swebench-images.yml index f06f9c3e0..c5d0820db 100644 --- a/.github/workflows/build-swebench-images.yml +++ b/.github/workflows/build-swebench-images.yml @@ -83,8 +83,6 @@ jobs: runs-on: labels: blacksmith-32vcpu-ubuntu-2204 - timeout-minutes: 180 - # Allow pushing to GHCR and commenting on issues permissions: contents: read diff --git a/.github/workflows/build-swtbench-images.yml b/.github/workflows/build-swtbench-images.yml index eca402d57..d92b16e0b 100644 --- a/.github/workflows/build-swtbench-images.yml +++ b/.github/workflows/build-swtbench-images.yml @@ -77,8 +77,6 @@ jobs: runs-on: labels: blacksmith-32vcpu-ubuntu-2204 - timeout-minutes: 180 - permissions: contents: read packages: write