Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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...

Loading