[bitbake-devel] [PATCH] runqueue: Improve 'mulitiple .bb files are due to be built' message

[Christopher Larson]

On Sun, Apr 10, 2016 at 3:10 AM Richard Purdie <
richard.purdie at linuxfoundation.org> wrote:

> When multiple recipes which both provide something are being built, bitbake
> informs us that most likely one of them provides something the other
> doesn't,
> which is usually correct, but unfortunately it's rather painful to figure
> out
> exactly what that is.
>
> This patch dumps two sets of information, one is the provides information
> for
> each recipe, filtered so only common components are removed. The other is
> a list
> of dependees on the recipe, since sometimes this can easily identify why
> something
> is being built.
>
> Its not straightforward for bitbake to obtain the information but since the
> warning/error code path isn't the normal one, we can afford to go through
> some
> less than optimal processing to aid debugging.
>
> Also provide the same information even if we're showing a warning since
> its still
> useful.
>
> [YOCTO #8032]
>
> Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
>

Nice work on this, thanks much. I think this will be a big help in
diagnosing issues of this sort. I have a few very minor comments below.

diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
> index 2990de7..ac3cdae 100644
> --- a/bitbake/lib/bb/runqueue.py
> +++ b/bitbake/lib/bb/runqueue.py
> @@ -261,6 +261,13 @@ class RunQueueData:
>          taskname = self.runq_task[task] + task_name_suffix
>          return "%s, %s" % (fn, taskname)
>
> +    def get_short_user_idstring(self, task, task_name_suffix = ""):
> +        fn = self.taskData.fn_index[self.runq_fnid[task]]
> +        pn = self.dataCache.pkg_fn[fn]
> +        taskname = self.runq_task[task] + task_name_suffix
> +        return "%s:%s" % (pn, taskname)
> +
> +
>      def get_task_id(self, fnid, taskname):
>          for listid in xrange(len(self.runq_fnid)):
>              if self.runq_fnid[listid] == fnid and self.runq_task[listid]
> == taskname:
> @@ -753,11 +760,72 @@ class RunQueueData:
>                          seen_pn.append(pn)
>                      else:
>                          bb.fatal("Multiple versions of %s are due to be
> built (%s). Only one version of a given PN should be built in any given
> build. You likely need to set PREFERRED_VERSION_%s to select the correct
> version or don't depend on multiple versions." % (pn, "
> ".join(prov_list[prov]), pn))
> -                msg = "Multiple .bb files are due to be built which each
> provide %s (%s)." % (prov, " ".join(prov_list[prov]))
> +                msg = "Multiple .bb files are due to be built which each
> provide %s:\n  %s" % (prov, "\n  ".join(prov_list[prov]))
> +                #
> +                # Construct a list of things which uniquely depend on
> each provider
> +                # since this may help the user figure out which
> dependency is triggering this warning
> +                #
> +                msg += "\nA list of tasks depending on these providers is
> shown and may help explain where the dependency comes from."
> +                deplist = {}
> +                commondeps = None
> +                for provfn in prov_list[prov]:
> +                    deps = set()
> +                    for task in xrange(len(self.runq_fnid)):
> +                        fn = taskData.fn_index[self.runq_fnid[task]]
> +                        if fn != provfn:
> +                            continue
>


I think this would be more pythonic:

    for task, fnid in enumerate(self.runq_fnid):
       fn = taskData.fn_index[fnid]


+                        for dep in self.runq_revdeps[task]:
> +                            fn = taskData.fn_index[self.runq_fnid[dep]]
> +                            if fn == provfn:
> +                                continue
> +                            deps.add(self.get_short_user_idstring(dep))
>


Here we look up the fn of dep, but then get_short_user_idstring() does so
as well, might be worth passing it in instead, unless it's used elsewhere
where the fn isn't yet available.


+                    if not commondeps:
> +                        commondeps = set(deps)
> +                    else:
> +                        commondeps &= deps
> +                    deplist[provfn] = deps
> +                for provfn in deplist:
> +                    msg += "\n%s has unique dependees:\n  %s" % (provfn,
> "\n  ".join(deplist[provfn] - commondeps))
> +                #
> +                # Construct a list of provides and runtime providers for
> each recipe
> +                # (rprovides has to cover RPROVIDES, PACKAGES,
> PACKAGES_DYNAMIC)
> +                #
> +                msg += "\nIt could be that one recipe provides something
> the other doesn't and should. The following provider and runtime provider
> differences may be helpful."
> +                provlist = {}
>


Might want to rename this variable to improve readability, just because
there's also prov_list :)


+                rprovlist = {}
> +                commonprovs = None
> +                commonrprovs = None
> +                for provfn in prov_list[prov]:
> +                    provides = set(self.dataCache.fn_provides[provfn])
> +                    rprovides = set()
> +                    for rprovide in self.dataCache.rproviders:
> +                        if provfn in self.dataCache.rproviders[rprovide]:
> +                            rprovides.add(rprovide)
> +                    for package in self.dataCache.packages:
> +                        if provfn in self.dataCache.packages[package]:
> +                            rprovides.add(package)
> +                    for package in self.dataCache.packages_dynamic:
> +                        if provfn in
> self.dataCache.packages_dynamic[package]:
> +                            rprovides.add(package)
>


Minor and largely personal preference either way, but these loops are
simple enough they could be comprehensions. Don't feel too strongly either
way on that.


+                    if not commonprovs:
> +                        commonprovs = set(provides)
> +                    else:
> +                        commonprovs &= provides
> +                    provlist[provfn] = provides
> +                    if not commonrprovs:
> +                        commonrprovs = set(rprovides)
> +                    else:
> +                        commonrprovs &= rprovides
> +                    rprovlist[provfn] = rprovides
> +                #msg += "\nCommon provides:\n  %s" % ("\n
> ".join(commonprovs))
> +                #msg += "\nCommon rprovides:\n  %s" % ("\n
> ".join(commonrprovs))
> +                for provfn in prov_list[prov]:
> +                    msg += "\n%s has unique provides:\n  %s" % (provfn,
> "\n  ".join(provlist[provfn] - commonprovs))
> +                    msg += "\n%s has unique rprovides:\n  %s" % (provfn,
> "\n  ".join(rprovlist[provfn] - commonrprovs))



Might want to check to see if the list of uniques is empty before
displaying its message, just to reduce possible confusion.

Thanks again,
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openembedded.org/pipermail/bitbake-devel/attachments/20160410/d7624827/attachment-0002.html>