From afc61d5d329f74418508911fdf7beaeba3353568 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 21 Jul 2018 00:20:15 +0200 Subject: [PATCH 1/9] Make name optional by defaulting to public_name for libraries This language feature requires dune 1.1 Signed-off-by: Rudi Grinberg --- src/jbuild.ml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/jbuild.ml b/src/jbuild.ml index 2f793065e10..0d4aa563576 100644 --- a/src/jbuild.ml +++ b/src/jbuild.ml @@ -805,7 +805,8 @@ module Library = struct let t = record (let%map buildable = Buildable.t - and name = field "name" library_name + and loc = loc + and name = field_o "name" library_name and public = Public_lib.public_name_field and synopsis = field_o "synopsis" string and install_c_headers = @@ -834,6 +835,22 @@ module Library = struct and project = Dune_project.get_exn () and dune_version = Syntax.get_exn Stanza.syntax in + let name = + match name, public with + | Some n, _ -> n + | None, Some { name ; _ } -> + if dune_version >= (1, 1) then + name + else + of_sexp_error loc "name field cannot be omitted before version 1.1" + | None, None -> + of_sexp_error loc ( + if dune_version >= (1, 1) then + "supply at least least one of name or public_name fields" + else + "name field is missing" + ) + in { name ; public ; synopsis From 1967c38a9a88d1bce2ae108543bb3bf343d89d6f Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 21 Jul 2018 00:34:00 +0200 Subject: [PATCH 2/9] Add locations to public names Signed-off-by: Rudi Grinberg --- src/install_rules.ml | 2 +- src/jbuild.ml | 20 +++++++++++--------- src/jbuild.mli | 10 ++++++---- src/lib.ml | 11 ++++++----- src/scope.ml | 5 +++-- src/super_context.ml | 3 ++- 6 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/install_rules.ml b/src/install_rules.ml index 4309392c228..ac07a15a7e9 100644 --- a/src/install_rules.ml +++ b/src/install_rules.ml @@ -304,7 +304,7 @@ module Gen(P : Params) = struct ~f:(fun { SC.Installable. dir; stanza; kind = dir_kind; scope; _ } -> let dir_contents = Dir_contents.get sctx ~dir in match stanza with - | Library ({ public = Some { package; sub_dir; name; _ } + | Library ({ public = Some { package; sub_dir; name = (_, name); _ } ; _ } as lib) -> List.map (lib_install_files ~dir ~sub_dir ~name lib ~scope ~dir_kind ~dir_contents) diff --git a/src/jbuild.ml b/src/jbuild.ml index 0d4aa563576..e5face4dd5b 100644 --- a/src/jbuild.ml +++ b/src/jbuild.ml @@ -640,20 +640,22 @@ end module Public_lib = struct type t = - { name : string + { name : Loc.t * string ; package : Package.t ; sub_dir : string option } + let name t = snd t.name + let public_name_field = map_validate (let%map project = Dune_project.get_exn () - and name = field_o "public_name" string in - (project, name)) - ~f:(fun (project, name) -> - match name with + and loc_name = field_o "public_name" (located string) in + (project, loc_name)) + ~f:(fun (project, loc_name) -> + match loc_name with | None -> Ok None - | Some s -> + | Some ((_, s) as loc_name) -> match String.split s ~on:'.' with | [] -> assert false | pkg :: rest -> @@ -664,7 +666,7 @@ module Public_lib = struct ; sub_dir = if rest = [] then None else Some (String.concat rest ~sep:"/") - ; name = s + ; name = loc_name }) | Error _ as e -> e) end @@ -838,7 +840,7 @@ module Library = struct let name = match name, public with | Some n, _ -> n - | None, Some { name ; _ } -> + | None, Some { name = (_loc, name) ; _ } -> if dune_version >= (1, 1) then name else @@ -893,7 +895,7 @@ module Library = struct let best_name t = match t.public with | None -> t.name - | Some p -> p.name + | Some p -> snd p.name end module Install_conf = struct diff --git a/src/jbuild.mli b/src/jbuild.mli index 2c432ff84c8..7713f3eeb85 100644 --- a/src/jbuild.mli +++ b/src/jbuild.mli @@ -137,11 +137,13 @@ end module Public_lib : sig type t = - { name : string (** Full public name *) - ; package : Package.t (** Package it is part of *) - ; sub_dir : string option (** Subdirectory inside the installation - directory *) + { name : Loc.t * string (** Full public name *) + ; package : Package.t (** Package it is part of *) + ; sub_dir : string option (** Subdirectory inside the installation + directory *) } + + val name : t -> string end module Sub_system_info : sig diff --git a/src/lib.ml b/src/lib.ml index d11f8f67805..14b4a6c6cb5 100644 --- a/src/lib.ml +++ b/src/lib.ml @@ -988,11 +988,12 @@ module DB = struct | None -> [(conf.name, Resolve_result.Found info)] | Some p -> - if p.name = conf.name then - [(p.name, Found info)] + let name = Jbuild.Public_lib.name p in + if name = conf.name then + [(name, Found info)] else - [ p.name , Found info - ; conf.name, Redirect (None, p.name) + [ name , Found info + ; conf.name, Redirect (None, name) ]) |> String.Map.of_list |> function @@ -1003,7 +1004,7 @@ module DB = struct if name = conf.name || match conf.public with | None -> false - | Some p -> name = p.name + | Some p -> name = Jbuild.Public_lib.name p then Some conf.buildable.loc else None) with diff --git a/src/scope.ml b/src/scope.ml index d8d7b709bd7..0da4f1e70e8 100644 --- a/src/scope.ml +++ b/src/scope.ml @@ -78,7 +78,7 @@ module DB = struct List.filter_map internal_libs ~f:(fun (_dir, lib) -> match lib.public with | None -> None - | Some p -> Some (p.name, lib.project)) + | Some p -> Some (Jbuild.Public_lib.name p, lib.project)) |> String.Map.of_list |> function | Ok x -> x @@ -87,7 +87,8 @@ module DB = struct List.filter_map internal_libs ~f:(fun (_dir, lib) -> match lib.public with | None -> None - | Some p -> Option.some_if (name = p.name) lib.buildable.loc) + | Some p -> Option.some_if (name = Jbuild.Public_lib.name p) + lib.buildable.loc) with | [] | [_] -> assert false | loc1 :: loc2 :: _ -> diff --git a/src/super_context.ml b/src/super_context.ml index c4eb27b6b38..776dc55e3a0 100644 --- a/src/super_context.ml +++ b/src/super_context.ml @@ -77,7 +77,8 @@ let internal_lib_names t = String.Set.add (match lib.public with | None -> acc - | Some { name; _ } -> String.Set.add acc name) + | Some { name = (_, name); _ } -> + String.Set.add acc name) lib.name | _ -> acc)) From 0fefb434c41f0da64028554bc5cc2339dc36d046 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 21 Jul 2018 17:14:13 +0200 Subject: [PATCH 3/9] Support for omitting the name field for executables Signed-off-by: Rudi Grinberg --- src/jbuild.ml | 61 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/src/jbuild.ml b/src/jbuild.ml index e5face4dd5b..ebb6d81532f 100644 --- a/src/jbuild.ml +++ b/src/jbuild.ml @@ -1039,6 +1039,12 @@ module Executables = struct ; buildable : Buildable.t } + let pluralize s ~multi = + if multi then + s + else + s ^ "s" + let common = let%map buildable = Buildable.t and (_ : bool) = field "link_executables" ~default:true @@ -1059,8 +1065,33 @@ module Executables = struct (loc, s)) and project = Dune_project.get_exn () and file_kind = Stanza.file_kind () + and dune_syntax = Syntax.get_exn Stanza.syntax + and loc = loc in fun names public_names ~multi -> + let names = + match names, public_names with + | _::_, _ -> names + | [], _::_ -> + if dune_syntax >= (1, 1) then + List.filter_map public_names ~f:(fun (loc, p) -> + match p with + | None -> + of_sexp_error loc "This executable must have a name field" + | Some s -> Some (loc, s)) + else + of_sexp_errorf loc + "%s field may not be omitted before dune version 1.1" + (pluralize ~multi "name") + | [], [] -> + if dune_syntax >= (1, 1) then + of_sexp_errorf loc "either the %s or the %s field must be present" + (pluralize ~multi "name") + (pluralize ~multi "public_name") + else + of_sexp_errorf loc "field %s is missing" + (pluralize ~multi "name") + in let t = { names ; link_flags @@ -1070,7 +1101,7 @@ module Executables = struct } in let has_public_name = - List.exists ~f:Option.is_some public_names + List.exists ~f:(fun (_, n) -> Option.is_some n) public_names in let to_install = match Link_mode.Set.best_install_mode t.modes with @@ -1093,7 +1124,7 @@ module Executables = struct | Byte -> ".bc" in List.map2 names public_names - ~f:(fun (_, name) pub -> + ~f:(fun (_, name) (_, pub) -> match pub with | None -> None | Some pub -> Some ({ Install_conf. @@ -1133,21 +1164,26 @@ module Executables = struct (t, Some { Install_conf. section = Bin; files; package }) let public_name = - string >>| function + located string >>| fun (loc, s) -> + (loc + , match s with | "-" -> None - | s -> Some s + | s -> Some s) let multi = record (let%map names, public_names = map_validate - (let%map names = field "names" (list (located string)) + (let%map names = field_o "names" (list (located string)) and pub_names = field_o "public_names" (list public_name) in (names, pub_names)) ~f:(fun (names, public_names) -> - match public_names with - | None -> Ok (names, List.map names ~f:(fun _ -> None)) - | Some public_names -> + match names, public_names with + | None, None -> Ok ([], []) + | None, Some p -> Ok ([], p) + | Some names, None -> + Ok (names, List.map names ~f:(fun _ -> (Loc.none, None))) + | Some names, Some public_names -> if List.length public_names = List.length names then Ok (names, public_names) else @@ -1158,10 +1194,13 @@ module Executables = struct let single = record - (let%map name = field "name" (located string) - and public_name = field_o "public_name" string + (let%map name = field_o "name" (located string) + and public_name = field_o "public_name" (located string) and f = common in - f [name] [public_name] ~multi:false) + f (Option.to_list name) + (match public_name with + | None -> [Loc.none, None] + | Some (loc, n) -> [loc, Some n]) ~multi:false) end module Rule = struct From aa1a86cb6b4cead38bd4a2bbdb2cdaf5a3612f98 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 21 Jul 2018 17:24:32 +0200 Subject: [PATCH 4/9] Add tests for missing name field Signed-off-by: Rudi Grinberg --- test/blackbox-tests/dune.inc | 10 +++++++ .../no-name-exes-syntax-1-0/dune | 1 + .../no-name-exes-syntax-1-0/dune-project | 1 + .../no-name-field/no-name-exes/dune-project | 1 + .../no-name-field/no-name-exes/exe/dune | 1 + .../no-name-field/no-name-exes/exe/foo.ml | 0 .../no-name-field/no-name-exes/exes/bar.ml | 0 .../no-name-field/no-name-exes/exes/baz.ml | 0 .../no-name-field/no-name-exes/exes/dune | 1 + .../no-name-field/no-name-lib-syntax-1-0/dune | 1 + .../no-name-lib-syntax-1-0/dune-project | 2 ++ .../test-cases/no-name-field/no-name-lib/dune | 1 + .../no-name-field/no-name-lib/dune-project | 1 + .../no-name-field/no-name-lib/foo.ml | 0 .../test-cases/no-name-field/run.t | 30 +++++++++++++++++++ 15 files changed, 50 insertions(+) create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes/dune-project create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes/exe/dune create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes/exe/foo.ml create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/bar.ml create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/baz.ml create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune-project create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune-project create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-lib/foo.ml create mode 100644 test/blackbox-tests/test-cases/no-name-field/run.t diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index c3d357ce479..05f3c1623e0 100644 --- a/test/blackbox-tests/dune.inc +++ b/test/blackbox-tests/dune.inc @@ -468,6 +468,14 @@ test-cases/no-installable-mode (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) +(alias + (name no-name-field) + (deps (package dune) (source_tree test-cases/no-name-field)) + (action + (chdir + test-cases/no-name-field + (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected))))) + (alias (name null-dep) (deps (package dune) (source_tree test-cases/null-dep)) @@ -782,6 +790,7 @@ (alias misc) (alias multiple-private-libs) (alias no-installable-mode) + (alias no-name-field) (alias null-dep) (alias ocaml-config-macro) (alias ocaml-syntax) @@ -868,6 +877,7 @@ (alias meta-gen) (alias misc) (alias no-installable-mode) + (alias no-name-field) (alias null-dep) (alias ocaml-config-macro) (alias ocaml-syntax) diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune b/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune new file mode 100644 index 00000000000..4f7dc3911e7 --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune @@ -0,0 +1 @@ +(executables (public_names foo bar)) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project b/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project new file mode 100644 index 00000000000..4dd84ec1833 --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project @@ -0,0 +1 @@ +(dune lang 1.0) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/dune-project b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/dune-project new file mode 100644 index 00000000000..6687faf298a --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/dune-project @@ -0,0 +1 @@ +(lang dune 1.1) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exe/dune b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exe/dune new file mode 100644 index 00000000000..232f3d6747a --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exe/dune @@ -0,0 +1 @@ +(executable (public_name foo)) diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exe/foo.ml b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exe/foo.ml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/bar.ml b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/bar.ml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/baz.ml b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/baz.ml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune new file mode 100644 index 00000000000..a635b2fc6dc --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune @@ -0,0 +1 @@ +(executables (public_names (bar baz)) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune b/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune new file mode 100644 index 00000000000..372eda8398f --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune @@ -0,0 +1 @@ +(library (public_name foo)) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune-project b/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune-project new file mode 100644 index 00000000000..01e7bbba8dd --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/dune-project @@ -0,0 +1,2 @@ +(lang dune 1.0) + diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune b/test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune new file mode 100644 index 00000000000..372eda8398f --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune @@ -0,0 +1 @@ +(library (public_name foo)) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune-project b/test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune-project new file mode 100644 index 00000000000..6687faf298a --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-lib/dune-project @@ -0,0 +1 @@ +(lang dune 1.1) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-lib/foo.ml b/test/blackbox-tests/test-cases/no-name-field/no-name-lib/foo.ml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/run.t b/test/blackbox-tests/test-cases/no-name-field/run.t new file mode 100644 index 00000000000..2a153f125f6 --- /dev/null +++ b/test/blackbox-tests/test-cases/no-name-field/run.t @@ -0,0 +1,30 @@ +the name field can be omitted for libraries when public_name is present + $ dune build --root no-name-lib + File "dune-project", line 1, characters 11-14: + Error: Version 1.1 of dune is not supported. + Supported versions: + - 0.0 + - 1.0 + [1] + +this isn't possible for older syntax <= (1, 0) + $ dune build --root no-name-lib-syntax-1-0 + File "dune", line 1, characters 9-26: + Error: You cannot declare items to be installed without adding a .opam file at the root of your project. + To declare elements to be installed as part of package "foo", add a "foo.opam" file at the root of your project. + [1] + +executable(s) stanza works the same way + + $ dune build --root no-name-exes + File "dune-project", line 1, characters 11-14: + Error: Version 1.1 of dune is not supported. + Supported versions: + - 0.0 + - 1.0 + [1] + + $ dune build --root no-name-exes-syntax-1-0 + File "dune-project", line 1, characters 0-15: + Error: Invalid first line, expected: (lang ) + [1] From 837e9e483a60d0a7fd700dd5768af5b2b8f2a2ec Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 21 Jul 2018 17:25:30 +0200 Subject: [PATCH 5/9] Bump version to 1.1 Signed-off-by: Rudi Grinberg --- .../blackbox-tests/test-cases/no-name-field/run.t | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/test/blackbox-tests/test-cases/no-name-field/run.t b/test/blackbox-tests/test-cases/no-name-field/run.t index 2a153f125f6..9021e13f6eb 100644 --- a/test/blackbox-tests/test-cases/no-name-field/run.t +++ b/test/blackbox-tests/test-cases/no-name-field/run.t @@ -1,10 +1,8 @@ the name field can be omitted for libraries when public_name is present $ dune build --root no-name-lib - File "dune-project", line 1, characters 11-14: - Error: Version 1.1 of dune is not supported. - Supported versions: - - 0.0 - - 1.0 + File "dune", line 1, characters 9-26: + Error: You cannot declare items to be installed without adding a .opam file at the root of your project. + To declare elements to be installed as part of package "foo", add a "foo.opam" file at the root of your project. [1] this isn't possible for older syntax <= (1, 0) @@ -17,11 +15,8 @@ this isn't possible for older syntax <= (1, 0) executable(s) stanza works the same way $ dune build --root no-name-exes - File "dune-project", line 1, characters 11-14: - Error: Version 1.1 of dune is not supported. - Supported versions: - - 0.0 - - 1.0 + File "exes/dune", line 1, characters 37-37: + Error: unclosed parenthesis at end of input [1] $ dune build --root no-name-exes-syntax-1-0 From 3fd28d57a10bfa878b1518da26f49066adb58106 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 21 Jul 2018 17:28:41 +0200 Subject: [PATCH 6/9] Fix missing name field tests Signed-off-by: Rudi Grinberg --- .../no-name-exes-syntax-1-0/dune-project | 2 +- .../no-name-exes-syntax-1-0/foo.opam | 0 .../no-name-field/no-name-exes/exes/dune | 2 +- .../no-name-field/no-name-exes/foo.opam | 0 .../no-name-lib-syntax-1-0/foo.opam | 0 .../no-name-field/no-name-lib/foo.opam | 0 .../test-cases/no-name-field/run.t | 18 ++++++------------ 7 files changed, 8 insertions(+), 14 deletions(-) create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/foo.opam create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-exes/foo.opam create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/foo.opam create mode 100644 test/blackbox-tests/test-cases/no-name-field/no-name-lib/foo.opam diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project b/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project index 4dd84ec1833..b2559fa0581 100644 --- a/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project @@ -1 +1 @@ -(dune lang 1.0) \ No newline at end of file +(lang dune 1.0) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/foo.opam b/test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/foo.opam new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune index a635b2fc6dc..97e291bcb96 100644 --- a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune +++ b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/exes/dune @@ -1 +1 @@ -(executables (public_names (bar baz)) \ No newline at end of file +(executables (public_names bar baz)) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-exes/foo.opam b/test/blackbox-tests/test-cases/no-name-field/no-name-exes/foo.opam new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/foo.opam b/test/blackbox-tests/test-cases/no-name-field/no-name-lib-syntax-1-0/foo.opam new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/no-name-lib/foo.opam b/test/blackbox-tests/test-cases/no-name-field/no-name-lib/foo.opam new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/no-name-field/run.t b/test/blackbox-tests/test-cases/no-name-field/run.t index 9021e13f6eb..0551cffac2c 100644 --- a/test/blackbox-tests/test-cases/no-name-field/run.t +++ b/test/blackbox-tests/test-cases/no-name-field/run.t @@ -1,25 +1,19 @@ the name field can be omitted for libraries when public_name is present $ dune build --root no-name-lib - File "dune", line 1, characters 9-26: - Error: You cannot declare items to be installed without adding a .opam file at the root of your project. - To declare elements to be installed as part of package "foo", add a "foo.opam" file at the root of your project. - [1] + Entering directory 'no-name-lib' this isn't possible for older syntax <= (1, 0) $ dune build --root no-name-lib-syntax-1-0 - File "dune", line 1, characters 9-26: - Error: You cannot declare items to be installed without adding a .opam file at the root of your project. - To declare elements to be installed as part of package "foo", add a "foo.opam" file at the root of your project. + File "dune", line 1, characters 0-27: + Error: name field cannot be omitted before version 1.1 [1] executable(s) stanza works the same way $ dune build --root no-name-exes - File "exes/dune", line 1, characters 37-37: - Error: unclosed parenthesis at end of input - [1] + Entering directory 'no-name-exes' $ dune build --root no-name-exes-syntax-1-0 - File "dune-project", line 1, characters 0-15: - Error: Invalid first line, expected: (lang ) + File "dune", line 1, characters 0-36: + Error: name field may not be omitted before dune version 1.1 [1] From 60bce9a1f50d9bd0123d45f5320ec4dbcb46ec33 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Mon, 30 Jul 2018 19:41:16 +0200 Subject: [PATCH 7/9] Remove silly use of List.filter_map List.map works just as well Signed-off-by: Rudi Grinberg --- src/jbuild.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jbuild.ml b/src/jbuild.ml index ebb6d81532f..5ea79430a4a 100644 --- a/src/jbuild.ml +++ b/src/jbuild.ml @@ -1074,11 +1074,11 @@ module Executables = struct | _::_, _ -> names | [], _::_ -> if dune_syntax >= (1, 1) then - List.filter_map public_names ~f:(fun (loc, p) -> + List.map public_names ~f:(fun (loc, p) -> match p with | None -> of_sexp_error loc "This executable must have a name field" - | Some s -> Some (loc, s)) + | Some s -> (loc, s)) else of_sexp_errorf loc "%s field may not be omitted before dune version 1.1" From 34b7f45753837bd876737da6795438bf42b25bfc Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Mon, 30 Jul 2018 19:45:30 +0200 Subject: [PATCH 8/9] Consistent error message Signed-off-by: Rudi Grinberg --- src/jbuild.ml | 3 ++- test/blackbox-tests/test-cases/no-name-field/run.t | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/jbuild.ml b/src/jbuild.ml index 5ea79430a4a..96b17bc5c33 100644 --- a/src/jbuild.ml +++ b/src/jbuild.ml @@ -844,7 +844,8 @@ module Library = struct if dune_version >= (1, 1) then name else - of_sexp_error loc "name field cannot be omitted before version 1.1" + of_sexp_error loc "name field cannot be omitted before version \ + 1.1 of the dune language" | None, None -> of_sexp_error loc ( if dune_version >= (1, 1) then diff --git a/test/blackbox-tests/test-cases/no-name-field/run.t b/test/blackbox-tests/test-cases/no-name-field/run.t index 0551cffac2c..e22bf93a3de 100644 --- a/test/blackbox-tests/test-cases/no-name-field/run.t +++ b/test/blackbox-tests/test-cases/no-name-field/run.t @@ -5,7 +5,7 @@ the name field can be omitted for libraries when public_name is present this isn't possible for older syntax <= (1, 0) $ dune build --root no-name-lib-syntax-1-0 File "dune", line 1, characters 0-27: - Error: name field cannot be omitted before version 1.1 + Error: name field cannot be omitted before version 1.1 of the dune language [1] executable(s) stanza works the same way From 8c458e6d53cd08086641c36dee9258079dcfb928 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Tue, 31 Jul 2018 09:37:29 +0200 Subject: [PATCH 9/9] Use option type for missing names and public names Signed-off-by: Rudi Grinberg --- src/jbuild.ml | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/jbuild.ml b/src/jbuild.ml index 96b17bc5c33..c6496d726ae 100644 --- a/src/jbuild.ml +++ b/src/jbuild.ml @@ -1046,7 +1046,13 @@ module Executables = struct else s ^ "s" - let common = + let common + (* : (Loc.t * string) list option + * -> (Loc.t * string) list option + * -> multi:bool + * -> unit + * -> t * Install_conf.t option Sexp.Of_sexp.t *) + = let%map buildable = Buildable.t and (_ : bool) = field "link_executables" ~default:true (Syntax.deleted_in Stanza.syntax (1, 0) >>> bool) @@ -1072,8 +1078,8 @@ module Executables = struct fun names public_names ~multi -> let names = match names, public_names with - | _::_, _ -> names - | [], _::_ -> + | Some names, _ -> names + | None, Some public_names -> if dune_syntax >= (1, 1) then List.map public_names ~f:(fun (loc, p) -> match p with @@ -1084,7 +1090,7 @@ module Executables = struct of_sexp_errorf loc "%s field may not be omitted before dune version 1.1" (pluralize ~multi "name") - | [], [] -> + | None, None -> if dune_syntax >= (1, 1) then of_sexp_errorf loc "either the %s or the %s field must be present" (pluralize ~multi "name") @@ -1102,7 +1108,10 @@ module Executables = struct } in let has_public_name = - List.exists ~f:(fun (_, n) -> Option.is_some n) public_names + (* user could omit public names by avoiding the field or writing - *) + match public_names with + | None -> false + | Some pns -> List.exists ~f:(fun (_, n) -> Option.is_some n) pns in let to_install = match Link_mode.Set.best_install_mode t.modes with @@ -1124,6 +1133,11 @@ module Executables = struct | Native | Best -> ".exe" | Byte -> ".bc" in + let public_names = + match public_names with + | None -> List.map names ~f:(fun _ -> (Loc.none, None)) + | Some pns -> pns + in List.map2 names public_names ~f:(fun (_, name) (_, pub) -> match pub with @@ -1180,16 +1194,13 @@ module Executables = struct (names, pub_names)) ~f:(fun (names, public_names) -> match names, public_names with - | None, None -> Ok ([], []) - | None, Some p -> Ok ([], p) - | Some names, None -> - Ok (names, List.map names ~f:(fun _ -> (Loc.none, None))) | Some names, Some public_names -> if List.length public_names = List.length names then - Ok (names, public_names) + Ok (Some names, Some public_names) else Error "The list of public names must be of the same \ - length as the list of names") + length as the list of names" + | names, public_names -> Ok (names, public_names)) and f = common in f names public_names ~multi:true) @@ -1198,10 +1209,10 @@ module Executables = struct (let%map name = field_o "name" (located string) and public_name = field_o "public_name" (located string) and f = common in - f (Option.to_list name) - (match public_name with - | None -> [Loc.none, None] - | Some (loc, n) -> [loc, Some n]) ~multi:false) + f (Option.map name ~f:List.singleton) + (Option.map public_name ~f:(fun (loc, s) -> + [loc, Some s])) + ~multi:false) end module Rule = struct