From 62f3b05fb310623b4043ea9fc30691068e90e93d Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Fri, 22 May 2026 22:12:38 +0200 Subject: [PATCH 1/3] feat: avoid stale cache --- src/BUILD | 12 ++++ src/incremental.py | 54 +++++++++++++++- src/incremental_dirty_build_test.py | 95 +++++++++++++++++++++++++++++ src/requirements.in | 1 + src/requirements.txt | 4 ++ 5 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 src/incremental_dirty_build_test.py diff --git a/src/BUILD b/src/BUILD index f12df2fa2..b4bf27c14 100644 --- a/src/BUILD +++ b/src/BUILD @@ -12,8 +12,10 @@ # ******************************************************************************* load("@aspect_rules_py//py:defs.bzl", "py_library") +load("@docs_as_code_hub_env//:requirements.bzl", "all_requirements") load("@rules_java//java:java_binary.bzl", "java_binary") load("@rules_python//python:pip.bzl", "compile_pip_requirements") +load("//:score_pytest.bzl", "score_pytest") # These are only exported because they're passed as files to the //docs.bzl # macros, and thus must be visible to other packages. They should only be @@ -30,6 +32,16 @@ exports_files( visibility = ["//visibility:public"], ) +score_pytest( + name = "incremental_dirty_build_test", + srcs = [ + "incremental_dirty_build_test.py", + "incremental.py", + ], + deps = all_requirements, + pytest_config = "//:pyproject.toml", +) + filegroup( name = "all_sources", srcs = glob( diff --git a/src/incremental.py b/src/incremental.py index 92449a24b..749ddc7dd 100644 --- a/src/incremental.py +++ b/src/incremental.py @@ -12,8 +12,10 @@ # ******************************************************************************* import argparse +import hashlib import logging import os +import shutil import sys import time from pathlib import Path @@ -26,6 +28,8 @@ logger = logging.getLogger(__name__) +_MODULE_HASH_FILE = ".module_bazel_hash" + def get_env(name: str) -> str: val = os.environ.get(name, None) @@ -35,6 +39,34 @@ def get_env(name: str) -> str: return val +def _compute_hash(files: list[Path]) -> str: + h = hashlib.sha256() + for f in sorted(files, key=str): + h.update(f.read_bytes()) + return h.hexdigest() + + +def clean_builddir_if_stale(build_dir: Path, sentinel_files: list[Path]) -> None: + """Delete build_dir if the previous build had warnings or any sentinel file changed.""" + warnings_txt = build_dir / "warnings.txt" + has_warnings = warnings_txt.exists() and warnings_txt.stat().st_size > 0 + + hash_file = build_dir / _MODULE_HASH_FILE + hash_changed = ( + hash_file.exists() + and hash_file.read_text().strip() != _compute_hash(sentinel_files) + ) + + if has_warnings: + print("Previous build had warnings. Removing _build to ensure a clean build.") + if has_warnings or hash_changed: + shutil.rmtree(build_dir) + + +def update_module_hash(build_dir: Path, sentinel_files: list[Path]) -> None: + (build_dir / _MODULE_HASH_FILE).write_text(_compute_hash(sentinel_files)) + + if __name__ == "__main__": parser = argparse.ArgumentParser() # Add debuging functionality @@ -73,9 +105,21 @@ def get_env(name: str) -> str: else: workspace = "" + build_dir = Path(workspace + "_build") + sentinel_files = [ + Path(workspace + "MODULE.bazel"), + Path(workspace + "MODULE.bazel.lock"), + Path(workspace + "BUILD"), + ] + clean_builddir_if_stale(build_dir, sentinel_files) + + warning_file = Path(workspace + "_build/warnings.txt") + base_arguments = [ workspace + get_env("SOURCE_DIRECTORY"), workspace + "_build", + "--warning-file", + str(warning_file), "-W", # treat warning as errors "--keep-going", # do not abort after one error "-T", # show details in case of errors in extensions @@ -134,7 +178,13 @@ def get_env(name: str) -> str: start_time = time.perf_counter() exit_code = sphinx_main(base_arguments) end_time = time.perf_counter() - duration = end_time - start_time - print(f"docs ({action}) finished in {duration:.1f} seconds") + print(f"docs ({action}) finished in {end_time - start_time:.1f} seconds") + + if exit_code == 0: + update_module_hash(build_dir, sentinel_files) + else: + with warning_file.open("a", encoding="utf-8") as f: + f.write("-" * 80 + "\n") + f.write(f"Build failed with exit code {exit_code}\n") sys.exit(exit_code) diff --git a/src/incremental_dirty_build_test.py b/src/incremental_dirty_build_test.py new file mode 100644 index 000000000..8d0bd33bb --- /dev/null +++ b/src/incremental_dirty_build_test.py @@ -0,0 +1,95 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +# Unit Tests of incremental.py + +from pathlib import Path + +from incremental import clean_builddir_if_stale, update_module_hash + +_BUILD = Path("/build") +_MODULE = Path("/MODULE.bazel") +_LOCK = Path("/MODULE.bazel.lock") + + +def _simulate_old_state(fs, warnings: str | None) -> None: + """Helper function to set up a build directory with an old hash and warnings.""" + + fs.create_dir(_BUILD) + fs.create_file(_MODULE, contents="stable") + fs.create_file(_LOCK, contents="old lock") + update_module_hash(_BUILD, [_MODULE, _LOCK]) + if warnings is not None: + fs.create_file(_BUILD / "warnings.txt", contents=warnings) + + +def test_clean_removes_build_dir_when_previous_build_had_warnings(fs) -> None: + """If warnings.txt exists and is not empty, the build dir is removed.""" + + _simulate_old_state(fs, warnings="WARNING: something went wrong") + + clean_builddir_if_stale(_BUILD, [_MODULE]) + + assert not _BUILD.exists() + + +def test_clean_keeps_build_dir_when_warnings_txt_is_empty(fs) -> None: + """If warnings.txt exists and is empty, the build dir is kept.""" + + _simulate_old_state(fs, warnings="") + + clean_builddir_if_stale(_BUILD, [_MODULE, _LOCK]) + + assert _BUILD.exists() + + +def test_clean_is_noop_when_warnings_txt_is_absent(fs) -> None: + """If warnings.txt does not exist, the build dir is kept (no error).""" + + _simulate_old_state(fs, warnings=None) + + clean_builddir_if_stale(_BUILD, [_MODULE, _LOCK]) + + assert _BUILD.exists() + + +def test_clean_is_noop_when_build_dir_is_absent(fs) -> None: + fs.create_file(_MODULE, contents="stable") + + clean_builddir_if_stale(_BUILD, [_MODULE]) + + +def test_module_changed_removes_build_dir_when_one_sentinel_file_changed(fs) -> None: + _simulate_old_state(fs, warnings=None) + + _LOCK.write_bytes(b"new lock") + clean_builddir_if_stale(_BUILD, [_MODULE, _LOCK]) + + assert not _BUILD.exists() + + +def test_module_changed_keeps_build_dir_when_all_sentinel_files_unchanged(fs) -> None: + _simulate_old_state(fs, warnings=None) + + clean_builddir_if_stale(_BUILD, [_MODULE, _LOCK]) + + assert _BUILD.exists() + + +def test_module_change_after_successful_build_forces_clean(fs) -> None: + _simulate_old_state(fs, warnings=None) + + _MODULE.write_bytes(b"version 2") + clean_builddir_if_stale(_BUILD, [_MODULE]) + + assert not _BUILD.exists() diff --git a/src/requirements.in b/src/requirements.in index aff2c3d05..04317ea12 100644 --- a/src/requirements.in +++ b/src/requirements.in @@ -35,4 +35,5 @@ bazel-runfiles # Local development pytest +pyfakefs basedpyright diff --git a/src/requirements.txt b/src/requirements.txt index 6c0b03a13..0219d853c 100644 --- a/src/requirements.txt +++ b/src/requirements.txt @@ -1062,6 +1062,10 @@ pydata-sphinx-theme==0.17.0 \ --hash=sha256:529c5631582cb3328cf4814fb9eb80611d1704c854406d282a75c9c86e3a1955 \ --hash=sha256:cec5c92f41f4a11541b6df8210c446b4aa9c3badb7fcf2db7893405b786d5c99 # via -r requirements.in +pyfakefs==6.2.0 \ + --hash=sha256:0968a49db692694ffed420e54a9f1cbae4636637b880e8ab09c8ccc0f11bd7ae \ + --hash=sha256:e59a36db447bf509ce9c97ab3d1510c08cc51895c5311325a560a5e5b5dc1940 + # via -r requirements.in pygithub==2.9.0 \ --hash=sha256:5e2b260ce327bffce9b00f447b65953ef7078ffe93e5a5425624a3075483927c \ --hash=sha256:a26abda1222febba31238682634cad11d8b966137ed6cc3c5e445b29a11cb0a4 From f4fd9fbf9d008e9619ba6b1abc34626fe737d29b Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Tue, 26 May 2026 10:25:48 +0200 Subject: [PATCH 2/3] improve print MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maximilian Sören Pollak Signed-off-by: Alexander Lanin --- src/incremental.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/incremental.py b/src/incremental.py index 749ddc7dd..e31328b8b 100644 --- a/src/incremental.py +++ b/src/incremental.py @@ -57,9 +57,8 @@ def clean_builddir_if_stale(build_dir: Path, sentinel_files: list[Path]) -> None and hash_file.read_text().strip() != _compute_hash(sentinel_files) ) - if has_warnings: - print("Previous build had warnings. Removing _build to ensure a clean build.") if has_warnings or hash_changed: + print("Previous build had warnings or the hash changed. Removing _build to ensure a clean build.") shutil.rmtree(build_dir) From f727650d0126198fee517141bd3370d58c061ca9 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Tue, 26 May 2026 10:41:40 +0200 Subject: [PATCH 3/3] fix: trigger clean build when hash file is missing If _build/ exists but .module_bazel_hash is absent (e.g. after upgrading from a version without hash tracking), the old behavior silently skipped the stale check and ran an incremental build against unknown state. Treat missing hash file as stale so the build dir is always cleaned in that case. Also add an early return for absent build_dir, which makes the intent explicit and avoids relying on warnings.txt not existing as an implicit guard against rmtree on a non-existent path. --- src/incremental.py | 11 ++++++++--- src/incremental_dirty_build_test.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/incremental.py b/src/incremental.py index e31328b8b..6a23e3905 100644 --- a/src/incremental.py +++ b/src/incremental.py @@ -48,17 +48,22 @@ def _compute_hash(files: list[Path]) -> str: def clean_builddir_if_stale(build_dir: Path, sentinel_files: list[Path]) -> None: """Delete build_dir if the previous build had warnings or any sentinel file changed.""" + if not build_dir.exists(): + return + warnings_txt = build_dir / "warnings.txt" has_warnings = warnings_txt.exists() and warnings_txt.stat().st_size > 0 hash_file = build_dir / _MODULE_HASH_FILE hash_changed = ( - hash_file.exists() - and hash_file.read_text().strip() != _compute_hash(sentinel_files) + not hash_file.exists() + or hash_file.read_text().strip() != _compute_hash(sentinel_files) ) if has_warnings or hash_changed: - print("Previous build had warnings or the hash changed. Removing _build to ensure a clean build.") + print( + "Previous build had warnings or the hash changed. Removing _build to ensure a clean build." + ) shutil.rmtree(build_dir) diff --git a/src/incremental_dirty_build_test.py b/src/incremental_dirty_build_test.py index 8d0bd33bb..2169a8765 100644 --- a/src/incremental_dirty_build_test.py +++ b/src/incremental_dirty_build_test.py @@ -93,3 +93,14 @@ def test_module_change_after_successful_build_forces_clean(fs) -> None: clean_builddir_if_stale(_BUILD, [_MODULE]) assert not _BUILD.exists() + + +def test_missing_hash_file_triggers_clean(fs) -> None: + """If _build/ exists but hash file is absent, treat as stale (e.g. upgrade from old version).""" + fs.create_dir(_BUILD) + fs.create_file(_MODULE, contents="stable") + # No hash file written + + clean_builddir_if_stale(_BUILD, [_MODULE]) + + assert not _BUILD.exists()