Add comments, fix indentation

1. Add comment explaining why dune-site.linker is virtual. 2. Add comment explaining why two opens in toplevel/linker.ml. 3. Fix indentation of dune stanzas in tests. Signed-off-by: Richard L Ford <[email protected]>
ocaml · Mar 7, 2023 · c495a83 · c495a83
1 parent 102b609
commit c495a83
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 119 deletions.
diff --git a/otherlibs/site/src/plugins/linker/dune b/otherlibs/site/src/plugins/linker/dune
@@ -1,5 +1,12 @@
 (library
  (name dune_site_backend)
  (public_name dune-site.linker)
+
+ ; The linker module is virtual because it has two implementations
+ ; for load.
+ ; dune-site.dynlink implements it using Dynlink.loadfile
+ ; dune-site.toplevel implements it using
+ ; Topdirs.loadfile (before 4.13.0) or Toploop.loadfile (otherwise).
+ ; dune-site.toplevel is needed for OCaml toplevels with plugins.
  (virtual_modules linker)
  (default_implementation dune-site.dynlink))
diff --git a/otherlibs/site/src/plugins/linker/toplevel/linker.ml b/otherlibs/site/src/plugins/linker/toplevel/linker.ml
@@ -1,3 +1,11 @@
+(* 
+  Prior to OCaml 4.13.0, [load_file] was in the Topdirs module.
+  Beginning with OCaml 4.13.0, load_file is in the Toploop module.
+  In order to be able to compile with OCaml versions either
+  before or after, open both modules and let the compiler
+  find [load_file] where it is defined.
+ *)
+
 open Topdirs [@@ocaml.warning "-33"]
 open Toploop [@@ocaml.warning "-33"]
 

diff --git a/test/blackbox-tests/test-cases/sites-plugin.t b/test/blackbox-tests/test-cases/sites-plugin.t
@@ -6,20 +6,20 @@ Test sites plugins (example from the manual)
   > (name app)
   > 
   > (package
-  > (name app)
-  > (sites (lib plugins)))
+  >  (name app)
+  >  (sites (lib plugins)))
   > EOF
 
   $ cat > dune <<EOF
   > (executable
-  > (public_name app)
-  > (modules sites app)
-  > (libraries app.register dune-site dune-site.plugins))
+  >  (public_name app)
+  >  (modules sites app)
+  >  (libraries app.register dune-site dune-site.plugins))
   > 
   > (library
-  > (public_name app.register)
-  > (name registration)
-  > (modules registration))
+  >  (public_name app.register)
+  >  (name registration)
+  >  (modules registration))
   > 
   > (generate_sites_module
   > (module sites)
@@ -48,20 +48,20 @@ Test sites plugins (example from the manual)
   > (generate_opam_files true)
   > 
   > (package
-  > (name plugin1))
+  >  (name plugin1))
   > EOF
 
   $ cat > plugin/dune <<EOF
   > (library
-  > (public_name plugin1.plugin1_impl)
-  > (name plugin1_impl)
-  > (modules plugin1_impl)
-  > (libraries app.register))
+  >  (public_name plugin1.plugin1_impl)
+  >  (name plugin1_impl)
+  >  (modules plugin1_impl)
+  >  (libraries app.register))
   > 
   > (plugin
-  > (name plugin1)
-  > (libraries plugin1.plugin1_impl)
-  > (site (app plugins)))
+  >  (name plugin1)
+  >  (libraries plugin1.plugin1_impl)
+  >  (site (app plugins)))
   > EOF
 
   $ cat > plugin/plugin1_impl.ml <<EOF

