Add support for LZMA/XZ compressed dpkg_src repositories (Packages.xz)

[gertvdijk]

According to the Debian Repository format wiki page:

Clients must support xz compression, and must support gzip and bzip2 if they want to use the files that are listed as usual use cases of these formats.
[...]
Servers should offer only xz compressed files [...]

Debian Buster (10) backports archive only has a Packages.xz file, and no longer a Packages.gz file. See e.g. https://snapshot.debian.org/archive/debian/20200217T085500Z/dists/buster-backports/main/binary-amd64/

dpkg_src() from package_manager/dpkg.bzl only supports a packages_gz_url parameter, and providing an XZ file there results in an error.

distroless/package_manager/dpkg.bzl

Lines 58 to 74 in 76c76a1

	_dpkg_src = repository_rule(
	_dpkg_src_impl,
	attrs = {
	"url": attr.string(),
	"arch": attr.string(),
	"distro": attr.string(),
	"snapshot": attr.string(),
	"packages_gz_url": attr.string(),
	"package_prefix": attr.string(),
	"sha256": attr.string(),
	"_dpkg_parser": attr.label(
	executable = True,
	default = Label("@dpkg_parser//file:downloaded"),
	cfg = "host",
	),
	},
	)

I think I understand the logic in these rules to create a PR. It seems that everything is designed around the presence of the .gz files only. Here's my proposed solution:

Introduce packages_url in favour of packages_gz_url (which then should be deprecated), auto-detecting the compression format based on the file extension in the URL, supporting gzip, lzma and uncompressed. Elegant and compatible, I'd say.

A bit less trivial is the other use case of the rule where packages_url is not given but mirror_url, snapshot, distro and arch are. Currently we assume there's a .gz file at the compiled location. If I would change that to .xz, it will break current use for several reasons (old or Google repos which only have .gz files, and sha256 will be different). Perhaps introduce a new parameter for that with the default to .gz to maintain compatibility?

[gertvdijk]

See master...gertvdijk:packages-xz-issue-463 for my approach here. This does not address the less trivial case yet, but this could be done in a separate change.

What is the workflow for updating the PAR in the package_manager/package_manager.bzl? First review this, then a maintainer can bump it?

[chanseokoh]

Awesome! dpkg_src is an internal component, so I'd just replace .gz with .xz everywhere and eliminate anything related to gzip or .gz, assuming that switching to .xz doesn't break the current master and it won't likely break for the foreseeable future. I think no deprecation or whatsoever is necessary.

What is the workflow for updating the PAR in the package_manager/package_manager.bzl? First review this, then a maintainer can bump it?

After we merge a change to package_manager/package_manager.bzl, I will probably be able to build a PAR myself and upload it at https://storage.googleapis.com/distroless/package_manager_tools/.../dpkg_parser.par.

[gertvdijk]

@chanseokoh Thanks for your reply!

dpkg_src is an internal component

Is it? I mean, in terms of use in other projects, it's very useful, at least for me:

load("@distroless//package_manager:dpkg.bzl", "dpkg_list", "dpkg_src")

I'd just replace .gz with .xz everywhere and eliminate anything related to gzip or .gz, assuming that switching to .xz doesn't break the current master and it won't likely break for the foreseeable future. I think no deprecation or whatsoever is necessary.

Have you read my last paragraph?

• Google repositories don't serve .xz, disobeying the current Debian repository format guidelines. So this will break at least some stuff.
• (If indeed not internal) Users currently using it would have to change the sha256.

After we merge a change to package_manager/package_manager.bzl, I will probably be able to build a PAR myself and upload it at https://storage.googleapis.com/distroless/package_manager_tools/.../dpkg_parser.par.

