Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpminspect complains about missing depedency that is provided transitively #1231

Closed
keszybz opened this issue Aug 3, 2023 · 6 comments · Fixed by #1309
Closed

rpminspect complains about missing depedency that is provided transitively #1231

keszybz opened this issue Aug 3, 2023 · 6 comments · Fixed by #1309
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@keszybz
Copy link

keszybz commented Aug 3, 2023

Subpackage systemd-tests on i686 carries 'Requires: libsystemd.so.0' which comes from subpackage systemd-libs but does not carry an explicit package version requirement. Please add 'Requires: systemd-libs = %{version}-%{release}' to the spec file to avoid the need to test interoperability between various combinations of old and new subpackages.

Suggested remedy:
Add the indicated explicit Requires to the spec file for the named subpackage. Subpackages depending on shared libraries in another subpackage must carry an explicit 'Requires: SUBPACKAGE_NAME = %{version}-%{release}' in the spec file.

The dependency is provided transitively:

$ rpm -qR systemd-tests|rg '^systemd'
systemd(x86-64) = 254~rc2-1.fc38
$ rpm -qR systemd|rg '^systemd-libs' 
systemd-libs = 254~rc2-1.fc38

It is a fairly common pattern to have such transitive deps between subpackages. I think rpminspect could figure out if a dependency is satisfied transitively among subpackages.

Also, I think the dependency needs to be archful. I.e. it should include (%_isa). In systemd, the systemd subpackage doesn't have this, which is probably an error. The advice given by rpminspect should include it, because libraries by definition are compiled code and archful.

@msrb
Copy link
Contributor

msrb commented Aug 24, 2023

Hmm 🤔 I'd argue that it is a good practice to list the dependencies explicitly. Do you see any problems or disadvantages with the explicit listing?

@keszybz
Copy link
Author

keszybz commented Aug 24, 2023

It's not that an explicit listing would be bad. But it might be unnecessary, and it is not an error to not have it. This warning is about subpackages of a single package, maintained together, and it's entirely reasonable for the maintainer to make use of transitive dependencies like this. If this was between different packages, then I'd argue that it should always be declared explicitly, because the other package may rearrange components at any time. But when it's all in a single spec file, the maintainer fully controls all the subpackages. In particular, the maintainer might want to not add those dependencies explicitly to keep down the complexity of the spec file.

In general, this is another of those cases where some advice might be useful in some cases, but it's very hard to figure out programatically if it applies. Rpminspect presents this as certain. I would very much prefer for rpminspect to not print the warning at all, so that we can get to point where warnings from rpminspect have no false positives, or almost no false positives, and then maintainers are motivated to fix the issues that are reported.

@dcantrell dcantrell added enhancement New feature or request question Further information is requested labels Sep 6, 2023
@dcantrell dcantrell self-assigned this Sep 6, 2023
@dcantrell
Copy link
Collaborator

rpminspect's behavior for this check comes from the longstanding practice that automatically generated shared object dependencies at rpmbuild time should be explicitly added as Requires in the spec file.

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_requires

The guidance on this topic these days is unclear to me, but it seems we still lean in that direction. Personally, I feel explicitly listing them is better than not because it gives a hard requires on the dependent package NVR rather than relying on the autodep which just gives the SONAME and could lead to a situation where you have version mismatches installed for those subpackages. Though honestly we wouldn't have that problem in the world of Linux if we [developers] were better about API versioning and stability, but we're not. :)

rpminspect does not unwrap explicit dependencies like you describe. I suppose I could add support for that, but it does feel like shaving the yak.

[Side topic... looking at systemd and systemd-libs, are those packages ever able to exist by themselves or do you always need to have both installed? I prefer fewer packages than more packages and if these two effectively act as a single package, I would recommend putting systemd-libs content in systemd.]

@keszybz
Copy link
Author

keszybz commented Sep 7, 2023

Personally, I feel explicitly listing them is better than not because it gives a hard requires on the dependent package NVR rather than relying on the autodep which just gives the SONAME and could lead to a situation where you have version mismatches installed for those subpackages.

Nope, in this case (as described in the original report), I have explicit deps with a fixed version. So autodep is not used here at all.

it does feel like shaving the yak.

