Skip to content
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

Cleanup handling of .opt and .exe #2543

Merged
merged 4 commits into from
Aug 12, 2019
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
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@
relies on the version written in the `dune-project` file and no
longer read `VERSION` or similar files (#2541, @diml)

- On Windows, an .exe suffix is no longer added implicitly to binary names that
already end in .exe. Second, when resolving binary names, .opt variants are no
longer chosen automatically. (#2543, @nojb)

1.11.0 (23/07/2019)
-------------------

Expand Down
17 changes: 15 additions & 2 deletions src/dune/context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,20 @@ let opam_config_var ~env ~cache var =
| Error _ ->
None ) )

let which ~cache ~path x = Table.find_or_add cache x ~f:(Bin.which ~path)
let best_prog dir prog =
let fn = Path.relative dir (prog ^ ".opt" ^ Bin.exe) in
if Bin.exists fn then
Some fn
else
let fn = Path.relative dir (prog ^ Bin.exe) in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option.some_if makes this a bit cleaner.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'm not convinced; if I use Option.some_if around the first call to Bin.exists then I need to pattern match on the result, which is not nice. And if I only use it around the second call to Bin.exists then the code loses its symmetry.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The symmetry was lost on me :) anyway it's just a suggestion, feel free to leave as is.

if Bin.exists fn then
Some fn
else
None

let which ~path prog = List.find_map path ~f:(fun dir -> best_prog dir prog)

let which ~cache ~path x = Table.find_or_add cache x ~f:(which ~path)

let ocamlpath_sep =
if Sys.cygwin then
Expand Down Expand Up @@ -273,7 +286,7 @@ let create ~(kind : Kind.t) ~path ~env ~env_nodes ~name ~merlin ~targets
let get_ocaml_tool prog =
match get_tool_using_findlib_config prog with
| None ->
Bin.best_prog dir prog
best_prog dir prog
| Some _ as x ->
x
in
Expand Down
24 changes: 8 additions & 16 deletions src/stdune/bin.ml
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,17 @@ let exists fn =
| _ ->
true

let best_prog dir prog =
let fn = Path.relative dir (prog ^ ".opt" ^ exe) in
if exists fn then
Some fn
let add_exe prog =
if String.is_suffix (String.lowercase prog) ~suffix:exe then
prog
else
let fn = Path.relative dir (prog ^ exe) in
if exists fn then
Some fn
else
None
prog ^ exe

let which ~path prog =
let rec search = function
| [] ->
None
| dir :: rest -> (
match best_prog dir prog with None -> search rest | Some fn -> Some fn )
in
search path
let prog = add_exe prog in
List.find_map path ~f:(fun dir ->
let fn = Path.relative dir prog in
Option.some_if (exists fn) fn)

let make ~path =
match which ~path "gmake" with None -> which ~path "make" | some -> some
7 changes: 3 additions & 4 deletions src/stdune/bin.mli
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ val cons_path : Path.t -> _PATH:string option -> string
(** Extension to append to executable filenames *)
val exe : string

(** Check if a file exists *)
val exists : Path.t -> bool

(** Look for a program in the PATH *)
val which : path:Path.t list -> string -> Path.t option

(** Return the .opt version of a tool if available. If the tool is not
available at all in the given directory, returns [None]. *)
val best_prog : Path.t -> string -> Path.t option

(** "make" program *)
val make : path:Path.t list -> Path.t option