diff --git a/test/blackbox-tests/test-cases/toplevel-plugin-fail.t b/test/blackbox-tests/test-cases/toplevel-plugin-fail.t
@@ -10,31 +10,31 @@ This is not allowed in toplevels, so it fails.
   > (map_workspace_root false)
   > 
   > (package
-  >   (name top_with_plugins)
-  >   (sites (lib top_plugins)))
+  >  (name top_with_plugins)
+  >  (sites (lib top_plugins)))
   > EOF
 
   $ cat > dune <<EOF
   > (executable
-  >   (public_name top_with_plugins)
-  >   (name top_with_plugins)
-  >   (modes byte)
-  >   (flags :standard -safe-string)
-  >   (modules sites top_with_plugins)
-  >   (link_flags (-linkall))
-  >   (libraries compiler-libs.toplevel
-  >     top_with_plugins.register dune-site dune-site.plugins 
-  >     dune-site.dynlink))
+  >  (public_name top_with_plugins)
+  >  (name top_with_plugins)
+  >  (modes byte)
+  >  (flags :standard -safe-string)
+  >  (modules sites top_with_plugins)
+  >  (link_flags (-linkall))
+  >  (libraries compiler-libs.toplevel
+  >   top_with_plugins.register dune-site dune-site.plugins 
+  >   dune-site.dynlink))
   > 
   > (library
-  >   (public_name top_with_plugins.register)
-  >   (modes byte)
-  >   (name registration)
-  >   (modules registration))
+  >  (public_name top_with_plugins.register)
+  >  (modes byte)
+  >  (name registration)
+  >  (modules registration))
   > 
   > (generate_sites_module
-  >   (module sites)
-  >   (plugins (top_with_plugins top_plugins)))
+  >  (module sites)
+  >  (plugins (top_with_plugins top_plugins)))
   > EOF
 
   $ cat > top_with_plugins.ml <<EOF
@@ -70,21 +70,21 @@ This is not allowed in toplevels, so it fails.
   > (wrapped_executables false)
   > (map_workspace_root false)
   > (package
-  >   (name top-plugin1))
+  >  (name top-plugin1))
   > EOF
 
   $ cat > plugin1/dune <<EOF
   > (library
-  >   (public_name top-plugin1.plugin1_impl)
-  >   (modes byte)
-  >   (name plugin1_impl)
-  >   (modules plugin1_impl)
-  >   (libraries top_with_plugins.register))
+  >  (public_name top-plugin1.plugin1_impl)
+  >  (modes byte)
+  >  (name plugin1_impl)
+  >  (modules plugin1_impl)
+  >  (libraries top_with_plugins.register))
   > 
   > (plugin
-  >   (name plugin1)
-  >   (libraries top-plugin1.plugin1_impl)
-  >   (site (top_with_plugins top_plugins)))
+  >  (name plugin1)
+  >  (libraries top-plugin1.plugin1_impl)
+  >  (site (top_with_plugins top_plugins)))
   > EOF
 
   $ cat > plugin1/plugin1_impl.ml <<EOF
@@ -106,21 +106,21 @@ This is not allowed in toplevels, so it fails.
   > (wrapped_executables false)
   > (map_workspace_root false)
   > (package
-  >   (name top-plugin2))
+  >  (name top-plugin2))
   > EOF
 
   $ cat > plugin2/dune <<EOF
   > (library
-  >   (public_name top-plugin2.plugin2_impl)
-  >   (modes byte)
-  >   (name plugin2_impl)
-  >   (modules plugin2_impl)
-  >   (libraries top_with_plugins.register))
+  >  (public_name top-plugin2.plugin2_impl)
+  >  (modes byte)
+  >  (name plugin2_impl)
+  >  (modules plugin2_impl)
+  >  (libraries top_with_plugins.register))
   > 
   > (plugin
-  >   (name plugin2)
-  >   (libraries top-plugin2.plugin2_impl)
-  >   (site (top_with_plugins top_plugins)))
+  >  (name plugin2)
+  >  (libraries top-plugin2.plugin2_impl)
+  >  (site (top_with_plugins top_plugins)))
   > EOF
 
   $ cat > plugin2/plugin2_impl.ml <<EOF

