From e65ca063b3f420024e6ee8713f4329444be1efb8 Mon Sep 17 00:00:00 2001 From: Gabriel Scherer Date: Wed, 30 Nov 2016 09:13:30 -0500 Subject: [PATCH] make sure that -just-plugin always stops after the plugin-build phase Previously -just-plugin would only be checked at the end of plugin build, and plugin build would only run if there is a plugin. This means that -just-plugin would be ignored if there was no myocamlbuild.ml: (ocamlbuild -just-plugin foo.byte) would build the target or not depending on whether a plugin source was present. --- Changes | 3 ++ Makefile | 2 +- src/plugin.ml | 90 ++++++++++++++++++++++++++++--------------- testsuite/internal.ml | 9 +++++ 4 files changed, 73 insertions(+), 31 deletions(-) diff --git a/Changes b/Changes index 3e71edca..d2dc6e02 100644 --- a/Changes +++ b/Changes @@ -26,6 +26,9 @@ NEXT_RELEASE: - #127: install ocamlbuild's man pages, missing since 4.02 (Gabriel Scherer) +- #130: make sure that -just-plugin always stops after the plugin-build phase + (Gabriel Scherer, report by whitequark) + - #132: add a rule producing .cmxs from .mllib files. Previously, .mllib files would only produce .cm{x,}a files, and .cmxs were only produced by a separate (and in most cases redundant) .mldylib file. diff --git a/Makefile b/Makefile index 6b116cb7..8c3ca7d5 100644 --- a/Makefile +++ b/Makefile @@ -76,8 +76,8 @@ PACK_CMO= $(addprefix src/,\ ocaml_compiler.cmo \ ocaml_tools.cmo \ ocaml_specific.cmo \ - plugin.cmo \ exit_codes.cmo \ + plugin.cmo \ hooks.cmo \ main.cmo \ ) diff --git a/src/plugin.ml b/src/plugin.ml index aad94688..062467e9 100644 --- a/src/plugin.ml +++ b/src/plugin.ml @@ -28,17 +28,30 @@ let plugin = "myocamlbuild" let plugin_file = plugin^".ml" let plugin_config_file = plugin^"_config.ml" let plugin_config_file_interface = plugin^"_config.mli" -let we_need_a_plugin () = !Options.plugin && sys_file_exists plugin_file -let we_have_a_plugin () = sys_file_exists ((!Options.build_dir/plugin)^(!Options.exe)) -let we_have_a_config_file () = sys_file_exists plugin_config_file -let we_have_a_config_file_interface () = sys_file_exists plugin_config_file_interface +let we_have_a_plugin_source () = + sys_file_exists plugin_file +let we_need_a_plugin_binary () = + !Options.plugin && we_have_a_plugin_source () +let we_have_a_plugin_binary () = + sys_file_exists ((!Options.build_dir/plugin)^(!Options.exe)) +let we_have_a_config_file () = + sys_file_exists plugin_config_file +let we_have_a_config_file_interface () = + sys_file_exists plugin_config_file_interface + +(* exported through plugin.mli *) +let we_need_a_plugin () = we_need_a_plugin_binary () module Make(U:sig end) = struct - let we_need_a_plugin = we_need_a_plugin () - let we_have_a_plugin = we_have_a_plugin () + let we_need_a_plugin_binary = we_need_a_plugin_binary () + let we_have_a_plugin_source = we_have_a_plugin_source () let we_have_a_config_file = we_have_a_config_file () let we_have_a_config_file_interface = we_have_a_config_file_interface () + let we_have_a_plugin_binary () = + (* this remains a function as it will change during the build *) + we_have_a_plugin_binary () + let up_to_date_or_copy fn = let fn' = !Options.build_dir/fn in Pathname.exists fn && @@ -50,11 +63,11 @@ module Make(U:sig end) = end end - let rebuild_plugin_if_needed () = + let rebuild_plugin () = let a = up_to_date_or_copy plugin_file in let b = (not we_have_a_config_file) || up_to_date_or_copy plugin_config_file in let c = (not we_have_a_config_file_interface) || up_to_date_or_copy plugin_config_file_interface in - if a && b && c && we_have_a_plugin then + if a && b && c && we_have_a_plugin_binary () then () (* Up to date *) (* FIXME: remove ocamlbuild_config.ml in _build/ if removed in parent *) else begin @@ -229,35 +242,52 @@ module Make(U:sig end) = Shell.chdir !Options.build_dir; Shell.rm_f (plugin^(!Options.exe)); Command.execute cmd; - if !Options.just_plugin then begin - Log.finish (); - raise Exit_OK; - end; end - let execute_plugin_if_needed () = - if we_need_a_plugin then - begin - rebuild_plugin_if_needed (); - Shell.chdir Pathname.pwd; - let runner = if !Options.native_plugin then N else !Options.ocamlrun in - let argv = List.tl (Array.to_list Sys.argv) in - let passed_argv = List.filter (fun s -> s <> "-plugin-option") argv in - let spec = S[runner; P(!Options.build_dir/plugin^(!Options.exe)); - A"-no-plugin"; atomize passed_argv] in - Log.finish (); - let rc = sys_command (Command.string_of_command_spec spec) in - raise (Exit_silently_with_code rc); + let execute_plugin () = + Shell.chdir Pathname.pwd; + let runner = if !Options.native_plugin then N else !Options.ocamlrun in + let argv = List.tl (Array.to_list Sys.argv) in + let passed_argv = List.filter (fun s -> s <> "-plugin-option") argv in + let spec = S[runner; P(!Options.build_dir/plugin^(!Options.exe)); + A"-no-plugin"; atomize passed_argv] in + Log.finish (); + let rc = sys_command (Command.string_of_command_spec spec) in + raise (Exit_silently_with_code rc) + + let main () = + if we_need_a_plugin_binary then begin + rebuild_plugin (); + if not (we_have_a_plugin_binary ()) then begin + Log.eprintf "Error: we failed to build the plugin"; + raise (Exit_with_code Exit_codes.rc_build_error); end - else if not (sys_file_exists plugin_file) && !Options.plugin_tags <> [] then + end; + (* if -just-plugin is passed by there is no plugin, nothing + happens, and we decided not to emit a warning: this lets + people that write ocamlbuild-driver scripts always run + a first phase (ocamlbuild -just-plugin ...) if they want to, + without having to test for the existence of a plugin + first. *) + if !Options.just_plugin then begin + Log.finish (); + raise Exit_OK; + end; + (* On the contrary, not having a plugin yet passing -plugin-tags + is probably caused by a user error that we should warn about; + for example people may incorrectly think that -plugin-tag + foo.ocamlbuild will enable foo's rules: they have to + explicitly use Foo in their plugin and it's best to warn if + they don't. *) + if not we_have_a_plugin_source && !Options.plugin_tags <> [] then eprintf "Warning: option -plugin-tag(s) has no effect \ - in absence of plugin file %S" plugin_file - else - () + in absence of plugin file %S" plugin_file; + if we_need_a_plugin_binary && we_have_a_plugin_binary () then + execute_plugin () end ;; let execute_plugin_if_needed () = let module P = Make(struct end) in - P.execute_plugin_if_needed () + P.main () ;; diff --git a/testsuite/internal.ml b/testsuite/internal.ml index 125d1c7e..df76c479 100644 --- a/testsuite/internal.ml +++ b/testsuite/internal.ml @@ -380,6 +380,15 @@ CAMLprim value hello_world(value unit) ] ~targets:("libtest.a", []) ();; +let () = test "JustNoPlugin" + ~description:"(ocamlbuild -just-plugin) should do nothing when no plugin is there" + ~options:[`no_ocamlfind; `just_plugin] + ~tree:[T.f "test.ml" ~content:{|print_endline "Hellow World"|};] + (* we check that the target is *not* built *) + ~matching:[_build [M.Not (M.f "test.cmo")]] + ~targets:("test.cmo", []) + ();; + let () = test "MldylibOverridesMllib" ~description:"Check that the rule producing a cmxs from a .mllib only \ triggers if there is no .mldylib"