indirect dependencies
I recently ran into a situation where due to a stupid typo (DEPEND instead of DEPENDS) my recipe happened to work for me when the required package was pulled in indirectly and failed for someone else.
It would be nice if such indirect dependencies triggered a QA warning, which they do not at the moment.
I started coding that, but ran into issues with false positives. I don't have the time and knowledge to address these problems, but wanted to shared the code anyway, in case that someone wants to continue.
Patrick Ohly (1):
insane.bbclass: add indirect-build-deps QA check
meta/classes/insane.bbclass | 52 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 7 deletions(-)
Signature · 1 line
2.1.4
PATCH 1/1 insane.bbclass: add indirect-build-deps QA check
Indirect dependencies occur when a recipe does not declare a depency on "foo" because it depends on "bar", which in turn depends on "foo". This happens to work, but is fragile and should better be avoided by listing "foo" explicitly, because:
* "bar" might not depend on "foo" in all configurations
* future changes to "bar" might remove the dependency on "bar"
The traditional "build-deps" QA check skips indirect dependencies because they are listed in the task dependencies. Checking for indirect dependencies remembers that instead of aborting the check and then later tries to filter out false positives by looking at DEPENDS.
Some common packages which can be assumed to be present are covered by INDIRECT_BUILD_DEPS_WHITELIST. That list needs to be reviewed and also take into account the build configuration - glibc is listed there at the moment, but might not actually be the configured base library.
However, there are at least two false positives left in the current code:
* aliases for recipes via PROVIDES
* unnecessarily linking against libs which are not really used
See the TODOs in the code.
--- meta/classes/insane.bbclass | 52 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 7 deletions(-)
meta/classes/insane.bbclass +45 -7
@@ -29,7 +29,7 @@ QA_SANE = "True" WARN_QA ?= "ldflags useless-rpaths rpaths staticdev libdir xorg-driver-abi \ textrel already-stripped incompatible-license files-invalid \ installed-vs-shipped compile-host-path install-host-path \ - pn-overrides infodir build-deps file-rdeps \ + pn-overrides infodir build-deps indirect-build-deps file-rdeps \ unknown-configure-option \ " ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \
@@ -42,6 +42,8 @@ ALL_QA = "${WARN_QA} ${ERROR_QA}" UNKNOWN_CONFIGURE_WHITELIST ?= "--enable-nls --disable-nls --disable-silent-rules --disable-dependency-tracking --with-libtool-sysroot" +INDIRECT_BUILD_DEPS_WHITELIST ?= "glibc" + # # dictionary for elf headers #
@@ -763,6 +765,8 @@ def package_qa_walk(path, warnfuncs, errorfuncs, skip, package, d): return len(errors) == 0 + + def package_qa_check_rdepends(pkg, pkgdest, skip, taskdeps, packages, d): # Don't do this check for kernel/module recipes, there aren't too many debug/development # packages and you can get false positives e.g. on kernel-module-lirc-dev
@@ -779,18 +783,37 @@ def package_qa_check_rdepends(pkg, pkgdest, skip, taskdeps, packages, d): rdepends = bb.utils.explode_deps(localdata.getVar('RDEPENDS', True) or "") # Now do the sanity check!!! - if "build-deps" not in skip: + skip_indirect = "indirect-build-deps" in skip + skip_direct = "build-deps" in skip + if not skip_direct and not skip_indirect: + indirect_whitelist = [] if skip_indirect else set((d.getVar('INDIRECT_BUILD_DEPS_WHITELIST', True) or '').split()) for rdepend in rdepends: + bb.note("DO PACKAGE QA: rdepend %s pkg %s" % (rdepend, pkg)) + if "-dbg" in rdepend and "debug-deps" not in skip: error_msg = "%s rdepends on %s" % (pkg,rdepend) sane = package_qa_handle_error("debug-deps", error_msg, d) if (not "-dev" in pkg and not "-staticdev" in pkg) and rdepend.endswith("-dev") and "dev-deps" not in skip: error_msg = "%s rdepends on %s" % (pkg, rdepend) sane = package_qa_handle_error("dev-deps", error_msg, d) - if rdepend not in packages: + internaldep = rdepend in packages + if not internaldep and not skip_indirect: + # Might be a name provided by some of the packages (example: + # libpam RPROVIDES libpam-suffix, which libpam-runtime RDEPENDS on). + # If we do not check for this, the rdepend will treated like + # an invalid indirect dependency although this is okay. + if rdepend in (d.getVar("PROVIDES", True) or "").split(): + internaldep = True + else: + for internalpkg in packages: + if rdepend in (d.getVar("RPROVIDES_%s" % internalpkg, True) or "").split(): + internaldep = True + break + if not internaldep: rdep_data = oe.packagedata.read_subpkgdata(rdepend, d) + indirect = False if rdep_data and 'PN' in rdep_data and rdep_data['PN'] in taskdeps: - continue + indirect = True if not rdep_data or not 'PN' in rdep_data: pkgdata_dir = d.getVar("PKGDATA_DIR", True) try:
@@ -802,9 +825,24 @@ def package_qa_check_rdepends(pkg, pkgdest, skip, taskdeps, packages, d): if rdep_data and 'PN' in rdep_data and rdep_data['PN'] in taskdeps: break if rdep_data and 'PN' in rdep_data and rdep_data['PN'] in taskdeps: - continue - error_msg = "%s rdepends on %s, but it isn't a build dependency?" % (pkg, rdepend) - sane = package_qa_handle_error("build-deps", error_msg, d) + indirect = True + if indirect: + if skip_indirect or rdepend in indirect_whitelist: + continue + # Need to avoid false positives when the potential indirect dependency is + # in fact listed as a direct dependency. Also need to handle aliases here. + depends = set((d.getVar('DEPENDS', True) or '').split()) + if rdepend in depends: + continue + # TODO: check PROVIDES of the rdepend recipe to see whether that is + # listed in DEPENDS. + error_msg = "%s rdepends on %s, but it only an indirect build dependency?" % (pkg, rdepend) + sane = package_qa_handle_error("indirect-build-deps", error_msg, d) + else: + if skip_direct: + continue + error_msg = "%s rdepends on %s, but it isn't a build dependency?" % (pkg, rdepend) + sane = package_qa_handle_error("build-deps", error_msg, d) if "file-rdeps" not in skip: ignored_file_rdeps = set(['/bin/sh', '/usr/bin/env', 'rtld(GNU_HASH)'])
Signature · 1 line
2.1.4
Re: [PATCH] insane.bbclass: add indirect-build-deps QA check
Quoted 5 lines · Patrick Ohly
However, there are at least two false positives left in the current code:
* aliases for recipes via PROVIDES
* unnecessarily linking against libs which are not really usedSee the TODOs in the code.
Actually, I had written more about that which didn't make it into the posted patch. Here's the more complete comment:
# TODO: check PROVIDES of the rdepend recipe to see whether that is
# listed in DEPENDS. Example: "smack" instead of "userspace-smack" in
# https://github.com/01org/meta-intel-iot-security/blob/master/meta-security-smack/recipes-extended/libpam/libpam_%25.bbappend which is an alias for # https://github.com/01org/meta-intel-iot-security/blob/master/meta-security-smack/recipes-security/smack/smack-userspace_git.bb
# TODO: pam-plugin-cracklib/lib/security/pam_cracklib.so is unecessarily # linked against libz.so.1, probably because of link flags from libcrack, # which is what libpam calls and (correctly) lists as DEPENDS. # Because libz is not listed as DEPENDS, we warn about it here # unnecessarily. This could be fairly common; not sure how to detect # it though, short of comparing symbol tables.
Quoted 4 lines
meta/classes/insane.bbclass +4
-0+ # TODO: check PROVIDES of the rdepend recipe to see whether that is + # listed in DEPENDS. + error_msg = "%s rdepends on %s, but it only an indirect build dependency?" % (pkg, rdepend) + sane = package_qa_handle_error("indirect-build-deps", error_msg, d)
Signature · 6 lines
Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter.