Make name and names fields optional when public_name or public_names are present (#1041)

Make name field optional when public_name is specified

When name is omitted, it will be defaulted to public_name

This feature is 1.1 only.

Author: rgrinberg

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)

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
@@ -805,7 +807,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 +837,23 @@ 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 = (_loc, name) ; _ }  ->
+           if dune_version >= (1, 1) then
+             name
+           else
+             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
+               "supply at least least one of name or public_name fields"
+             else
+               "name field is missing"
+           )
+       in
        { name
        ; public
        ; synopsis
@@ -876,7 +896,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
@@ -1020,7 +1040,19 @@ module Executables = struct
     ; buildable  : Buildable.t
     }
 
-  let common =
+  let pluralize s ~multi =
+    if multi then
+      s
+    else
+      s ^ "s"
+
+  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)
@@ -1040,8 +1072,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
+        | Some names, _ -> names
+        | None, Some public_names ->
+          if dune_syntax >= (1, 1) then
+            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 -> (loc, s))
+          else
+            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")
+              (pluralize ~multi "public_name")
+          else
+            of_sexp_errorf loc "field %s is missing"
+              (pluralize ~multi "name")
+      in
       let t =
         { names
         ; link_flags
@@ -1051,7 +1108,10 @@ module Executables = struct
         }
       in
       let has_public_name =
-        List.exists ~f:Option.is_some 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
@@ -1073,8 +1133,13 @@ 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 ->
+            ~f:(fun (_, name) (_, pub) ->
               match pub with
               | None -> None
               | Some pub -> Some ({ Install_conf.
@@ -1114,35 +1179,40 @@ 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
+             | 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)
 
   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.map name ~f:List.singleton)
+         (Option.map public_name ~f:(fun (loc, s) ->
+            [loc, Some s]))
+         ~multi:false)
 end
 
 module Rule = struct

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

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

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 :: _ ->

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))
 

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)

test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune

@@ -0,0 +1 @@
+(executables (public_names foo bar))

test/blackbox-tests/test-cases/no-name-field/no-name-exes-syntax-1-0/dune-project

@@ -0,0 +1 @@
+(lang dune 1.0)