deduplicates dune plugins and ignores empty plugins (#1569)

Removes duplicating packages, provided that they have the same
implementation.

When dune executes a binary (either via `dune exec` or `dune test`) it
sets an internal `DUNE_DIR_LOCATIONS` environment variable, which
affects the selection of plugins in bap, even if bap is installed from
an indepenent repository. I am still not sure if this is a bug or a
feature of dune, but to prevent double-loading we have to deduplicate
plugins. We silently remove duplicating names only if all
implementation that match the name are the same. We compare
implementation using the sum of md5 sums of files that comprise the
plugin folder. The choice of md5 sum as the implementation witness
instead of parsing the META files allows us to preserve the
abstraction of dune plugins, as the contents of the folder is the
implementation detail of dune. This increases the probability that
this code will still work, when they will change the implementation.

In addition, this change arounds another subtle bug in the dune plugin
system. When a plugin is removed with `opam remove`, opam leaves an
empty folder in the plugin folder, which we should ignore, otherwise
dune plugin loader will fail on it.

Author: ivg

lib/bap_plugins/bap_plugins.ml

@@ -306,17 +306,66 @@ module Plugins = struct
         | Ok p -> Some p
         | Error _ -> None)
 
+  (* returns a sorted deduplicated list of dune plugin names.
+
+     dune sites might have duplicating folders, so we have to be
+     careful that we do not try to load the same plugin twice. *)
   let list_packages () =
+    let require cnd k = if cnd then k () else None in
+
+    (* for each occurence of [name] in the search paths
+       returns its digest. The digest is the sum of md5 sums
+       of all files that comprise the plugin folder. *)
+    let digests name =
+      Bap_common.Plugins.paths |>
+      List.filter_map ~f:(fun dir ->
+          let path = Filename.concat dir name in
+          require (Sys.file_exists path) @@ fun () ->
+          require (Sys.is_directory path) @@ fun () ->
+          let contents =
+            Sys.readdir path |>
+            Array.map ~f:(Filename.concat path) in
+          require (Array.length contents > 0) @@ fun () ->
+          Option.return @@
+          Array.sum ~f:Md5.digest_file_blocking (module struct
+            type t = Md5.t
+            let zero = Md5.digest_string ""
+            let (+) x y =
+              Md5.digest_string (Md5.to_binary x ^ Md5.to_binary y)
+          end) contents) in
+
+    (* removes duplicating names provided that they have the same
+       implementation. The implementation is digested as the md5 sum
+       of files comprising the plugin folder.  *)
+    let deduplicate names =
+      let init = Map.empty (module String) in
+      List.fold names ~init ~f:(fun plugins name ->
+          Map.update plugins name ~f:(function
+              | None -> digests name
+              | Some ds -> ds @ digests name)) |>
+      Map.filteri ~f:(fun ~key:name ~data:digests ->
+          match digests with
+          | [] -> false
+          | d::ds when List.for_all ds ~f:(Md5.equal d) -> true
+          | _ ->
+            invalid_argf "The plugin %S has ambiguous implementations"
+              name ()) |>
+      Map.keys in
+
     Bap_common.Plugins.list () |>
+    deduplicate |>
     List.map ~f:Plugin.of_package
 
+
+
   let list ?env ?provides ?library () =
     list_bundles ?env ?provides ?library () @
     list_packages ()
 
   let collect ?env ?provides ?library () =
     collect_bundles ?env ?provides ?library () @
-    List.map ~f:Result.return @@list_packages ()
+    List.map ~f:Result.return @@
+    list_packages ()
 
   let loaded,finished = Future.create ()