Cool. Any plans to replace this with @rules_python's py_binary? I find it very developer-unfriendly to edit code that isn't used at runtime because of the PAR step in between. (Happy to create a PR for that too, if there's no compelling reasons to keep using PARs.)

[chanseokoh]

Thanks! Your contribution is highly appreciated!

Is it? I mean, in terms of use in other projects, it's very useful, at least for me:

I understand, but this is actually what I'd like to avoid. I don't want to make things unnecessarily public for the sake of being nice, thinking that some build/utility stuff may be useful for others. Once people start depending on it, we then need to keep supporting it. People will constantly report feature requests or breakages. Considering how this repo is being maintained at the current state, I think we should just focus on building and publishing actual images, and not about releasing some internal build stuff. We should minimize public surface and maintenance burden. We don't intend to support this kind of use case, and that's not the goal of this repo. For a similar reason, we've closed a PR like this.

Have you read my last paragraph?

Yes, I did, and I actually didn't understand the Google repositories part. What and where is the Google repository and what are they serving exactly? I don't think we use any Google repository with dpkg_src.

(If indeed not internal) Users currently using it would have to change the sha256.

I think you are talking about users building images from source. I'd say they should sync with master. But keeping the old PAR isn't a big deal, so their source will keep working. But in the end, once we fix the problem you mentioned below, this won't be an issue anymore. Or, are you talking about something else? In any case, the goal of this repo is not really about supporting other users building images from out-of-date source.

Cool. Any plans to replace this with @rules_python's py_binary? I find it very developer-unfriendly to edit code that isn't used at runtime because of the PAR step in between. (Happy to create a PR for that too, if there's no compelling reasons to keep using PARs.)

I have no expertise in Python, but I really want to close #434 (comment), which I think is what you are talking about: we currently have this "two-step"-like builds, where we first need to build a PAR and then upload it to somewhere. I have no idea why it was set up like this. In #434, different people suggested different options, but as I don't have Python expertise, I don't really get them.

[gertvdijk]

I understand, but this is actually what I'd like to avoid. [...]

Okay, got it. Sad, but from your point I understand. :)

Please note that the current version explicitly exports this to the public, hence people may rely on this:

distroless/package_manager/BUILD

Line 7 in e95325f

visibility = ["//visibility:public"],

Yes, I did, and I actually didn't understand the Google repositories part. What and where is the Google repository and what are they serving exactly? I don't think we use any Google repository with dpkg_src.

Sorry, I should have been more specific. I meant the Google APT repositories like bazel-apt as referenced in the README here.

• http://storage.googleapis.com/bazel-apt/dists/stable/jdk1.8/binary-amd64/Packages.gz - OK, exists.

• http://storage.googleapis.com/bazel-apt/dists/stable/jdk1.8/binary-amd64/Packages.xz - NOK, 404.

• http://storage.googleapis.com/bazel-apt/dists/stable/jdk1.8/binary-amd64/Packages - OK, exists.

This is non-conforming Debian guidelines. Shall we just remove this from the README too then?

I think you are talking about users building images from source

No, I was referring to simple use of dpkg_src() by users, not using packages_gz_url, but the alternative syntax with mirror etc, their sha256 will then fail. But if that is considered internal-only functions, then let's just disregard that. :)

I have no expertise in Python, but I really want to close #434 (comment), which I think is what you are talking about: we currently have this "two-step"-like builds, where we first need to build a PAR and then upload it to somewhere. I have no idea why it was set up like this. In #434, different people suggested different options, but as I don't have Python expertise, I don't really get them.

Thanks for the pointers! If there's no reason to keep this in, then let's move to modern way of using Python in Bazel (still a bit yuck, but okay). I can work on this later this week so we can indeed close #434 too (in my project the shebang is #!/usr/bin/env python and works with PY3).

update: I've underestimated it a bit; in a quick attempt I see that I have to work in a repository_ctx for executing the dpkg_parser at Bazel analysis time. The PAR file is loaded from external sources and thereby avoids a cyclic dependency on the @distroless workspace where the dpkg_parser is.

[chanseokoh]

Okay, got it. Sad, but from your point I understand. :)

Yeah, sorry. We should be able to freely change any part of the repository code at will at any frequency. Our development velocity shouldn't be slowed down by assuming this as a public interface for no good reason.

Please note that the current version explicitly exports this to the public, hence people may rely on this:

I am not a Bazel expert either, so I didn't know what this really means. But let me see if I can remove this. (UPDATE: filed and merged #469.)

Sorry, I should have been more specific. I meant the Google APT repositories like bazel-apt as referenced in the README here.

Aha, I see. You can remove these when you drop everything about gzip (if you intend to file a PR).

update: I've underestimated it a bit; in a quick attempt I see that I have to work in a repository_ctx for executing the dpkg_parser at Bazel analysis time. The PAR file is loaded from external sources and thereby avoids a cyclic dependency on the @distroless workspace where the dpkg_parser is.

Hmm... I cannot say I really understood the details, but I think I get the problem at a high level. Probably you should update the details on #434, but do we still have a way to fix this "two-step" setup (i.e., building dpkg_parser and using it in one pass)? Thanks for looking into it.

[chanseokoh]

do we still have a way to fix this "two-step" setup (i.e., building dpkg_parser and using it in one pass)?

I am fine with running Bazel builds twice, as long as we can get rid of the restriction that we have to upload a PAR. For example, the build instructions would be to first run bazel build //package_manager:dpkg_parser.par and then bazel build ... or bazel run ... This will require changing package_manager/package_manager.bzl to get the PAR file from ./bazel-bin/package_manager/dpkg_parser.par. As mentioned in #434 (comment), http_file supports reading from a local path. UPDATE: package_manager.bzl isn't even necessary (#477).

[chanseokoh]

update: I've underestimated it a bit; in a quick attempt I see that I have to work in a repository_ctx for executing the dpkg_parser at Bazel analysis time. The PAR file is loaded from external sources and thereby avoids a cyclic dependency on the @distroless workspace where the dpkg_parser is.

do we still have a way to fix this "two-step" setup (i.e., building dpkg_parser and using it in one pass)?

I am fine with running Bazel builds twice, as long as we can get rid of the restriction that we have to upload a PAR. For example, the build instructions would be to first run bazel build //package_manager:dpkg_parser.par and then bazel build ... or bazel run ...

This is done with #477.

[chanseokoh]

Fixed by #781, I believe.