diff --git a/test/blackbox-tests/test-cases/toplevel-plugin-fail2.t b/test/blackbox-tests/test-cases/toplevel-plugin-fail2.t
@@ -12,30 +12,30 @@ This is not allowed in toplevels, so it fails.
   > (map_workspace_root false)
   > 
   > (package
-  >   (name top_with_plugins)
-  >   (sites (lib top_plugins)))
+  >  (name top_with_plugins)
+  >  (sites (lib top_plugins)))
   > EOF
 
   $ cat > dune <<EOF
   > (executable
-  >   (public_name top_with_plugins)
-  >   (name top_with_plugins)
-  >   (modes byte)
-  >   (flags :standard -safe-string)
-  >   (modules sites top_with_plugins)
-  >   (link_flags (-linkall))
-  >   (libraries compiler-libs.toplevel
-  >     top_with_plugins.register dune-site dune-site.plugins))
+  >  (public_name top_with_plugins)
+  >  (name top_with_plugins)
+  >  (modes byte)
+  >  (flags :standard -safe-string)
+  >  (modules sites top_with_plugins)
+  >  (link_flags (-linkall))
+  >  (libraries compiler-libs.toplevel
+  >   top_with_plugins.register dune-site dune-site.plugins))
   > 
   > (library
-  >   (public_name top_with_plugins.register)
-  >   (modes byte)
-  >   (name registration)
-  >   (modules registration))
+  >  (public_name top_with_plugins.register)
+  >  (modes byte)
+  >  (name registration)
+  >  (modules registration))
   > 
   > (generate_sites_module
-  >   (module sites)
-  >   (plugins (top_with_plugins top_plugins)))
+  >  (module sites)
+  >  (plugins (top_with_plugins top_plugins)))
   > EOF
 
   $ cat > top_with_plugins.ml <<EOF
@@ -76,16 +76,16 @@ This is not allowed in toplevels, so it fails.
 
   $ cat > plugin1/dune <<EOF
   > (library
-  >   (public_name top-plugin1.plugin1_impl)
-  >   (modes byte)
-  >   (name plugin1_impl)
-  >   (modules plugin1_impl)
-  >   (libraries top_with_plugins.register))
+  >  (public_name top-plugin1.plugin1_impl)
+  >  (modes byte)
+  >  (name plugin1_impl)
+  >  (modules plugin1_impl)
+  >  (libraries top_with_plugins.register))
   > 
   > (plugin
-  >   (name plugin1)
-  >   (libraries top-plugin1.plugin1_impl)
-  >   (site (top_with_plugins top_plugins)))
+  >  (name plugin1)
+  >  (libraries top-plugin1.plugin1_impl)
+  >  (site (top_with_plugins top_plugins)))
   > EOF
 
   $ cat > plugin1/plugin1_impl.ml <<EOF
@@ -107,21 +107,21 @@ This is not allowed in toplevels, so it fails.
   > (wrapped_executables false)
   > (map_workspace_root false)
   > (package
-  >   (name top-plugin2))
+  >  (name top-plugin2))
   > EOF
 
   $ cat > plugin2/dune <<EOF
   > (library
-  >   (public_name top-plugin2.plugin2_impl)
-  >   (modes byte)
-  >   (name plugin2_impl)
-  >   (modules plugin2_impl)
-  >   (libraries top_with_plugins.register))
+  >  (public_name top-plugin2.plugin2_impl)
+  >  (modes byte)
+  >  (name plugin2_impl)
+  >  (modules plugin2_impl)
+  >  (libraries top_with_plugins.register))
   > 
   > (plugin
-  >   (name plugin2)
-  >   (libraries top-plugin2.plugin2_impl)
-  >   (site (top_with_plugins top_plugins)))
+  >  (name plugin2)
+  >  (libraries top-plugin2.plugin2_impl)
+  >  (site (top_with_plugins top_plugins)))
   > EOF
 
   $ cat > plugin2/plugin2_impl.ml <<EOF