[PATCH] insane.bbclass: add indirect-build-deps QA check

[Patrick Ohly <patrick.ohly@intel.com>]

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.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/insane.bbclass | 52 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 156ae5a..70f110c 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -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)'])
-- 
2.1.4