From aae6540ae5dfff9dbe269d547f56a88822beaa75 Mon Sep 17 00:00:00 2001 From: akritkbehera Date: Thu, 8 Oct 2026 07:36:58 +0200 Subject: [PATCH 1/3] 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/3] 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") From 4bef5baafea039075f57fba841d7e7ef43961b63 Mon Sep 17 00:00:00 2001 From: Predrag Buncic Date: Fri, 9 Oct 2026 00:23:18 +0200 Subject: [PATCH 3/3] status/plan: hash like build (shared hash inputs, init.sh mode, defaults store) One prepare_hash_inputs for build and status; status takes legacy_initdotsh and system.remote_store from the defaults; plan falls back to the local hash after an unsigned remote hit. --- bits_helpers/build.py | 257 +++++++++++++++++++++++------------------ bits_helpers/plan.py | 11 +- bits_helpers/status.py | 130 +++++---------------- docs/REFERENCE.md | 11 +- tests/test_plan.py | 10 ++ tests/test_status.py | 180 +++++++++++++++++------------ 6 files changed, 306 insertions(+), 293 deletions(-) diff --git a/bits_helpers/build.py b/bits_helpers/build.py index 136cac9e..395344f7 100644 --- a/bits_helpers/build.py +++ b/bits_helpers/build.py @@ -107,6 +107,128 @@ def add_initdotsh_hash_marker(defaults_meta, enabled) -> None: env["BITS_INITDOTSH_FROM_MODULES"] = "1" +def resolve_initdotsh_mode(args) -> bool: + """Set ``args.initdotshFromModules``: the --initdotsh-from-modules / + --legacy-initdotsh flag, else legacy when BITS_LEGACY_INITDOTSH is set (the + aliBuild wrapper sets it), else from modules (the default). Returns whether + the choice was explicit (flag or env var): only then does it win over a + defaults file's ``legacy_initdotsh`` (apply_defaults_legacy_initdotsh).""" + explicit = (getattr(args, "initdotshFromModules", None) is not None + or os.environ.get("BITS_LEGACY_INITDOTSH", "").strip() != "") + if getattr(args, "initdotshFromModules", None) is None: + args.initdotshFromModules = os.environ.get( + "BITS_LEGACY_INITDOTSH", "").strip().lower() not in ("1", "true", "yes", "on") + return explicit + + +def defaults_store_url(defaults_meta): + """The binary store the active defaults name (``system: remote_store``, else a + top-level ``remote_store``), as ``(url, writable)``: normalised, with a + trailing ``::rw`` taken as writable. ``("", False)`` when there is none.""" + system = defaults_meta.get("system", {}) or {} + url = system.get("remote_store", defaults_meta.get("remote_store")) + if not url: + return "", False + url = str(url).strip() + writable = url.endswith("::rw") + from bits_helpers.sync import normalise_store_url + return normalise_store_url(url[:-4] if writable else url), writable + + +def prepare_hash_inputs(spec, specs, defaults, default_vars, config_dir, + branch_basename="", branch_stream="", devel=None, + devel_version=None) -> None: + """Resolve what storeHashes() hashes for *spec*: tag and commit, version, + sources, patches, variables and recipe text. `bits build` and `bits status` + both use it, so that they compute the same package identity. + + *devel* is called for a development package with a source, once its tag is + resolved; it sets commit_hash, devel_hash and tag from the checkout. + *devel_version*, if set, then replaces a development package's version + (--devel-prefix). *spec* needs its scm_refs when it has a source. + """ + spec["commit_hash"] = "0" + # version_from: — take version (and, for a source-less package, tag + + # commit_hash) directly from a named defaults variable. Runs before the tag + # defaulting / source blocks so a synthetic package can be versioned by e.g. + # the LCG release without a source. No-op unless the recipe sets version_from. + apply_version_from(spec, default_vars) + if "tag" not in spec: + spec["tag"] = spec["version"] + if "source" in spec: + # Tag may contain date params like %(year)s, %(month)s, %(day)s, %(hour), + # plus any variable from the defaults profile / the recipe (e.g. an + # override of tag: "%(release)s" driven by variables: release:). + spec["tag"] = resolve_tag(spec, default_vars) + # First, we try to resolve the "tag" as a branch name, and use its tip as + # the commit_hash. If it's not a branch, it must be a tag or a raw commit + # hash, so we use it directly. A development package then takes its + # commit from the checkout (devel). + spec["commit_hash"] = spec["scm_refs"].get("refs/heads/" + spec["tag"], spec["tag"]) + if spec.get("is_devel_pkg") and devel is not None: + devel(spec) + + if "sources" in spec: + # Expand a templated tag (e.g. "v%(version)s") for tarball sources too. + # The git branch above only resolves it when a `source:` is present, so a + # tarball-only recipe kept the raw tag, which then leaked into commit_hash + # and the SOURCES/// path. No-op for literal tags. + spec["tag"] = resolve_tag(spec, default_vars) + spec["sources"] = [resolveLocalPath(config_dir, s) for s in spec["sources"]] + spec["commit_hash"] = spec["tag"] + # Version may contain date params like tag, plus %(commit_hash)s, + # %(short_hash)s and %(tag)s. + spec["version"] = resolve_version(spec, defaults, branch_basename, branch_stream) + + spec.setdefault("variables", OrderedDict(spec.get("variables", {}))) + variables = spec["variables"] + if "Python" in spec.get("requires", []): + # Find the Python package spec safely + python_version_str = "" + py_spec = specs.get("Python") + if isinstance(py_spec, dict): + python_version_str = (py_spec.get("version", "") or "").replace("v", "") + python_version = python_version_str.split(".") if python_version_str else [] + + # Safely extract major, minor, patch versions + major = python_version[0] if len(python_version) > 0 else "0" + minor = python_version[1] if len(python_version) > 1 else "0" + patch = python_version[2] if len(python_version) > 2 else "0" + + # Populate variables dictionary + variables.update({ + "python_major_version": major, + "python_minor_version": minor, + "python_patch_version": patch, + "python_major_minor": f"{major}.{minor}", + "python_major_minor_str": f"{major}{minor}", + }) + for k, v in variables.items(): + variables[k] = resolve_spec_data(spec, v, 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, src, defaults, branch_basename, branch_stream) for src in spec["sources"]] + if "patches" in spec: + spec["patches"] = [resolve_spec_data(spec, p, defaults, branch_basename, branch_stream) for p in spec["patches"]] + # Variables defined in the active --defaults profile's `variables:` block are + # available to every recipe body. When a recipe does not itself opt into + # expansion (no `variables:` / `expand_recipe: true`) we expand it in SOFT + # mode: only known variables are substituted and any other %(...)s / bare % + # is left untouched, so profile-wide variables never clobber or break a + # recipe that happens to contain a literal %(...)s or shell `%`. + default_vars = default_vars or None + recipe_opts_in = bool(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) + + if spec.get("is_devel_pkg") and devel_version is not None: + spec["version"] = devel_version + + def _prefetch_package(spec, sync_helper, work_dir, build_arch, source_arch=None) -> None: """Background task: prefetch the prebuilt tarball + all source archives. @@ -2882,12 +3004,7 @@ def defaultsReader(): # does it win over a defaults-file request below; the CLI flags set # initdotshFromModules to True/False (None when absent), env is # BITS_LEGACY_INITDOTSH. - _initdotsh_explicit = (getattr(args, "initdotshFromModules", None) is not None - or os.environ.get("BITS_LEGACY_INITDOTSH", "").strip() != "") - if getattr(args, "initdotshFromModules", None) is None: - _legacy_env = os.environ.get("BITS_LEGACY_INITDOTSH", "").strip().lower() in ( - "1", "true", "yes", "on") - args.initdotshFromModules = not _legacy_env + _initdotsh_explicit = resolve_initdotsh_mode(args) # Preserve the CLI-supplied disables: parseDefaults appends the config-dir # disables to this list in place, so the re-parse below must start clean. _cli_disable = list(args.disable) @@ -3199,12 +3316,8 @@ def _system_opt(key, top_default): # built-in arch default. '::rw' shorthand is honoured as on the CLI. if (not getattr(args, "remoteStoreExplicit", False) and not getattr(args, "no_remote_store", False)): - _rs = _system_opt("remote_store", None) + _rs, _rw = defaults_store_url(defaultsMeta) if _rs: - _rs = str(_rs).strip() - from bits_helpers.sync import normalise_store_url - _rw = _rs.endswith("::rw") - _rs = normalise_store_url(_rs[:-4] if _rw else _rs) if _rw and not getattr(args, "writeStore", ""): args.writeStore = _rs args.remoteStore = _rs @@ -3653,10 +3766,9 @@ def _build_row(pkg): buildTargets = [] - # Resolve the tag to the actual commit ref + # Resolve the tag to the actual commit ref, and every other hash input. for p in buildOrder: spec = specs[p] - spec["commit_hash"] = "0" develPackageBranch = "" # This is a development package (i.e. a local directory named like # spec["package"]), but there is no "source" key in its bits recipe, @@ -3670,103 +3782,28 @@ def _build_row(pkg): "source code from this directory, add a 'source:' key to " "{recipe}.sh instead." .format(package=p, recipe=p.lower())) - - # version_from: — take version (and, for a source-less package, tag + - # commit_hash) directly from a named defaults variable. Runs before the tag - # defaulting / source blocks so a synthetic package can be versioned by e.g. - # the LCG release without a source. No-op unless the recipe sets version_from. - apply_version_from(spec, defaultsMeta.get("variables")) - if "tag" not in spec: - spec["tag"] = spec["version"] - if "source" in spec: - # Tag may contain date params like %(year)s, %(month)s, %(day)s, %(hour), - # plus any variable from the defaults profile / the recipe (e.g. an - # override of tag: "%(release)s" driven by variables: release:). - spec["tag"] = resolve_tag(spec, defaultsMeta.get("variables")) - # First, we try to resolve the "tag" as a branch name, and use its tip as - # the commit_hash. If it's not a branch, it must be a tag or a raw commit - # hash, so we use it directly. Finally if the package is a development - # one, we use the name of the branch as commit_hash. - assert "scm_refs" in spec - try: - spec["commit_hash"] = spec["scm_refs"]["refs/heads/" + spec["tag"]] - except KeyError: - spec["commit_hash"] = spec["tag"] - # We are in development mode, we need to rebuild if the commit hash is - # different or if there are extra changes on top. - if spec["is_devel_pkg"]: - # Devel package: we get the commit hash from the checked source, not from remote. - out = spec["scm"].checkedOutCommitName(directory=spec["source"]) - spec["commit_hash"] = out.strip() - local_hash, untracked = hash_local_changes(spec) - untrackedFilesDirectories.extend(untracked) - spec["devel_hash"] = spec["commit_hash"] + local_hash - out = spec["scm"].branchOrRef(directory=spec["source"]) - develPackageBranch = out.replace("/", "-") - spec["tag"] = args.develPrefix if "develPrefix" in args else develPackageBranch - spec["commit_hash"] = "0" - - if "sources" in spec: - # Expand a templated tag (e.g. "v%(version)s") for tarball sources too. - # The git branch above only resolves it when a `source:` is present, so a - # tarball-only recipe kept the raw tag, which then leaked into commit_hash - # and the SOURCES/// path. No-op for literal tags. - spec["tag"] = resolve_tag(spec, defaultsMeta.get("variables")) - for i, s in enumerate(spec["sources"]): - resolved = resolveLocalPath(args.configDir, s) - spec["sources"][i] = resolved - spec["commit_hash"] = spec["tag"] - # Version may contain date params like tag, plus %(commit_hash)s, - # %(short_hash)s and %(tag)s. - spec["version"] = resolve_version(spec, args.defaults, branch_basename, branch_stream) - - spec.setdefault("variables", OrderedDict(spec.get("variables", {}))) - variables = spec["variables"] - if "Python" in spec.get("requires", []): - # Find the Python package spec safely - python_version_str = "" - py_spec = specs.get("Python") - if isinstance(py_spec, dict): - python_version_str = (py_spec.get("version", "") or "").replace("v", "") - python_version = python_version_str.split(".") if python_version_str else [] - - # Safely extract major, minor, patch versions - major = python_version[0] if len(python_version) > 0 else "0" - minor = python_version[1] if len(python_version) > 1 else "0" - patch = python_version[2] if len(python_version) > 2 else "0" - - # Populate variables dictionary - variables.update({ - "python_major_version": major, - "python_minor_version": minor, - "python_patch_version": patch, - "python_major_minor": f"{major}.{minor}", - "python_major_minor_str": f"{major}{minor}", - }) - for k, v in variables.items(): - variables[k] = resolve_spec_data(spec, v, args.defaults, branch_basename, branch_stream) - if "source" in spec: - spec["source"] = resolve_spec_data(spec, spec["source"], args.defaults, branch_basename, branch_stream) - if "sources" in spec: - spec["sources"] = [resolve_spec_data(spec, src, args.defaults, branch_basename, branch_stream) for src in spec["sources"]] - if "patches" in spec: - spec["patches"] = [resolve_spec_data(spec, p, args.defaults, branch_basename, branch_stream) for p in spec["patches"]] - # Variables defined in the active --defaults profile's `variables:` block are - # available to every recipe body. When a recipe does not itself opt into - # expansion (no `variables:` / `expand_recipe: true`) we expand it in SOFT - # mode: only known variables are substituted and any other %(...)s / bare % - # is left untouched, so profile-wide variables never clobber or break a - # recipe that happens to contain a literal %(...)s or shell `%`. - default_vars = defaultsMeta.get("variables") or None - recipe_opts_in = bool(variables or spec.get("expand_recipe", False)) - if recipe_opts_in or default_vars: - spec["recipe"] = resolve_spec_data(spec, spec["recipe"], args.defaults, - branch_basename, branch_stream, - default_vars=default_vars, - strict=recipe_opts_in) - - if spec["is_devel_pkg"] and "develPrefix" in args and args.develPrefix != "ali-master": - spec["version"] = args.develPrefix + assert "source" not in spec or "scm_refs" in spec + + # We are in development mode, we need to rebuild if the commit hash is + # different or if there are extra changes on top. + def _devel(spec): + nonlocal develPackageBranch + # Devel package: we get the commit hash from the checked source, not from remote. + out = spec["scm"].checkedOutCommitName(directory=spec["source"]) + spec["commit_hash"] = out.strip() + local_hash, untracked = hash_local_changes(spec) + untrackedFilesDirectories.extend(untracked) + spec["devel_hash"] = spec["commit_hash"] + local_hash + out = spec["scm"].branchOrRef(directory=spec["source"]) + develPackageBranch = out.replace("/", "-") + spec["tag"] = args.develPrefix if "develPrefix" in args else develPackageBranch + spec["commit_hash"] = "0" + + prepare_hash_inputs( + spec, specs, args.defaults, defaultsMeta.get("variables"), args.configDir, + branch_basename, branch_stream, devel=_devel, + devel_version=(args.develPrefix if "develPrefix" in args + and args.develPrefix != "ali-master" else None)) # Decide what is the main package we are building and at what commit. # diff --git a/bits_helpers/plan.py b/bits_helpers/plan.py index ecbac871..aaadbe39 100644 --- a/bits_helpers/plan.py +++ b/bits_helpers/plan.py @@ -99,12 +99,17 @@ def classify(spec, arch, work_dir, sync_helper, can_list, write_store, trusted, if rev is not None: state = INSTALLED if _installed(spec, work_dir, eff, rev, h) else LOCAL_TARBALL return state, h, rev + # A remote tarball the signed manifest does not vouch for is discarded by + # the build, which then builds as if the store had none (the hash-policy + # fallback below included). + unsigned = None if can_list: for h in remote: rev = pick_revision(sync_helper.list_store_tarballs(eff, h), spec, eff, forced=forced) if rev is not None: if trusted is not None and h not in trusted: - return REMOTE_UNSIGNED, h, rev + unsigned = (h, rev) + break return REMOTE, h, rev if (spec.get("_revision_policy_hash_injected") and not write_store and not is_virtual_package(spec)): @@ -119,7 +124,9 @@ def classify(spec, arch, work_dir, sync_helper, can_list, write_store, trusted, 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"] + return (REMOTE_UNSIGNED if unsigned else BUILD), spec["hash"], spec["revision"] + if unsigned: + return (REMOTE_UNSIGNED,) + unsigned 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 f67c49ef..543bb9f4 100644 --- a/bits_helpers/status.py +++ b/bits_helpers/status.py @@ -28,7 +28,6 @@ import re import shutil import sys -from collections import OrderedDict from glob import glob from os.path import abspath, basename, dirname, join from pathlib import Path @@ -39,11 +38,7 @@ 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, @@ -142,82 +137,6 @@ def _try_populate_refs(spec: dict, reference_sources: str, package: str) -> None spec.setdefault("scm_refs", {}) -def _resolve_commit_hash(spec: dict, default_vars=None) -> None: - """Set spec["commit_hash"] from scm_refs, falling back to the tag string.""" - if "tag" not in spec: - spec["tag"] = spec["version"] - # Expand date-based tags (%(year)s etc.) and defaults/recipe variables, so - # `bits status` resolves the same tag the build does. - try: - spec["tag"] = resolve_tag(spec, default_vars) - except (KeyError, ValueError): - pass - if "source" not in spec: - # 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. - spec["commit_hash"] = ( - scm_refs.get("refs/heads/" + spec["tag"]) - or spec["tag"] - ) - - -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. @@ -440,7 +359,10 @@ def _emit_json(rows: List[dict], architecture: str) -> None: def doStatus(args, parser) -> None: """Resolve the dependency tree and report the build state of each package.""" # Deferred heavy imports (build.py pulls in jinja2, analytics, etc.) - from bits_helpers.build import storeHook, hash_local_changes + from bits_helpers.build import ( + add_initdotsh_hash_marker, apply_defaults_legacy_initdotsh, + defaults_store_url, hash_local_changes, prepare_hash_inputs, + resolve_initdotsh_mode, storeHook) from bits_helpers.hashing import storeHashes from bits_helpers.log import dieOnError from bits_helpers.git import git @@ -455,24 +377,23 @@ def doStatus(args, parser) -> None: resolve_config_dir(args) # ── Defaults and overrides ───────────────────────────────────────────────── + # The init.sh mode is part of the defaults-release hash: decide it as build + # does (BITS_LEGACY_INITDOTSH, else a defaults file's legacy_initdotsh, else + # from modules), so that status computes the same package identities. + initdotsh_explicit = resolve_initdotsh_mode(args) + 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" + add_initdotsh_hash_marker(meta, args.initdotshFromModules) return meta, body err, overrides, taps, defaults_meta = parseDefaults( args.disable, defaults_reader, debug, args.architecture, args.configDir ) dieOnError(err, err) + apply_defaults_legacy_initdotsh(args, defaults_meta, initdotsh_explicit) raw_architecture = args.architecture args.architecture = compute_combined_arch(defaults_meta, args.defaults, raw_architecture) @@ -604,6 +525,12 @@ def defaults_reader(): remote_store_url = (getattr(args, "remoteStore", "") or os.environ.get("BITS_REMOTE_STORE") or os.environ.get("REMOTE_STORE") or "") + # As in build: without a store from the command line or environment, the + # one the defaults name (system: remote_store, ::rw for a writable one). + if not remote_store_url and not getattr(args, "no_remote_store", False): + remote_store_url, defaults_rw = defaults_store_url(defaults_meta) + if defaults_rw and not write_store_url: + write_store_url = remote_store_url if remote_store_url.endswith("::rw"): if write_store_url: parser.error("cannot specify ::rw and --write-store at the same time") @@ -662,29 +589,28 @@ def defaults_reader(): else: spec.setdefault("scm_refs", {}) - # 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"]: + def devel(spec): try: out = spec["scm"].checkedOutCommitName(directory=spec["source"]) spec["commit_hash"] = out.strip() local_hash, _ = hash_local_changes(spec) spec["devel_hash"] = spec["commit_hash"] + local_hash out = spec["scm"].branchOrRef(directory=spec["source"]) - dev_branch = out.replace("/", "-") - spec["tag"] = (getattr(args, "develPrefix", None) or dev_branch) + spec["tag"] = out.replace("/", "-") spec["commit_hash"] = "0" except Exception as exc: - debug("Could not compute devel_hash for %s: %s", p, exc) + debug("Could not compute devel_hash for %s: %s", spec["package"], exc) spec.setdefault("devel_hash", "") + # Resolve exactly the hash inputs that doBuild prepares before + # storeHashes (the same function): 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. + prepare_hash_inputs( + spec, specs, args.defaults, defaults_meta.get("variables"), + args.configDir, branch_basename, branch_stream, devel=devel) + # Compute build hashes (same as doBuild main loop) consider_relocation = ( raw_architecture.startswith("osx") diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index d2f8d12a..2e05b6e3 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -3020,9 +3020,14 @@ 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. 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. +read-only store, or none, 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, and its dependents then hash in that local hash. With a +writable store (`--write-store`, `::rw`, also from the defaults' +`system: remote_store`) it keeps the remote hash. Uploads keep using the same +content-addressed store paths. `bits status` and `bits plan` make the same +choice; without `--check-store`, or for a store that cannot be listed, `bits +status` cannot see what the store holds and assumes a local build. Omitting `revision_policy` retains the existing revision-counter behavior. #### How the install path changes diff --git a/tests/test_plan.py b/tests/test_plan.py index 818f95fe..0eeaf54c 100644 --- a/tests/test_plan.py +++ b/tests/test_plan.py @@ -98,6 +98,16 @@ def test_hash_policy_local_fallback_finds_installed_local_build(self): self._installed(LH, LH) self.assertEqual(self._classify(**spec), (plan.INSTALLED, LH, LH)) + def test_hash_policy_unsigned_remote_falls_back_to_local_hash(self): + """Build discards an unsigned remote tarball and builds under the local hash.""" + spec = _spec(revision_policy="hash", _revision_policy_hash_injected=True, + force_revision=RH, hash=RH, revision=RH) + store = _Store(["zlib-1.3-%s.%s.tar.gz" % (RH, ARCH)]) + self.assertEqual(self._classify(store, trusted={}, **spec), + (plan.REMOTE_UNSIGNED, LH, LH)) + self.assertEqual(self._classify(store, trusted={RH: "sha"}, **spec), + (plan.REMOTE, RH, RH)) + def test_hash_policy_fallback_does_not_change_virtual_package_hash(self): spec = _spec( provides_repository=True, _revision_policy_hash_injected=True, diff --git a/tests/test_status.py b/tests/test_status.py index e5ef8bf4..b9e31b09 100644 --- a/tests/test_status.py +++ b/tests/test_status.py @@ -24,13 +24,11 @@ _emit_json, _emit_table, _is_already_installed, - _prepare_spec_for_hash, - _resolve_commit_hash, _scan_local_tars, _try_populate_refs, ) from bits_helpers.hashing import storeHashes -from bits_helpers.build import add_initdotsh_hash_marker +from bits_helpers.build import add_initdotsh_hash_marker, prepare_hash_inputs # ── Helpers ──────────────────────────────────────────────────────────────────── @@ -138,75 +136,49 @@ def test_git_failure_falls_back_to_empty(self): self.assertEqual(spec.get("scm_refs"), {}) -# ── _resolve_commit_hash ─────────────────────────────────────────────────────── +# ── prepare_hash_inputs (shared with build) ─────────────────────────────────── -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" +class TestPrepareHashInputs(unittest.TestCase): + """The hash inputs status resolves, with build's own function.""" - 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 _prepare(self, spec, specs=None, default_vars=None): + spec.setdefault("scm_refs", {}) + prepare_hash_inputs(spec, specs or {spec["package"]: spec}, ["release"], + default_vars or {}, "/recipes") def test_branch_ref_resolved(self): spec = _make_spec(tag="main") spec["scm_refs"] = {"refs/heads/main": "deadbeef"} - _resolve_commit_hash(spec) + self._prepare(spec) self.assertEqual(spec["commit_hash"], "deadbeef") def test_tag_falls_back_to_tag_string(self): spec = _make_spec(tag="v1.0") - spec["scm_refs"] = {} - _resolve_commit_hash(spec) + self._prepare(spec) self.assertEqual(spec["commit_hash"], "v1.0") def test_no_source_sets_zero(self): spec = _make_spec() del spec["source"] - spec["scm_refs"] = {} - _resolve_commit_hash(spec) + self._prepare(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.""" + """Tarball-only recipes 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._prepare(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._prepare(spec) 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"]) @@ -215,11 +187,32 @@ def test_hash_inputs_expand_recipe_variables_like_build(self): def test_date_tag_expanded(self): spec = _make_spec(tag="v%(year)s-1") - spec["scm_refs"] = {} - _resolve_commit_hash(spec) - # The tag should be expanded (year substituted) + self._prepare(spec) self.assertNotIn("%(year)s", spec["tag"]) + def test_python_version_variables(self): + """A package requiring Python gets build's python_* variables.""" + spec = _make_spec(pkg="pyext") + spec["requires"] = ["Python"] + spec["recipe"] = "pip install --prefix lib/python%(python_major_minor)s" + python = _make_spec(pkg="Python", version="v3.12.4") + self._prepare(spec, specs={"pyext": spec, "Python": python}) + self.assertEqual(spec["variables"]["python_major_minor_str"], "312") + self.assertEqual(spec["recipe"], "pip install --prefix lib/python3.12") + + def test_devel_package_tag_before_version(self): + """The devel callback runs before the version is resolved.""" + spec = _make_spec(version="%(tag)s", tag="v1.0", is_devel=True) + + def devel(s): + s["tag"] = "my-branch" + s["commit_hash"] = "0" + spec["scm_refs"] = {} + prepare_hash_inputs(spec, {"mylib": spec}, ["release"], {}, "/recipes", + devel=devel) + self.assertEqual(spec["version"], "my-branch") + self.assertEqual(spec["commit_hash"], "0") + # ── _is_already_installed ────────────────────────────────────────────────────── @@ -620,6 +613,14 @@ def _make_recipe_dir(self, pkg, version="1.0", tag=None, requires=None): def test_defaults_release_hash_matches_build_hash(self): """Status must hash defaults-release like build's defaults reader.""" + self._check_defaults_release_hash(legacy=False) + + def test_defaults_legacy_initdotsh_matches_build_hash(self): + """A defaults file selecting the legacy init.sh (alidist) drops the marker + in status too, as build's apply_defaults_legacy_initdotsh does.""" + self._check_defaults_release_hash(legacy=True) + + def _check_defaults_release_hash(self, legacy): from copy import deepcopy import argparse import bits_helpers.status as status_mod @@ -629,6 +630,8 @@ def test_defaults_release_hash_matches_build_hash(self): ("DCMAKE_BUILD_TYPE", "Release"), ]) defaults_meta = {"env": base_env.copy(), "variables": {}} + if legacy: + defaults_meta["system"] = {"legacy_initdotsh": True} status_spec = OrderedDict({ "package": "defaults-release", "version": "vCMS", @@ -668,7 +671,9 @@ def get_packages(*args, **kwargs): patch("bits_helpers.repo_provider.fetch_repo_providers_iteratively", return_value={}), \ patch("bits_helpers.build.storeHook"), \ + patch.dict(os.environ), \ patch("sys.stdout", new_callable=StringIO) as out: + os.environ.pop("BITS_LEGACY_INITDOTSH", None) parser = argparse.ArgumentParser() parser.error = lambda msg: (_ for _ in ()).throw(SystemExit(msg)) status_mod.doStatus(args, parser) @@ -678,7 +683,7 @@ def get_packages(*args, **kwargs): # 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) + add_initdotsh_hash_marker(build_meta, enabled=not legacy) build_spec = OrderedDict({ "package": "defaults-release", "version": "vCMS", @@ -741,22 +746,39 @@ 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): + def test_write_store_keeps_hash_policy_on_remote_hash(self): """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 + self.assertEqual(self._hash_policy_status(write_store="b3://bits-write"), self.RH) + + def test_no_store_hash_policy_uses_local_hash(self): + """Without a writable store, a package to build gets its local hash.""" + self.assertEqual(self._hash_policy_status(), self.LH) + + def test_write_store_from_defaults(self): + """A writable store named in the defaults (system: remote_store: …::rw) + counts as for build.""" + meta = {"system": {"remote_store": "b3://bits-write::rw"}} + self.assertEqual(self._hash_policy_status(defaults_meta=meta), self.RH) + + def test_cli_store_wins_over_defaults(self): + """A read-only store from the command line wins over the defaults' ::rw.""" + meta = {"system": {"remote_store": "b3://bits-write::rw"}} + self.assertEqual( + self._hash_policy_status(defaults_meta=meta, remote_store="b3://bits-read"), + self.LH) + + def test_no_remote_store_ignores_defaults_store(self): + meta = {"system": {"remote_store": "b3://bits-write::rw"}} + self.assertEqual( + self._hash_policy_status(defaults_meta=meta, no_remote_store=True), self.LH) + + RH = "aabbcc" + "0" * 34 + LH = "ddeeff" + "0" * 34 + def _hash_policy_status(self, write_store="", remote_store="", defaults_meta=None, + no_remote_store=False): + """The hash status reports for a hash-policy package not built anywhere.""" + rh, lh = self.RH, self.LH spec = _make_spec(pkg="mylib", version="1.0", remote_revision_hash=rh, local_revision_hash=lh, remote_hashes=[rh], local_hashes=[lh]) @@ -765,7 +787,6 @@ def test_write_store_keeps_hash_policy_on_remote_hash(self, mock_hook, 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] @@ -776,24 +797,31 @@ def fake_store_hashes(p, specs, considerRelocation): 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") + args = self._make_args(["mylib"], json_output=True, write_store=write_store) + args.remoteStore = remote_store + args.no_remote_store = no_remote_store + import argparse 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) + parser = argparse.ArgumentParser() + parser.error = lambda msg: (_ for _ in ()).throw(SystemExit(msg)) + with patch("bits_helpers.status.getPackageList", side_effect=fake_get_pkg_list), \ + patch("bits_helpers.status.parseDefaults", + return_value=(None, {}, {}, defaults_meta or {})), \ + patch("bits_helpers.status.readDefaults", return_value=None), \ + patch("bits_helpers.hashing.storeHashes", side_effect=fake_store_hashes), \ + patch("bits_helpers.build.storeHook"), \ + patch("bits_helpers.status.topological_sort", return_value=["mylib"]), \ + patch("bits_helpers.status.compute_combined_arch", return_value=self.arch), \ + patch("bits_helpers.status.prunePaths"), \ + patch.dict(os.environ), \ + patch("sys.stdout", new_callable=StringIO) as mock_out: + for v in ("BITS_REMOTE_STORE", "REMOTE_STORE", "BITS_WRITE_STORE", "WRITE_STORE"): + os.environ.pop(v, None) + status_mod.doStatus(args, parser) + package = json.loads(mock_out.getvalue())["packages"][0] self.assertEqual(package["state"], BUILD_FROM_SOURCE) + return package["hash"] @patch("bits_helpers.status.getPackageList") @patch("bits_helpers.status.parseDefaults")