From 69819ebf88d9b0b3706b591cd927415285c8d955 Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Fri, 11 Sep 2026 18:44:45 -0400 Subject: [PATCH] fix: retain apt package source identity Keep indexes from different URLs separate even when suite, component, and architecture match. Scope snapshot facts by source URL and retain the selected package's URLs instead of reconstructing them from the suite. Continue reading existing v2 locks and refresh their provenance when packages are resolved. Cover mixed snapshot/rolling sources and package URLs that differ from the suite's URLs. --- apt/extensions.bzl | 43 ++++++++++-------------- apt/private/lockfile.bzl | 12 +++++++ apt/private/translate_dependency_set.bzl | 6 +--- apt/private/util.bzl | 6 ++-- apt/tests/facts_test.bzl | 6 ++-- apt/tests/lockfile_test.bzl | 33 ++++++++++++++++++ 6 files changed, 69 insertions(+), 37 deletions(-) diff --git a/apt/extensions.bzl b/apt/extensions.bzl index 5969d27..8166073 100644 --- a/apt/extensions.bzl +++ b/apt/extensions.bzl @@ -23,7 +23,7 @@ def _get_auth(mctx, urls): netrc = read_user_netrc(mctx) return use_netrc(netrc, urls, {}) -def _start_downloads(mctx, urls, dist, comp, arch, integrity, index_type, cached_format = None): +def _start_downloads(mctx, urls, dist, comp, arch, integrity, index_type, source_id, cached_format = None): """Initiate all format downloads for a given index type with block=False. If cached_format is set, only that extension is attempted — avoiding @@ -63,7 +63,7 @@ def _start_downloads(mctx, urls, dist, comp, arch, integrity, index_type, cached # Without this, the uncompressed variant ("") and a decompressed # .xz/.gz/.bz2 would both write to the same final path. ext_name = ext.lstrip(".") if ext else "raw" - output = "{}/{}/{}/{}{}".format(target_triple, url_idx, ext_name, index_type, ext) + output = "{}/{}/{}/{}/{}{}".format(source_id, target_triple, url_idx, ext_name, index_type, ext) if index_type == "Packages": dist_url = "{}/dists/{}/{}/binary-{}/{}{}".format(url, dist, comp, arch, index_type, ext) else: @@ -98,10 +98,7 @@ def _resolve_downloads(mctx, tokens, index_type, dist, comp, arch): if download.success: decompress_r = mctx.execute(cmd + [output]) if decompress_r.return_code == 0: - target_triple = "{}/{}/{}".format(dist, comp, arch) - - # Decompressed file lives in its own ext_name subdirectory - result = ("{}/{}/{}/{}".format(target_triple, url_idx, ext_name, index_type), url, download.integrity, ext) + result = (output.removesuffix(ext) if ext else output, url, download.integrity, ext) continue failed_attempts.append((url + "/.../" + index_type + ext, download, decompress_r)) if result != None: @@ -129,7 +126,7 @@ def _resolve_downloads(mctx, tokens, index_type, dist, comp, arch): {} """.format(len(failed_attempts), "\n".join(attempt_messages))) -def _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats): +def _fetch_and_parse_sources(mctx, repo, glock, snapshot_indices, formats): """Fetch all package indices and contents in parallel, then parse them. Returns the set (as a dict) of fact keys that belong to the current sources, @@ -143,8 +140,8 @@ def _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats): # Deduplicate: multiple dict entries can map to the same logical source # (one entry per URL in the urls list). Only process each unique - # (dist, component, architecture) combination once. - dedup_key = "{}/{}/{}".format(dist, component, architecture) + # (URLs, dist, component, architecture) combination once. + dedup_key = util.index_fact_key(dist, component, architecture, "Packages", urls) if dedup_key in seen: continue seen[dedup_key] = True @@ -158,6 +155,9 @@ def _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats): cnt_fact_key = util.index_fact_key(dist, component, architecture, "Contents", urls) used_keys[pkg_fact_key] = True used_keys[cnt_fact_key] = True + if urls and all([util.is_snapshot_uri(url) for url in urls]): + snapshot_indices[pkg_fact_key] = True + snapshot_indices[cnt_fact_key] = True # Check cached format info to avoid 404 warnings on subsequent runs cached_pkg_format = formats.get(pkg_fact_key) @@ -175,6 +175,7 @@ def _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats): architecture, glock.facts().get(pkg_fact_key, ""), "Packages", + source_id = len(seen), cached_format = cached_pkg_format, ) @@ -188,6 +189,7 @@ def _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats): architecture, glock.facts().get(cnt_fact_key, ""), "Contents", + source_id = len(seen), cached_format = cached_cnt_format, ) @@ -206,7 +208,7 @@ def _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats): for (urls, dist, comp, arch, pkg_tokens, cnt_tokens, pkg_fk, cnt_fk) in pending: mctx.report_progress("resolving Package indices: {}/{} for {}".format(dist, comp, arch)) (output, url, integrity, ext) = _resolve_downloads(mctx, pkg_tokens, "Packages", dist, comp, arch) - if dist in snapshot_suites: + if pkg_fk in snapshot_indices: glock.facts()[pkg_fk] = integrity formats[pkg_fk] = ext @@ -221,7 +223,7 @@ def _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats): if contents_result != None: (output, url, integrity, ext) = contents_result - if dist in snapshot_suites: + if cnt_fk in snapshot_indices: glock.facts()[cnt_fk] = integrity formats[cnt_fk] = ext @@ -278,15 +280,7 @@ def _distroless_extension(mctx): for lock in mod.tags.lock ]) - # First pass over sources_list: classify suites as snapshot or rolling - snapshot_suites = {} - for mod in mctx.modules: - for sl in mod.tags.sources_list: - uris = [uri.removeprefix("mirror+") for uri in sl.uris] - is_snapshot = len(uris) > 0 and all([util.is_snapshot_uri(uri) for uri in uris]) - if is_snapshot: - for suite in sl.suites: - snapshot_suites[suite] = True + snapshot_indices = {} repo = deb_repository.new() resolver = dependency_resolver.new(repo) @@ -317,7 +311,7 @@ def _distroless_extension(mctx): # Fetch all sources_list in parallel and parse them. `used_keys` is the set # of fact keys for the current sources, used below to prune stale facts. - used_keys = _fetch_and_parse_sources(mctx, repo, glock, snapshot_suites, formats) + used_keys = _fetch_and_parse_sources(mctx, repo, glock, snapshot_indices, formats) sources = glock.sources() dependency_sets = glock.dependency_sets() @@ -504,10 +498,7 @@ def _distroless_extension(mctx): deb_import( name = repo_name, target_name = repo_name, - urls = [ - uri + "/" + package["filename"] - for uri in sources[package["suite"]]["uris"] - ], + urls = package["urls"], sha256 = package["sha256"], mergedusr = mergedusr, depends_on = package["depends_on"], @@ -541,7 +532,7 @@ def _distroless_extension(mctx): glock.facts(), formats, used_keys, - snapshot_suites, + snapshot_indices, ) return mctx.extension_metadata( facts = {"indices": cacheable_indices, "formats": cacheable_formats}, diff --git a/apt/private/lockfile.bzl b/apt/private/lockfile.bzl index bdb2af8..70519b6 100644 --- a/apt/private/lockfile.bzl +++ b/apt/private/lockfile.bzl @@ -40,6 +40,8 @@ def _package_key(package, arch = None): def _add_package(lock, package, arch = None): k = _package_key(package, arch) if k in lock.packages: + # Refresh provenance from the selected index when reading an older lock. + lock.packages[k]["urls"] = [root + "/" + package["Filename"] for root in package["Roots"]] return lock.packages[k] = { "name": package["Package"], @@ -47,6 +49,7 @@ def _add_package(lock, package, arch = None): "architecture": package["Architecture"], "sha256": package["SHA256"], "filename": package["Filename"], + "urls": [root + "/" + package["Filename"] for root in package["Roots"]], "suite": package["Dist"], "section": package["Section"], "size": int(package["Size"]), @@ -116,6 +119,15 @@ def _from_json(mctx, content): if lock["version"] != 2: fail("lock file version %d is not supported anymore. please upgrade your lock file" % lock["version"]) + # Existing v2 locks only recorded source URLs at suite granularity. + # Preserve their download locations until a selected index refreshes them. + for package in lock.get("packages", {}).values(): + if "urls" not in package: + package["urls"] = [ + root + "/" + package["filename"] + for root in lock["sources"][package["suite"]]["uris"] + ] + lock = struct( version = lock["version"], dependency_sets = lock["dependency_sets"] if "dependency_sets" in lock else dict(), diff --git a/apt/private/translate_dependency_set.bzl b/apt/private/translate_dependency_set.bzl index d0147bf..cf52ddd 100644 --- a/apt/private/translate_dependency_set.bzl +++ b/apt/private/translate_dependency_set.bzl @@ -152,7 +152,6 @@ def _translate_dependency_set_impl(rctx): package_template = rctx.read(rctx.attr.package_template) lockf = lockfile.from_json(rctx, rctx.attr.lock_content) - sources = lockf.sources() packages = lockf.packages() dependency_sets = lockf.dependency_sets() dependency_set = dependency_sets[rctx.attr.depset_name] @@ -209,10 +208,7 @@ Please unify the versions manually, or use separate `apt.install` calls (with di control_targets = '"@%s//:control"' % repo_name, src = '"@%s//:data"' % repo_name, deps = package_deps_for_architecture(packages, package, architecture, mergedusr = rctx.attr.mergedusr), - urls = [ - uri + "/" + package["filename"] - for uri in sources[package["suite"]]["uris"] - ], + urls = package["urls"], name = package["name"], arch = package["architecture"], sha256 = package["sha256"], diff --git a/apt/private/util.bzl b/apt/private/util.bzl index 901dea1..066a2e2 100644 --- a/apt/private/util.bzl +++ b/apt/private/util.bzl @@ -61,14 +61,14 @@ def _index_fact_key(dist, component, architecture, index_type, urls): url_token = "|".join(sorted_deduplicated_urls) return "{}/{}/{}/{}/{}".format(dist, component, architecture, index_type, url_token) -def _prune_uncacheable_facts(indices, formats, used_keys, snapshot_suites): +def _prune_uncacheable_facts(indices, formats, used_keys, snapshot_indices): """Keep only the facts that can be cached. `used_keys` holds the fact keys produced for this run's sources (see `index_fact_key`). Entries left over from a previous snapshot URL are not in `used_keys`, so they get dropped here instead of accumulating across runs. - `snapshot_suites` holds a list of suites from snapshots, + `snapshot_indices` holds the fact keys from snapshot sources, because we don't want to cache rolling suites. Returns `(cacheable_indices, cacheable_formats)`. @@ -76,7 +76,7 @@ def _prune_uncacheable_facts(indices, formats, used_keys, snapshot_suites): cacheable_indices = { k: v for k, v in indices.items() - if k in used_keys and k.split("/")[0] in snapshot_suites + if k in used_keys and k in snapshot_indices } cacheable_formats = { k: v diff --git a/apt/tests/facts_test.bzl b/apt/tests/facts_test.bzl index a1054f0..c1fe4da 100644 --- a/apt/tests/facts_test.bzl +++ b/apt/tests/facts_test.bzl @@ -45,16 +45,16 @@ def _prune_facts_test(ctx): old_key = util.index_fact_key("bookworm", "main", "amd64", "Packages", _TEST_SNAPSHOT_1) new_key = util.index_fact_key("bookworm", "main", "amd64", "Packages", _TEST_SNAPSHOT_2) - rolling_key = util.index_fact_key("sid", "main", "amd64", "Packages", ["https://deb.debian.org/debian"]) + rolling_key = util.index_fact_key("bookworm", "main", "amd64", "Packages", ["https://deb.debian.org/debian"]) indices = {old_key: "sha256-OLD", new_key: "sha256-NEW", rolling_key: "sha256-ROLLING"} formats = {old_key: ".xz", new_key: ".xz", rolling_key: ".xz"} # Only the current sources' keys are used this run. used_keys = {new_key: True, rolling_key: True} - snapshot_suites = {"bookworm": True} + snapshot_indices = {new_key: True} - (cacheable_indices, cacheable_formats) = util.prune_uncacheable_facts(indices, formats, used_keys, snapshot_suites) + (cacheable_indices, cacheable_formats) = util.prune_uncacheable_facts(indices, formats, used_keys, snapshot_indices) # The stale previous-URL entry is dropped, and indices from rolling indexes are not cached. asserts.equals(env, {new_key: "sha256-NEW"}, cacheable_indices) diff --git a/apt/tests/lockfile_test.bzl b/apt/tests/lockfile_test.bzl index 384b710..6b2be40 100644 --- a/apt/tests/lockfile_test.bzl +++ b/apt/tests/lockfile_test.bzl @@ -63,4 +63,37 @@ def _add_source_merges_architectures_test(ctx): add_source_merges_architectures_test = unittest.make(_add_source_merges_architectures_test) def lockfile_tests(): + package_source_urls_test(name = _TEST_SUITE_PREFIX + "package_source_urls") add_source_merges_architectures_test(name = _TEST_SUITE_PREFIX + "add_source_merges_architectures") + +def _package_source_urls_test(ctx): + env = unittest.begin(ctx) + lock = lockfile.empty(struct()) + package = { + "Architecture": "amd64", + "Dist": "noble", + "Filename": "pool/main/e/example_1_amd64.deb", + "Package": "example", + "Roots": ["https://example.org/ppa"], + "SHA256": "abc", + "Section": "libs", + "Size": "1", + "Version": "1", + } + lock.add_source("noble", ["deb"], ["https://example.org/ubuntu"], ["main"], ["amd64"]) + lock.add_package(package) + key = lockfile.package_key(package) + expected = ["https://example.org/ppa/" + package["Filename"]] + asserts.equals(env, expected, lock.packages()[key]["urls"]) + + # A v2 lock without per-package URLs remains readable. Resolving its + # package again must replace the old suite URL with the selected PPA URL. + legacy = json.decode(lock.as_json()) + legacy["packages"][key].pop("urls") + restored = lockfile.from_json(struct(), json.encode(legacy)) + asserts.equals(env, ["https://example.org/ubuntu/" + package["Filename"]], restored.packages()[key]["urls"]) + restored.add_package(package) + asserts.equals(env, expected, restored.packages()[key]["urls"]) + return unittest.end(env) + +package_source_urls_test = unittest.make(_package_source_urls_test)