From aae6540ae5dfff9dbe269d547f56a88822beaa75 Mon Sep 17 00:00:00 2001 From: akritkbehera Date: Thu, 8 Oct 2026 07:36:58 +0200 Subject: [PATCH 1/2] fix(hashing): align build, plan, and status package identities Try remote reuse before falling back to the local hash for read-only hash-policy builds. Use that hash for revision labels and store paths, and recognize existing local installs on subsequent runs. Make plan and status follow the same selection rules, including writable stores, explicit revisions, and virtual packages. Resolve status source URLs, tags, versions, patches, variables, and recipe text before hashing so it looks up the same artifacts as build. Include regression coverage, hash lookup diagnostics, and documentation of the remote-first local fallback. --- bits_helpers/args.py | 5 + bits_helpers/build.py | 265 +++++++++++++++++++--------------- bits_helpers/hashing.py | 20 ++- bits_helpers/plan.py | 16 +- bits_helpers/status.py | 161 ++++++++++++++++++--- docs/REFERENCE.md | 8 +- tests/test_args.py | 1 + tests/test_build.py | 126 +++++++++++++++- tests/test_plan.py | 23 +++ tests/test_revision_policy.py | 54 +++++++ tests/test_status.py | 97 ++++++++++++- 11 files changed, 627 insertions(+), 149 deletions(-) diff --git a/bits_helpers/args.py b/bits_helpers/args.py index daa3125c..de1e697a 100644 --- a/bits_helpers/args.py +++ b/bits_helpers/args.py @@ -772,6 +772,11 @@ def add_status_arguments(subparsers, ctx): "--remote-store", dest="remoteStore", metavar="STORE", default="", help="Remote binary store URL. Used only when --check-store is given.", ) + status_parser.add_argument( + "--write-store", dest="writeStore", metavar="STORE", default="", + help=("Writable binary store URL. Used to predict whether a hash-policy " + "package will use its remote or local hash."), + ) status_parser.add_argument( "--no-remote-store", dest="no_remote_store", action="store_true", default=False, help="Disable any remote store (even if a default is configured).", diff --git a/bits_helpers/build.py b/bits_helpers/build.py index 2b043377..307daea2 100644 --- a/bits_helpers/build.py +++ b/bits_helpers/build.py @@ -1965,8 +1965,8 @@ def build_one_package(p, ctx): "this version coexist the convenience symlink will be silently " "overwritten.", spec["package"], spec["package"], spec["version"], ) - # Hash was already computed; align spec["hash"] to the remote store - # (forced revisions are never prefixed with "local"). + # Keep the remote hash for lookup and reuse. A policy-injected label can + # fall back to the local hash after the remote reuse attempt below. spec["hash"] = spec["remote_revision_hash"] else: # Normal revision-counter logic: scan existing symlinks and find the @@ -2205,126 +2205,135 @@ def build_one_package(p, ctx): # dangling symlinks — e.g. created by a previous interrupted run that # wrote fetch_symlinks() entries before the actual tarball existed — # are NOT mistaken for a successfully installed package. - if os.path.islink(hashPath) and os.path.isdir(hashPath): - fileHash = spec["hash"] - else: - fileHash = readHashFile(hashFile) # Development packages have their own rebuild-detection logic above. # spec["hash"] is only useful here for regular packages. - if fileHash == spec["hash"] and not spec["is_devel_pkg"]: - # If we get here, we know we are in sync with whatever remote store. We - # can therefore create a directory which contains all the packages which - # were used to compile this one. - debug("Package %s was correctly compiled. Moving to next one.", spec["package"]) - # own_hash arch bridge (reuse compatibility): an own_hash package (the - # toolchain) is installed ONLY under its build-type-NEUTRAL arch. A consumer - # whose init.sh was generated by the current initdotsh sources it from that - # neutral arch, but a consumer — or the own_hash package's own init.sh — - # reused from an OLDER store tarball carries a baked path under the build arch - # ($BITS_ARCH_PREFIX). Materialise a build-arch -> neutral symlink so those - # older references resolve, instead of forcing a toolchain rebuild. Only when - # the effective (neutral) arch actually differs from the build arch. - _bridge_eff = effective_arch(spec, args.architecture) - if spec.get("own_hash") and _bridge_eff != args.architecture: - _bridge_neutral = _pkg_install_path(workDir, _bridge_eff, spec) - _bridge_build = _pkg_install_path(workDir, args.architecture, spec) - if os.path.isdir(_bridge_neutral) and not os.path.exists(_bridge_build): + def _handle_already_installed(hashPath): + hashFile = hashPath + "/.build-hash" + if os.path.islink(hashPath) and os.path.isdir(hashPath): + fileHash = spec["hash"] + else: + fileHash = readHashFile(hashFile) + if fileHash == spec["hash"] and not spec["is_devel_pkg"]: + # If we get here, the installed package matches the selected hash. It is + # already usable, so we can register its dependency closure and skip work. + debug("Package %s is already installed with the expected hash.", spec["package"]) + # own_hash arch bridge (reuse compatibility): an own_hash package (the + # toolchain) is installed ONLY under its build-type-NEUTRAL arch. A consumer + # whose init.sh was generated by the current initdotsh sources it from that + # neutral arch, but a consumer — or the own_hash package's own init.sh — + # reused from an OLDER store tarball carries a baked path under the build arch + # ($BITS_ARCH_PREFIX). Materialise a build-arch -> neutral symlink so those + # older references resolve, instead of forcing a toolchain rebuild. Only when + # the effective (neutral) arch actually differs from the build arch. + _bridge_eff = effective_arch(spec, args.architecture) + if spec.get("own_hash") and _bridge_eff != args.architecture: + _bridge_neutral = _pkg_install_path(workDir, _bridge_eff, spec) + _bridge_build = _pkg_install_path(workDir, args.architecture, spec) + if os.path.isdir(_bridge_neutral) and not os.path.exists(_bridge_build): + try: + os.makedirs(os.path.dirname(_bridge_build), exist_ok=True) + symlink(os.path.relpath(_bridge_neutral, os.path.dirname(_bridge_build)), + _bridge_build) + debug("own_hash arch bridge: %s -> %s", _bridge_build, _bridge_neutral) + except OSError as _bridge_exc: + debug("own_hash arch bridge symlink failed (%s)", _bridge_exc) + # If using incremental builds, next time we execute the script we need to remove + # the placeholders which avoid rebuilds. + if spec["is_devel_pkg"] and "incremental_recipe" in spec: + unlink(hashFile) + if "obsolete_tarball" in spec: + unlink(realpath(spec["obsolete_tarball"])) + unlink(spec["obsolete_tarball"]) + # We can now delete the INSTALLROOT and BUILD directories, + # assuming the package is not a development one. We also can + # delete the SOURCES in case we have aggressive-cleanup enabled. + if not spec["is_devel_pkg"] and args.autoCleanup: + cleanupDirs = [buildRoot, + join(workDir, "INSTALLROOT", spec["hash"])] + if args.aggressiveCleanup: + cleanupDirs.append(join(workDir, "SOURCES", spec["package"])) + debug("Cleaning up:\n%s", "\n".join(cleanupDirs)) + + for d in cleanupDirs: + shutil.rmtree(d.encode("utf8"), True) try: - os.makedirs(os.path.dirname(_bridge_build), exist_ok=True) - symlink(os.path.relpath(_bridge_neutral, os.path.dirname(_bridge_build)), - _bridge_build) - debug("own_hash arch bridge: %s -> %s", _bridge_build, _bridge_neutral) - except OSError as _bridge_exc: - debug("own_hash arch bridge symlink failed (%s)", _bridge_exc) - # If using incremental builds, next time we execute the script we need to remove - # the placeholders which avoid rebuilds. - if spec["is_devel_pkg"] and "incremental_recipe" in spec: - unlink(hashFile) - if "obsolete_tarball" in spec: - unlink(realpath(spec["obsolete_tarball"])) - unlink(spec["obsolete_tarball"]) - # We can now delete the INSTALLROOT and BUILD directories, - # assuming the package is not a development one. We also can - # delete the SOURCES in case we have aggressive-cleanup enabled. - if not spec["is_devel_pkg"] and args.autoCleanup: - cleanupDirs = [buildRoot, - join(workDir, "INSTALLROOT", spec["hash"])] - if args.aggressiveCleanup: - cleanupDirs.append(join(workDir, "SOURCES", spec["package"])) - debug("Cleaning up:\n%s", "\n".join(cleanupDirs)) - - for d in cleanupDirs: - shutil.rmtree(d.encode("utf8"), True) - try: - unlink(join(buildWorkDir, "BUILD", spec["package"] + "-latest")) - if "develPrefix" in args: - unlink(join(buildWorkDir, "BUILD", spec["package"] + "-latest-" + args.develPrefix)) - except Exception: - pass + unlink(join(buildWorkDir, "BUILD", spec["package"] + "-latest")) + if "develPrefix" in args: + unlink(join(buildWorkDir, "BUILD", spec["package"] + "-latest-" + args.develPrefix)) + except Exception: + pass + try: + rmdir(join(buildWorkDir, "BUILD")) + rmdir(join(workDir, "INSTALLROOT")) + except Exception: + pass + # The install dir is present at the right hash, but that does NOT prove the + # content tarball is in the write store: a store wipe (or the first publish + # from a node whose local cache stayed warm) leaves a package installed + # locally yet absent from S3. Don't assume "installed => published" — when a + # write store is configured and the local content tarball is present, push + # it. upload_symlinks_and_tarball HEAD-skips when the object already exists, + # so this is a cheap no-op in the common case. Guards mirror doFinalSync + # (skip local revisions, repository packages, non-redistributable binaries). + from bits_helpers.sync import binary_redistributable + if getattr(syncHelper, "writeStore", "") \ + and not spec["revision"].startswith("local") \ + and not spec.get("provides_repository") \ + and binary_redistributable(spec): + _eff_arch = effective_arch(spec, args.architecture) + _tarname = "%s-%s.%s.tar.gz" % (spec["package"], ver_rev(spec), _eff_arch) + _local_tar = os.path.join(workDir, resolve_store_path(_eff_arch, spec["hash"]), _tarname) + # Gate on the WRITE-store object itself (a HEAD), never the manifest or the + # read store: after a store wipe the package is installed locally but the + # S3 object is gone. + # The existence HEAD is itself best-effort: _s3_key_exists re-raises any + # non-404 (403 expired creds, 503 SlowDown), and a connection timeout is + # not even a ClientError — so an un-guarded call here would propagate and + # fail an ALREADY-INSTALLED (reused) package, which has an empty BUILD dir + # and no log. Under a -j build doing one HEAD per reused package a transient + # throttle/timeout is near-certain, so we must never let it fail the package: + # on error we fall through and try the (idempotent) publish anyway. + _in_write_store = False + try: + _in_write_store = syncHelper.writestore_has_tarball(spec, args.architecture) + except Exception as exc: + warning("%s@%s write-store check failed (%s); will try to publish anyway", + spec["package"], spec["version"], exc) + if _in_write_store: + debug("%s@%s already in the write store; nothing to publish.", + spec["package"], spec["version"]) + elif os.path.isfile(_local_tar): + # Absent from the store but present locally: publish it. Best-effort — a + # store hiccup (expired creds / transient network) must NOT fail an + # otherwise-complete package, unlike a freshly built one in doFinalSync. + try: + syncHelper.upload_symlinks_and_tarball(spec) + info("%s@%s [uploaded from local cache]", spec["package"], spec["version"]) + except Exception as exc: + warning("%s@%s store sync failed: %s", spec["package"], spec["version"], exc) + else: + warning("%s@%s installed locally but absent from the store and no local " + "tarball — rebuild to publish it.", spec["package"], spec["version"]) + # Record in the build manifest that this package was already installed. + if getattr(args, "manifest", None) is not None: + args.manifest.add_package(spec, "already_installed", + effective_architecture=effective_arch(spec, args.architecture)) + # Touch the sentinel so the cleanup command knows this package was used. try: - rmdir(join(buildWorkDir, "BUILD")) - rmdir(join(workDir, "INSTALLROOT")) + from bits_helpers.cleanup import touch_sentinel as _touch_sentinel + _touch_sentinel(workDir, args.architecture, spec["package"], ver_rev(spec)) except Exception: pass - # The install dir is present at the right hash, but that does NOT prove the - # content tarball is in the write store: a store wipe (or the first publish - # from a node whose local cache stayed warm) leaves a package installed - # locally yet absent from S3. Don't assume "installed => published" — when a - # write store is configured and the local content tarball is present, push - # it. upload_symlinks_and_tarball HEAD-skips when the object already exists, - # so this is a cheap no-op in the common case. Guards mirror doFinalSync - # (skip local revisions, repository packages, non-redistributable binaries). - from bits_helpers.sync import binary_redistributable - if getattr(syncHelper, "writeStore", "") \ - and not spec["revision"].startswith("local") \ - and not spec.get("provides_repository") \ - and binary_redistributable(spec): - _eff_arch = effective_arch(spec, args.architecture) - _tarname = "%s-%s.%s.tar.gz" % (spec["package"], ver_rev(spec), _eff_arch) - _local_tar = os.path.join(workDir, resolve_store_path(_eff_arch, spec["hash"]), _tarname) - # Gate on the WRITE-store object itself (a HEAD), never the manifest or the - # read store: after a store wipe the package is installed locally but the - # S3 object is gone. - # The existence HEAD is itself best-effort: _s3_key_exists re-raises any - # non-404 (403 expired creds, 503 SlowDown), and a connection timeout is - # not even a ClientError — so an un-guarded call here would propagate and - # fail an ALREADY-INSTALLED (reused) package, which has an empty BUILD dir - # and no log. Under a -j build doing one HEAD per reused package a transient - # throttle/timeout is near-certain, so we must never let it fail the package: - # on error we fall through and try the (idempotent) publish anyway. - _in_write_store = False - try: - _in_write_store = syncHelper.writestore_has_tarball(spec, args.architecture) - except Exception as exc: - warning("%s@%s write-store check failed (%s); will try to publish anyway", - spec["package"], spec["version"], exc) - if _in_write_store: - debug("%s@%s already in the write store; nothing to publish.", - spec["package"], spec["version"]) - elif os.path.isfile(_local_tar): - # Absent from the store but present locally: publish it. Best-effort — a - # store hiccup (expired creds / transient network) must NOT fail an - # otherwise-complete package, unlike a freshly built one in doFinalSync. - try: - syncHelper.upload_symlinks_and_tarball(spec) - info("%s@%s [uploaded from local cache]", spec["package"], spec["version"]) - except Exception as exc: - warning("%s@%s store sync failed: %s", spec["package"], spec["version"], exc) - else: - warning("%s@%s installed locally but absent from the store and no local " - "tarball — rebuild to publish it.", spec["package"], spec["version"]) - # Record in the build manifest that this package was already installed. - if getattr(args, "manifest", None) is not None: - args.manifest.add_package(spec, "already_installed", - effective_architecture=effective_arch(spec, args.architecture)) - # Touch the sentinel so the cleanup command knows this package was used. - try: - from bits_helpers.cleanup import touch_sentinel as _touch_sentinel - _touch_sentinel(workDir, args.architecture, spec["package"], ver_rev(spec)) - except Exception: - pass + return True + return False + + if _handle_already_installed(hashPath): return + if os.path.islink(hashPath) and os.path.isdir(hashPath): + fileHash = spec["hash"] + else: + fileHash = readHashFile(hashFile) if fileHash != "0": debug("Mismatch between local area (%s) and the one which I should build (%s). Redoing.", fileHash, spec["hash"]) @@ -2425,6 +2434,34 @@ def build_one_package(p, ctx): debug("Trusted reuse: %s@%s verified against signed manifest", spec["package"], spec["hash"]) + # A hash-policy package first looks up the remote identity, so read-only + # builds can reuse CI artifacts. Only after that lookup misses do we switch + # the label and store path to the local identity for this build. + from bits_helpers.hashing import apply_local_hash_fallback + remote_eff = effective_arch(spec, args.architecture) + remote_link = os.path.join( + workDir, resolve_links_path(remote_eff, spec["package"]), + "{}-{}.{}.tar.gz".format(spec["package"], ver_rev(spec), remote_eff)) + remote_hash = spec["hash"] + if apply_local_hash_fallback( + spec, getattr(syncHelper, "writeStore", ""), spec["cachedTarball"]): + if spec["hash"] != remote_hash: + call_ignoring_oserrors(unlink, remote_link) + create_version_link(spec, args.architecture, workDir) + local_hash_dir = os.path.join( + workDir, resolve_store_path(effective_arch(spec, args.architecture), spec["hash"])) + local_tarballs = [t for t in glob(os.path.join(local_hash_dir, "*gz")) + if os.path.isfile(t)] + spec["cachedTarball"] = _select_cached_tarball( + local_tarballs, spec, effective_arch(spec, args.architecture)) + + local_hash_path = _pkg_install_path( + workDir, effective_arch(spec, args.architecture), spec) + buildRoot = join(buildWorkDir, "BUILD", spec["hash"]) + if _handle_already_installed(local_hash_path): + return + shutil.rmtree(local_hash_path.encode("utf-8"), True) + # The actual build script. fp = open(dirname(realpath(__file__))+'/build_template.sh') diff --git a/bits_helpers/hashing.py b/bits_helpers/hashing.py index 3fb8b650..ae11ceb9 100644 --- a/bits_helpers/hashing.py +++ b/bits_helpers/hashing.py @@ -113,14 +113,24 @@ def normalize_recipe_for_hash(recipe): def _apply_revision_policy(spec): - """Use package identity as a label without overriding explicit revisions. - - Development packages keep their counter (localN) revisions: they are built - from a local checkout under their local hash, so the remote hash as label - would name a different build of the same package.""" + """Inject a remote hash label, except for development packages.""" if (spec.get("revision_policy") == "hash" and "force_revision" not in spec and not spec.get("is_devel_pkg")): spec["force_revision"] = spec["remote_revision_hash"] + spec["_revision_policy_hash_injected"] = True + + +def apply_local_hash_fallback(spec, write_store, reusable): + """Use the local hash label only after remote reuse failed on a read-only build.""" + if (spec.get("_revision_policy_hash_injected") and not write_store + and not reusable): + local_hash = spec["local_revision_hash"] + spec["force_revision"] = local_hash + spec["revision"] = local_hash + spec["hash"] = local_hash + spec["_revision_policy_local_fallback"] = True + return True + return False def storeHashes(package, specs, considerRelocation): diff --git a/bits_helpers/plan.py b/bits_helpers/plan.py index a0cf44ea..ecbac871 100644 --- a/bits_helpers/plan.py +++ b/bits_helpers/plan.py @@ -15,7 +15,7 @@ from bits_helpers.log import banner, debug from bits_helpers.sync import binary_redistributable from bits_helpers.rev_index import revision_from_tarball -from bits_helpers.utilities import resolve_store_path +from bits_helpers.utilities import is_virtual_package, resolve_store_path INSTALLED = "installed" LOCAL_TARBALL = "local tarball" @@ -106,6 +106,20 @@ def classify(spec, arch, work_dir, sync_helper, can_list, write_store, trusted, if trusted is not None and h not in trusted: return REMOTE_UNSIGNED, h, rev return REMOTE, h, rev + if (spec.get("_revision_policy_hash_injected") and not write_store + and not is_virtual_package(spec)): + from bits_helpers.hashing import apply_local_hash_fallback + local_hash = spec["local_revision_hash"] + local_files = glob(os.path.join( + work_dir, resolve_store_path(eff, local_hash), "*.tar.gz")) + local_rev = pick_revision(local_files, spec, eff, forced=local_hash) + if local_rev is not None: + state = (INSTALLED if _installed(spec, work_dir, eff, local_rev, local_hash) + else LOCAL_TARBALL) + apply_local_hash_fallback(spec, write_store, reusable=False) + return state, local_hash, local_rev + apply_local_hash_fallback(spec, write_store, reusable=False) + return BUILD, spec["hash"], spec["revision"] if forced is not None or write_store: return BUILD, spec["remote_revision_hash"], forced or "" return BUILD, spec["local_revision_hash"], "" diff --git a/bits_helpers/status.py b/bits_helpers/status.py index f1acb7e5..d329615e 100644 --- a/bits_helpers/status.py +++ b/bits_helpers/status.py @@ -39,10 +39,14 @@ from bits_helpers.log import debug, info, warning, banner from bits_helpers.packages import getPackageList from bits_helpers.utilities import ( + apply_version_from, prunePaths, + resolve_spec_data, resolve_tag, + resolve_version, topological_sort, ver_rev, + is_virtual_package, ) from bits_helpers.defaults import parseDefaults, readDefaults, validateDefaults from bits_helpers.arch import ( @@ -149,8 +153,12 @@ def _resolve_commit_hash(spec: dict, default_vars=None) -> None: except (KeyError, ValueError): pass if "source" not in spec: - # Package has no source (e.g. a meta-package); commit_hash is "0". - spec.setdefault("commit_hash", "0") + # Tarball-source recipes use their resolved tag (usually the version) + # as commit_hash, matching doBuild. Only truly source-less packages + # such as defaults-release use "0". Leaving tarball recipes at the + # initialization value "0" changes their hashes and prevents status + # from finding the artifacts build published. + spec["commit_hash"] = spec["tag"] if "sources" in spec else "0" return scm_refs = spec.get("scm_refs", {}) # Prefer branch head; fall back to literal tag / commit hash string. @@ -160,6 +168,56 @@ def _resolve_commit_hash(spec: dict, default_vars=None) -> None: ) +def _prepare_spec_for_hash(spec: dict, defaults: list, default_vars: dict, + config_dir: str, branch_basename: str = "", + branch_stream: str = "") -> None: + """Resolve hash inputs the same way build.py does before storeHashes().""" + from bits_helpers.paths import resolveLocalPath + + apply_version_from(spec, default_vars) + if "tag" not in spec: + spec["tag"] = spec["version"] + + if "source" in spec: + spec["tag"] = resolve_tag(spec, default_vars) + spec["commit_hash"] = ( + spec.get("scm_refs", {}).get("refs/heads/" + spec["tag"]) + or spec["tag"] + ) + if "sources" in spec: + spec["tag"] = resolve_tag(spec, default_vars) + spec["sources"] = [resolveLocalPath(config_dir, source) + for source in spec["sources"]] + spec["commit_hash"] = spec["tag"] + elif "source" not in spec and not spec.get("version_from"): + spec["commit_hash"] = "0" + + spec["version"] = resolve_version( + spec, defaults, branch_basename, branch_stream) + spec.setdefault("variables", OrderedDict(spec.get("variables", {}))) + for key, value in spec["variables"].items(): + spec["variables"][key] = resolve_spec_data( + spec, value, defaults, branch_basename, branch_stream) + if "source" in spec: + spec["source"] = resolve_spec_data( + spec, spec["source"], defaults, branch_basename, branch_stream) + if "sources" in spec: + spec["sources"] = [resolve_spec_data( + spec, source, defaults, branch_basename, branch_stream) + for source in spec["sources"]] + if "patches" in spec: + spec["patches"] = [resolve_spec_data( + spec, patch, defaults, branch_basename, branch_stream) + for patch in spec["patches"]] + + default_vars = default_vars or None + recipe_opts_in = bool(spec["variables"] or spec.get("expand_recipe", False)) + if recipe_opts_in or default_vars: + spec["recipe"] = resolve_spec_data( + spec, spec["recipe"], defaults, branch_basename, branch_stream, + default_vars=default_vars, strict=recipe_opts_in) + + def _fetch_refs_with_clone(spec: dict, reference_sources: str, package: str) -> None: """Populate spec["scm_refs"] by cloning / fetching the reference repo. @@ -220,6 +278,10 @@ def _scan_local_tars(spec: dict, work_dir: str, architecture: str) -> bool: if not match: continue rev_hash = match.group(1) + if (spec.get("_revision_policy_local_fallback") + and rev_hash == spec.get("hash") + and name.endswith("-{}.{}.tar.gz".format(spec["hash"], spec_arch))): + return True if "local" in name: if rev_hash in spec.get("local_hashes", []): return True @@ -240,7 +302,7 @@ def _is_already_installed(spec: dict, work_dir: str, architecture: str) -> bool: def _classify(spec: dict, work_dir: str, architecture: str, - sync_helper=None) -> str: + sync_helper=None, write_store=None) -> str: """Return the state string for one resolved package spec.""" pkg = spec["package"] @@ -265,22 +327,35 @@ def _classify(spec: dict, work_dir: str, architecture: str, eff = effective_arch(spec, architecture) if store_can_list(sync_helper): for h in spec.get("remote_hashes", []): - if pick_revision(sync_helper.list_store_tarballs(eff, h), spec, eff) is not None: + names = sync_helper.list_store_tarballs(eff, h) + debug("Remote status lookup for %s@%s hash %s: %d candidate(s)", + pkg, spec.get("version", "?"), h, len(names)) + if pick_revision(names, spec, eff) is not None: return FROM_REMOTE_STORE - return BUILD_FROM_SOURCE - try: - sync_helper.fetch_tarball(spec) - tar_hash_dir = join( - work_dir, - "TARS", effective_arch(spec, architecture), - "store", spec["hash"][:2], spec["hash"], - ) - tarballs = [t for t in glob(join(tar_hash_dir, "*gz")) - if os.path.isfile(t)] - if tarballs: - return FROM_REMOTE_STORE - except Exception: - pass + else: + try: + sync_helper.fetch_tarball(spec) + tar_hash_dir = join( + work_dir, + "TARS", effective_arch(spec, architecture), + "store", spec["hash"][:2], spec["hash"], + ) + tarballs = [t for t in glob(join(tar_hash_dir, "*gz")) + if os.path.isfile(t)] + if tarballs: + return FROM_REMOTE_STORE + except Exception: + pass + has_write_store = (bool(getattr(sync_helper, "writeStore", "")) + if write_store is None else bool(write_store)) + if (spec.get("_revision_policy_hash_injected") and not has_write_store + and not is_virtual_package(spec)): + from bits_helpers.hashing import apply_local_hash_fallback + apply_local_hash_fallback(spec, write_store=False, reusable=False) + if _is_already_installed(spec, work_dir, architecture): + return ALREADY_INSTALLED + if _scan_local_tars(spec, work_dir, architecture): + return FROM_STORE return BUILD_FROM_SOURCE @@ -509,13 +584,32 @@ def doStatus(args, parser) -> None: # Processed in topological order so dependency hashes are available. rows: List[dict] = [] + # Resolve the same read/write store inputs as `bits build`. Status does not + # write anything, but the presence of a write store changes the hash-policy + # label selected for packages that are not reusable from the read store. + write_store_url = (getattr(args, "writeStore", "") + or os.environ.get("BITS_WRITE_STORE") + or os.environ.get("WRITE_STORE") or "") + remote_store_url = (getattr(args, "remoteStore", "") + or os.environ.get("BITS_REMOTE_STORE") + or os.environ.get("REMOTE_STORE") or "") + if remote_store_url.endswith("::rw"): + if write_store_url: + parser.error("cannot specify ::rw and --write-store at the same time") + remote_store_url = remote_store_url[:-4] + write_store_url = remote_store_url + if getattr(args, "no_remote_store", False): + remote_store_url = "" + if not remote_store_url and write_store_url: + remote_store_url = write_store_url + # Optional remote store for --check-store probing sync_helper = None - if getattr(args, "checkStore", False) and getattr(args, "remoteStore", ""): + if getattr(args, "checkStore", False) and remote_store_url: try: from bits_helpers.sync import remote_from_url sync_helper = remote_from_url( - args.remoteStore, "", args.architecture, work_dir, + remote_store_url, "", args.architecture, work_dir, getattr(args, "insecure", False) ) except Exception as exc: @@ -523,6 +617,15 @@ def doStatus(args, parser) -> None: hash_error_pkgs: List[str] = [] + status_write_store = bool(write_store_url) + _branch_err, _branch_value = git( + ("symbolic-ref", "-q", "HEAD"), directory=args.configDir, check=False) + branch_basename = (re.sub("refs/heads/", "", _branch_value) + if _branch_err == 0 else "") + branch_stream = re.sub("-patches$", "", branch_basename) + if branch_stream == branch_basename: + branch_stream = "" + for p in buildOrder: spec = specs[p] @@ -548,8 +651,13 @@ def doStatus(args, parser) -> None: else: spec.setdefault("scm_refs", {}) - # Resolve commit hash - _resolve_commit_hash(spec, defaults_meta.get("variables")) + # Resolve exactly the hash inputs that doBuild prepares before + # storeHashes: tags/versions, templated sources and patches, variables, + # and recipe text. Hashing the raw recipe fields makes status look under + # a different store key from the build (especially for %(var)s URLs). + _prepare_spec_for_hash( + spec, args.defaults, defaults_meta.get("variables"), args.configDir, + branch_basename, branch_stream) # Devel package: compute devel_hash from local changes if spec["is_devel_pkg"]: @@ -574,6 +682,9 @@ def doStatus(args, parser) -> None: try: storeHook(p, specs, args.defaults[0]) storeHashes(p, specs, considerRelocation=consider_relocation) + debug("Status hashes for %s: remote %s; local %s", p, + spec.get("remote_revision_hash", "?"), + spec.get("local_revision_hash", "?")) except Exception as exc: debug("Hash computation failed for %s: %s", p, exc) hash_error_pkgs.append(p) @@ -594,7 +705,11 @@ def doStatus(args, parser) -> None: spec["hash"] = spec["remote_revision_hash"] spec["revision"] = "1" - state = _classify(spec, work_dir, args.architecture, sync_helper) + state = _classify(spec, work_dir, args.architecture, sync_helper, + write_store=status_write_store) + if spec["is_devel_pkg"]: + # Build disables remote writes after reaching a development package. + status_write_store = False version_str = spec.get("version", "?") if spec["is_devel_pkg"]: version_str = "{} (dev)".format(version_str) diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 05cd94fb..d2f8d12a 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -3019,9 +3019,11 @@ packages keep counter (`localN`) revisions: they are built from a local checkout under their local hash. The hash includes tracked dependencies; this is the package build hash, not its source commit hash. Install paths and tarball names use `-` with -no `local` prefix, through the existing forced-revision mechanism. Uploads keep -using the same content-addressed store paths. Omitting `revision_policy` retains -the existing revision-counter behavior. +no `local` prefix, through the existing forced-revision mechanism. With a +read-only store, bits checks for the remote hash first and reuses it when +available; if it must build the package locally, it uses the local hash as the +label and store path. Uploads keep using the same content-addressed store paths. +Omitting `revision_policy` retains the existing revision-counter behavior. #### How the install path changes diff --git a/tests/test_args.py b/tests/test_args.py index 4bab45f5..716b65fe 100644 --- a/tests/test_args.py +++ b/tests/test_args.py @@ -61,6 +61,7 @@ class FakeExit(Exception): ((), "build --force-unknown-architecture zlib --no-remote-store --remote-store rsync://test.local/", [("noSystem", None), ("remoteStore", "")]), ((), "build zlib --architecture slc7_x86-64" , [("noSystem", None), ("preferSystem", False), ("remoteStore", "https://s3.cern.ch/swift/v1/alibuild-repo")]), ((), "build zlib --architecture ubuntu1804_x86-64" , [("noSystem", None), ("preferSystem", False), ("remoteStore", "")]), + ((), "status zlib --write-store b3://bits-write" , [("action", "status"), ("writeStore", "b3://bits-write")]), ((), "build zlib -a slc7_x86-64" , [("docker", False), ("dockerImage", None), ("docker_extra_args", ["--network=host", _MOCK_CPUSET_ARG])]), ((), "build zlib -a slc7_x86-64 --docker-image registry.cern.ch/alisw/some-builder" , [("docker", True), ("dockerImage", "registry.cern.ch/alisw/some-builder")]), ((), "build zlib -a slc7_x86-64 --docker" , [("docker", True), ("dockerImage", "gitlab-registry.cern.ch/bits/containers/x86_64-slc7:latest")]), diff --git a/tests/test_build.py b/tests/test_build.py index 50da82a8..a81e1cc8 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -7,15 +7,16 @@ import platform import re import sys +import tempfile import unittest # Assuming you are using the mock library to ... mock things from unittest.mock import call, patch, MagicMock, DEFAULT from io import StringIO from collections import OrderedDict -from bits_helpers.utilities import resolve_tag +from bits_helpers.utilities import resolve_tag, resolve_store_path from bits_helpers.recipe import parseRecipe -from bits_helpers.build import doBuild +from bits_helpers.build import doBuild, build_one_package, _BuildLoopCtx from bits_helpers.hashing import storeHashes from bits_helpers.initdotsh import generate_initdotsh @@ -731,5 +732,126 @@ def test_initdotsh_reuse_orders_prereq_before_reused(self) -> None: "local prerequisite Tools must be sourced before reused CMake") +class HashPolicyBuildFallbackTest(unittest.TestCase): + """Exercise the remote-first hash-policy decision in build_one_package.""" + + remote_hash = "a" * 40 + local_hash = "b" * 40 + + class StopAtBuildScript(Exception): + pass + + def _run_package(self, work_dir, *, remote_tarball=False): + arch = TEST_ARCHITECTURE + remote_hash = self.remote_hash + spec = { + "package": "hash-policy-test", "version": "1.0", + "revision_policy": "hash", "is_devel_pkg": False, + "remote_revision_hash": self.remote_hash, + "local_revision_hash": self.local_hash, + "remote_hashes": [self.remote_hash], + "local_hashes": [self.local_hash], + "requires": [], "build_requires": [], "runtime_requires": [], + } + + class ReadOnlyStore: + writeStore = "" + + def fetch_symlinks(self, _spec): + pass + + def fetch_tarball(self, _spec): + if remote_tarball: + directory = os.path.join( + work_dir, resolve_store_path(arch, remote_hash)) + os.makedirs(directory, exist_ok=True) + path = os.path.join( + directory, + "hash-policy-test-1.0-{}.{}.tar.gz".format(remote_hash, arch)) + with _REAL_OPEN(path, "wb") as stream: + stream.write(b"remote") + + sync = ReadOnlyStore() + args = Namespace( + architecture=arch, defaults=["release"], workDir=work_dir, + autoCleanup=False, aggressiveCleanup=False, docker=False, + builders=2, referenceSources="", manifest=None, + ) + cfg = Namespace( + reuse_overlay=None, build_local=[], reuse_policy="strict", + store_integrity=False, require_signed_reuse=False, + parallel_sources=1, + ) + ctx = _BuildLoopCtx( + args=args, cfg=cfg, specs={spec["package"]: spec}, + workDir=work_dir, syncHelper=sync, scheduler=None, + mainPackage=spec["package"], raw_architecture=arch, + develPackageBranch="", defaultsMeta={}, buildTargets="", + packages=[], prefetch_executor=None, cmake_prefix_env=False, + specs_for_checksum_phase=[], monitoredDirs={}, + ) + + real_open = _REAL_OPEN + + def stop_at_script(path, *open_args, **open_kwargs): + if os.path.basename(os.fspath(path)) == "build_template.sh": + raise self.StopAtBuildScript() + return real_open(path, *open_args, **open_kwargs) + + def read_hash(path): + try: + with real_open(path) as stream: + return stream.read().strip("\n") + except OSError: + return "0" + + with patch("bits_helpers.build.storeHook"), \ + patch("bits_helpers.build.log_current_package"), \ + patch("bits_helpers.build.readHashFile", side_effect=read_hash), \ + patch("bits_helpers.build.open", side_effect=stop_at_script, create=True): + try: + result = build_one_package(spec["package"], ctx) + except self.StopAtBuildScript as stopped: + result = stopped + return result, spec + + def test_read_only_miss_uses_local_label_and_next_run_skips_build(self): + with tempfile.TemporaryDirectory() as work_dir: + result, spec = self._run_package(work_dir) + self.assertIsInstance(result, self.StopAtBuildScript) + self.assertEqual(self.local_hash, spec["hash"]) + self.assertEqual(self.local_hash, spec["revision"]) + spec_hash = spec["hash"] + install_dir = os.path.join( + work_dir, TEST_ARCHITECTURE, "hash-policy-test", + "1.0-" + spec_hash) + os.makedirs(install_dir, exist_ok=True) + with _REAL_OPEN(os.path.join(install_dir, ".build-hash"), "w") as stream: + stream.write(spec_hash) + + result, spec = self._run_package(work_dir) + self.assertIsNone(result) + self.assertEqual(self.local_hash, spec["hash"]) + self.assertEqual(self.local_hash, spec["revision"]) + + def test_remote_match_is_preferred_to_existing_local_install(self): + with tempfile.TemporaryDirectory() as work_dir: + local_install = os.path.join( + work_dir, TEST_ARCHITECTURE, "hash-policy-test", + "1.0-" + self.local_hash) + os.makedirs(local_install, exist_ok=True) + with _REAL_OPEN(os.path.join(local_install, ".build-hash"), "w") as stream: + stream.write(self.local_hash) + + result, spec = self._run_package(work_dir, remote_tarball=True) + + self.assertIsInstance(result, self.StopAtBuildScript) + self.assertEqual(self.remote_hash, spec["hash"]) + self.assertEqual(self.remote_hash, spec["revision"]) + self.assertTrue(spec["cachedTarball"].endswith( + "hash-policy-test-1.0-{}.{}.tar.gz".format( + self.remote_hash, TEST_ARCHITECTURE))) + + if __name__ == '__main__': unittest.main() diff --git a/tests/test_plan.py b/tests/test_plan.py index 36b48166..818f95fe 100644 --- a/tests/test_plan.py +++ b/tests/test_plan.py @@ -82,6 +82,29 @@ def test_build_hash_follows_write_store(self): self.assertEqual(self._classify(_Store(), write_store=True), (plan.BUILD, RH, "")) self.assertEqual(self._classify(_Store(), write_store=False), (plan.BUILD, LH, "")) + def _hash_policy_local_fallback(self): + spec = _spec( + revision_policy="hash", _revision_policy_hash_injected=True, + force_revision=RH, hash=RH, revision=RH) + self._tarball(LH, LH) + return spec + + def test_hash_policy_local_fallback_finds_local_tarball(self): + spec = self._hash_policy_local_fallback() + self.assertEqual(self._classify(**spec), (plan.LOCAL_TARBALL, LH, LH)) + + def test_hash_policy_local_fallback_finds_installed_local_build(self): + spec = self._hash_policy_local_fallback() + self._installed(LH, LH) + self.assertEqual(self._classify(**spec), (plan.INSTALLED, LH, LH)) + + def test_hash_policy_fallback_does_not_change_virtual_package_hash(self): + spec = _spec( + provides_repository=True, _revision_policy_hash_injected=True, + force_revision=RH, hash=RH, revision=RH) + self.assertEqual(self._classify(**spec), (plan.BUILD, RH, RH)) + self.assertEqual(spec["hash"], RH) + def test_devel(self): self.assertEqual(self._classify(is_devel_pkg=True)[0], plan.DEVEL) diff --git a/tests/test_revision_policy.py b/tests/test_revision_policy.py index 55290fee..3a085675 100644 --- a/tests/test_revision_policy.py +++ b/tests/test_revision_policy.py @@ -145,6 +145,60 @@ def test_devel_package_keeps_counter_revisions(self): _apply_revision_policy(spec) self.assertEqual(spec["force_revision"], self.HASH) + def test_build_hash_fallback_waits_until_remote_reuse_misses(self): + from bits_helpers.hashing import apply_local_hash_fallback + spec = {"_revision_policy_hash_injected": True, + "remote_revision_hash": self.HASH, + "local_revision_hash": "de" + "ad" * 19, + "force_revision": self.HASH, "revision": self.HASH, + "hash": self.HASH} + self.assertFalse(apply_local_hash_fallback(spec, False, reusable=True)) + self.assertEqual(spec["hash"], self.HASH) + self.assertEqual(spec["force_revision"], self.HASH) + + self.assertTrue(apply_local_hash_fallback(spec, False, reusable=False)) + self.assertEqual(spec["hash"], spec["local_revision_hash"]) + self.assertEqual(spec["force_revision"], spec["local_revision_hash"]) + self.assertFalse(apply_local_hash_fallback(spec, True, reusable=False)) + + def test_plan_reuses_remote_hash_before_local_build_fallback(self): + from bits_helpers.plan import REMOTE, classify + spec = {"package": "app", "version": "1", "revision_policy": "hash", + "_revision_policy_hash_injected": True, + "force_revision": self.HASH, "remote_revision_hash": self.HASH, + "local_revision_hash": "de" + "ad" * 19, + "remote_hashes": [self.HASH], "local_hashes": [], + "hash": self.HASH, "requires": []} + + class ReadOnlyStore: + writeStore = "" + + @staticmethod + def list_store_tarballs(arch, pkg_hash): + return ["app-1-{}.{}.tar.gz".format(pkg_hash, arch)] + + with tempfile.TemporaryDirectory() as work_dir: + state, pkg_hash, revision = classify( + spec, self.ARCH, work_dir, ReadOnlyStore(), + can_list=True, write_store=False, trusted=None) + self.assertEqual((state, pkg_hash, revision), + (REMOTE, self.HASH, self.HASH)) + + def test_status_uses_local_hash_after_remote_lookup_misses(self): + from bits_helpers.status import BUILD_FROM_SOURCE, _classify + spec = {"package": "app", "version": "1", "revision_policy": "hash", + "_revision_policy_hash_injected": True, + "force_revision": self.HASH, "remote_revision_hash": self.HASH, + "local_revision_hash": "de" + "ad" * 19, + "remote_hashes": [self.HASH], "local_hashes": [], + "hash": self.HASH, "revision": self.HASH} + with tempfile.TemporaryDirectory() as work_dir: + state = _classify(spec, work_dir, self.ARCH, sync_helper=None, + write_store=False) + self.assertEqual(state, BUILD_FROM_SOURCE) + self.assertEqual(spec["hash"], spec["local_revision_hash"]) + self.assertEqual(spec["revision"], spec["local_revision_hash"]) + def test_untracked_label_fatal_only_under_hash_policy(self): from bits_helpers import build specs = {"u": {}, "h": {"revision_policy": "hash"}, diff --git a/tests/test_status.py b/tests/test_status.py index 6280c445..0e97f283 100644 --- a/tests/test_status.py +++ b/tests/test_status.py @@ -24,6 +24,7 @@ _emit_json, _emit_table, _is_already_installed, + _prepare_spec_for_hash, _resolve_commit_hash, _scan_local_tars, _try_populate_refs, @@ -157,6 +158,33 @@ def test_no_source_sets_zero(self): _resolve_commit_hash(spec) self.assertEqual(spec["commit_hash"], "0") + def test_tarball_sources_use_tag_as_commit_hash(self): + """Tarball-only recipes must hash like build, which uses the tag.""" + spec = _make_spec(tag="1.0") + del spec["source"] + spec["sources"] = ["https://example.org/source-1.0.tar.gz"] + spec["commit_hash"] = "0" + spec["scm_refs"] = {} + _resolve_commit_hash(spec) + self.assertEqual(spec["commit_hash"], "1.0") + + def test_hash_inputs_expand_recipe_variables_like_build(self): + spec = _make_spec(tag="%(gmpVersion)s") + del spec["source"] + spec["commit_hash"] = "0" + spec["variables"] = OrderedDict(gmpVersion="6.3.0") + spec["sources"] = ["https://example.org/gmp-%(gmpVersion)s.tar.gz"] + spec["patches"] = ["gmp-%(gmpVersion)s.patch"] + spec["recipe"] = "build %(gmpVersion)s" + + _prepare_spec_for_hash(spec, ["release"], {}, "/recipes") + + self.assertEqual(spec["tag"], "6.3.0") + self.assertEqual(spec["commit_hash"], "6.3.0") + self.assertEqual(spec["sources"], ["https://example.org/gmp-6.3.0.tar.gz"]) + self.assertEqual(spec["patches"], ["gmp-6.3.0.patch"]) + self.assertEqual(spec["recipe"], "build 6.3.0") + def test_date_tag_expanded(self): spec = _make_spec(tag="v%(year)s-1") spec["scm_refs"] = {} @@ -324,6 +352,18 @@ def test_build_from_source(self): spec["revision"] = "1" self.assertEqual(_classify(spec, self.tmp, self.arch), BUILD_FROM_SOURCE) + def test_hash_policy_fallback_does_not_change_virtual_package_hash(self): + rh = "aabb" + "0" * 36 + lh = "ccdd" + "0" * 36 + spec = self._spec(pkg="recipe-loader", source="", + remote_revision_hash=rh, local_revision_hash=lh, + remote_hashes=[rh], local_hashes=[lh]) + spec.update(provides_repository=True, _revision_policy_hash_injected=True, + force_revision=rh, hash=rh, revision=rh) + self.assertEqual(_classify(spec, self.tmp, self.arch), BUILD_FROM_SOURCE) + self.assertEqual(spec["hash"], rh) + self.assertEqual(spec["revision"], rh) + def test_local_checkout_will_rebuild(self): spec = self._spec(pkg="MyAnalysis", version="dev", is_devel=True, devel_hash="newhash", deps_hash="") @@ -497,7 +537,7 @@ def tearDown(self): def _make_args(self, pkgname, json_output=False, check_store=False, force_tracked=True, no_devel=None, disable=None, - force_rebuild=None, fetch_repos=False): + force_rebuild=None, fetch_repos=False, write_store=""): import argparse args = argparse.Namespace( pkgname = pkgname, @@ -513,6 +553,7 @@ def _make_args(self, pkgname, json_output=False, check_store=False, force_rebuild = force_rebuild or [], fetchRepos = fetch_repos, remoteStore = "", + writeStore = write_store, no_remote_store = False, checkStore = check_store, json_output = json_output, @@ -594,6 +635,60 @@ def fake_store_hashes(p, specs, considerRelocation): data = json.loads(mock_out.getvalue()) self.assertEqual(data["packages"][0]["state"], BUILD_FROM_SOURCE) + @patch("bits_helpers.status.getPackageList") + @patch("bits_helpers.status.parseDefaults") + @patch("bits_helpers.status.readDefaults") + @patch("bits_helpers.hashing.storeHashes") + @patch("bits_helpers.build.storeHook") + def test_write_store_keeps_hash_policy_on_remote_hash(self, mock_hook, + mock_store_hashes, + mock_read_defaults, + mock_parse_defaults, + mock_get_package_list): + """A writable store means a missing hash-policy package keeps remote identity.""" + rh = "aabbcc" + "0" * 34 + lh = "ddeeff" + "0" * 34 + mock_parse_defaults.return_value = (None, {}, {}, {}) + mock_read_defaults.return_value = None + + spec = _make_spec(pkg="mylib", version="1.0", + remote_revision_hash=rh, local_revision_hash=lh, + remote_hashes=[rh], local_hashes=[lh]) + spec["revision_policy"] = "hash" + + def fake_get_pkg_list(*args, **kwargs): + kwargs["specs"]["mylib"] = spec + return ([], ["mylib"], set(), None) + mock_get_package_list.side_effect = fake_get_pkg_list + + def fake_store_hashes(p, specs, considerRelocation): + package = specs[p] + package["remote_revision_hash"] = rh + package["local_revision_hash"] = lh + package["remote_hashes"] = [rh] + package["local_hashes"] = [lh] + package["force_revision"] = rh + package["_revision_policy_hash_injected"] = True + package["deps_hash"] = "" + mock_store_hashes.side_effect = fake_store_hashes + + args = self._make_args(["mylib"], json_output=True, + write_store="b3://bits-write") + import bits_helpers.status as status_mod + with patch("sys.stdout", new_callable=StringIO) as mock_out: + with patch("bits_helpers.status.topological_sort", return_value=["mylib"]): + with patch("bits_helpers.status.compute_combined_arch", return_value=self.arch): + with patch("bits_helpers.status.prunePaths"): + import argparse + parser = argparse.ArgumentParser() + parser.error = lambda msg: (_ for _ in ()).throw(SystemExit(msg)) + status_mod.doStatus(args, parser) + data = json.loads(mock_out.getvalue()) + + package = data["packages"][0] + self.assertEqual(package["hash"], rh) + self.assertEqual(package["state"], BUILD_FROM_SOURCE) + @patch("bits_helpers.status.getPackageList") @patch("bits_helpers.status.parseDefaults") @patch("bits_helpers.status.readDefaults") From fa8b6d1d3dc48f48215748bf10b0321e4fa49a64 Mon Sep 17 00:00:00 2001 From: akritkbehera Date: Thu, 8 Oct 2026 07:36:58 +0200 Subject: [PATCH 2/2] fix(status): include the build's defaults init.sh hash marker Include BITS_INITDOTSH_FROM_MODULES in status defaults hash inputs unless legacy init.sh mode is selected. This prevents defaults-release and its dependents from being reported as needing a rebuild due to hash drift. Extract the build-side marker helper and test defaults-release hash parity, including the complete dependency fields in the test fixture. --- bits_helpers/build.py | 18 ++++--- bits_helpers/status.py | 17 +++++-- tests/test_status.py | 106 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 9 deletions(-) diff --git a/bits_helpers/build.py b/bits_helpers/build.py index 307daea2..136cac9e 100644 --- a/bits_helpers/build.py +++ b/bits_helpers/build.py @@ -96,6 +96,17 @@ def apply_defaults_legacy_initdotsh(args, defaults_meta, explicit) -> bool: return True +def add_initdotsh_hash_marker(defaults_meta, enabled) -> None: + """Add the module-based init.sh mode to the defaults package hash inputs.""" + if not enabled: + return + env = defaults_meta.get("env") + if not isinstance(env, dict): + env = OrderedDict() + defaults_meta["env"] = env + env["BITS_INITDOTSH_FROM_MODULES"] = "1" + + def _prefetch_package(spec, sync_helper, work_dir, build_arch, source_arch=None) -> None: """Background task: prefetch the prebuilt tarball + all source archives. @@ -2859,12 +2870,7 @@ def defaultsReader(): # reconstruction on it. In legacy mode (--legacy-initdotsh) nothing is added, # so its hashes are byte-identical to the pre-modules default (alidist tarballs # stay reusable). - if getattr(args, "initdotshFromModules", False): - from collections import OrderedDict as _OD - # An empty `env:` block parses to None, so setdefault would keep it None. - if not isinstance(meta.get("env"), dict): - meta["env"] = _OD() - meta["env"]["BITS_INITDOTSH_FROM_MODULES"] = "1" + add_initdotsh_hash_marker(meta, getattr(args, "initdotshFromModules", False)) return meta, body # Deriving the dependency env from the dependencies' modulefiles is the default. # --legacy-initdotsh (CLI) or BITS_LEGACY_INITDOTSH=1 (the environment — the diff --git a/bits_helpers/status.py b/bits_helpers/status.py index d329615e..f67c49ef 100644 --- a/bits_helpers/status.py +++ b/bits_helpers/status.py @@ -455,9 +455,20 @@ def doStatus(args, parser) -> None: resolve_config_dir(args) # ── Defaults and overrides ───────────────────────────────────────────────── - defaults_reader = lambda: readDefaults( - args.configDir, args.defaults, parser.error, args.architecture - ) + def defaults_reader(): + meta, body = readDefaults( + args.configDir, args.defaults, parser.error, args.architecture + ) + # Build hashes this marker into defaults-release by default. Keep status + # on the same identity; the aliBuild compatibility environment selects + # legacy hashes and therefore omits it. + legacy_initdotsh = os.environ.get("BITS_LEGACY_INITDOTSH", "").strip().lower() + if legacy_initdotsh not in ("1", "true", "yes", "on"): + if not isinstance(meta.get("env"), dict): + meta["env"] = OrderedDict() + meta["env"]["BITS_INITDOTSH_FROM_MODULES"] = "1" + return meta, body + err, overrides, taps, defaults_meta = parseDefaults( args.disable, defaults_reader, debug, args.architecture, args.configDir ) diff --git a/tests/test_status.py b/tests/test_status.py index 0e97f283..e5ef8bf4 100644 --- a/tests/test_status.py +++ b/tests/test_status.py @@ -29,6 +29,8 @@ _scan_local_tars, _try_populate_refs, ) +from bits_helpers.hashing import storeHashes +from bits_helpers.build import add_initdotsh_hash_marker # ── Helpers ──────────────────────────────────────────────────────────────────── @@ -139,6 +141,32 @@ def test_git_failure_falls_back_to_empty(self): # ── _resolve_commit_hash ─────────────────────────────────────────────────────── class TestResolveCommitHash(unittest.TestCase): + def test_defaults_hash_includes_build_initdotsh_mode_marker(self): + spec = OrderedDict({ + "package": "defaults-release", + "version": "vCMS", + "tag": "vCMS", + "recipe": "", + "pkg_family": "", + "commit_hash": "0", + "scm_refs": {}, + "is_devel_pkg": False, + "requires": [], + "env": OrderedDict([ + ("CXXSTD", "20"), + ("DCMAKE_BUILD_TYPE", "Release"), + ]), + }) + spec["env"]["BITS_INITDOTSH_FROM_MODULES"] = "1" + + storeHashes("defaults-release", {"defaults-release": spec}, False) + + self.assertEqual( + spec["remote_revision_hash"], + "0028228fa91a801048bac07afed5974c27e293dc", + ) + self.assertEqual(spec["local_revision_hash"], spec["remote_revision_hash"]) + def test_branch_ref_resolved(self): spec = _make_spec(tag="main") spec["scm_refs"] = {"refs/heads/main": "deadbeef"} @@ -590,6 +618,84 @@ def _make_recipe_dir(self, pkg, version="1.0", tag=None, requires=None): fh.write("package: defaults-release\nversion: \"1\"\n---\n") return config_dir + def test_defaults_release_hash_matches_build_hash(self): + """Status must hash defaults-release like build's defaults reader.""" + from copy import deepcopy + import argparse + import bits_helpers.status as status_mod + + base_env = OrderedDict([ + ("CXXSTD", "20"), + ("DCMAKE_BUILD_TYPE", "Release"), + ]) + defaults_meta = {"env": base_env.copy(), "variables": {}} + status_spec = OrderedDict({ + "package": "defaults-release", + "version": "vCMS", + "tag": "vCMS", + "recipe": "", + "pkg_family": "", + "revision_policy": "hash", + "commit_hash": "0", + "scm_refs": {}, + "is_devel_pkg": False, + "requires": [], + "build_requires": [], + "runtime_requires": [], + "env": OrderedDict(), + }) + + def parse_defaults(disable, reader, *args): + meta, _body = reader() + return None, {"defaults-release": {"env": meta["env"]}}, {}, meta + + def get_packages(*args, **kwargs): + status_spec["env"] = kwargs["overrides"]["defaults-release"]["env"] + kwargs["specs"]["defaults-release"] = status_spec + return [], ["defaults-release"], set(), None + + args = self._make_args(["defaults-release"], json_output=True) + with patch("bits_helpers.status.readDefaults", + return_value=(deepcopy(defaults_meta), "")), \ + patch("bits_helpers.status.parseDefaults", side_effect=parse_defaults), \ + patch("bits_helpers.status.getPackageList", side_effect=get_packages), \ + patch("bits_helpers.status.topological_sort", + return_value=["defaults-release"]), \ + patch("bits_helpers.status.compute_combined_arch", return_value=self.arch), \ + patch("bits_helpers.status.prunePaths"), \ + patch("bits_helpers.repo_provider.resolve_config_dir"), \ + patch("bits_helpers.repo_provider.load_always_on_providers", return_value={}), \ + patch("bits_helpers.repo_provider.fetch_repo_providers_iteratively", + return_value={}), \ + patch("bits_helpers.build.storeHook"), \ + patch("sys.stdout", new_callable=StringIO) as out: + parser = argparse.ArgumentParser() + parser.error = lambda msg: (_ for _ in ()).throw(SystemExit(msg)) + status_mod.doStatus(args, parser) + status_hash = json.loads(out.getvalue())["packages"][0]["hash"] + + # Build uses this helper in its defaults reader before creating the + # defaults-release package spec. Hash that build-side spec independently + # and require status to report the same identity. + build_meta = {"env": base_env.copy()} + add_initdotsh_hash_marker(build_meta, enabled=True) + build_spec = OrderedDict({ + "package": "defaults-release", + "version": "vCMS", + "tag": "vCMS", + "recipe": "", + "pkg_family": "", + "revision_policy": "hash", + "commit_hash": "0", + "scm_refs": {}, + "is_devel_pkg": False, + "requires": [], + "env": build_meta["env"], + }) + storeHashes("defaults-release", {"defaults-release": build_spec}, False) + + self.assertEqual(status_hash, build_spec["remote_revision_hash"]) + @patch("bits_helpers.status.getPackageList") @patch("bits_helpers.status.parseDefaults") @patch("bits_helpers.status.readDefaults")