Prevent apt lockfiles from containing multiple versions

apt/private/lockfile.bzl

@@ -37,10 +37,24 @@ def _short_package_key(package, arch = None):
 def _package_key(package, arch = None):
     return _make_package_key(package["Dist"], package["Package"], package["Version"], arch or package["Architecture"])
 
-def _add_package(lock, package, arch = None):
-    k = _package_key(package, arch)
-    if k in lock.packages:
+def _package_identity(name, arch):
+    return "%s:%s" % (name, arch)
+
+def _index_packages(packages):
+    package_index = {}
+    for package_key in packages:
+        (_, name, arch, _) = _parse_package_key(package_key)
+        identity = _package_identity(name, arch)
+        if identity not in package_index:
+            package_index[identity] = package_key
+    return package_index
+
+def _add_package(lock, package_index, package, arch = None):
+    target_arch = arch or package["Architecture"]
+    identity = _package_identity(package["Package"], target_arch)
+    if identity in package_index:
         return
+    k = _package_key(package, arch)
     lock.packages[k] = {
         "name": package["Package"],
         "version": package["Version"],
@@ -52,12 +66,16 @@ def _add_package(lock, package, arch = None):
         "size": int(package["Size"]),
         "depends_on": [],
     }
+    package_index[identity] = k
 
-def _add_package_dependency(lock, package, dependency, arch = None):
-    k = _package_key(package, arch)
-    if k not in lock.packages:
+def _add_package_dependency(lock, package_index, package, dependency, arch = None):
+    target_arch = arch or package["Architecture"]
+    k = package_index.get(_package_identity(package["Package"], target_arch))
+    if not k:
         fail("illegal state: %s is not in the lockfile." % package["Package"])
-    sk = _package_key(dependency, arch)
+    sk = package_index.get(_package_identity(dependency["Package"], target_arch))
+    if not sk:
+        fail("illegal state: %s is not in the lockfile." % dependency["Package"])
     if sk in lock.packages[k]["depends_on"]:
         return
     lock.packages[k]["depends_on"].append(sk)
@@ -82,11 +100,12 @@ def _add_source(lock, suite, types, uris, components, architectures):
     }
 
 def _create(mctx, lock):
+    package_index = _index_packages(lock.packages)
     return struct(
         has_package = lambda *args, **kwargs: _has_package(lock, *args, **kwargs),
         add_source = lambda *args, **kwargs: _add_source(lock, *args, **kwargs),
-        add_package = lambda *args, **kwargs: _add_package(lock, *args, **kwargs),
-        add_package_dependency = lambda *args, **kwargs: _add_package_dependency(lock, *args, **kwargs),
+        add_package = lambda *args, **kwargs: _add_package(lock, package_index, *args, **kwargs),
+        add_package_dependency = lambda *args, **kwargs: _add_package_dependency(lock, package_index, *args, **kwargs),
         packages = lambda: lock.packages,
         sources = lambda: lock.sources,
         dependency_sets = lambda: lock.dependency_sets,
@@ -126,15 +145,20 @@ def _from_json(mctx, content):
     return _create(mctx, lock)
 
 def _merge(mctx, locks):
-    mlock = _empty(mctx)
-    packages = mlock.packages()
-    facts = mlock.facts()
+    packages = {}
+    facts = {}
     for lock in locks:
         for (key, pkg) in lock.packages().items():
             packages[key] = pkg
         for (key, fact) in lock.facts().items():
             facts[key] = fact
-    return mlock
+    return _create(mctx, struct(
+        version = 2,
+        dependency_sets = {},
+        packages = packages,
+        sources = {},
+        facts = facts,
+    ))
 
 lockfile = struct(
     empty = _empty,

apt/tests/lockfile_test.bzl

@@ -5,6 +5,18 @@ load("//apt/private:lockfile.bzl", "lockfile")
 
 _TEST_SUITE_PREFIX = "lockfile/"
 
+def _package(name, version, dist, architecture = "amd64"):
+    return {
+        "Architecture": architecture,
+        "Dist": dist,
+        "Filename": "pool/{}/{}_{}_{}.deb".format(name, name, version, architecture),
+        "Package": name,
+        "SHA256": "0" * 64,
+        "Section": "libs",
+        "Size": "1",
+        "Version": version,
+    }
+
 # Regression test for `_add_source`: previously, calling `add_source` twice
 # for the same suite (e.g. once per architecture, as `apt_deb_repository`
 # does when resolving multiple architectures for the same suite) made the
@@ -62,5 +74,45 @@ def _add_source_merges_architectures_test(ctx):
 
 add_source_merges_architectures_test = unittest.make(_add_source_merges_architectures_test)
 
+def _add_package_reuses_name_and_architecture_test(ctx):
+    env = unittest.begin(ctx)
+
+    lock = lockfile.empty(struct())
+    updated_libssl = _package(
+        name = "libssl3t64",
+        version = "3.0.13-0ubuntu3.9",
+        dist = "noble-updates",
+    )
+    release_libssl = _package(
+        name = "libssl3t64",
+        version = "3.0.13-0ubuntu3",
+        dist = "noble",
+    )
+    librabbitmq = _package(
+        name = "librabbitmq4",
+        version = "0.11.0-1build2",
+        dist = "noble",
+    )
+
+    lock.add_package(updated_libssl)
+    lock.add_package(release_libssl)
+    lock.add_package(librabbitmq)
+    lock.add_package_dependency(librabbitmq, release_libssl)
+
+    updated_libssl_key = lockfile.package_key(updated_libssl)
+    release_libssl_key = lockfile.package_key(release_libssl)
+    librabbitmq_key = lockfile.package_key(librabbitmq)
+    packages = lock.packages()
+
+    asserts.equals(env, 2, len(packages))
+    asserts.true(env, updated_libssl_key in packages)
+    asserts.false(env, release_libssl_key in packages)
+    asserts.equals(env, [updated_libssl_key], packages[librabbitmq_key]["depends_on"])
+
+    return unittest.end(env)
+
+add_package_reuses_name_and_architecture_test = unittest.make(_add_package_reuses_name_and_architecture_test)
+
 def lockfile_tests():
     add_source_merges_architectures_test(name = _TEST_SUITE_PREFIX + "add_source_merges_architectures")
+    add_package_reuses_name_and_architecture_test(name = _TEST_SUITE_PREFIX + "add_package_reuses_name_and_architecture")