#1256 file and dir dependencies: upgrade SHOULD NOT to MUST NOT
Merged by ngompa. Opened by zbyszek.
zbyszek/packaging-committee filelists  into  master

Download 1256.patch

We have had the rule that those dependencies are not kosher for as long as I
remember. It was put in place because file lists in repository metadata are
split in two parts: "primary" and "filelists", with only a subset of paths in
the first file. Nevertheless, the rule wasn't very closely followed because it
didn't have any practical effect: dnf would download and load both lists anyway.

With dnf5, behaviour is changed:

  1. Right now, any dependency on a path that is not in primary simply fails
    (rhbz#2173051). This might or might not be a bug.

  2. Longer-term, the goal is to not download filelists at all, until required
    by some operation that the user does. For this to work correctly we need to
    get rid of all path dependencies in packages. 2. should happen even if 1. is
    resolved by changing to download and load filepath metadata if required by a
    package.

So the language is upgraded to "MUST NOT". An exception is added for
explicitly-provided paths. Those are sometimes useful to coordinate
functionality between packages, and those Provides are always in primary, so
they are excluded from the prohibition.

The rule is extended to also cover directories. I suspect that the
omission was just clumsy language and not intentional.

Hard reject. Bugs should be fixed, and file dependencies are useful.

-1

Did you read the text I wrote, esp. point 2.?

Yes. I don't agree with that premise at all.

Aren't some file lists always included in primary metadata? I don't think it makes sense to ban them entirely, if there is a subset that can be used without additional cost.

OK, I'll try to provide more context.

Dnf operations require repository metadata. Currently (with dnf < 5), this means both primary.xml.zck and filelists.xml.zck, e.g. 32MB + 50MB. Both are downloaded and loaded into memory. This is painful for users with slow download speeds because dnf will download 80+ MB of data. Especially for small install operations, where the rpm or rpms could be a few MBs or less, the actual package download is dwarfed by the repo metadata download. It is also painful for users on slow machines and machines with limited memory, because bigger metadata requires more CPU and memory. This issue is not very visible for people with fast CPUs and disk and fast internet, i.e. probably most Fedora developers don't feel it directly, but it is a frequent complaint from users. In particular, outside reviews of Fedora often start with a complaint "why is dnf so slow, it starts by downloading 80 MB of stuff when I want to install a single package". Similarly, this extra metadata is very visible when using an rpi or another tiny machine.

Not downloading and not loading filelists has been a long-standing RFE. I remember discussing it at Flock in 2018, but the idea is much older. Dnf5 is able to not download and load file lists by default. It is also faster for other reasons, but downloading ~60% less metadata is obviously beneficial.

(Also note that filelists.xml is long: fedora-38 has 73851 <file> entires in primary.xml and 8162602 in filelists.xml. Loading this additional data into the dependency graph will always have a cost.)

This is not about avoiding a bug, but about resolving a longstanding RFE. The premise is that we should make this (not requiring filelists.xml for normal dnf operations) work correctly and reap the benefit of smaller metadata downloads and faster dnf operations. This means that we have to get rid of the path dependencies in packages. They were discouraged ("SHOULD NOT"), but not disallowed. In fact this change affects a small number of packages: approx. 10 and it will be very easy to adjust them all. I plan to create pull requests for those.

Bugs should be fixed, and file dependencies are useful.

File dependencies are useful, and we want to allow users to use them. dnf install /whatever/path is great, and works, and we want to keep it that way. But we don't want to use file dependencies (outside of the set listed in the guidelines) in packages. It's just a handful of packages but we pay a significant cost on every dnf update.

I went through the packages:

sng - https://bugzilla.redhat.com/show_bug.cgi?id=1731702 - FIXED
sqlninja - https://bugzilla.redhat.com/show_bug.cgi?id=1731703
rt - https://bugzilla.redhat.com/show_bug.cgi?id=1731700 - PR SUBMITTED
lightsquid - https://bugzilla.redhat.com/show_bug.cgi?id=1731696
kata-containers - https://src.fedoraproject.org/rpms/kata-containers/pull-request/4
glusterfs - https://bugzilla.redhat.com/show_bug.cgi?id=1731689
libebml - https://bugzilla.redhat.com/show_bug.cgi?id=1731694
libmatroska - https://bugzilla.redhat.com/show_bug.cgi?id=1731695

(ncid - https://bugzilla.redhat.com/show_bug.cgi?id=2173616)

7 packages to fix, I think we could manage that if we tried.

Aren't some file lists always included in primary metadata? I don't think it makes sense to ban them entirely, if there is a subset that can be used without additional cost.

Yes. That's why text says

Packages MUST NOT include such dependencies outside of the following directories: * +/usr/bin+, * +/usr/sbin+, * `+/etc+

(There's a caveat that createrepo_c faithfully reproduced earlier bugs in rpm and actual list included in primary.xml is a bit wider. But the guidelines always ignored that, and I think they should. We can adjust createrepo_cif appropriate later to narrow the list to the actually wanted one.)

Two points:

  1. I disagree in general. Certainly we should discourage such dependencies (which we do) and explain why. We used to have an explanation, but of course it was bogus since dnf just downloads everything. We kept the SHOULD rule there just in case dnf got better in the future, and if that's happening then we can add it back. But it looks like dnf isn't really getting better here, just getting broken.
  2. It would be much better if such a change could be reworded so that there is no "as an exception" text down below. I know that wording it either way is kind of awkward because you have two exceptions (the list of approved directories and the "file provides" bit) but I think it can be made clearer than what's here.

Obviously if this dnf5 breakage is accepted via the proper change procedure with FESCo approval, then the guidelines will be hanging with no questions asked and those packages will have to be fixed. But until then, this is a bug in dnf5 that really needs to be fixed there. Patching around things with packaging guidelines is really suboptimal.

FWIW I think this makes a good rule and I agree we should ban such requirements. What others say seems orthogonal to that.

But it looks like dnf isn't really getting better here, just getting broken.

What? It is getting better. Dnf5 does not load filelists for normal transactions RIGHT NOW. This already works.

Obviously if this dnf5 breakage is accepted via the proper change procedure with FESCo approval

What breakage? Please be more specific. I think we're talking past one another.

It doesn't load them at all, even if it needs to do so in order to resolve dependencies. This leaves packages unable to be installed unless you change a dnf configuration setting. Or am I missing something? I asked explicitly and this is what was told is the current dnf5 behavior.

The old yum behavior was to load the file lists only if necessary.

rebased onto 7c730f0b6879c2b58d5e63e05bdade2355eca4e5

It would be much better if such a change could be reworded so that there is no "as an exception" text down below.

I reworded the text.

It doesn't load them at all, even if it needs to do so in order to resolve dependencies.

It loads them if a path is specified explicitly by the user. I.e. both dnf5 install ngingx and dnf5 install /usr/lib/systemd/system/nginx.service work. The difference is that the latter downloads "filelists".

Please consider the following: whether dnf5 is "fixed" to automatically download and load "filelists" whenever a non-standard file dependency is encountered in some package is not really important. (Maybe this would be nicer, but maybe it's hard to implement, or maybe it'll be implemented later. It doesn't really matter at this point.) We want the optimization of not requiring "filelists" to happen for normal package installations. For this to work fully, we need to get rid of the file dependencies in packages. If we leave non-standard file dependencies in packages, a user who installs even one such package would be pessimized by having to download "filelists" for every operation as long as that package is installed. We get fairly nice benefits from such a small adjustment.

If we leave non-standard file dependencies in packages, a user who installs even one such package would be pessimized by having to download "filelists" for every operation as long as that package is installed.

There are two problems with this:

  1. There is no such thing as "non-standard file dependencies"
  2. This will already happen when people using COPRs or third party projects anyway

That is ignoring the fact that I think this isn't a good policy change to make in the first place. I want filelists to be downloaded opportunistically when it detects a need for it. There is wiring in libsolv for being able to do it, it is up to the DNF team to actually do it. If they don't, then we will need DNF to load the file lists anyway because we cannot guarantee the resolvability of third party packages without opportunistic loading unless we always load filelists.

I reworded the text.

Thanks, that looks much better.

It loads them if a path is specified explicitly by the user. I.e. both dnf5 install ngingx and dnf5 install /usr/lib/systemd/system/nginx.service work. The difference is that the latter downloads "filelists".

I take it to mean that I was not mistaken, and that it doesn't load them if a package has a dependency on a path outside of those few paths. So if I, say, try to install the "rt" package (which I think has a dependency on "/usr/share/fonts/google-droid/DroidSans.ttf") then it will simply fail to install.

Anyway, it's not that I'm not generally supportive of trying to speed up dnf by reducing file list downloads. It was quite sad that we lost that functionality when the switch to dnf first happened. But there is more going on here than changing seven packages and calling this good. There is a whole process for making this kind of breaking change which goes well beyond changing the packaging guidelines, and I think it should be followed.

Please consider the following: whether dnf5 is "fixed" to automatically download and load "filelists" whenever a non-standard file dependency is encountered in some package is not really important.

And I believe it is the fundamental issue here.

We want the optimization of not requiring "filelists" to happen for normal package installations.

Agreed, which is why the guidelines discourage that and of course it is already the case.

For this to work fully, we need to get rid of the file dependencies in packages.

And that is where you lose me. No, to get it work fully dnf needs to do what yum used to do: download the extra metadata only when it needs to do so. Using policy as a hack to get around its inability to do that is just not the proper way. And even ignoring that, this isn't the way to change the policy.

If we leave non-standard file dependencies in packages, a user who installs even one such package would be pessimized by having to download "filelists" for every operation as long as that package is installed. We get fairly nice benefits from such a small adjustment.

That makes no sense to me and I can't imagine why that would be a requirement. You would need it when resolving dependencies, if you encounter a file dependency that you can't resolve using the file data already downloaded. That would be on installation or updating only of packages which have these dependencies, not of all packages. Again, yum already worked this way.

Certainly, you can do without this type of dependency. You can even do without file dependencies altogether. (Doesn't Debian?) And if the distribution as a whole wants to ban file dependencies outside of a specific set of directories or even change that set over time, it could. But that isn't the packaging committee's call to make.

rebased onto 9cc15507079ee5c03f8157d3a76bdb202406e5a1

Based on the feedback, I updated the patch to NOT say "MUST NOT". The text is reordered a bit. See the commit description for details.

There is a whole process for making this kind of breaking change which goes well beyond changing the packaging guidelines, and I think it should be followed.

Effectively this patch is now a small cleanup, so I don't think it requires any special process. Please let me know if you think otherwise.

s/depencies/dependencies/

rebased onto 6a2df576234fa65a393bb4484170a51ecf172f3b

Updated.

The current change looks good. :thumbsup:

Since this now is a non-controversial clarification, can we please merge it?

rebased onto e5671d7e9838bc8b1a39b186bdde01fb2f313899

Pull-Request has been merged by ngompa

Metadata