Well… I'd say that the the implementation of the original check was the shaving of the yak. It must have been quite a bit of work, but it was done without apparently considering various way in which the check is insufficient.

looking at systemd and systemd-libs, are those packages ever able to exist by themselves or do you always need to have both installed?

systemd-libs provides public systemd libraries (libsystemd, libudev), which other programs can link to. It also provides various nss modules. All of those can work without systemd (even on systems with a different init system to a large extent). So yes, systemd-libs can exist by itself, and in containers and mock and such will often be installed without the big systemd package.

@keszybz
Copy link
Author

keszybz commented Sep 7, 2023

U2FsdGVkX1 pushed a commit to fedora-riscv/systemd that referenced this issue Nov 2, 2023
@dcantrell
Copy link
Collaborator

Personally, I feel explicitly listing them is better than not because it gives a hard requires on the dependent package NVR rather than relying on the autodep which just gives the SONAME and could lead to a situation where you have version mismatches installed for those subpackages.

Nope, in this case (as described in the original report), I have explicit deps with a fixed version. So autodep is not used here at all.

Right, I did see that. I was explaining more the origin of what rpminspect is doing. Unfortunately a lot of the "why" of the rpmdeps check has been lost to time, but it was all built for issues showing up in RHEL at the time. I was not trying to claim it makes sense or not.

it does feel like shaving the yak.

Well… I'd say that the the implementation of the original check was the shaving of the yak. It must have been quite a bit of work, but it was done without apparently considering various way in which the check is insufficient.

I did the implementation in rpminspect basically by reimplementing what rpmdiff had been doing as my first order of business was feature parity. And yes, it was quite a bit of work.

I knew that its first iteration would present problems like this, but I did not have a good way to determine what packages in the wild would present these issues. My goal is to make the check mostly useful for the purposes of shipping a functional distribution.

My yak shaving comment was meant to imply that we can keep going down the path of handling more and more RPM dependency cases, but at some point we [the distribution] needs a packaging policy in place that says how we will use the features of the packaging system and what self-imposed limits we will have. That's sort of the objective of rpminspect is to enforce the rules we make for ourselves. Your not wrong that this functionality is fine in RPM packages, but I want to see that cross-referenced with packaging policy.

looking at systemd and systemd-libs, are those packages ever able to exist by themselves or do you always need to have both installed?

systemd-libs provides public systemd libraries (libsystemd, libudev), which other programs can link to. It also provides various nss modules. All of those can work without systemd (even on systems with a different init system to a large extent). So yes, systemd-libs can exist by itself, and in containers and mock and such will often be installed without the big systemd package.

Noted, thanks.

@dcantrell dcantrell added this to the v1.13 milestone Nov 7, 2023
@dcantrell dcantrell added in progress Work on this issue is currently underway, but that does not imply it will be done quickly. and removed question Further information is requested labels Nov 7, 2023
dcantrell added a commit to dcantrell/rpminspect that referenced this issue Nov 30, 2023
A package with automatic shared library dependencies usually require
an explicit Requires on the %{name} = %{version}-%{release}.  This is
done to ensure subpackages don't drift from the main package when
performing upgrades.  However, it is also possible to have those
explicit Requires handled by transitive Requires.  That is having an
explicit Requires on a package that then itself has an explicit
Requires on the package you need.  This commit implements that check
when looking for explicit Requires and treats it as satisifed.

Fixes: rpminspect#1231

Signed-off-by: David Cantrell <dcantrell@redhat.com>
@dcantrell dcantrell removed the in progress Work on this issue is currently underway, but that does not imply it will be done quickly. label Nov 30, 2023
dcantrell added a commit that referenced this issue Dec 5, 2023
A package with automatic shared library dependencies usually require
an explicit Requires on the %{name} = %{version}-%{release}.  This is
done to ensure subpackages don't drift from the main package when
performing upgrades.  However, it is also possible to have those
explicit Requires handled by transitive Requires.  That is having an
explicit Requires on a package that then itself has an explicit
Requires on the package you need.  This commit implements that check
when looking for explicit Requires and treats it as satisifed.

Fixes: #1231

Signed-off-by: David Cantrell <dcantrell@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants