Skip to content

Commit

Permalink
Look for META files rather than just directories to list plugins (oca…
Browse files Browse the repository at this point in the history
…ml#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>
  • Loading branch information
3 people authored and MA0010 committed Jun 5, 2024
1 parent 4d06899 commit 8eb6944
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 7 deletions.
3 changes: 3 additions & 0 deletions doc/changes/10458.md
Original file line number Diff line number Diff line change
@@ -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)
23 changes: 16 additions & 7 deletions otherlibs/dune-site/src/plugins/plugins.ml
Original file line number Diff line number Diff line change
@@ -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 =
Expand Down Expand Up @@ -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
Expand Down
96 changes: 96 additions & 0 deletions otherlibs/dune-site/test/opam-uninstall.t
Original file line number Diff line number Diff line change
@@ -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...

0 comments on commit 8eb6944

Please sign in to comment.