From 3c88a4f5b964866e0ec1b956667722f7334d1c02 Mon Sep 17 00:00:00 2001 From: Lucas Pluvinage Date: Thu, 27 Jun 2019 17:29:01 +0200 Subject: [PATCH 1/3] Check user-written input of implentations to make sure it actually implements the given vlib Signed-off-by: Lucas Pluvinage --- src/lib.ml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/lib.ml b/src/lib.ml index 7eb286bbbc7..d3f6a7a21ac 100644 --- a/src/lib.ml +++ b/src/lib.ml @@ -141,6 +141,14 @@ module Error = struct [ Pp.text "No solution found for this select form." ] + + let not_an_implementation_of ~vlib ~impl = + make + [ Pp.textf "%S is not an implementation of %S." + (Lib_name.to_string (Lib_info.name impl)) + (Lib_name.to_string (Lib_info.name vlib)) + ] + let dependency_cycle cycle = make [ Pp.text "Dependency cycle detected between the following libraries:" @@ -891,16 +899,23 @@ let rec instantiate db name info ~stack ~hidden = Variant.pp variant Lib_name.pp name) in + let actually_implements_vlib (lib : lib Or_exn.t) = + let* lib = lib in + let* vlib = Option.value ~default:(Error.not_an_implementation_of ~vlib:info ~impl:lib.info) lib.implements in + if Lib_name.equal vlib.name name + then Ok lib + else Error.not_an_implementation_of ~vlib:info ~impl:lib.info + in let default_implementation = Lib_info.default_implementation info - |> Option.map ~f:(fun l -> lazy (resolve l)) in + |> Option.map ~f:(fun l -> lazy (resolve l |> actually_implements_vlib)) in let resolved_implementations = Lib_info.virtual_ info |> Option.map ~f:(fun _ -> lazy ( (* TODO this can be made even lazier as we don't need to resolve all variants at once *) Lib_info.known_implementations info - |> Variant.Map.map ~f:resolve)) + |> Variant.Map.map ~f:(fun l -> resolve l |> actually_implements_vlib))) in let requires, pps, resolved_selects = let pps = Lib_info.pps info in From 025cf8e5915be84844339d2443b65be3ead1787c Mon Sep 17 00:00:00 2001 From: Lucas Pluvinage Date: Fri, 28 Jun 2019 10:08:32 +0200 Subject: [PATCH 2/3] add tests for the implementation check. Signed-off-by: Lucas Pluvinage --- test/blackbox-tests/dune.inc | 22 +++++++++++++++++++ .../exe/dune | 4 ++++ .../exe/dune-project | 2 ++ .../exe/exe.ml | 1 + .../prj1/dune | 10 +++++++++ .../prj1/dune-project | 2 ++ .../prj1/vlibfoo-ext.opam | 0 .../prj1/vlibfoo.mli | 1 + .../prj2/bar.ml | 1 + .../prj2/dune | 4 ++++ .../prj2/dune-project | 2 ++ .../prj2/not-an-implementation.opam | 0 .../variants-wrong-external-declaration/run.t | 7 ++++++ .../vlib-wrong-default-impl/dune-project | 2 ++ .../vlib-wrong-default-impl/exe/dune | 9 ++++++++ .../vlib-wrong-default-impl/exe/exe.ml | 1 + .../vlib-wrong-default-impl/implfoo/bar.ml | 1 + .../vlib-wrong-default-impl/implfoo/dune | 3 +++ .../test-cases/vlib-wrong-default-impl/run.t | 7 ++++++ .../vlib-wrong-default-impl/vlibfoo/dune | 5 +++++ .../vlibfoo/vlibfoo.mli | 1 + 21 files changed, 85 insertions(+) create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune-project create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/exe.ml create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune-project create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/vlibfoo-ext.opam create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/vlibfoo.mli create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/bar.ml create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune-project create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/not-an-implementation.opam create mode 100644 test/blackbox-tests/test-cases/variants-wrong-external-declaration/run.t create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/dune-project create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/dune create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/exe.ml create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/bar.ml create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/dune create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/run.t create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/dune create mode 100644 test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/vlibfoo.mli diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index ba32477858b..5f7c5704d62 100644 --- a/test/blackbox-tests/dune.inc +++ b/test/blackbox-tests/dune.inc @@ -1490,6 +1490,16 @@ test-cases/variants-only-package (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) +(alias + (name variants-wrong-external-declaration) + (deps + (package dune) + (source_tree test-cases/variants-wrong-external-declaration)) + (action + (chdir + test-cases/variants-wrong-external-declaration + (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) + (alias (name vlib) (deps (package dune) (source_tree test-cases/vlib)) @@ -1506,6 +1516,14 @@ test-cases/vlib-default-impl (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) +(alias + (name vlib-wrong-default-impl) + (deps (package dune) (source_tree test-cases/vlib-wrong-default-impl)) + (action + (chdir + test-cases/vlib-wrong-default-impl + (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) + (alias (name windows-diff) (deps (package dune) (source_tree test-cases/windows-diff)) @@ -1721,8 +1739,10 @@ (alias variants-external-declaration-conflict) (alias variants-multi-project) (alias variants-only-package) + (alias variants-wrong-external-declaration) (alias vlib) (alias vlib-default-impl) + (alias vlib-wrong-default-impl) (alias windows-diff) (alias workspaces) (alias wrapped-false-main-module-name) @@ -1885,8 +1905,10 @@ (alias variants-external-declaration-conflict) (alias variants-multi-project) (alias variants-only-package) + (alias variants-wrong-external-declaration) (alias vlib) (alias vlib-default-impl) + (alias vlib-wrong-default-impl) (alias windows-diff) (alias workspaces) (alias wrapped-false-main-module-name) diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune new file mode 100644 index 00000000000..fb2c70786fd --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune @@ -0,0 +1,4 @@ +(executable + (name exe) + (libraries vlibfoo-ext) + (variants somevariant)) diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune-project b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune-project new file mode 100644 index 00000000000..8a603347b8d --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/dune-project @@ -0,0 +1,2 @@ +(lang dune 1.10) +(using library_variants 0.1) diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/exe.ml b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/exe.ml new file mode 100644 index 00000000000..abb3a1931c1 --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/exe/exe.ml @@ -0,0 +1 @@ +let () = Vlibfoo.implme () \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune new file mode 100644 index 00000000000..ec7d16112c1 --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune @@ -0,0 +1,10 @@ +(library + (name vlibfoo) + (public_name vlibfoo-ext) + (virtual_modules vlibfoo) +) + +(external_variant + (virtual_library vlibfoo) + (variant somevariant) + (implementation not-an-implementation)) diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune-project b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune-project new file mode 100644 index 00000000000..51cd215341f --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/dune-project @@ -0,0 +1,2 @@ +(lang dune 1.10) +(using library_variants 0.2) diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/vlibfoo-ext.opam b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/vlibfoo-ext.opam new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/vlibfoo.mli b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/vlibfoo.mli new file mode 100644 index 00000000000..56a52c4170a --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj1/vlibfoo.mli @@ -0,0 +1 @@ +val implme : unit -> unit diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/bar.ml b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/bar.ml new file mode 100644 index 00000000000..e7126a66014 --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/bar.ml @@ -0,0 +1 @@ +let implme () = print_endline "foobar" diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune new file mode 100644 index 00000000000..9f0dd16274c --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune @@ -0,0 +1,4 @@ +(library + (name not_an_implementation) + (public_name not-an-implementation) +) diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune-project b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune-project new file mode 100644 index 00000000000..8a603347b8d --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/dune-project @@ -0,0 +1,2 @@ +(lang dune 1.10) +(using library_variants 0.1) diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/not-an-implementation.opam b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/prj2/not-an-implementation.opam new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/variants-wrong-external-declaration/run.t b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/run.t new file mode 100644 index 00000000000..df80c2e037f --- /dev/null +++ b/test/blackbox-tests/test-cases/variants-wrong-external-declaration/run.t @@ -0,0 +1,7 @@ +External declaration of an implementation, when the given library is not an +implementation, should fail. + + $ dune build exe/exe.exe + Error: "not-an-implementation" is not an implementation of "vlibfoo-ext". + -> required by executable exe in exe/dune:2 + [1] diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/dune-project b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/dune-project new file mode 100644 index 00000000000..8a603347b8d --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/dune-project @@ -0,0 +1,2 @@ +(lang dune 1.10) +(using library_variants 0.1) diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/dune b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/dune new file mode 100644 index 00000000000..7e316a71cef --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/dune @@ -0,0 +1,9 @@ +(executable + (name exe) + (libraries vlibfoo) +) + +(alias + (name default) + (action + (run ./exe.exe))) diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/exe.ml b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/exe.ml new file mode 100644 index 00000000000..abb3a1931c1 --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/exe/exe.ml @@ -0,0 +1 @@ +let () = Vlibfoo.implme () \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/bar.ml b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/bar.ml new file mode 100644 index 00000000000..e11e2d7309a --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/bar.ml @@ -0,0 +1 @@ +let implme () = () \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/dune b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/dune new file mode 100644 index 00000000000..e3700268500 --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/implfoo/dune @@ -0,0 +1,3 @@ +(library + (name not_an_implem) +) diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/run.t b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/run.t new file mode 100644 index 00000000000..d773b85741a --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/run.t @@ -0,0 +1,7 @@ +Check that dune makes a proper error if the default implementation of a virtual +library is not actually an implementation of the virtual library. + + $ dune build @default + Error: "not_an_implem" is not an implementation of "vlibfoo". + -> required by executable exe in exe/dune:2 + [1] diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/dune b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/dune new file mode 100644 index 00000000000..17d24f531da --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/dune @@ -0,0 +1,5 @@ +(library + (name vlibfoo) + (virtual_modules vlibfoo) + (default_implementation not_an_implem) +) diff --git a/test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/vlibfoo.mli b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/vlibfoo.mli new file mode 100644 index 00000000000..56a52c4170a --- /dev/null +++ b/test/blackbox-tests/test-cases/vlib-wrong-default-impl/vlibfoo/vlibfoo.mli @@ -0,0 +1 @@ +val implme : unit -> unit From 3f90cc06de1a3da4f34fa73ece0ac085a5ab2038 Mon Sep 17 00:00:00 2001 From: Lucas Pluvinage Date: Fri, 28 Jun 2019 14:29:40 +0200 Subject: [PATCH 3/3] add changes Signed-off-by: Lucas Pluvinage --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index cec4cda7e74..1db3f13fcd5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,9 @@ 1.11.0 (unreleased) ------------------- +- Check that selected implementations (either by variants or default + implementations) are indeed implementations. (#2328, @TheLortex, review by ?) + - Don't reserve the `Ppx` toplevel module name for ppx rewriters (#2242, @diml) - Redesign of the library variant feature according to the #2134 proposal. The