From bbf5d62ccfec6e5ff92b41bc22a4e7e906a39bc8 Mon Sep 17 00:00:00 2001 From: Javier Maestro Date: Thu, 19 Sep 2024 01:07:49 +0100 Subject: [PATCH] fix: NVIDIA CUDA flat repos don't follow Debian repo spec Although the Debian repo spec for 'Filename' (see https://wiki.debian.org/DebianRepository/Format#Filename) clearly says that 'Filename' should be relative to the base directory of the repo and should be in canonical form (i.e. without '.' or '..') there are cases where this is not honored. In those cases we try to work around this by assuming 'Filename' is relative to the sources.list directory/ so we combine them and normalize the new 'Filename' path. Note that, so far, only the NVIDIA CUDA repos needed this workaround so maybe this heuristic will break for other repos that don't conform to the Debian repo spec. --- MODULE.bazel | 4 +- MODULE.bazel.lock | 12 ++-- WORKSPACE.bazel | 11 +++ apt/private/BUILD.bazel | 3 + apt/private/package_index.bzl | 72 ++++++++++++++++++- apt/private/pkg.bzl | 2 +- apt/tests/package_index_test.bzl | 32 ++++++++- examples/debian_flat_repo/BUILD.bazel | 52 ++++++++++---- .../nvidia_ubuntu2404_cuda.lock.json | 23 ++++++ .../nvidia_ubuntu2404_cuda.yaml | 23 ++++++ .../debian_flat_repo/test_linux_amd64.yaml | 1 + .../debian_flat_repo/test_linux_arm64.yaml | 9 +++ 12 files changed, 218 insertions(+), 26 deletions(-) create mode 100644 examples/debian_flat_repo/nvidia_ubuntu2404_cuda.lock.json create mode 100644 examples/debian_flat_repo/nvidia_ubuntu2404_cuda.yaml create mode 100644 examples/debian_flat_repo/test_linux_arm64.yaml diff --git a/MODULE.bazel b/MODULE.bazel index c1b7932..11e75be 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -6,7 +6,7 @@ module( compatibility_level = 1, ) -bazel_dep(name = "bazel_skylib", version = "1.5.0") +bazel_dep(name = "bazel_skylib", version = "1.7.1") bazel_dep(name = "aspect_bazel_lib", version = "2.7.9") bazel_lib_toolchains = use_extension("@aspect_bazel_lib//lib:extensions.bzl", "toolchains") @@ -21,7 +21,7 @@ use_repo(bazel_lib_toolchains, "yq_linux_s390x") use_repo(bazel_lib_toolchains, "yq_windows_amd64") bazel_dep(name = "gazelle", version = "0.34.0", dev_dependency = True, repo_name = "bazel_gazelle") -bazel_dep(name = "bazel_skylib_gazelle_plugin", version = "1.5.0", dev_dependency = True) +bazel_dep(name = "bazel_skylib_gazelle_plugin", version = "1.7.1", dev_dependency = True) bazel_dep(name = "buildifier_prebuilt", version = "6.1.2", dev_dependency = True) bazel_dep(name = "platforms", version = "0.0.10", dev_dependency = True) bazel_dep(name = "rules_oci", version = "2.0.0-rc0", dev_dependency = True) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 4079645..dafe198 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -20,12 +20,12 @@ "https://bcr.bazel.build/modules/bazel_skylib/1.2.1/MODULE.bazel": "f35baf9da0efe45fa3da1696ae906eea3d615ad41e2e3def4aeb4e8bc0ef9a7a", "https://bcr.bazel.build/modules/bazel_skylib/1.3.0/MODULE.bazel": "20228b92868bf5cfc41bda7afc8a8ba2a543201851de39d990ec957b513579c5", "https://bcr.bazel.build/modules/bazel_skylib/1.4.1/MODULE.bazel": "a0dcb779424be33100dcae821e9e27e4f2901d9dfd5333efe5ac6a8d7ab75e1d", - "https://bcr.bazel.build/modules/bazel_skylib/1.4.2/MODULE.bazel": "3bd40978e7a1fac911d5989e6b09d8f64921865a45822d8b09e815eaa726a651", "https://bcr.bazel.build/modules/bazel_skylib/1.5.0/MODULE.bazel": "32880f5e2945ce6a03d1fbd588e9198c0a959bb42297b2cfaf1685b7bc32e138", "https://bcr.bazel.build/modules/bazel_skylib/1.6.1/MODULE.bazel": "8fdee2dbaace6c252131c00e1de4b165dc65af02ea278476187765e1a617b917", - "https://bcr.bazel.build/modules/bazel_skylib/1.6.1/source.json": "082ed5f9837901fada8c68c2f3ddc958bb22b6d654f71dd73f3df30d45d4b749", - "https://bcr.bazel.build/modules/bazel_skylib_gazelle_plugin/1.5.0/MODULE.bazel": "10757f9d22ebe137930a0a677269be86d2986e8abf6b84522d631920a7267743", - "https://bcr.bazel.build/modules/bazel_skylib_gazelle_plugin/1.5.0/source.json": "2c5fb7b2ad5e07bfcc90e1661c3703adb8069ea6b3d9121f647d4288d8b48748", + "https://bcr.bazel.build/modules/bazel_skylib/1.7.1/MODULE.bazel": "3120d80c5861aa616222ec015332e5f8d3171e062e3e804a2a0253e1be26e59b", + "https://bcr.bazel.build/modules/bazel_skylib/1.7.1/source.json": "f121b43eeefc7c29efbd51b83d08631e2347297c95aac9764a701f2a6a2bb953", + "https://bcr.bazel.build/modules/bazel_skylib_gazelle_plugin/1.7.1/MODULE.bazel": "c76b9d256c77c31754c5ac306d395fd47946d8d7470bea2474c3add17b334c3d", + "https://bcr.bazel.build/modules/bazel_skylib_gazelle_plugin/1.7.1/source.json": "25a87991a554369633d706f924f67ca3eb4d9200af1bba7e57dceb85eb9198e4", "https://bcr.bazel.build/modules/buildifier_prebuilt/6.1.2/MODULE.bazel": "2ef4962c8b0b6d8d21928a89190755619254459bc67f870dc0ccb9ba9952d444", "https://bcr.bazel.build/modules/buildifier_prebuilt/6.1.2/source.json": "19fb45ed3f0d55cbff94e402c39512940833ae3a68f9cbfd9518a1926b609c7c", "https://bcr.bazel.build/modules/buildozer/7.1.2/MODULE.bazel": "2e8dd40ede9c454042645fd8d8d0cd1527966aa5c919de86661e62953cd73d84", @@ -813,7 +813,7 @@ "@@gazelle~//:extensions.bzl%go_deps": { "general": { "bzlTransitiveDigest": "Taobh9Bi1JpF4jHwuw6x9ceWDpHtCjGmS8VXbxLOqH8=", - "usagesDigest": "OF7bvO+xWblkUXgQsIPSIr048t6z7ZKSjlR8twO/efg=", + "usagesDigest": "dKNKvQJECrWkRG++E5OdDZZAclQbs9RKSkeI+WU2EpA=", "recordedFileInputs": { "@@rules_go~//go.mod": "a7143f329c2a3e0b983ce74a96c0c25b0d0c59d236d75f7e1b069aadd988d55e", "@@gazelle~//go.sum": "7469786f3930030c430969cedae951e6947cb40f4a563dac94a350659c0fedc4", @@ -1688,7 +1688,7 @@ }, "@@rules_oci~//oci:extensions.bzl%oci": { "general": { - "bzlTransitiveDigest": "6G6tDFJTPCtKyxon8Br4ev91dRdgBbCkorJmAgiIagc=", + "bzlTransitiveDigest": "3HRH6B82zu14f5XyCoQciUqaObjafWuRersO7BtZxGU=", "usagesDigest": "Pu/P+SVB+Qbdzl3wU3n5aOMsoK6nTX8UJYRkP9qCPpE=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, diff --git a/WORKSPACE.bazel b/WORKSPACE.bazel index 7c99d2d..d0e1aa6 100644 --- a/WORKSPACE.bazel +++ b/WORKSPACE.bazel @@ -57,6 +57,17 @@ load("@bullseye_rproject//:packages.bzl", "bullseye_rproject_packages") bullseye_rproject_packages() +# bazel run @nvidia_ubuntu2404_cuda//:lock +deb_index( + name = "nvidia_ubuntu2404_cuda", + lock = "//examples/debian_flat_repo:nvidia_ubuntu2404_cuda.lock.json", + manifest = "//examples/debian_flat_repo:nvidia_ubuntu2404_cuda.yaml", +) + +load("@nvidia_ubuntu2404_cuda//:packages.bzl", "nvidia_ubuntu2404_cuda_packages") + +nvidia_ubuntu2404_cuda_packages() + deb_index( name = "apt_security", manifest = "//examples/debian_snapshot_security:security.yaml", diff --git a/apt/private/BUILD.bazel b/apt/private/BUILD.bazel index f930b43..23d312f 100644 --- a/apt/private/BUILD.bazel +++ b/apt/private/BUILD.bazel @@ -42,6 +42,9 @@ bzl_library( name = "pkg", srcs = ["pkg.bzl"], visibility = ["//apt:__subpackages__"], + deps = [ + "@bazel_skylib//lib:paths", + ], ) bzl_library( diff --git a/apt/private/package_index.bzl b/apt/private/package_index.bzl index 64682d9..3f7f680 100644 --- a/apt/private/package_index.bzl +++ b/apt/private/package_index.bzl @@ -1,5 +1,6 @@ "package index" +load("@bazel_skylib//lib:paths.bzl", "paths") load(":version.bzl", version_lib = "version") def _fetch_package_index(rctx, source): @@ -56,6 +57,56 @@ def _fetch_package_index(rctx, source): return rctx.read(source.output) +def _parse_url(url): + if "://" not in url: + fail("Invalid URL: %s" % url) + + scheme, url_ = url.split("://", 1) + + path = "/" + + if "/" in url_: + host, path_ = url_.split("/", 1) + path += path_ + else: + host = url_ + + return struct(scheme = scheme, host = host, path = path) + +def _make_file_url(pkg, source): + filename = pkg["Filename"] + + invalid_filename = not paths.is_normalized( + filename, + look_for_same_level_references = True, + ) + + if invalid_filename: + # NOTE: + # Although the Debian repo spec for 'Filename' (see + # https://wiki.debian.org/DebianRepository/Format#Filename) clearly + # says that 'Filename' should be relative to the base directory of the + # repo and should be in canonical form (i.e. without '.' or '..') there + # are cases where this is not honored. + # + # In those cases we try to work around this by assuming 'Filename' is + # relative to the sources.list "index path" (e.g. directory/ for flat + # repos) so we combine them and normalize the new 'Filename' path. + # + # Note that, so far, only the NVIDIA CUDA repos needed this workaround + # so maybe this heuristic will break for other repos that don't conform + # to the Debian repo spec. + filename = paths.normalize(paths.join(source.index_path, filename)) + + base_url = _parse_url(source.base_url) + file_url = "{}://{}{}".format( + base_url.scheme, + base_url.host, + paths.join(base_url.path, filename), + ) + + return file_url, invalid_filename + def _package_set(packages, keys, package): for key in keys[:-1]: if key not in packages: @@ -66,6 +117,9 @@ def _package_set(packages, keys, package): def _parse_package_index(packages, contents, source): last_key = "" pkg = {} + total_pkgs = 0 + out_of_spec = [] + for group in contents.split("\n\n"): for line in group.split("\n"): if line.strip() == "": @@ -92,7 +146,10 @@ def _parse_package_index(packages, contents, source): pkg[key] = value if len(pkg.keys()) != 0: - pkg["Root"] = source.base_url + pkg["FileUrl"], invalid_filename = _make_file_url(pkg, source) + + if invalid_filename: + out_of_spec.append(pkg["Package"]) # NOTE: workaround for multi-arch flat repos arch = source.arch if pkg["Architecture"] == "all" else pkg["Architecture"] @@ -104,6 +161,9 @@ def _parse_package_index(packages, contents, source): ) last_key = "" pkg = {} + total_pkgs += 1 + + return out_of_spec, total_pkgs def _package_get(packages, arch, name, version = None): versions = packages.get(arch, {}).get(name, {}) @@ -123,7 +183,13 @@ def _index(rctx, manifest): output = _fetch_package_index(rctx, source) rctx.report_progress("Parsing package index: %s" % index) - _parse_package_index(packages, output, source) + out_of_spec, total_pkgs = _parse_package_index(packages, output, source) + + if out_of_spec: + count = len(out_of_spec) + pct = int(100.0 * count / total_pkgs) + msg = "Warning: index {} has {} packages ({}%) with invalid 'Filename' fields" + print(msg.format(index, count, pct)) return struct( packages = packages, @@ -287,6 +353,8 @@ package_index = struct( parse_depends = _parse_depends, # NOTE: these are exposed here for testing purposes, DO NOT USE OTHERWISE _fetch_package_index = _fetch_package_index, + _parse_url = _parse_url, + _make_file_url = _make_file_url, _parse_package_index = _parse_package_index, _package_set = _package_set, _package_get = _package_get, diff --git a/apt/private/pkg.bzl b/apt/private/pkg.bzl index 68c6d7a..26790d1 100644 --- a/apt/private/pkg.bzl +++ b/apt/private/pkg.bzl @@ -4,7 +4,7 @@ def _pkg_from_index(package, arch): return struct( name = package["Package"], version = package["Version"], - url = "%s/%s" % (package["Root"], package["Filename"]), + url = package["FileUrl"], sha256 = package["SHA256"], arch = arch, dependencies = [], diff --git a/apt/tests/package_index_test.bzl b/apt/tests/package_index_test.bzl index f9b0813..954ad4f 100644 --- a/apt/tests/package_index_test.bzl +++ b/apt/tests/package_index_test.bzl @@ -34,6 +34,22 @@ def _fetch_package_index_test(ctx): fetch_package_index_test = unittest.make(_fetch_package_index_test) +def _parse_url_test(ctx): + env = unittest.begin(ctx) + + parameters = { + "http://mirror.com": struct(scheme = "http", host = "mirror.com", path = "/"), + "http://mirror.com/foo/bar": struct(scheme = "http", host = "mirror.com", path = "/foo/bar"), + } + + for url, expected in parameters.items(): + actual = package_index._parse_url(url) + asserts.equals(env, expected, actual) + + return unittest.end(env) + +parse_url_test = unittest.make(_parse_url_test) + def _parse_package_index_test(ctx): env = unittest.begin(ctx) @@ -50,7 +66,7 @@ def _parse_package_index_test(ctx): package_index._parse_package_index(actual, output, source) asserts.equals(env, "foo", actual[arch][name][version]["Package"]) - asserts.equals(env, url, actual[arch][name][version]["Root"]) + asserts.true(env, actual[arch][name][version]["FileUrl"].startswith(url)) return unittest.end(env) @@ -107,6 +123,8 @@ def _index_test(ctx): url = "http://mirror.com" + source = mock.manifest(url, arch, name).sources[0] + mock_rctx = mock.rctx( read = mock.read(output), download = mock.download(success = True), @@ -116,10 +134,17 @@ def _index_test(ctx): actual = package_index._index(mock_rctx, mock.manifest(url, arch, name)) expected_pkg = mock.pkg(arch, name, version) - expected_pkg["Root"] = url + file_url, _ = package_index._make_file_url(expected_pkg, source) + expected_pkg["FileUrl"] = file_url actual_pkg = actual.package_get(arch, name, version) - asserts.equals(env, expected_pkg, actual_pkg) + + # NOTE: we compare key-by-key because the error output of + # asserts.equals(env, expected_pkg, actual_pkg) is quite + # hard to read + asserts.equals(env, expected_pkg.keys(), actual_pkg.keys()) + for key in expected_pkg.keys(): + asserts.equals(env, expected_pkg[key], actual_pkg[key]) expected_packages = {arch: {name: {version: expected_pkg}}} asserts.equals(env, expected_packages, actual.packages) @@ -230,6 +255,7 @@ parse_depends_test = unittest.make(_parse_depends_test) def package_index_tests(): fetch_package_index_test(name = _TEST_SUITE_PREFIX + "_fetch_package_index") + parse_url_test(name = _TEST_SUITE_PREFIX + "_parse_url") parse_package_index_test(name = _TEST_SUITE_PREFIX + "_parse_package_index") package_set_get_test(name = _TEST_SUITE_PREFIX + "_package_set_get") index_test(name = _TEST_SUITE_PREFIX + "_index") diff --git a/examples/debian_flat_repo/BUILD.bazel b/examples/debian_flat_repo/BUILD.bazel index d491029..f536061 100644 --- a/examples/debian_flat_repo/BUILD.bazel +++ b/examples/debian_flat_repo/BUILD.bazel @@ -5,28 +5,51 @@ load("@rules_oci//oci:defs.bzl", "oci_image", "oci_load") PACKAGES = [ "@bullseye//dpkg", "@bullseye//apt", +] + +PACKAGES_AMD64 = PACKAGES + [ "@bullseye_rproject//r-mathlib", + "@nvidia_ubuntu2404_cuda//nvidia-container-toolkit-base", +] + +PACKAGES_ARM64 = PACKAGES + [ + "@nvidia_ubuntu2404_cuda//nvidia-container-toolkit-base", ] # Creates /var/lib/dpkg/status with installed package information. dpkg_status( name = "dpkg_status", - controls = [ - "%s/amd64:control" % package - for package in PACKAGES - ], + controls = select({ + "@platforms//cpu:x86_64": [ + "%s/amd64:control" % package + for package in PACKAGES_AMD64 + ], + "@platforms//cpu:arm64": [ + "%s/arm64:control" % package + for package in PACKAGES_ARM64 + ], + }), ) oci_image( name = "apt", - architecture = "amd64", + architecture = select({ + "@platforms//cpu:x86_64": "amd64", + "@platforms//cpu:arm64": "arm64", + }), os = "linux", tars = [ ":dpkg_status", - ] + [ - "%s/amd64" % package - for package in PACKAGES - ], + ] + select({ + "@platforms//cpu:x86_64": [ + "%s/amd64" % package + for package in PACKAGES_AMD64 + ], + "@platforms//cpu:arm64": [ + "%s/arm64" % package + for package in PACKAGES_ARM64 + ], + }), ) oci_load( @@ -39,10 +62,15 @@ oci_load( container_structure_test( name = "test", - configs = ["test_linux_amd64.yaml"], + configs = select({ + "@platforms//cpu:x86_64": ["test_linux_amd64.yaml"], + "@platforms//cpu:arm64": ["test_linux_arm64.yaml"], + }), image = ":apt", - target_compatible_with = [ - "@platforms//cpu:x86_64", + target_compatible_with = select({ + "@platforms//cpu:x86_64": ["@platforms//cpu:x86_64"], + "@platforms//cpu:arm64": ["@platforms//cpu:arm64"], + }) + [ "@platforms//os:linux", ], ) diff --git a/examples/debian_flat_repo/nvidia_ubuntu2404_cuda.lock.json b/examples/debian_flat_repo/nvidia_ubuntu2404_cuda.lock.json new file mode 100644 index 0000000..12cd9ce --- /dev/null +++ b/examples/debian_flat_repo/nvidia_ubuntu2404_cuda.lock.json @@ -0,0 +1,23 @@ +{ + "packages": { + "nvidia-container-toolkit-base": { + "amd64": { + "arch": "amd64", + "dependencies": [], + "name": "nvidia-container-toolkit-base", + "sha256": "8184d04f88215de4f630e4f5ba24d9bf7e64a7a597ba2e3c6fbd94f86bea0599", + "url": "https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2404/x86_64/nvidia-container-toolkit-base_1.16.1-1_amd64.deb", + "version": "1.16.1-1" + }, + "arm64": { + "arch": "arm64", + "dependencies": [], + "name": "nvidia-container-toolkit-base", + "sha256": "dfc068e5ff69274351e59376078d9bda6a6c95423c7de1619b6a54aa9ba0f494", + "url": "https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2404/arm64/nvidia-container-toolkit-base_1.16.1-1_arm64.deb", + "version": "1.16.1-1" + } + } + }, + "version": 2 +} \ No newline at end of file diff --git a/examples/debian_flat_repo/nvidia_ubuntu2404_cuda.yaml b/examples/debian_flat_repo/nvidia_ubuntu2404_cuda.yaml new file mode 100644 index 0000000..9d1fd8c --- /dev/null +++ b/examples/debian_flat_repo/nvidia_ubuntu2404_cuda.yaml @@ -0,0 +1,23 @@ +# Packages for examples/debian_flat_repo. +# +# Anytime this file is changed, the lockfile needs to be regenerated. +# +# To generate the nvidia_cuda.lock.json run the following command +# +# bazel run @nvidia_ubuntu2404_cuda//:lock +# +# See debian_package_index at WORKSPACE.bazel +version: 1 + +sources: + - channel: ubuntu2404/x86_64/ + url: https://developer.download.nvidia.com/compute/cuda/repos + - channel: ubuntu2404/arm64/ + url: https://developer.download.nvidia.com/compute/cuda/repos + +archs: + - amd64 + - arm64 + +packages: + - nvidia-container-toolkit-base diff --git a/examples/debian_flat_repo/test_linux_amd64.yaml b/examples/debian_flat_repo/test_linux_amd64.yaml index 4e9d1d8..95c703c 100644 --- a/examples/debian_flat_repo/test_linux_amd64.yaml +++ b/examples/debian_flat_repo/test_linux_amd64.yaml @@ -7,3 +7,4 @@ commandTests: expectedOutput: - Listing\.\.\. - r-mathlib/now 4.4.1-1~bullseyecran.0 amd64 \[installed,local\] + - nvidia-container-toolkit-base/now 1.16.1-1 amd64 \[installed,local\] diff --git a/examples/debian_flat_repo/test_linux_arm64.yaml b/examples/debian_flat_repo/test_linux_arm64.yaml new file mode 100644 index 0000000..7af4d70 --- /dev/null +++ b/examples/debian_flat_repo/test_linux_arm64.yaml @@ -0,0 +1,9 @@ +schemaVersion: "2.0.0" + +commandTests: + - name: "apt list --installed" + command: "apt" + args: ["list", "--installed"] + expectedOutput: + - Listing\.\.\. + - nvidia-container-toolkit-base/now 1.16.1-1 arm64 \[installed,local\]