[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 4/4] rpm: Choose providers better (RHBZ#1266918).



On Tuesday 13 October 2015 13:54:31 Richard W.M. Jones wrote:
> In the referenced bug, a customer had installed a web browser called
> 'palemoon'.  The RPM of this web browser provides and requires various
> core libraries, such as:
> 
>   Provides: libnss3.so()(64bit) # normally provided by 'nss'
>   Requires: libxul.so()(64bit)  # normally provided by 'firefox'
> 
> Our previous algorithm -- inherited from the days when we used to run
> 'rpm' commands -- takes every provider of a particular requirement and
> adds it to a big list, so if any other package requires
> 'libnss3.so()(64bit)', then both 'nss' and 'palemoon' are added to the
> package list.
> 
> Yum used to handle this differently - it used to only pick the package
> with the shortest name.  Later on, before yum was retired, it had a
> more complex decision algorithm described here:
> http://yum.baseurl.org/wiki/CompareProviders
> 
> This change makes supermin use the shortest name algorithm, so in the
> case above, it always picks 'nss' over 'palemoon'.
> 
> There is a second possible problem which is not fixed by the current
> patch set: If a package both provides and requires the same
> dependency, we should ignore that dependency.  For example, 'palemoon'
> both Provides and Requires 'libxul.so()(64bit)', so if 'palemoon' is
> pulled in, then 'firefox' would not be an additional requirement.
> Because we store all the packages in a big list, we lose track of
> where a dependency originates from, so it is not easy to implement
> this second change.
> ---
>  src/rpm.ml | 55 +++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 39 insertions(+), 16 deletions(-)
> 
> diff --git a/src/rpm.ml b/src/rpm.ml
> index 4d31472..d3ab7da 100644
> --- a/src/rpm.ml
> +++ b/src/rpm.ml
> @@ -210,8 +210,42 @@ let rpm_package_name pkg =
>  let rpm_get_package_database_mtime () =
>    (lstat "/var/lib/rpm/Packages").st_mtime
>  
> -(* Memo of resolved provides. *)
> -let rpm_providers = Hashtbl.create 13
> +(* Return the best provider of a particular RPM requirement.
> + *
> + * There may be multiple, or no providers.  In case there are multiple,
> + * choose the one with the shortest name (as yum used to).
> + *
> + * We could do better here: http://yum.baseurl.org/wiki/CompareProviders
> + *)
> +let provider =
> +  (* Memo of resolved provides. *)
> +  let rpm_providers = Hashtbl.create 13 in
> +  fun req ->
> +    try Hashtbl.find rpm_providers req
> +    with Not_found ->
> +      try
> +        (* 'providers' here is an array of just package names. *)
> +        let providers = rpm_pkg_whatprovides (get_rpm ()) req in
> +        let providers = Array.to_list providers in
> +        (* --whatprovides will return duplicate identical packages, so: *)
> +        let providers = sort_uniq providers in
> +        match providers with
> +        | [] -> None
> +        | [name] -> Some name
> +        | names ->
> +           if !settings.debug >= 2 then
> +             printf "supermin: rpm: multiple providers: requirement %s: providers: %s\n"
> +                    req (String.concat " " names);
> +           let cmp name1 name2 =
> +             let len1 = String.length name1 and len2 = String.length name2 in
> +             if len1 <> len2 then compare len1 len2
> +             else compare name1 name2 in
> +           let names = List.sort cmp names in
> +           let best_name = List.hd names in
> +           if !settings.debug >= 2 then
> +             printf "supermin: rpm: multiple providers: picked %s\n" best_name;
> +           Some best_name
> +      with Not_found -> None

This is nicer indeed, although there are two issues:

- the caching of providers (rpm_providers) is not done, so it will do
  the search every time (and things like "/bin/sh provided by bash"
  appear a lot in packages)

- the old code was checking each provided name was really a package
  (using rpm_package_of_string).  The new code should then pick the
  first one in the list that is actually installed (i.e. with a Some
  result of rpm_package_of_string); the two cases
| +        | [name] -> Some name
| +        | names ->
could be joined together, printing the debug output only if there is
more than one element. I hope List.sort on one element should be a
no-op...

Thanks,
-- 
Pino Toscano

Attachment: signature.asc
Description: This is a digitally signed message part.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]