From 72b7dbcf5f21cf4442e71eccc25cb47b4517104c Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Fri, 7 Feb 2020 09:03:55 -0500 Subject: [PATCH 01/10] Correct doc comment on atom converter Signed-off-by: Shon Feder --- src/dune_lang/t.mli | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dune_lang/t.mli b/src/dune_lang/t.mli index fed1b0d4b09..08dca6a68d8 100644 --- a/src/dune_lang/t.mli +++ b/src/dune_lang/t.mli @@ -10,8 +10,8 @@ type t = | List of t list | Template of Template.t -(** [atom s] convert the string [s] to an Atom. @raise Exn.Code_error if [s] - does not satisfy [Atom.is_valid s]. *) +(** [atom s] convert the string [s] to an Atom. + NOTE No validity check is performed. *) val atom : string -> t val atom_or_quoted_string : string -> t From b0ff6f2b27516847b8e5264972aa1ac4c3899a2f Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Fri, 7 Feb 2020 21:27:22 -0500 Subject: [PATCH 02/10] Add Dune_lang.Atom.of_valid_string Signed-off-by: Shon Feder --- src/dune_lang/atom.ml | 6 ++++++ src/dune_lang/atom.mli | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/dune_lang/atom.ml b/src/dune_lang/atom.ml index 0178f9d5b18..8535a64a861 100644 --- a/src/dune_lang/atom.ml +++ b/src/dune_lang/atom.ml @@ -45,6 +45,12 @@ let of_string s = A s let to_string (A s) = s +let of_valid_string s = + if is_valid s then + Some (A s) + else + None + let print (A s) = if is_valid s then s diff --git a/src/dune_lang/atom.mli b/src/dune_lang/atom.mli index 770d0712fc1..42729368373 100644 --- a/src/dune_lang/atom.mli +++ b/src/dune_lang/atom.mli @@ -10,6 +10,8 @@ val of_string : string -> t val to_string : t -> string +val of_valid_string : string -> t option + val print : t -> string val of_int : int -> t From 9cd77d9b5a2c54864b8f7f5dc06989a15d4d83e2 Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Wed, 5 Feb 2020 18:20:57 -0500 Subject: [PATCH 03/10] [#3046] Fix dune init validation and error handling - Make dune init exec NAME use the value of NAME for private modules (closes #3088) - Uses Cmdliner Arg.conv converters to guard against invalid inputs (closes #3046) - Adds library name validation checks to relevant inputs - Adds an ADT to better represent the logic of the public_name option - The Dune_init module now expects the value of relevant options to be of type Dune_lang.Atom.t - Minor improvements to the inline documentation - Move 2 CLI only functions into the init.ml CLI module Signed-off-by: Shon Feder --- bin/init.ml | 81 +++++++++++++++++++++++++++++++++---- src/dune/dune_init.ml | 90 +++++++++++++++++++++--------------------- src/dune/dune_init.mli | 20 +++++----- 3 files changed, 128 insertions(+), 63 deletions(-) diff --git a/bin/init.ml b/bin/init.ml index d75ecb900e1..fe6cbd17569 100644 --- a/bin/init.ml +++ b/bin/init.ml @@ -2,18 +2,80 @@ open Stdune open Import open Dune.Dune_init +(** {1 Helper functions} *) + +(** {2 Validation} *) + (* TODO(shonfeder): Remove when nested subcommands are available *) let validate_component_options kind unsupported_options = let report_invalid_option = function | _, false -> () (* The option wasn't supplied *) | option_name, true -> User_error.raise - [ Pp.textf "The %s component does not support the %s option" + [ Pp.textf "The `%s' component does not support the `--%s' option" (Kind.to_string kind) option_name ] in List.iter ~f:report_invalid_option unsupported_options +(** {2 Cmdliner Argument Converts }*) + +let atom_parser s = + match Dune_lang.Atom.of_valid_string s with + | Some s -> Ok s + | None -> Error (`Msg "expected a valid dune atom") + +let component_name_parser s = + let err_msg () = + User_error.make + [ Pp.textf "invalid component name `%s'" s ] + ~hints:[ Lib_name.Local.valid_format_doc ] + |> User_message.to_string + |> fun m -> `Msg m + in + let open Result.O in + let* atom = atom_parser s in + let* _ = Lib_name.Local.of_string s |> Result.map_error ~f:err_msg in + Ok atom + +let atom_conv = + let printer ppf atom = + Format.pp_print_string ppf (Dune_lang.Atom.to_string atom) + in + Arg.conv (atom_parser, printer) + +let component_name_conv = + let printer ppf atom = + Format.pp_print_string ppf (Dune_lang.Atom.to_string atom) + in + Arg.conv (component_name_parser, printer) + +let public_name_conv = + let open Component.Options in + let parser = function + | "" -> Ok Use_name + | s -> component_name_parser s |> Result.map ~f:(fun a -> Public_name a) + in + let printer ppf public_name = + Format.pp_print_string ppf (public_name_to_string public_name) + in + Arg.conv (parser, printer) + +(** {2 Status reporting} *) + +let print_completion kind name = + let open Pp.O in + Console.print_user_message + (User_message.make + [ Pp.tag (Pp.verbatim "Success") ~tag:User_message.Style.Ok + ++ Pp.textf ": initialized %s component named " (Kind.to_string kind) + ++ Pp.tag + (Pp.verbatim (Dune_lang.Atom.to_string name)) + ~tag:User_message.Style.Kwd + ]) + +(** {1 CLI} *) + let doc = "Initialize dune components" let man = @@ -61,25 +123,29 @@ let term = required & pos 0 (some (enum Kind.commands)) None & info [] ~docv:"INIT_KIND") - and+ name = Arg.(required & pos 1 (some string) None & info [] ~docv:"NAME") + and+ name = + Arg.( + required & pos 1 (some component_name_conv) None & info [] ~docv:"NAME") and+ path = Arg.(value & pos 2 (some string) None & info [] ~docv:"PATH") and+ libraries = Arg.( value - & opt (list string) [] + & opt (list component_name_conv) [] & info [ "libs" ] ~docv:"LIBRARIES" - ~doc:"Libraries on which the component depends") + ~doc: + "A comma separated list of libraries on which the component depends") and+ pps = Arg.( value - & opt (list string) [] + & opt (list atom_conv) [] & info [ "ppx" ] ~docv:"PREPROCESSORS" - ~doc:"ppx preprocessors used by the component") + ~doc: + "A comma separated list of ppx preprocessors used by the component") and+ public = (* TODO(shonfeder): Move to subcommands {lib, exe} once implemented *) Arg.( value - & opt ~vopt:(Some "") (some string) None + & opt ~vopt:(Some Component.Options.Use_name) (some public_name_conv) None & info [ "public" ] ~docv:"PUBLIC_NAME" ~doc: "If called with an argument, make the component public under the \ @@ -111,7 +177,6 @@ let term = $(b,e[sy]). Defaults to $(b,opam). Only applicable for \ $(b,project) components.") in - validate_component_name name; Common.set_common common_term ~targets:[]; let open Component in let context = Init_context.make path in diff --git a/src/dune/dune_init.ml b/src/dune/dune_init.ml index ec5aa4d2c9e..6710467a31b 100644 --- a/src/dune/dune_init.ml +++ b/src/dune/dune_init.ml @@ -193,19 +193,27 @@ module Component = struct module Options = struct module Common = struct type t = - { name : string - ; libraries : string list - ; pps : string list + { name : Dune_lang.Atom.t + ; libraries : Dune_lang.Atom.t list + ; pps : Dune_lang.Atom.t list } end + type public_name = + | Use_name + | Public_name of Dune_lang.Atom.t + + let public_name_to_string = function + | Use_name -> "" + | Public_name p -> Dune_lang.Atom.to_string p + module Executable = struct - type t = { public : string option } + type t = { public : public_name option } end module Library = struct type t = - { public : string option + { public : public_name option ; inline_tests : bool } end @@ -271,11 +279,12 @@ module Component = struct open Dune_lang module Field = struct - let atoms = List.map ~f:atom + let atoms : Atom.t list -> Dune_lang.t list = + List.map ~f:(fun x -> Atom x) - let public_name name = List [ atom "public_name"; atom name ] + let public_name name = List [ atom "public_name"; Atom name ] - let name name = List [ atom "name"; atom name ] + let name name = List [ atom "name"; Atom name ] let inline_tests = List [ atom "inline_tests" ] @@ -288,11 +297,9 @@ module Component = struct | args -> [ f args ] let common (options : Options.Common.t) = - let optional_fields = - optional_field ~f:libraries options.libraries - @ optional_field ~f:pps options.pps - in - name options.name :: optional_fields + name options.name + :: ( optional_field ~f:libraries options.libraries + @ optional_field ~f:pps options.pps ) end let make kind common_options fields = @@ -309,21 +316,25 @@ module Component = struct elem :: set let public_name_field ~default = function - | None -> [] - | Some "" -> [ Field.public_name default ] - | Some n -> [ Field.public_name n ] + | (None : Options.public_name option) -> [] + | Some Use_name -> [ Field.public_name default ] + | Some (Public_name name) -> [ Field.public_name name ] let executable (common : Options.Common.t) (options : Options.Executable.t) = let public_name = public_name_field ~default:common.name options.public in - make "executable" { common with name = "main" } public_name + make "executable" common public_name let library (common : Options.Common.t) (options : Options.Library.t) = let common, inline_tests = if not options.inline_tests then (common, []) else - let pps = add_to_list_set "ppx_inline_test" common.pps in + let pps = + add_to_list_set + (Dune_lang.Atom.of_string "ppx_inline_test") + common.pps + in ({ common with pps }, [ Field.inline_tests ]) in let public_name = public_name_field ~default:common.name options.public in @@ -344,7 +355,7 @@ module Component = struct |> add_stanza_to_dune_file ~project:context.project ~dir in let bin_ml = - let name = "main.ml" in + let name = sprintf "%s.ml" (Dune_lang.Atom.to_string common.name) in let content = sprintf "let () = print_endline \"Hello, World!\"\n" in File.make_text dir name content in @@ -368,7 +379,7 @@ module Component = struct |> add_stanza_to_dune_file ~project:context.project ~dir in let test_ml = - let name = sprintf "%s.ml" common.name in + let name = sprintf "%s.ml" (Dune_lang.Atom.to_string common.name) in let content = "" in File.make_text dir name content in @@ -398,8 +409,9 @@ module Component = struct in bin { context = { context with dir = Path.relative dir "bin" } - ; options = { public = Some common.name } - ; common = { common with libraries } + ; options = { public = Some (Options.Public_name common.name) } + ; common = + { common with libraries; name = Dune_lang.Atom.of_string "main" } } in bin_target @ lib_target @ test_target @@ -410,7 +422,9 @@ module Component = struct src { context = { context with dir = Path.relative dir "lib" } ; options = - { public = Some common.name; inline_tests = options.inline_tests } + { public = Some (Options.Public_name common.name) + ; inline_tests = options.inline_tests + } ; common } in @@ -426,8 +440,13 @@ module Component = struct let proj ({ context; common; options } as opts : Options.Project.t Options.t) = let ({ template; pkg; _ } : Options.Project.t) = options in - let dir = Path.relative context.dir common.name in - let name = Package.Name.parse_string_exn (Loc.none, common.name) in + let dir = + Path.relative context.dir (Dune_lang.Atom.to_string common.name) + in + let name = + Package.Name.parse_string_exn + (Loc.none, Dune_lang.Atom.to_string common.name) + in let proj_target = let files = match (pkg : Options.Project.Pkg.t) with @@ -475,24 +494,3 @@ module Component = struct in List.concat_map ~f:create target |> List.iter ~f:report_uncreated_file end - -let validate_component_name name = - match Lib_name.Local.of_string name with - | Ok _ -> () - | _ -> - User_error.raise - [ Pp.textf - "A component named '%s' cannot be created because it is an invalid \ - library name." - name - ] - ~hints:[ Lib_name.Local.valid_format_doc ] - -let print_completion kind name = - let open Pp.O in - Console.print_user_message - (User_message.make - [ Pp.tag (Pp.verbatim "Success") ~tag:User_message.Style.Ok - ++ Pp.textf ": initialized %s component named " (Kind.to_string kind) - ++ Pp.tag (Pp.verbatim name) ~tag:User_message.Style.Kwd - ]) diff --git a/src/dune/dune_init.mli b/src/dune/dune_init.mli index ac98de70714..7aa56773217 100644 --- a/src/dune/dune_init.mli +++ b/src/dune/dune_init.mli @@ -32,19 +32,25 @@ module Component : sig module Options : sig module Common : sig type t = - { name : string - ; libraries : string list - ; pps : string list + { name : Dune_lang.Atom.t + ; libraries : Dune_lang.Atom.t list + ; pps : Dune_lang.Atom.t list } end + type public_name = + | Use_name + | Public_name of Dune_lang.Atom.t + + val public_name_to_string : public_name -> string + module Executable : sig - type t = { public : string option } + type t = { public : public_name option} end module Library : sig type t = - { public : string option + { public : public_name option ; inline_tests : bool } end @@ -99,7 +105,3 @@ module Component : sig ['options] is *) val init : 'options t -> unit end - -val validate_component_name : string -> unit - -val print_completion : Kind.t -> string -> unit From e9b8e5b8e6cac6cfdafd313a8f4badab233c8263 Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Thu, 6 Feb 2020 19:57:25 -0500 Subject: [PATCH 04/10] [#3046] Add test cases against reported bug Signed-off-by: Shon Feder --- test/blackbox-tests/dune.inc | 10 ++++++ .../test-cases/github3046/run.t | 34 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 test/blackbox-tests/test-cases/github3046/run.t diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index 7457efa9fba..075e23f337d 100644 --- a/test/blackbox-tests/dune.inc +++ b/test/blackbox-tests/dune.inc @@ -970,6 +970,14 @@ test-cases/github2848 (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) +(rule + (alias github3046) + (deps (package dune) (source_tree test-cases/github3046)) + (action + (chdir + test-cases/github3046 + (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) + (rule (alias github534) (deps (package dune) (source_tree test-cases/github534)) @@ -2236,6 +2244,7 @@ (alias github2629) (alias github2681) (alias github2848) + (alias github3046) (alias github534) (alias github568) (alias github597) @@ -2484,6 +2493,7 @@ (alias github2629) (alias github2681) (alias github2848) + (alias github3046) (alias github534) (alias github568) (alias github597) diff --git a/test/blackbox-tests/test-cases/github3046/run.t b/test/blackbox-tests/test-cases/github3046/run.t new file mode 100644 index 00000000000..a81fc40049e --- /dev/null +++ b/test/blackbox-tests/test-cases/github3046/run.t @@ -0,0 +1,34 @@ +---------------------------------------------------------------------------------- +Testsuite for https://github.com/ocaml/dune/issues/3046 +`dune init` should raise proper errors when syntacticaly invalid arguments +are given as paramters +---------------------------------------------------------------------------------- + +`dune init exe main --libs="str gsl"` returns an informative parsing error + + $ dune init exe main --libs="str gsl" + dune: option `--libs': invalid element in list (`str gsl'): expected a valid + dune atom + Usage: dune init [OPTION]... INIT_KIND NAME [PATH] + Try `dune init --help' or `dune --help' for more information. + [1] + +`dune init lib foo --ppx="foo bar"` returns an informative parsing error + + $ dune init lib foo --ppx="foo bar" + dune: option `--ppx': invalid element in list (`foo bar'): expected a valid + dune atom + Usage: dune init [OPTION]... INIT_KIND NAME [PATH] + Try `dune init --help' or `dune --help' for more information. + [1] + +`dune init lib foo --public="some/invalid&name!"` returns an informative parsing error + + $ dune init lib foo --public="some/invalid&name!" + dune: option `--public': invalid component name `some/invalid&name!' + Hint: library names must be non-empty and composed only of the + following + characters: 'A'..'Z', 'a'..'z', '_' or '0'..'9' + Usage: dune init [OPTION]... INIT_KIND NAME [PATH] + Try `dune init --help' or `dune --help' for more information. + [1] From 7dbac0d8801c7a715ff02bd565205ad7bb063417 Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Fri, 7 Feb 2020 22:32:34 -0500 Subject: [PATCH 05/10] [#3046] Update tests Signed-off-by: Shon Feder --- .../blackbox-tests/test-cases/dune-init/run.t | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/test/blackbox-tests/test-cases/dune-init/run.t b/test/blackbox-tests/test-cases/dune-init/run.t index 114f6eea446..3312b2a91ac 100644 --- a/test/blackbox-tests/test-cases/dune-init/run.t +++ b/test/blackbox-tests/test-cases/dune-init/run.t @@ -215,10 +215,10 @@ Safety and Validation Will not overwrite existing files - $ dune init exe test_bin ./existing_project/bin + $ dune init exe main ./existing_project/bin Warning: File existing_project/bin/main.ml was not created because it already exists - Success: initialized executable component named test_bin + Success: initialized executable component named main $ cat ./existing_project/bin/main.ml () = print_endline "Goodbye" @@ -239,10 +239,12 @@ Comments in dune files are preserved Will not create components with invalid names $ dune init lib invalid-component-name ./_test_lib - Error: A component named 'invalid-component-name' cannot be created because - it is an invalid library name. - Hint: library names must be non-empty and composed only of the following - characters: 'A'..'Z', 'a'..'z', '_' or '0'..'9' + dune: NAME argument: invalid component name `invalid-component-name' + Hint: library names must be non-empty and composed only of the + following + characters: 'A'..'Z', 'a'..'z', '_' or '0'..'9' + Usage: dune init [OPTION]... INIT_KIND NAME [PATH] + Try `dune init --help' or `dune --help' for more information. [1] $ test -f ./_test_lib [1] @@ -259,10 +261,11 @@ Will fail and inform user when invalid component command is given Will fail and inform user when an invalid option is given to a component $ dune init test test_foo --public - Error: The test component does not support the public option + Error: The `test' component does not support the `--public' option [1] $ dune init exe test_exe --inline-tests - Error: The executable component does not support the inline-tests option + Error: The `executable' component does not support the `--inline-tests' + option [1] Adding fields to existing stanzas @@ -278,14 +281,14 @@ Adding fields to existing stanzas is currently not supported A preexisting dune stanza conflicts with a generated stanza: Generated stanza: - (executable (name main) (libraries test_lib2)) + (executable (name test_bin) (libraries test_lib2)) Pre-existing stanza: - (executable (name main) (libraries test_lib1)) + (executable (name test_bin) (libraries test_lib1)) [1] $ cat ./_test_bin/dune (executable - (name main) + (name test_bin) (libraries test_lib1)) Creating projects From e12218509fada02f3e0eca48a53c7ab31d5eb66c Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Sat, 8 Feb 2020 01:41:58 -0500 Subject: [PATCH 06/10] [#3046] Cleanup formatting Signed-off-by: Shon Feder --- bin/init.ml | 100 +++++++++++++++++++++++++--------------------------- 1 file changed, 49 insertions(+), 51 deletions(-) diff --git a/bin/init.ml b/bin/init.ml index fe6cbd17569..2a319798359 100644 --- a/bin/init.ml +++ b/bin/init.ml @@ -18,13 +18,15 @@ let validate_component_options kind unsupported_options = in List.iter ~f:report_invalid_option unsupported_options -(** {2 Cmdliner Argument Converts }*) +(** {2 Cmdliner Argument Converters }*) let atom_parser s = match Dune_lang.Atom.of_valid_string s with | Some s -> Ok s | None -> Error (`Msg "expected a valid dune atom") +let atom_printer ppf a = Format.pp_print_string ppf (Dune_lang.Atom.to_string a) + let component_name_parser s = let err_msg () = User_error.make @@ -38,17 +40,9 @@ let component_name_parser s = let* _ = Lib_name.Local.of_string s |> Result.map_error ~f:err_msg in Ok atom -let atom_conv = - let printer ppf atom = - Format.pp_print_string ppf (Dune_lang.Atom.to_string atom) - in - Arg.conv (atom_parser, printer) +let atom_conv = Arg.conv (atom_parser, atom_printer) -let component_name_conv = - let printer ppf atom = - Format.pp_print_string ppf (Dune_lang.Atom.to_string atom) - in - Arg.conv (component_name_parser, printer) +let component_name_conv = Arg.conv (component_name_parser, atom_printer) let public_name_conv = let open Component.Options in @@ -119,63 +113,67 @@ let term = and+ kind = (* TODO(shonfeder): Replace with nested subcommand once we have support for that *) - Arg.( - required - & pos 0 (some (enum Kind.commands)) None - & info [] ~docv:"INIT_KIND") + let docv = "INIT_KIND" in + Arg.(required & pos 0 (some (enum Kind.commands)) None & info [] ~docv) and+ name = - Arg.( - required & pos 1 (some component_name_conv) None & info [] ~docv:"NAME") - and+ path = Arg.(value & pos 2 (some string) None & info [] ~docv:"PATH") + let docv = "NAME" in + Arg.(required & pos 1 (some component_name_conv) None & info [] ~docv) + and+ path = + let docv = "PATH" in + Arg.(value & pos 2 (some string) None & info [] ~docv) and+ libraries = - Arg.( - value - & opt (list component_name_conv) [] - & info [ "libs" ] ~docv:"LIBRARIES" - ~doc: - "A comma separated list of libraries on which the component depends") + let docv = "LIBRARIES" in + let doc = + "A comma separated list of libraries on which the component depends" + in + Arg.(value & opt (list component_name_conv) [] & info [ "libs" ] ~docv ~doc) and+ pps = - Arg.( - value - & opt (list atom_conv) [] - & info [ "ppx" ] ~docv:"PREPROCESSORS" - ~doc: - "A comma separated list of ppx preprocessors used by the component") + let docv = "PREPROCESSORS" in + let doc = + "A comma separated list of ppx preprocessors used by the component" + in + Arg.(value & opt (list atom_conv) [] & info [ "ppx" ] ~docv ~doc) and+ public = (* TODO(shonfeder): Move to subcommands {lib, exe} once implemented *) + let docv = "PUBLIC_NAME" in + let doc = + "If called with an argument, make the component public under the given \ + PUBLIC_NAME. If supplied without an argument, use NAME." + in Arg.( value & opt ~vopt:(Some Component.Options.Use_name) (some public_name_conv) None - & info [ "public" ] ~docv:"PUBLIC_NAME" - ~doc: - "If called with an argument, make the component public under the \ - given PUBLIC_NAME. If supplied without an argument, use NAME.") + & info [ "public" ] ~docv ~doc) and+ inline_tests = - (* TODO Move to subcommand lib once implemented *) - Arg.( - value & flag - & info [ "inline-tests" ] ~docv:"USE_INLINE_TESTS" - ~doc: - "Whether to use inline tests. Only applicable for $(b,library) and \ - $(b,project) components.") + (* TODO(shonfeder): Move to subcommand [lib] once implemented *) + let docv = "USE_INLINE_TESTS" in + let doc = + "Whether to use inline tests. Only applicable for $(b,library) and \ + $(b,project) components." + in + Arg.(value & flag & info [ "inline-tests" ] ~docv ~doc) and+ template = + let docv = "PROJECT_KIND" in + let doc = + "The kind of project to initialize. Valid options are $(b,e[xecutable]) \ + or $(b,l[ibrary]). Defaults to $(b,executable). Only applicable for \ + $(b,project) components." + in Arg.( value & opt (some (enum Component.Options.Project.Template.commands)) None - & info [ "kind" ] ~docv:"PROJECT_KIND" - ~doc: - "The kind of project to initialize. Valid options are \ - $(b,e[xecutable]) or $(b,l[ibrary]). Defaults to $(b,executable). \ - Only applicable for $(b,project) components.") + & info [ "kind" ] ~docv ~doc) and+ pkg = + let docv = "PACKAGE_MANAGER" in + let doc = + "Which package manager to use. Valid options are $(b,o[pam]) or \ + $(b,e[sy]). Defaults to $(b,opam). Only applicable for $(b,project) \ + components." + in Arg.( value & opt (some (enum Component.Options.Project.Pkg.commands)) None - & info [ "pkg" ] ~docv:"PACKAGE_MANAGER" - ~doc: - "Which package manager to use. Valid options are $(b,o[pam]) or \ - $(b,e[sy]). Defaults to $(b,opam). Only applicable for \ - $(b,project) components.") + & info [ "pkg" ] ~docv ~doc) in Common.set_common common_term ~targets:[]; let open Component in From a538d71d245a98f08bf38f4d1ed966360faba22a Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Sat, 8 Feb 2020 17:02:30 -0500 Subject: [PATCH 07/10] [#3046] Rename of_valid_string to parse As per @aalekseyev's suggestion Signed-off-by: Shon Feder --- bin/init.ml | 2 +- src/dune_lang/atom.ml | 2 +- src/dune_lang/atom.mli | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/bin/init.ml b/bin/init.ml index 2a319798359..7d31a09af7c 100644 --- a/bin/init.ml +++ b/bin/init.ml @@ -21,7 +21,7 @@ let validate_component_options kind unsupported_options = (** {2 Cmdliner Argument Converters }*) let atom_parser s = - match Dune_lang.Atom.of_valid_string s with + match Dune_lang.Atom.parse s with | Some s -> Ok s | None -> Error (`Msg "expected a valid dune atom") diff --git a/src/dune_lang/atom.ml b/src/dune_lang/atom.ml index 8535a64a861..f97286a8b14 100644 --- a/src/dune_lang/atom.ml +++ b/src/dune_lang/atom.ml @@ -45,7 +45,7 @@ let of_string s = A s let to_string (A s) = s -let of_valid_string s = +let parse s = if is_valid s then Some (A s) else diff --git a/src/dune_lang/atom.mli b/src/dune_lang/atom.mli index 42729368373..979389b7021 100644 --- a/src/dune_lang/atom.mli +++ b/src/dune_lang/atom.mli @@ -10,7 +10,9 @@ val of_string : string -> t val to_string : t -> string -val of_valid_string : string -> t option +(** [parse s] is [Some (a:t)] if [s] can be a valid atom according to [is_valid] + otherwise it is [None] *) +val parse : string -> t option val print : t -> string From 94c0b3d9189b9dda977d93a486b37fc9d01a4283 Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Sat, 8 Feb 2020 18:46:27 -0500 Subject: [PATCH 08/10] [#3046] Update testing recipe Generated after merging in upstream changes Signed-off-by: Shon Feder --- test/blackbox-tests/dune.inc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index b6b87164fcd..f66af759dc5 100644 --- a/test/blackbox-tests/dune.inc +++ b/test/blackbox-tests/dune.inc @@ -1213,7 +1213,9 @@ (action (chdir test-cases/github3046 - (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) + (progn + (run %{exe:cram.exe} run.t -sanitizer %{bin:sanitizer}) + (diff? run.t run.t.corrected))))) (rule (alias github534) From bda820cb56d247c2c7431f3c33ab584e42b5b2e8 Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Sat, 8 Feb 2020 18:48:03 -0500 Subject: [PATCH 09/10] [#3046] Add TODO for separating out public_name types Signed-off-by: Shon Feder --- src/dune/dune_init.ml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dune/dune_init.ml b/src/dune/dune_init.ml index 6710467a31b..6da882f4901 100644 --- a/src/dune/dune_init.ml +++ b/src/dune/dune_init.ml @@ -199,6 +199,8 @@ module Component = struct } end + (** TODO(shonfeder): Use separate types for executables and libs (which + would use Lib_name.t) *) type public_name = | Use_name | Public_name of Dune_lang.Atom.t From ae463a17dd3ccfd46e78ef8709326fee936a7a90 Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Sat, 8 Feb 2020 18:52:32 -0500 Subject: [PATCH 10/10] [#3046] Update CHANGES.md Signed-off-by: Shon Feder --- CHANGES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index fa34d5cedce..97a0a360102 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,12 @@ 2.3.0 (unreleased) ------------------ +- Improve validation and error handling of arguments to `dune init` (#3103, fixes + #3046, @shonfeder) + +- `dune init exec NAME` now uses the `NAME` argument for private modules (#3103, + fixes #3088, @shonfeder) + - Avoid linear walk to detect children, this should greatly improve performance when a target has a large number of dependencies (#2959, @ejgallego, @aalekseyev, @Armael)