Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/install_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
116 changes: 93 additions & 23 deletions src/jbuild.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand All @@ -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
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions src/jbuild.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions src/lib.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/scope.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 :: _ ->
Expand Down
3 changes: 2 additions & 1 deletion src/super_context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
10 changes: 10 additions & 0 deletions test/blackbox-tests/dune.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(executables (public_names foo bar))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(lang dune 1.0)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(lang dune 1.1)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(executable (public_name foo))
Empty file.
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(executables (public_names bar baz))
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(library (public_name foo))
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
(lang dune 1.0)

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(library (public_name foo))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(lang dune 1.1)
Empty file.
Empty file.
19 changes: 19 additions & 0 deletions test/blackbox-tests/test-cases/no-name-field/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
the name field can be omitted for libraries when public_name is present
$ dune build --root no-name-lib
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 0-27:
Error: name field cannot be omitted before version 1.1 of the dune language
[1]

executable(s) stanza works the same way

$ dune build --root no-name-exes
Entering directory 'no-name-exes'

$ dune build --root no-name-exes-syntax-1-0
File "dune", line 1, characters 0-36:
Error: name field may not be omitted before dune version 1.1
[1]