Skip to content

Commit

Permalink
fix: NVIDIA CUDA flat repos don't follow Debian repo spec
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jjmaestro committed Nov 26, 2024
1 parent f2cc792 commit ea90090
Show file tree
Hide file tree
Showing 14 changed files with 203 additions and 24 deletions.
7 changes: 6 additions & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,9 @@ apt.install(
lock = "//examples/ubuntu_snapshot:noble.lock.json",
manifest = "//examples/ubuntu_snapshot:noble.yaml",
)
use_repo(apt, "apt_security", "apt_security_resolve", "bullseye", "bullseye_nolock", "bullseye_rproject", "noble", "shared_dependencies")
apt.install(
name = "nvidia_ubuntu2404_cuda",
lock = "//examples/debian_flat_repo:nvidia_ubuntu2404_cuda.lock.json",
manifest = "//examples/debian_flat_repo:nvidia_ubuntu2404_cuda.yaml",
)
use_repo(apt, "apt_security", "apt_security_resolve", "bullseye", "bullseye_nolock", "bullseye_rproject", "noble", "nvidia_ubuntu2404_cuda", "shared_dependencies")
1 change: 1 addition & 0 deletions apt/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ bzl_library(
deps = [
":nested_dict",
":version_constraint",
"@bazel_skylib//lib:paths",
],
)

Expand Down
57 changes: 55 additions & 2 deletions apt/private/apt_deb_repository.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"https://wiki.debian.org/DebianRepository"

load("@bazel_skylib//lib:paths.bzl", "paths")
load(":nested_dict.bzl", "nested_dict")
load(":util.bzl", "util")
load(":version_constraint.bzl", "version_constraint")

def _fetch_package_index(rctx, source):
Expand Down Expand Up @@ -62,9 +64,46 @@ def _fetch_package_index(rctx, source):

return rctx.read(source.output)

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 = util.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 _parse_package_index(state, 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() == "":
Expand All @@ -91,10 +130,17 @@ def _parse_package_index(state, contents, source):
pkg[key] = value

if len(pkg.keys()) != 0:
pkg["Root"] = source.base_url
pkg["File-Url"], invalid_filename = _make_file_url(pkg, source)

if invalid_filename:
out_of_spec.append(pkg["Package"])

_add_package(state, pkg)
last_key = ""
pkg = {}
total_pkgs += 1

return out_of_spec, total_pkgs

def _add_package(state, package):
state.packages.set(
Expand Down Expand Up @@ -124,7 +170,13 @@ def _new(rctx, manifest):
output = _fetch_package_index(rctx, source)

rctx.report_progress("Parsing package index: %s" % index)
_parse_package_index(state, output, source)
out_of_spec, total_pkgs = _parse_package_index(state, 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(
package_versions = lambda arch, name: state.packages.get((arch, name), {}).keys(),
Expand All @@ -136,6 +188,7 @@ deb_repository = struct(
new = _new,
__test__ = struct(
_fetch_package_index = _fetch_package_index,
_make_file_url = _make_file_url,
_parse_package_index = _parse_package_index,
),
)
2 changes: 1 addition & 1 deletion apt/private/pkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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["File-Url"],
sha256 = package["SHA256"],
arch = arch,
dependencies = [],
Expand Down
17 changes: 17 additions & 0 deletions apt/private/util.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@ def _get_dupes(list_):

return sorted(sets.to_list(dupes))

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 _sanitize(str):
return str.replace("+", "-p-").replace(":", "-").replace("~", "_")

Expand All @@ -28,6 +44,7 @@ def _warning(rctx, message):
util = struct(
escape = _escape,
get_dupes = _get_dupes,
parse_url = _parse_url,
sanitize = _sanitize,
warning = _warning,
)
9 changes: 7 additions & 2 deletions apt/tests/apt_deb_repository_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,15 @@ def new_setup(pkgs = None):
mock_value.MANIFEST_LABEL,
)

packages_index_content = mock.packages_index_content(*pkgs)

source = mock_manifest.sources[0]

for idx in range(len(pkgs)):
pkg = pkgs[idx]
file_url, _ = deb_repository.__test__._make_file_url(pkg, source)
pkg["File-Url"] = file_url

packages_index_content = mock.packages_index_content(*pkgs)

mock_rctx = mock.rctx(
read = mock.read(packages_index_content),
download = mock.download(success = True),
Expand Down
5 changes: 2 additions & 3 deletions apt/tests/mocks.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ _PKG_INDEX = {
"Package": _PKG_LOCK_V2["name"],
"Architecture": _PKG_LOCK_V2["arch"],
"Version": _PKG_LOCK_V2["version"],
"Root": "/".join(_PKG_LOCK_V2["url"].split("/")[:-1]),
"File-Url": _PKG_LOCK_V2["url"],
"Filename": _PKG_LOCK_V2["url"].split("/")[-1],
"SHA256": _PKG_LOCK_V2["sha256"],
}
Expand All @@ -80,7 +80,7 @@ _PKG_INDEX_DEPS = [
{
"Package": d["name"],
"Version": d["version"],
"Root": "http://nowhere",
"File-Url": "http://nowhere/foo.deb",
"Filename": "foo.deb",
"SHA256": "deadbeef" * 8,
}
Expand Down Expand Up @@ -187,7 +187,6 @@ def _rctx(**kwargs):
def _pkg(package, **kwargs):
defaults = {
"Package": package,
"Root": _URL,
"Filename": "/foo/bar/pkg.deb",
"SHA256": "deadbeef" * 8,
}
Expand Down
3 changes: 1 addition & 2 deletions apt/tests/pkg_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ def _from_index_test(ctx):

asserts.equals(env, arch, p.arch)
asserts.equals(env, mock_value.PKG_INDEX["Package"], p.name)
asserts.true(env, p.url.startswith(mock_value.PKG_INDEX["Root"]))
asserts.true(env, p.url.endswith(mock_value.PKG_INDEX["Filename"]))
asserts.true(env, p.url, mock_value.PKG_INDEX["File-Url"])

return unittest.end(env)

Expand Down
18 changes: 17 additions & 1 deletion apt/tests/util_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ def _escape_test(ctx):

for s, expected in parameters.items():
actual = util.escape(s)
asserts.equals(env, expected, actual)

return unittest.end(env)

Expand All @@ -41,6 +40,22 @@ def _get_dupes_test(ctx):

get_dupes_test = unittest.make(_get_dupes_test)

def _parse_url_test(ctx):
env = unittest.begin(ctx)

parameters = {
"https://mirror.com": struct(scheme = "https", 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 = util.parse_url(url)
asserts.equals(env, expected, actual)

return unittest.end(env)

parse_url_test = unittest.make(_parse_url_test)

def _sanitize_test(ctx):
env = unittest.begin(ctx)

Expand All @@ -61,4 +76,5 @@ sanitize_test = unittest.make(_sanitize_test)
def util_tests():
escape_test(name = _TEST_SUITE_PREFIX + "escape")
get_dupes_test(name = _TEST_SUITE_PREFIX + "get_dupes")
parse_url_test(name = _TEST_SUITE_PREFIX + "parse_url")
sanitize_test(name = _TEST_SUITE_PREFIX + "sanitize")
52 changes: 40 additions & 12 deletions examples/debian_flat_repo/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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",
],
)
23 changes: 23 additions & 0 deletions examples/debian_flat_repo/nvidia_ubuntu2404_cuda.lock.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"packages": {
"nvidia-container-toolkit-base": {
"amd64": {
"arch": "amd64",
"dependencies": [],
"name": "nvidia-container-toolkit-base",
"sha256": "a28af3f9a0e88ae04a9f346375c85f4609c8320dbaf1136af7522b54a24f53be",
"url": "https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2404/x86_64/nvidia-container-toolkit-base_1.17.2-1_amd64.deb",
"version": "1.17.2-1"
},
"arm64": {
"arch": "arm64",
"dependencies": [],
"name": "nvidia-container-toolkit-base",
"sha256": "55f242746c244b2582fc354f57b5cf7fa467b8dec5105fc39f9f46602b8db590",
"url": "https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2404/arm64/nvidia-container-toolkit-base_1.17.2-1_arm64.deb",
"version": "1.17.2-1"
}
}
},
"version": 2
}
23 changes: 23 additions & 0 deletions examples/debian_flat_repo/nvidia_ubuntu2404_cuda.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions examples/debian_flat_repo/test_linux_amd64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ commandTests:
expectedOutput:
- Listing\.\.\.
- r-mathlib/now 4.4.2-1~bullseyecran.0 amd64 \[installed,local\]
- nvidia-container-toolkit-base/now 1.17.2-1 amd64 \[installed,local\]
9 changes: 9 additions & 0 deletions examples/debian_flat_repo/test_linux_arm64.yaml
Original file line number Diff line number Diff line change
@@ -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.17.2-1 arm64 \[installed,local\]

0 comments on commit ea90090

Please sign in to comment.