From c656befc3bcf7d3b293d32347fdda77e405400df Mon Sep 17 00:00:00 2001 From: Charles Wang Date: Sun, 2 Aug 2026 09:47:04 -0700 Subject: [PATCH 1/5] perf(box): speed cold docker builds and fix update --check estimates Replace apt nodejs meta-packages with the official Node binary, bump cryptography to a cp312 wheel, and hash the target-ref Dockerfile so forward version jumps no longer report a false ~90s cached estimate. Also surface live docker-build detail on the progress bar and use algorithms.AES for BluFi so the crypto bump stays compatible. Co-authored-by: Cursor --- box/lager/blufi/security/aes.py | 11 +- box/lager/docker/box.Dockerfile | 42 ++++- cli/commands/utility/update.py | 280 +++++++++++++++++++++++++++----- cli/tests/test_update_gate.py | 84 ++++++++++ 4 files changed, 360 insertions(+), 57 deletions(-) diff --git a/box/lager/blufi/security/aes.py b/box/lager/blufi/security/aes.py index 2bcd4cc3..5d7972de 100644 --- a/box/lager/blufi/security/aes.py +++ b/box/lager/blufi/security/aes.py @@ -8,9 +8,9 @@ # path with removal upstream calls imminent but has not scheduled. from cryptography.hazmat.decrepit.ciphers.modes import CFB except ImportError: - # Older installs predate the decrepit package entirely -- the box - # runtime pins cryptography==38.0.4 (box.Dockerfile) and the unit - # floor is >=42. Same class either way, byte-identical output. + # Older installs predate the decrepit package entirely. Box runtime + # pins cryptography==43.0.3 (box.Dockerfile); unit floor is >=42. + # Same class either way, byte-identical output. from cryptography.hazmat.primitives.ciphers.modes import CFB # https://cryptography.io/en/latest/hazmat/primitives/symmetric-encryption/#cryptography.hazmat.primitives.ciphers.algorithms.AES @@ -19,7 +19,10 @@ class BlufiAES(object): def __init__(self, key, iv): self.key = key self.iv = iv - self.cipher = Cipher(algorithms.AES128(self.key), CFB(self.iv)) + # AES128 was a fixed-size alias; AES(key) is the stable API across + # cryptography versions (and what manylinux wheels for 3.12 expect). + # BluFi always uses a 16-byte key. + self.cipher = Cipher(algorithms.AES(self.key), CFB(self.iv)) self.encryptor = self.cipher.encryptor() self.decryptor = self.cipher.decryptor() diff --git a/box/lager/docker/box.Dockerfile b/box/lager/docker/box.Dockerfile index c8f456b0..b942e8af 100644 --- a/box/lager/docker/box.Dockerfile +++ b/box/lager/docker/box.Dockerfile @@ -5,16 +5,18 @@ ENV PYTHONDONTWRITEBYTECODE=1 ENV PYTHONUNBUFFERED=1 ENV PYTHONPATH=/app/lager +# flex/bison/ccache/ninja are NOT here: they are build-only for uldaq (next +# stage) and would bloat every image. Node is installed from the official +# tarball below — Debian's `nodejs npm` meta-packages pull ~400 unused +# `node-*` packages and dominate cold-build time. RUN apt-get update && apt-get install -y ca-certificates libusb-1.0-0-dev libudev-dev \ libhidapi-dev git gcc python-dev-is-python3 python3-pip python3-venv xz-utils build-essential bluetooth ssh openssh-client \ cups-client lpr zlib1g-dev wget libjpeg-dev libpng-dev libfreetype6-dev \ fswebcam automake g++ libtool libleptonica-dev make pkg-config libpango1.0-dev gdb-multiarch tesseract-ocr libtesseract-dev \ - flex bison ccache ninja-build \ libturbojpeg0-dev v4l-utils \ wireless-tools \ tini \ gnupg \ - nodejs npm \ openocd \ && wget -qO /usr/share/keyrings/phidgets.gpg https://www.phidgets.com/gpgkey/pubring.gpg \ && echo deb [signed-by=/usr/share/keyrings/phidgets.gpg] http://www.phidgets.com/debian bookworm main > /etc/apt/sources.list.d/phidgets.list \ @@ -27,14 +29,33 @@ RUN apt-get update && apt-get install -y ca-certificates libusb-1.0-0-dev libude virtualenv \ && : +# Node.js + npm from the official binary tarball (~50MB) instead of apt's +# `nodejs` meta-package (~500MB of unused node-* tooling). Needed at runtime +# so start_box.sh can `npm install -g` box_config.npm_packages. Multi-arch: +# dpkg arch → node dist arch. +# Pin + verify: Debian-signed apt packages are replaced by an upstream tarball, +# so HTTPS alone is not enough — check SHASUMS256.txt before extracting. +ENV NODE_VERSION=20.18.1 +RUN arch="$(dpkg --print-architecture)" \ + && case "$arch" in \ + amd64) node_arch=x64 ;; \ + arm64) node_arch=arm64 ;; \ + armhf) node_arch=armv7l ;; \ + *) echo "unsupported arch for Node.js: $arch" >&2; exit 1 ;; \ + esac \ + && cd /tmp \ + && tarball="node-v${NODE_VERSION}-linux-${node_arch}.tar.xz" \ + && wget -q "https://nodejs.org/dist/v${NODE_VERSION}/${tarball}" \ + && wget -q "https://nodejs.org/dist/v${NODE_VERSION}/SHASUMS256.txt" \ + && grep " ${tarball}$" SHASUMS256.txt | sha256sum -c - \ + && tar -xJf "${tarball}" --strip-components=1 -C /usr/local \ + && rm -f "${tarball}" SHASUMS256.txt \ + && node --version && npm --version + # PicoScope SDK is mounted from host at runtime via start_box.sh # The SDK library is at /opt/picoscope/lib/libps2000.so on the host # This avoids the systemd reload error that occurs when installing in Docker - - - - # MCC uldaq C library -- required by uldaq Python bindings for USB-202 DAQ devices # See: https://github.com/mccdaq/uldaq # @@ -56,7 +77,10 @@ RUN apt-get update && apt-get install -y ca-certificates libusb-1.0-0-dev libude # a memory-safety bug rather than close it, so the warnings stay: noisy once # per image build, and honest. Fixing it means patching upstream source at # build time or dropping the library -- upstream has not released since 2022. -RUN apt-get update && apt-get install -y autoconf automake libtool libusb-1.0-0-dev && rm -rf /var/lib/apt/lists/* \ +# +# flex/bison stay in this layer only (autoreconf); not needed at runtime. +RUN apt-get update && apt-get install -y autoconf automake libtool libusb-1.0-0-dev flex bison \ + && rm -rf /var/lib/apt/lists/* \ && git clone --depth 1 --branch v1.2.1 https://github.com/mccdaq/uldaq.git /tmp/uldaq \ && cd /tmp/uldaq \ && autoreconf -i \ @@ -111,7 +135,9 @@ RUN --mount=type=cache,target=/root/.cache/pip,sharing=locked \ 'simplejson==3.18.0' \ 'labjack-ljm==1.23.0' \ 'pygdbmi==0.11.0.0' \ - 'cryptography==38.0.4' \ + # 38.0.4 has no cp312 wheel — every cold pip layer compiled Rust from + # source (~2-3 min). >=42 ships manylinux wheels for python 3.12. + 'cryptography==43.0.3' \ 'psycopg2-binary==2.9.9' \ 'pyvisa-py==0.5.2' \ 'PyVISA==1.11.3' \ diff --git a/cli/commands/utility/update.py b/cli/commands/utility/update.py index 6323c730..f27c4514 100644 --- a/cli/commands/utility/update.py +++ b/cli/commands/utility/update.py @@ -205,10 +205,23 @@ def _flatten_shell_cmd(): # Files whose contents legitimately invalidate the cached Docker image. # Dockerfile is the universal one; requirements.txt may not exist on every box # revision, so we sha256sum it only when present. +# +# Working-tree paths (post-flatten layout on the box) vs git blob paths (repo +# keeps the `box/` prefix). Target-ref hashing uses the git paths so --check +# can compare the upcoming pull's build inputs to /etc/lager/build-hash without +# checking it out. Individual blobs are listed separately from the source tree +# because `_build_hash_shell_cmd` hashes `_BUILD_HASH_INPUTS` first and then +# walks `_BUILD_HASH_SOURCE_DIRS` — Dockerfile/requirements therefore appear +# twice in the aggregate, and the at-ref hasher must match that composition. _BUILD_HASH_INPUTS = [ '~/box/lager/docker/box.Dockerfile', '~/box/lager/requirements.txt', ] +_BUILD_HASH_GIT_BLOBS = [ + # (git path under ~/box, absolute working-tree path after flatten) + ('box/lager/docker/box.Dockerfile', '~/box/lager/docker/box.Dockerfile'), + ('box/lager/requirements.txt', '~/box/lager/requirements.txt'), +] # Source trees whose contents also invalidate the cached image. The Dockerfile # `COPY`s these into the image, so a pure-Python change must wipe it: without @@ -220,6 +233,9 @@ def _flatten_shell_cmd(): _BUILD_HASH_SOURCE_DIRS = [ '~/box/lager', ] +# Git-tree prefix corresponding to `_BUILD_HASH_SOURCE_DIRS` after flatten +# (`box/lager/...` blob → `$HOME/box/lager/...` working-tree path). +_BUILD_HASH_GIT_SOURCE_PREFIX = 'box/lager' def _build_hash_shell_cmd(): @@ -267,6 +283,59 @@ def _build_hash_shell_cmd(): ) +def _build_hash_at_ref_shell_cmd(git_ref): + """Shell snippet: hash build inputs at ``git_ref`` without checking out. + + Emits the same aggregate sha256 as `_build_hash_shell_cmd` when the + working tree matches ``git_ref`` for those files — ``sha256sum`` lines use + the post-flatten absolute paths so they compose identically (including the + deliberate double-count of Dockerfile/requirements: once via the individual + inputs list, once via the source-tree walk). Missing blobs at the ref are + skipped (same as a missing working-tree file). Empty output means nothing + was measurable. + """ + # Sanitize: only allow refs that git will accept as a single argument + # (branch, tag, SHA, origin/main). Reject shell metacharacters. + if not git_ref or any(c in git_ref for c in ' \t\n\r;|&$`\\"\'<>(){}[]'): + return 'echo ""' + # POSIX only — the remote login shell may be dash, and the repo's probe + # tests execute these snippets under `sh`. No bash pattern substitution + # and no `printf -v`. + # + # `git show | sha256sum` prints " -"; the sed rewrites the "-" to + # the absolute working-tree path so each line is byte-identical to what + # `sha256sum ` produces in `_build_hash_shell_cmd`. The aggregate + # then uses the same `out=$(...)` + `echo "$out" | sha256sum` composition, + # so a ref whose blobs match the working tree yields the same digest. + clauses = [] + for git_path, abs_tilde in _BUILD_HASH_GIT_BLOBS: + abs_shell = abs_tilde.replace('~', '$HOME', 1) + # `&&` inside a clause (skip missing blobs), `;` between clauses so a + # missing requirements.txt does not suppress the Dockerfile line. + clauses.append( + f'git cat-file -e {git_ref}:{git_path} 2>/dev/null && ' + f'git show {git_ref}:{git_path} | sha256sum | ' + f'sed "s| -$| {abs_shell}|"' + ) + # Source-tree walk: same files as `find ~/box/lager ... | sort -z`, but + # read from the git object database. Paths are mapped from the pre-flatten + # git prefix (`box/lager/...`) to the post-flatten absolute path. + src_prefix = _BUILD_HASH_GIT_SOURCE_PREFIX + clauses.append( + f'git ls-tree -r --name-only {git_ref} {src_prefix} 2>/dev/null | ' + f'grep -v "/__pycache__/" | grep -v "\\.pyc$" | sort | ' + f'while IFS= read -r path; do ' + f'abs="$HOME/box/${{path#box/}}"; ' + f'git show {git_ref}:"$path" | sha256sum | sed "s| -$| $abs|"; ' + f'done' + ) + return ( + 'out=$(cd "$HOME/box" 2>/dev/null && { ' + + '; '.join(clauses) + + '; }); [ -n "$out" ] && echo "$out" | sha256sum | cut -d" " -f1' + ) + + def _read_build_hash(ssh_runner): """Return the sha256 of the box's *current* docker-build inputs. @@ -279,6 +348,38 @@ def _read_build_hash(ssh_runner): return r.stdout.strip() if r.returncode == 0 else '' +def _read_build_hash_at_ref(ssh_runner, git_ref): + """Return the sha256 of docker-build inputs at ``git_ref`` (no checkout).""" + r = ssh_runner(_build_hash_at_ref_shell_cmd(git_ref)) + return r.stdout.strip() if r.returncode == 0 else '' + + +def _preview_deps_status(*, force, stored_hash, working_hash, target_hash, needs_pull): + """Classify Dockerfile/requirements/source drift for ``--check`` preview. + + Returns ``(deps_will_change, deps_status)``. When ``needs_pull``, prefer + ``target_hash`` (blob hash at the ref about to be checked out) over the + working-tree hash — otherwise a forward jump whose *current* tree still + matches the stored hash falsely reports "~90s (cached build)". + """ + if force: + return True, 'forced clean rebuild (--force: image + cargo/npm volumes wiped)' + if needs_pull: + if target_hash: + if _build_hash_mismatch(target_hash, stored_hash): + return True, ( + 'will trigger fresh build ' + '(target Dockerfile, requirements or box source differ)' + ) + return False, 'cache valid (target matches last build)' + # Could not measure the target (sparse checkout, odd ref, …). Be + # honest rather than claiming cache-valid from the pre-pull tree. + return True, 'unknown until pull (could not hash target build inputs)' + if _build_hash_mismatch(working_hash, stored_hash): + return True, 'will trigger fresh build (Dockerfile, requirements or box source changed)' + return False, 'cache valid (no rebuild)' + + def _read_box_source_version(ssh_runner): """Return the `__version__` string declared in `cli/__init__.py` at the box's current HEAD, or empty. @@ -447,6 +548,49 @@ def _pull_shell_script(target_version, git_ref): ) +def _docker_build_line_summary(line): + """Extract a short human label from a BuildKit / docker build log line. + + Returns None for noise (blank, pure progress hashes, cache hits with no + payload). Used to feed ``ProgressBar.set_detail`` during the container + build so the bar shows *what* is slow. + """ + s = (line or '').strip() + if not s: + return None + # BuildKit: "#15 3.2 Setting up nodejs (20.x)" or "#12 [5/20] RUN pip ..." + if s.startswith('#'): + # Drop the leading "#N" / "#N M.M" prefix. + rest = s.lstrip('#').split(None, 1) + if len(rest) < 2: + return None + payload = rest[1] + # "#12 0.5 " timed lines — strip leading seconds if present. + parts = payload.split(None, 1) + if parts and parts[0].replace('.', '', 1).isdigit() and len(parts) > 1: + payload = parts[1] + payload = payload.strip() + if not payload or payload.startswith('DONE ') or payload == 'CACHED': + return None + return payload[:60] + # Classic builder / pip noise that still indicates progress. + for prefix in ( + 'Step ', + 'Collecting ', + 'Downloading ', + 'Building wheel ', + 'Installing collected', + 'Setting up ', + 'Unpacking ', + 'Compiling ', + 'Downloading crates', + 'Installing ', + ): + if s.startswith(prefix): + return s[:60] + return None + + def _build_hash_mismatch(new_hash, stored_hash): """True when the docker-build inputs changed relative to the last successful build. @@ -486,7 +630,7 @@ def _rebuild_gate_verdict(facts, *, git_sync_confirmed, needs_pull, def _deps_preview(new_hash, stored_hash, *, force, needs_flatten, - is_rollback, commits_ahead): + is_rollback, commits_ahead, target_hash='', needs_pull=False): """What `--check` reports about the docker-build cache. Returns ``(status_text, rebuild_certain, unmeasured)``. @@ -498,7 +642,7 @@ def _deps_preview(new_hash, stored_hash, *, force, needs_flatten, "Deps: cache valid (no rebuild) / Estimated: ~90s (cached build)" immediately before a ten-minute clean rebuild. - Two distinct failures were behind that: + Three distinct failures were behind that: * `needs_flatten` was ignored here, though the gate treats it as a definite rebuild trigger. The flatten moves every source file, and the @@ -506,36 +650,60 @@ def _deps_preview(new_hash, stored_hash, *, force, needs_flatten, its digest — so a moved file necessarily changes it. * `_build_hash_mismatch` returns False both for "measured, unchanged" and for "could not measure", and this rendered both as "cache valid". + * The only digest on hand was the *pre-pull* working tree's, so a forward + jump whose current tree still matched the stored hash read as "cache + valid" however much the target ref changed the image recipe. + + ``target_hash`` closes the third: the same aggregate digest, measured + against the ref about to be checked out and read straight out of the + object DB without a checkout (`_read_build_hash_at_ref`). When a pull is + coming and that measurement succeeded it — not the working tree — is what + the build will be keyed on, which also turns the old "older ref may + differ" guess into a measured answer. An empty ``target_hash`` means the + measurement failed (sparse checkout, odd ref) and the pre-pull tree is + all there is; that is reported as unknown rather than as cache-valid. `unmeasured` deliberately does NOT imply a rebuild: the gate does not treat it as one either, so predicting one would over-estimate exactly as badly as the old text under-estimated. It only stops the preview claiming knowledge it does not have. """ - unmeasured = not new_hash or not stored_hash - rebuild_certain = _build_hash_mismatch(new_hash, stored_hash) or needs_flatten + # Whichever digest the upcoming build will actually be keyed on. + at_target = bool(needs_pull and target_hash) + measured = target_hash if at_target else new_hash + + unmeasured = not measured or not stored_hash + rebuild_certain = _build_hash_mismatch(measured, stored_hash) or needs_flatten if force: return ('forced clean rebuild (--force: image + cargo/npm volumes wiped)', rebuild_certain, unmeasured) - if _build_hash_mismatch(new_hash, stored_hash): - return ('will trigger fresh build (Dockerfile, requirements or box source changed)', + if _build_hash_mismatch(measured, stored_hash): + what = 'target Dockerfile' if at_target else 'Dockerfile' + return (f'will trigger fresh build ({what}, requirements or box source changed)', rebuild_certain, unmeasured) if needs_flatten: return ('will rebuild (flatten moves every source path)', rebuild_certain, unmeasured) - if is_rollback or commits_ahead > 0: + if needs_pull and not target_hash: + # A pull is coming and the target ref's build inputs could not be + # read, so the pre-pull tree matching the stored hash proves nothing + # about what the pull is going to land. + return ('unknown until pull (could not hash target build inputs)', + rebuild_certain, unmeasured) + if not at_target and (is_rollback or commits_ahead > 0): # Probe measured the *current* (pre-pull) Dockerfile/requirements, so # a backward jump can still trigger a rebuild we can't predict # without actually pulling. Be honest about the unknown. return ('unknown until pull (older ref may differ)', rebuild_certain, unmeasured) if unmeasured: - why = ('no successful build recorded yet' if new_hash + why = ('no successful build recorded yet' if measured else 'box source not where the probe looks') return (f'cache state unknown, auto-invalidation skipped ({why})', rebuild_certain, unmeasured) - return ('cache valid (no rebuild)', rebuild_certain, unmeasured) + return ('cache valid (target matches last build)' if at_target + else 'cache valid (no rebuild)', rebuild_certain, unmeasured) def _deployed_version_stale(tree_version, etc_version_raw): @@ -594,6 +762,7 @@ def __init__(self, total_steps): self.total_steps = total_steps self.current_step = 0 self.current_task = "" + self._base_task = "" self.start_time = time.time() self._stop_event = threading.Event() self._render_thread = None @@ -629,6 +798,7 @@ def _periodic_render(self): def update(self, task_name): """Advance to the next step and render.""" self.current_step += 1 + self._base_task = task_name self.current_task = task_name self._render() # Lazy-start the periodic thread on first step so we don't tick a @@ -636,6 +806,23 @@ def update(self, task_name): if self._tty: self._start_periodic_thread() + def set_detail(self, detail): + """Refresh the in-flight label with a live detail suffix. + + Used during long steps (Docker build) so the bar shows what is + actually running, e.g. ``Building container... [pip install ...]``. + Does not advance the step counter. No-op when ``detail`` is empty. + """ + detail = (detail or '').strip() + if not detail: + return + # Keep the suffix short so _layout can still fit the base label. + if len(detail) > 50: + detail = detail[:47] + '...' + with self._lock: + self.current_task = f'{self._base_task} [{detail}]' + self._render() + def _format_elapsed_time(self): """Format elapsed time as human-readable string.""" elapsed = int(time.time() - self.start_time) @@ -1409,11 +1596,19 @@ def _parse_fetch_result(result): current_version_raw = facts.get('ETC_VERSION', '').strip() current_box_version = current_version_raw.split('|', 1)[0] if current_version_raw else '(unknown)' - # --check does no git pull, so the probe's build-hash inputs still - # reflect the tree that would be built — no extra round-trip needed. - _check_new_hash = facts.get('BUILD_HASH_NEW', '') + # --check does no git pull. Hash the *target* ref's Dockerfile when a + # pull is pending so a forward jump (e.g. 0.31 → main) that changes + # the image recipe is not misreported as "~90s (cached)" from the + # pre-pull working tree matching /etc/lager/build-hash. + _check_working_hash = facts.get('BUILD_HASH_NEW', '') _check_stored_hash = facts.get('BUILD_HASH_STORED', '') - deps_will_change = _build_hash_mismatch(_check_new_hash, _check_stored_hash) + _check_target_hash = '' + if needs_pull and git_sync_confirmed: + _check_target_hash = _read_build_hash_at_ref( + run_ssh_command_with_output, git_ref, + ) + # The preview itself is rendered further down, once `needs_flatten` + # and the container/deploy facts are in hand -- see `_deps_preview`. container_down = facts.get('LAGER_RUNNING', '') == '0' @@ -1450,11 +1645,13 @@ def _parse_fetch_result(result): ) deps_status, deps_rebuild_certain, deps_unmeasured = _deps_preview( - _check_new_hash, _check_stored_hash, + _check_working_hash, _check_stored_hash, force=force, needs_flatten=needs_flatten, is_rollback=is_rollback, commits_ahead=commits_ahead or 0, + target_hash=_check_target_hash, + needs_pull=needs_pull, ) if force: @@ -1479,7 +1676,9 @@ def _parse_fetch_result(result): # in the Dockerfile cache; assume a real build. An unmeasured hash # lands here too — over-estimating a cached build costs the # operator nothing, while under-estimating a fresh one is how - # "~90s" preceded ten minutes of waiting. + # "~90s" preceded ten minutes of waiting. Kept as a floor even + # now that a measured `target_hash` can clear `rebuild_certain`: + # the digest covers the image recipe, not every COPY layer. container_status = 'will restart' est = '~6 min (fresh build possible)' else: @@ -2264,37 +2463,28 @@ def _docker_supports_buildkit(): 'DOCKER_BUILDKIT=1 docker build -f docker/box.Dockerfile -t lager .']) build_output_lines = [] - if verbose: - # Stream output in verbose mode - process = subprocess.Popen( - ssh_cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - text=True, - encoding='utf-8', - errors='replace', - bufsize=1 - ) - if process.stdout: - for line in process.stdout: + # Always read the build stream so non-verbose mode can still surface a + # live detail on the progress bar (and so failures still get the last + # 20 lines). Verbose additionally echoes each line to the terminal. + process = subprocess.Popen( + ssh_cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + encoding='utf-8', + errors='replace', + bufsize=1, + ) + if process.stdout: + for line in process.stdout: + build_output_lines.append(line.rstrip()) + if verbose: click.echo(f' {line}', nl=False) - build_output_lines.append(line.rstrip()) - return_code = process.wait(timeout=600) - else: - # Silent mode - capture output for error reporting - process = subprocess.Popen( - ssh_cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - text=True, - encoding='utf-8', - errors='replace', - ) - # Read and store output for potential error reporting - if process.stdout: - for line in process.stdout: - build_output_lines.append(line.rstrip()) - return_code = process.wait(timeout=600) + elif progress: + summary = _docker_build_line_summary(line) + if summary: + progress.set_detail(summary) + return_code = process.wait(timeout=600) if return_code != 0: if progress: diff --git a/cli/tests/test_update_gate.py b/cli/tests/test_update_gate.py index 3f3e0e76..45d93f14 100644 --- a/cli/tests/test_update_gate.py +++ b/cli/tests/test_update_gate.py @@ -12,9 +12,12 @@ import pytest from cli.commands.utility.update import ( + _build_hash_at_ref_shell_cmd, _build_hash_mismatch, _deployed_version_stale, + _docker_build_line_summary, _parse_probe_output, + _preview_deps_status, _probe_shell_script, _pull_shell_script, _rebuild_gate_verdict, @@ -191,6 +194,87 @@ def test_old_probe_output_leaves_host_keys_absent(self): assert 'HOST_CLI_VERSION' not in facts +class TestPreviewDepsStatus: + """`--check` must hash the *target* ref when a pull is pending. + + Repro (JUL-4): 121 commits behind, pre-pull Dockerfile still matched the + stored hash → old code said "cache valid / ~90s", then the pull changed + the Dockerfile and the real update took ~6 min. + """ + + def test_forward_jump_target_mismatch_reports_fresh_build(self): + change, status = _preview_deps_status( + force=False, + stored_hash=SHA_A, + working_hash=SHA_A, # pre-pull tree still matches + target_hash=SHA_B, # origin/main Dockerfile differs + needs_pull=True, + ) + assert change is True + assert 'target Dockerfile' in status + + def test_forward_jump_target_match_reports_cache_valid(self): + change, status = _preview_deps_status( + force=False, + stored_hash=SHA_A, + working_hash=SHA_A, + target_hash=SHA_A, + needs_pull=True, + ) + assert change is False + assert 'target matches' in status + + def test_unmeasurable_target_is_unknown_not_cache_valid(self): + change, status = _preview_deps_status( + force=False, + stored_hash=SHA_A, + working_hash=SHA_A, + target_hash='', + needs_pull=True, + ) + assert change is True + assert 'unknown until pull' in status + + def test_in_sync_uses_working_tree_hash(self): + change, status = _preview_deps_status( + force=False, + stored_hash=SHA_A, + working_hash=SHA_B, + target_hash='', + needs_pull=False, + ) + assert change is True + assert 'Dockerfile or requirements changed' in status + + +class TestBuildHashAtRefShellCmd: + def test_emits_git_show_for_dockerfile_blob(self): + script = _build_hash_at_ref_shell_cmd('origin/main') + assert 'git show origin/main:box/lager/docker/box.Dockerfile' in script + assert 'git cat-file -e origin/main:box/lager/docker/box.Dockerfile' in script + + def test_rejects_metacharacters(self): + assert _build_hash_at_ref_shell_cmd('main; rm -rf /') == 'echo ""' + assert _build_hash_at_ref_shell_cmd('') == 'echo ""' + + +class TestDockerBuildLineSummary: + def test_buildkit_run_line(self): + assert 'pip3 install' in ( + _docker_build_line_summary('#12 [8/20] RUN pip3 install cryptography') or '' + ) + + def test_buildkit_timed_setting_up(self): + assert 'Setting up' in ( + _docker_build_line_summary('#15 3.2 Setting up nodejs (20.x)') or '' + ) + + def test_ignores_cached_and_blank(self): + assert _docker_build_line_summary('#5 CACHED') is None + assert _docker_build_line_summary('') is None + assert _docker_build_line_summary(' ') is None + + class TestGateIgnoresHostCliFacts: def test_host_cli_mismatch_never_forces_rebuild(self): # A missing/stale host CLI reconciles on the fast path; it must never From 189a13fb06eb4ebe1d5071af56a76b6aa0240b62 Mon Sep 17 00:00:00 2001 From: Charles Wang Date: Mon, 3 Aug 2026 10:03:47 -0700 Subject: [PATCH 2/5] fix(update): make target-ref build hash POSIX-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The at-ref hasher used bash pattern substitution and `printf -v`, both of which are silently wrong under dash — the digest would never match the stored working-tree hash on a box whose login shell is /bin/sh, making every --check report a spurious rebuild. Rewrite with `git show | sha256sum | sed` and the same `out=$(...)` + `echo "$out" | sha256sum` composition as the working-tree hasher, so the two are byte-identical by construction. Add tests that execute both snippets under `sh` against a fake box layout rather than asserting on substrings. Co-authored-by: Cursor --- cli/tests/test_update_gate.py | 81 +++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/cli/tests/test_update_gate.py b/cli/tests/test_update_gate.py index 45d93f14..f1e3c194 100644 --- a/cli/tests/test_update_gate.py +++ b/cli/tests/test_update_gate.py @@ -6,6 +6,7 @@ mismatch predicate, and the early-exit verdict (including container liveness). """ import os +import shutil import stat import subprocess @@ -14,6 +15,7 @@ from cli.commands.utility.update import ( _build_hash_at_ref_shell_cmd, _build_hash_mismatch, + _build_hash_shell_cmd, _deployed_version_stale, _docker_build_line_summary, _parse_probe_output, @@ -257,6 +259,85 @@ def test_rejects_metacharacters(self): assert _build_hash_at_ref_shell_cmd('main; rm -rf /') == 'echo ""' assert _build_hash_at_ref_shell_cmd('') == 'echo ""' + def test_uses_only_posix_shell_constructs(self): + # The remote login shell may be dash. Bash pattern substitution and + # `printf -v` both silently changed the digest under /bin/sh. + script = _build_hash_at_ref_shell_cmd('origin/main') + assert 'printf -v' not in script + assert '/#' not in script + + +@pytest.mark.skipif( + shutil.which('sha256sum') is None, + reason='needs GNU sha256sum (present on boxes and CI; macOS ships shasum)', +) +class TestBuildHashAtRefMatchesWorkingTree: + """Execute both hashers under ``sh`` against a fake box layout. + + This is the invariant #12 depends on: the target-ref digest must equal the + working-tree digest that `/etc/lager/build-hash` stores, or every --check + would report a spurious rebuild. Verified end-to-end (real git, real + sha256sum, dash-compatible) rather than by asserting on substrings. + """ + + DOCKERFILE_GIT_PATH = 'box/lager/docker/box.Dockerfile' + DOCKERFILE_TREE_PATH = 'lager/docker/box.Dockerfile' + + def _fake_box(self, tmp_path, dockerfile_body): + """Build $HOME/box as the boxes have it: git tracks the `box/` prefix, + the working tree is the flattened layout.""" + home = tmp_path / 'home' + box = home / 'box' + for rel in (self.DOCKERFILE_GIT_PATH, self.DOCKERFILE_TREE_PATH): + path = box / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(dockerfile_body) + env = dict(os.environ, HOME=str(home)) + run = lambda *args: subprocess.run( + args, cwd=box, env=env, capture_output=True, text=True, timeout=30, + ) + run('git', 'init', '-q', '-b', 'main') + run('git', 'config', 'user.email', 'test@example.com') + run('git', 'config', 'user.name', 'test') + run('git', 'add', self.DOCKERFILE_GIT_PATH) + commit = run('git', 'commit', '-q', '-m', 'dockerfile') + assert commit.returncode == 0, commit.stderr + return home, env + + def _sh(self, snippet, env, cwd): + result = subprocess.run( + ['sh'], input=snippet, text=True, capture_output=True, + env=env, cwd=cwd, timeout=30, + ) + return result.stdout.strip() + + def test_ref_digest_equals_working_tree_digest(self, tmp_path): + home, env = self._fake_box(tmp_path, 'FROM python:3.12-slim\n') + working = self._sh(_build_hash_shell_cmd(), env, home / 'box') + at_ref = self._sh(_build_hash_at_ref_shell_cmd('HEAD'), env, home) + assert working, 'working-tree hasher produced nothing' + assert at_ref == working + + def test_ref_digest_differs_when_target_dockerfile_changes(self, tmp_path): + home, env = self._fake_box(tmp_path, 'FROM python:3.12-slim\n') + before = self._sh(_build_hash_at_ref_shell_cmd('HEAD'), env, home) + box = home / 'box' + (box / self.DOCKERFILE_GIT_PATH).write_text('FROM python:3.13-slim\n') + subprocess.run( + ['git', 'commit', '-qam', 'bump base'], cwd=box, env=env, + capture_output=True, text=True, timeout=30, + ) + after = self._sh(_build_hash_at_ref_shell_cmd('HEAD'), env, home) + # The JUL-4 case: working tree still matches the stored hash while the + # ref about to be checked out does not. + working = self._sh(_build_hash_shell_cmd(), env, box) + assert after != before + assert after != working + + def test_missing_ref_yields_empty_not_a_bogus_digest(self, tmp_path): + home, env = self._fake_box(tmp_path, 'FROM python:3.12-slim\n') + assert self._sh(_build_hash_at_ref_shell_cmd('no-such-ref'), env, home) == '' + class TestDockerBuildLineSummary: def test_buildkit_run_line(self): From 0b6af3cae442791a09c55f4e15c455228c252d3f Mon Sep 17 00:00:00 2001 From: Charles Wang Date: Thu, 13 Aug 2026 13:36:00 -0700 Subject: [PATCH 3/5] test(update): cover source-tree at-ref hash after main rebase Main's build-hash now walks ~/box/lager; keep the equivalence tests and preview assertions aligned so a Dockerfile-only at-ref hasher cannot regress silently. Co-authored-by: Cursor --- cli/tests/test_update_gate.py | 40 +++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/cli/tests/test_update_gate.py b/cli/tests/test_update_gate.py index f1e3c194..3ad323cc 100644 --- a/cli/tests/test_update_gate.py +++ b/cli/tests/test_update_gate.py @@ -246,7 +246,7 @@ def test_in_sync_uses_working_tree_hash(self): needs_pull=False, ) assert change is True - assert 'Dockerfile or requirements changed' in status + assert 'Dockerfile, requirements or box source changed' in status class TestBuildHashAtRefShellCmd: @@ -255,6 +255,13 @@ def test_emits_git_show_for_dockerfile_blob(self): assert 'git show origin/main:box/lager/docker/box.Dockerfile' in script assert 'git cat-file -e origin/main:box/lager/docker/box.Dockerfile' in script + def test_emits_source_tree_walk(self): + # Must match main's `_BUILD_HASH_SOURCE_DIRS` composition or --check + # spuriously reports a rebuild against stored hashes that include + # every file under ~/box/lager. + script = _build_hash_at_ref_shell_cmd('origin/main') + assert 'git ls-tree -r --name-only origin/main box/lager' in script + def test_rejects_metacharacters(self): assert _build_hash_at_ref_shell_cmd('main; rm -rf /') == 'echo ""' assert _build_hash_at_ref_shell_cmd('') == 'echo ""' @@ -282,16 +289,25 @@ class TestBuildHashAtRefMatchesWorkingTree: DOCKERFILE_GIT_PATH = 'box/lager/docker/box.Dockerfile' DOCKERFILE_TREE_PATH = 'lager/docker/box.Dockerfile' + SOURCE_GIT_PATH = 'box/lager/nets/net.py' + SOURCE_TREE_PATH = 'lager/nets/net.py' - def _fake_box(self, tmp_path, dockerfile_body): + def _fake_box(self, tmp_path, dockerfile_body, source_body='print("ok")\n'): """Build $HOME/box as the boxes have it: git tracks the `box/` prefix, - the working tree is the flattened layout.""" + the working tree is the flattened layout. Includes a source file so + the `_BUILD_HASH_SOURCE_DIRS` walk is exercised (not just Dockerfile). + """ home = tmp_path / 'home' box = home / 'box' - for rel in (self.DOCKERFILE_GIT_PATH, self.DOCKERFILE_TREE_PATH): + for rel, body in ( + (self.DOCKERFILE_GIT_PATH, dockerfile_body), + (self.DOCKERFILE_TREE_PATH, dockerfile_body), + (self.SOURCE_GIT_PATH, source_body), + (self.SOURCE_TREE_PATH, source_body), + ): path = box / rel path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(dockerfile_body) + path.write_text(body) env = dict(os.environ, HOME=str(home)) run = lambda *args: subprocess.run( args, cwd=box, env=env, capture_output=True, text=True, timeout=30, @@ -299,7 +315,7 @@ def _fake_box(self, tmp_path, dockerfile_body): run('git', 'init', '-q', '-b', 'main') run('git', 'config', 'user.email', 'test@example.com') run('git', 'config', 'user.name', 'test') - run('git', 'add', self.DOCKERFILE_GIT_PATH) + run('git', 'add', self.DOCKERFILE_GIT_PATH, self.SOURCE_GIT_PATH) commit = run('git', 'commit', '-q', '-m', 'dockerfile') assert commit.returncode == 0, commit.stderr return home, env @@ -334,6 +350,18 @@ def test_ref_digest_differs_when_target_dockerfile_changes(self, tmp_path): assert after != before assert after != working + def test_ref_digest_differs_when_source_file_changes(self, tmp_path): + home, env = self._fake_box(tmp_path, 'FROM python:3.12-slim\n') + before = self._sh(_build_hash_at_ref_shell_cmd('HEAD'), env, home) + box = home / 'box' + (box / self.SOURCE_GIT_PATH).write_text('print("changed")\n') + subprocess.run( + ['git', 'commit', '-qam', 'touch source'], cwd=box, env=env, + capture_output=True, text=True, timeout=30, + ) + after = self._sh(_build_hash_at_ref_shell_cmd('HEAD'), env, home) + assert after != before + def test_missing_ref_yields_empty_not_a_bogus_digest(self, tmp_path): home, env = self._fake_box(tmp_path, 'FROM python:3.12-slim\n') assert self._sh(_build_hash_at_ref_shell_cmd('no-such-ref'), env, home) == '' From 6865111946c332981d542839eb567cc25e9d8c94 Mon Sep 17 00:00:00 2001 From: DanielRMErskine Date: Fri, 14 Aug 2026 15:09:43 -0700 Subject: [PATCH 4/5] Reconcile the target-ref build hash with main's flatten-aware preview Two corrections to the same `lager update --check` estimate landed in the same week against the same code. main extracted `_deps_preview()`, giving it the `needs_flatten` rebuild trigger and a measured/unmeasurable distinction; the three commits below this one added target-ref build hashing so that a forward jump stops reporting `~90s (cached build)` off a pre-pull tree. Neither was written with knowledge of the other, and each shipped its own helper for the job. Keeping `_preview_deps_status` would have quietly reverted the flatten fix. It never reads `needs_flatten`, so a box still on the `box/` subdir layout that is also behind its target would go back to printing "cache valid" immediately before a ten-minute rebuild -- precisely the case the flatten fix was written for. `_deps_preview()` therefore stays as the single helper and gains `target_hash` and `needs_pull`. A successfully measured target ref now replaces the working tree as the basis for the whole preview, which also converts the old "unknown until pull (older ref may differ)" guess on rollbacks and branch switches into a measured answer. `needs_flatten` still forces a rebuild whatever that digest says, and a target that could not be measured reports unknown rather than falling back to the pre-pull tree and calling the cache valid. `_preview_deps_status` is removed; its four tests move onto `_deps_preview`, joined by a fifth that pins the flatten-beats-matching-target case this reconciliation could have regressed unnoticed. Also in this commit: - `test/COVERAGE.md` counts regenerated: unit (cli) 1258 -> 1274, total gated 3342 -> 3358. - The new Dockerfile comment claimed flex/bison/ccache/ninja were "build-only for uldaq (next stage)". Only flex and bison are, and they do move to that stage; nothing in this repo invokes ccache or ninja-build anywhere, so they are dropped outright rather than relocated. The comment now says that. - CHANGELOG entries for the target-ref hashing, the cold-build speedups, and the live build detail on the progress bar -- including the operational note that a box carrying globally-installed npm packages needs one `--force` update, because Node 18 -> 20 is an ABI break and only `--force` wipes the `lager-npm-global` volume that survives an ordinary rebuild. - Two test comments now describe the forward-jump condition they cover, which is what tells a reader when the case applies. --- CHANGELOG.md | 56 ++++++++++++++++++++++++ box/lager/docker/box.Dockerfile | 11 +++-- cli/commands/utility/update.py | 26 ----------- cli/tests/test_update_gate.py | 76 ++++++++++++++++++--------------- test/COVERAGE.md | 4 +- 5 files changed, 107 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98ec0b8c..c36f3a91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,38 @@ All notable changes to the Lager platform are documented here. For detailed rele and false of the file that grants `apt-get` one line above them. Treat anyone holding the box login account's SSH key as holding root on that box. +- **Cold box image builds spend less time installing things nothing uses.** + Three changes to `box.Dockerfile`, all of them about build time rather than + behavior: + + Node and npm now come from the official upstream tarball, verified against + its `SHASUMS256.txt` before extraction, instead of Debian's `nodejs npm` + meta-packages — which pull in roughly 400 `node-*` packages the box never + touches. `start_box.sh` needs npm only to install the packages named in + `box_config.npm_packages`, which the tarball provides. + + `cryptography` moves from 38.0.4 to 43.0.3. The old pin has no cp312 wheel, + so every cold pip layer compiled it from Rust source; 43.0.3 ships a + manylinux wheel for the image's Python. The BluFi cipher, its only consumer + in this tree, switches from `algorithms.AES128` to `algorithms.AES` — stable + across both versions and byte-identical for BluFi's fixed 16-byte key. + + `flex` and `bison` move into the uldaq layer, the only stage whose + `autoreconf` needs them. `ccache` and `ninja-build` are dropped outright: + nothing in this repo invokes either, and no build here was wired to use + them. + + **A box carrying globally-installed npm packages should be updated once with + `--force`.** Node's major version moves from 18 to 20, and the + `lager-npm-global` volume holding those packages survives an ordinary image + rebuild — only `--force` wipes it. Any package with a compiled native module + needs reinstalling under the new ABI. + +- **The update progress bar names what the container build is currently + doing** — `Building container... [pip install ...]` — instead of holding one + unchanging label for the several minutes a cold build takes. Parsed from + BuildKit's own step output; `--verbose` is unchanged. + ### Fixed - **Every successful `lager update` that rebuilt the container warned that its @@ -117,6 +149,30 @@ All notable changes to the Lager platform are documented here. For detailed rele gate would rebuild. `--check`'s exit code now accounts for a pending flatten too, so a box needing one no longer reports `Nothing to do`. +- **`lager update --check` still promised a cached build when the ref it was + about to check out changed the image recipe.** The companion to the case + above, and the more common one. The probe measured the Dockerfile, + requirements and box source in the box's *current* working tree — which on a + box a long way behind its target still matched `/etc/lager/build-hash` + exactly. The preview printed `Estimated: ~90s (cached build)`; the pull then + landed a different Dockerfile and the update took the full six minutes. + + `--check` now reads those same build inputs at the target ref, straight out + of the box's git object database via `git cat-file` / `git show` — no + checkout, no mutation, and one extra SSH round-trip on the `--check` path + only. The snippet is composed to emit a byte-identical digest to the + working-tree hasher for an identical tree, so the two are comparable by + construction rather than by coincidence; tests execute both under `sh` + against a fixture repo and assert the digests agree. + + When the target ref can be measured it replaces the working tree as the + basis for the whole preview, which also turns the old + `unknown until pull (older ref may differ)` guess on rollbacks and branch + switches into a measured answer. When it cannot be measured — a sparse + checkout, an odd ref — the preview says so rather than falling back to the + pre-pull tree and calling the cache valid. A pending flatten still forces a + rebuild whatever the target digest says. + - **The sudoers bootstrap snippet printed by `lager box-config mount` wrote a strict subset of the file it was overwriting.** It teed a single `NOPASSWD: /bin/mkdir, /bin/chown` line over `/etc/sudoers.d/lager-box-config`, diff --git a/box/lager/docker/box.Dockerfile b/box/lager/docker/box.Dockerfile index b942e8af..0a36d031 100644 --- a/box/lager/docker/box.Dockerfile +++ b/box/lager/docker/box.Dockerfile @@ -5,10 +5,13 @@ ENV PYTHONDONTWRITEBYTECODE=1 ENV PYTHONUNBUFFERED=1 ENV PYTHONPATH=/app/lager -# flex/bison/ccache/ninja are NOT here: they are build-only for uldaq (next -# stage) and would bloat every image. Node is installed from the official -# tarball below — Debian's `nodejs npm` meta-packages pull ~400 unused -# `node-*` packages and dominate cold-build time. +# flex/bison are not here: they moved to the uldaq stage below, the only +# place that needs them (autoreconf). ccache/ninja-build are dropped +# outright -- nothing in this repo invokes either and no build here was +# wired to use them, so they were pure image weight; a container script +# that wants them has to install them itself. Node comes from the official +# tarball below rather than Debian's `nodejs npm` meta-packages, which pull +# ~400 unused `node-*` packages and dominate cold-build time. RUN apt-get update && apt-get install -y ca-certificates libusb-1.0-0-dev libudev-dev \ libhidapi-dev git gcc python-dev-is-python3 python3-pip python3-venv xz-utils build-essential bluetooth ssh openssh-client \ cups-client lpr zlib1g-dev wget libjpeg-dev libpng-dev libfreetype6-dev \ diff --git a/cli/commands/utility/update.py b/cli/commands/utility/update.py index f27c4514..b30e7ce5 100644 --- a/cli/commands/utility/update.py +++ b/cli/commands/utility/update.py @@ -354,32 +354,6 @@ def _read_build_hash_at_ref(ssh_runner, git_ref): return r.stdout.strip() if r.returncode == 0 else '' -def _preview_deps_status(*, force, stored_hash, working_hash, target_hash, needs_pull): - """Classify Dockerfile/requirements/source drift for ``--check`` preview. - - Returns ``(deps_will_change, deps_status)``. When ``needs_pull``, prefer - ``target_hash`` (blob hash at the ref about to be checked out) over the - working-tree hash — otherwise a forward jump whose *current* tree still - matches the stored hash falsely reports "~90s (cached build)". - """ - if force: - return True, 'forced clean rebuild (--force: image + cargo/npm volumes wiped)' - if needs_pull: - if target_hash: - if _build_hash_mismatch(target_hash, stored_hash): - return True, ( - 'will trigger fresh build ' - '(target Dockerfile, requirements or box source differ)' - ) - return False, 'cache valid (target matches last build)' - # Could not measure the target (sparse checkout, odd ref, …). Be - # honest rather than claiming cache-valid from the pre-pull tree. - return True, 'unknown until pull (could not hash target build inputs)' - if _build_hash_mismatch(working_hash, stored_hash): - return True, 'will trigger fresh build (Dockerfile, requirements or box source changed)' - return False, 'cache valid (no rebuild)' - - def _read_box_source_version(ssh_runner): """Return the `__version__` string declared in `cli/__init__.py` at the box's current HEAD, or empty. diff --git a/cli/tests/test_update_gate.py b/cli/tests/test_update_gate.py index 3ad323cc..1576181b 100644 --- a/cli/tests/test_update_gate.py +++ b/cli/tests/test_update_gate.py @@ -19,7 +19,7 @@ _deployed_version_stale, _docker_build_line_summary, _parse_probe_output, - _preview_deps_status, + _deps_preview, _probe_shell_script, _pull_shell_script, _rebuild_gate_verdict, @@ -196,58 +196,66 @@ def test_old_probe_output_leaves_host_keys_absent(self): assert 'HOST_CLI_VERSION' not in facts -class TestPreviewDepsStatus: +class TestDepsPreviewAtTargetRef: """`--check` must hash the *target* ref when a pull is pending. - Repro (JUL-4): 121 commits behind, pre-pull Dockerfile still matched the - stored hash → old code said "cache valid / ~90s", then the pull changed - the Dockerfile and the real update took ~6 min. + Field repro: a box ~120 commits behind whose pre-pull Dockerfile still + matched the stored hash. The preview read that as "cache valid / ~90s"; + the pull then changed the Dockerfile and the real update took ~6 min. """ + # Everything that is not about the target ref, held constant. + BASE = dict(force=False, needs_flatten=False, is_rollback=False, + commits_ahead=0) + def test_forward_jump_target_mismatch_reports_fresh_build(self): - change, status = _preview_deps_status( - force=False, - stored_hash=SHA_A, - working_hash=SHA_A, # pre-pull tree still matches - target_hash=SHA_B, # origin/main Dockerfile differs + status, rebuild_certain, _ = _deps_preview( + SHA_A, SHA_A, # pre-pull tree still matches stored + target_hash=SHA_B, # ref about to be checked out differs needs_pull=True, + **self.BASE, ) - assert change is True + assert rebuild_certain is True assert 'target Dockerfile' in status def test_forward_jump_target_match_reports_cache_valid(self): - change, status = _preview_deps_status( - force=False, - stored_hash=SHA_A, - working_hash=SHA_A, - target_hash=SHA_A, - needs_pull=True, + status, rebuild_certain, _ = _deps_preview( + SHA_A, SHA_A, target_hash=SHA_A, needs_pull=True, **self.BASE, ) - assert change is False + assert rebuild_certain is False assert 'target matches' in status def test_unmeasurable_target_is_unknown_not_cache_valid(self): - change, status = _preview_deps_status( - force=False, - stored_hash=SHA_A, - working_hash=SHA_A, - target_hash='', - needs_pull=True, + status, _, _ = _deps_preview( + SHA_A, SHA_A, target_hash='', needs_pull=True, **self.BASE, ) - assert change is True assert 'unknown until pull' in status + assert 'cache valid' not in status def test_in_sync_uses_working_tree_hash(self): - change, status = _preview_deps_status( - force=False, - stored_hash=SHA_A, - working_hash=SHA_B, - target_hash='', - needs_pull=False, + status, rebuild_certain, _ = _deps_preview( + SHA_B, SHA_A, target_hash='', needs_pull=False, **self.BASE, ) - assert change is True + assert rebuild_certain is True assert 'Dockerfile, requirements or box source changed' in status + def test_flatten_still_wins_over_a_matching_target_hash(self): + # Regression guard for the reconciliation of this change with the + # flatten fix. A box on the old `box/` subdir layout rebuilds + # whatever the target digest says, because the flatten moves every + # source path. Reading the target ref must not resurrect the "cache + # valid" claim that `needs_flatten` exists to prevent. + status, rebuild_certain, _ = _deps_preview( + SHA_A, SHA_A, + target_hash=SHA_A, + needs_pull=True, + force=False, needs_flatten=True, is_rollback=False, + commits_ahead=0, + ) + assert rebuild_certain is True + assert 'cache valid' not in status + assert 'flatten' in status + class TestBuildHashAtRefShellCmd: def test_emits_git_show_for_dockerfile_blob(self): @@ -344,8 +352,8 @@ def test_ref_digest_differs_when_target_dockerfile_changes(self, tmp_path): capture_output=True, text=True, timeout=30, ) after = self._sh(_build_hash_at_ref_shell_cmd('HEAD'), env, home) - # The JUL-4 case: working tree still matches the stored hash while the - # ref about to be checked out does not. + # The forward-jump case: working tree still matches the stored hash + # while the ref about to be checked out does not. working = self._sh(_build_hash_shell_cmd(), env, box) assert after != before assert after != working diff --git a/test/COVERAGE.md b/test/COVERAGE.md index 7e9734f4..51ab061a 100644 --- a/test/COVERAGE.md +++ b/test/COVERAGE.md @@ -36,13 +36,13 @@ are not. | Job (status context) | Path | Tests | |---|---|---:| -| `unit (cli)` | `test/unit/cli/` + `cli/tests/` | 1258 (+2 xfailed) | +| `unit (cli)` | `test/unit/cli/` + `cli/tests/` | 1274 (+2 xfailed) | | `unit (box)` | `test/unit/box/` | 1603 | | `unit (measurement)` | `test/unit/measurement/` | 105 | | `unit (blufi)` | `test/unit/blufi/` | 89 | | `unit (mcp)` | `test/mcp/unit/` | 166 | | `unit (root)` | `test/unit/test_*.py`, `test/test_*.py` | 121 (+1 skipped) | -| | **Total gated** | **3342** | +| | **Total gated** | **3358** | Each suite gets its own job because they need incompatible `sys.modules` states for the name `lager`: `test/unit/measurement/conftest.py` registers a placeholder whose `__init__` never runs From 3748404dc1f462e683d7da99f56542877e18de28 Mon Sep 17 00:00:00 2001 From: DanielRMErskine Date: Fri, 14 Aug 2026 15:57:57 -0700 Subject: [PATCH 5/5] Say why the rollback estimate stays pessimistic The comment claimed the build digest covers the image recipe but not every COPY layer. It covers both: the hash walks all of ~/box/lager, and every COPY source in box.Dockerfile resolves inside that tree. The floor is still correct, for a different reason. A matching digest proves the build inputs are unchanged, not that Docker still holds the layers built from them -- a `docker builder prune` between updates rebuilds everything with the digest identical. --- cli/commands/utility/update.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cli/commands/utility/update.py b/cli/commands/utility/update.py index b30e7ce5..baf0eee1 100644 --- a/cli/commands/utility/update.py +++ b/cli/commands/utility/update.py @@ -1650,9 +1650,12 @@ def _parse_fetch_result(result): # in the Dockerfile cache; assume a real build. An unmeasured hash # lands here too — over-estimating a cached build costs the # operator nothing, while under-estimating a fresh one is how - # "~90s" preceded ten minutes of waiting. Kept as a floor even - # now that a measured `target_hash` can clear `rebuild_certain`: - # the digest covers the image recipe, not every COPY layer. + # "~90s" preceded ten minutes of waiting. Deliberately kept as a + # floor even when a measured `target_hash` cleared + # `rebuild_certain`: the digest proves the build *inputs* match, + # not that Docker still holds the layers built from them — a + # `docker builder prune` between updates rebuilds everything with + # the digest unchanged. container_status = 'will restart' est = '~6 min (fresh build possible)' else: