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

Forbid #require in dune files in OCaml syntax #938

Merged
3 commits merged into from Jul 2, 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: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ next
when not defined by the user and make `@@default` be the default
target (#926, @diml)

- Forbid `#require` in `dune` files in OCaml syntax (#938, @diml)

- Add `%{profile}` variable. (#938, @rgrinberg)

1.0+beta20 (10/04/2018)
Expand Down
3 changes: 3 additions & 0 deletions plugin/jbuild_plugin.mli
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@ module V1 : sig
(** [send s] send [s] to jbuilder. [s] should be the contents of a
jbuild file following the specification described in the manual. *)
val send : string -> unit

(** Execute a command and read it's output *)
val run_and_read_lines : string -> string list
end
61 changes: 48 additions & 13 deletions src/jbuild_load.ml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module Jbuilds = struct

type requires = No_requires | Unix

let extract_requires path str =
let extract_requires path str ~kind =
let rec loop n lines acc =
match lines with
| [] -> acc
Expand All @@ -52,28 +52,40 @@ module Jbuilds = struct
match Scanf.sscanf line "#require %S" (fun x -> x) with
| exception _ -> acc
| s ->
match String.split s ~on:',' with
| [] -> acc
| ["unix"] -> Unix
| _ ->
let start =
{ Lexing.
pos_fname = Path.to_string path
let loc : Loc.t =
let start : Lexing.position =
{ pos_fname = Path.to_string path
; pos_lnum = n
; pos_cnum = 0
; pos_bol = 0
}
in
Loc.fail
{ start; stop = { start with pos_cnum = String.length line } }
{ start; stop = { start with pos_cnum = String.length line } }
in
(match (kind : File_tree.Dune_file.Kind.t) with
| Jbuild -> ()
| Dune ->
Loc.fail loc
"#require is no longer supported in dune files.\n\
You can use the following function instead of \
Unix.open_process_in:\n\
\n\
\ (** Execute a command and read it's output *)\n\
\ val run_and_read_lines : string -> string list");
match String.split s ~on:',' with
| [] -> acc
| ["unix"] -> Unix
| _ ->
Loc.fail loc
"Using libraries other that \"unix\" is not supported.\n\
See the manual for details.";
in
loop (n + 1) lines acc
in
loop 1 (String.split str ~on:'\n') No_requires

let create_plugin_wrapper (context : Context.t) ~exec_dir ~plugin ~wrapper ~target =
let create_plugin_wrapper (context : Context.t) ~exec_dir ~plugin ~wrapper
~target ~kind =
let plugin_contents = Io.read_file plugin in
Io.with_file_out wrapper ~f:(fun oc ->
let ocamlc_config =
Expand Down Expand Up @@ -106,6 +118,29 @@ module Jbuild_plugin = struct
let oc = open_out_bin %S in
output_string oc s;
close_out oc

let run_and_read_lines cmd =
let tmp_fname = Filename.temp_file "dune" ".output" in
at_exit (fun () -> Sys.remove tmp_fname);
let n =
Printf.ksprintf Sys.command "%%s > %%s" cmd (Filename.quote tmp_fname)
in
let rec loop ic acc =
match input_line ic with
| exception End_of_file -> close_in ic; List.rev acc
| line -> loop ic (line :: acc)
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to the Io module? Note that I see so much opportunity for code reuse, but keeping the application code uncluttered is nice.

Copy link
Author

Choose a reason for hiding this comment

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

It's not a trivial move since this is for plugins

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah, that's right. Bit of a shame.

in
let output = loop (open_in tmp_fname) [] in
if n = 0 then
output
else begin
Printf.ksprintf failwith
"Command failed: %%s\n\
Exit code: %%d\n\
Output:\n\
%%s"
cmd n (String.concat "\n" output)
end
end
end
# 1 %S
Expand All @@ -115,7 +150,7 @@ end
ocamlc_config
(Path.reach ~from:exec_dir target)
(Path.to_string plugin) plugin_contents);
extract_requires plugin plugin_contents
extract_requires plugin plugin_contents ~kind

let eval { jbuilds; ignore_promoted_rules } ~(context : Context.t) =
let open Fiber.O in
Expand All @@ -132,7 +167,7 @@ end
ensure_parent_dir_exists generated_jbuild;
let requires =
create_plugin_wrapper context ~exec_dir:dir ~plugin:file ~wrapper
~target:generated_jbuild
~target:generated_jbuild ~kind
in
let context = Option.value context.for_host ~default:context in
let cmas =
Expand Down