Look for META files rather than just directories to list plugins (#10458

)

* Add a test including the uninstallation of an OPAM plugin package

Signed-off-by: Samuel Hym <samuel.hym@rustyne.lautre.net>

* Look for META files rather than just directories to list plugins

If the plugin directory contains anything but directories that each
contain a `META` file, loading plugins fails

This can happen naturally in particular in the following scenario:
- install a package providing a plugin to a command in another package
- and remove that plugin package.
This leaves an empty directory which triggers that failure.

Co-authored-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Signed-off-by: Samuel Hym <samuel.hym@rustyne.lautre.net>

* Update otherlibs/dune-site/test/opam-uninstall.t

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

* Add a changelog entry

Signed-off-by: Samuel Hym <samuel.hym@rustyne.lautre.net>

---------

Signed-off-by: Samuel Hym <samuel.hym@rustyne.lautre.net>
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Co-authored-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Co-authored-by: Etienne Millon <me@emillon.org>

doc/changes/10458.md

@@ -0,0 +1,3 @@
+- Make `dune-site`'s `load_all` function look for `META` files so that it
+  doesn't fail on empty directories in the plugin directory (#10458, fixes
+  #10457, @shym)

otherlibs/dune-site/src/plugins/plugins.ml

@@ -1,11 +1,22 @@
 open Dune_site.Private_
 module Data = Dune_site_plugins_data
 
-let readdir dirs =
-  List.concat
-    (List.map
-       (fun dir -> Array.to_list (Sys.readdir dir))
-       (List.filter Sys.file_exists dirs))
+let meta_fn = "META"
+
+let readdir =
+  let ( / ) = Filename.concat in
+  let readdir_noexn dir =
+    try Sys.readdir dir with
+    | Sys_error _ -> [||]
+  in
+  fun dirs ->
+    List.concat
+      (List.map
+         (fun dir ->
+           List.filter
+             (fun entry -> Sys.file_exists (dir / entry / meta_fn))
+             (Array.to_list (readdir_noexn dir)))
+         dirs)
 ;;
 
 let rec lookup dirs file =
@@ -208,8 +219,6 @@ let load file ~pkg =
   { Meta_parser.name = Some pkg; entries }
 ;;
 
-let meta_fn = "META"
-
 let lookup_and_load_one_dir ~dir ~pkg =
   let meta_file = Filename.concat dir meta_fn in
   if Sys.file_exists meta_file

otherlibs/dune-site/test/opam-uninstall.t

@@ -0,0 +1,96 @@
+Test an approximation of an OPAM install / uninstall of a plugin package
+
+This takes the sites-plugin.t blackbox test extracted from the manual and
+changes its end to cover:
+- dune install of both the application and its plugin,
+- dune uninstall of the plugin, leaving an empty directory as OPAM would.
+
+  $ cat > dune-project <<EOF
+  > (lang dune 3.8)
+  > (using dune_site 0.1)
+  > (name app)
+  > 
+  > (package
+  >  (name app)
+  >  (sites (lib plugins)))
+  > EOF
+
+  $ cat > dune <<EOF
+  > (executable
+  >  (public_name app)
+  >  (modules sites app)
+  >  (libraries app.register dune-site dune-site.plugins))
+  > 
+  > (library
+  >  (public_name app.register)
+  >  (name registration)
+  >  (modules registration))
+  > 
+  > (generate_sites_module
+  >  (module sites)
+  >  (plugins (app plugins)))
+  > EOF
+
+  $ cat > registration.ml <<EOF
+  > let todo : (unit -> unit) Queue.t = Queue.create ()
+  > EOF
+
+  $ cat > app.ml <<EOF
+  > (* load all the available plugins *)
+  > let () = Sites.Plugins.Plugins.load_all ()
+  > 
+  > let () = print_endline "Main app starts..."
+  > (* Execute the code registered by the plugins *)
+  > let () = Queue.iter (fun f -> f ()) Registration.todo
+  > EOF
+
+
+  $ mkdir plugin
+  $ cat > plugin/dune-project <<EOF
+  > (lang dune 3.8)
+  > (using dune_site 0.1)
+  > 
+  > (generate_opam_files true)
+  > 
+  > (package
+  >  (name plugin1))
+  > EOF
+
+  $ cat > plugin/dune <<EOF
+  > (library
+  >  (public_name plugin1.plugin1_impl)
+  >  (name plugin1_impl)
+  >  (modules plugin1_impl)
+  >  (libraries app.register))
+  > 
+  > (plugin
+  >  (name plugin1)
+  >  (libraries plugin1.plugin1_impl)
+  >  (site (app plugins)))
+  > EOF
+
+  $ cat > plugin/plugin1_impl.ml <<EOF
+  > let () =
+  > print_endline "Registration of Plugin1";
+  > Queue.add (fun () -> print_endline "Plugin1 is doing something...") Registration.todo
+  > EOF
+
+  $ dune build @install
+  $ dune install --prefix _install
+
+  $ OCAMLPATH=_install/lib:$OCAMLPATH _install/bin/app
+  Registration of Plugin1
+  Main app starts...
+  Plugin1 is doing something...
+
+  $ dune uninstall --prefix _install plugin1
+
+Unfortunately, the fact that the `lib/app/plugins/plugin1` directory should be
+removed along with plugin1 is lost in the OPAM metadata, so we simulate this
+issue by recreating this empty directory.
+
+  $ mkdir -p _install/lib/app/plugins/plugin1
+
+  $ OCAMLPATH=_install/lib:$OCAMLPATH _install/bin/app
+  Main app starts...
+