diff --git a/src/jbuild.ml b/src/jbuild.ml index 9fdbe577..c5c16f1d 100644 --- a/src/jbuild.ml +++ b/src/jbuild.ml @@ -951,7 +951,9 @@ module Stanzas = struct | None -> [Executables exe] | Some i -> [Executables exe; Install i] - let rec v1 pkgs : Stanza.t list Sexp.Of_sexp.t = + exception Include_loop of Path.t * (Loc.t * Path.t) list + + let rec v1 pkgs ~file ~include_stack : Stanza.t list Sexp.Of_sexp.t = sum [ cstr "library" (Library.v1 pkgs @> nil) (fun x -> [Library x]) ; cstr "executable" (Executables.v1_single pkgs @> nil) execs @@ -969,21 +971,24 @@ module Stanzas = struct (* Just for validation and error messages *) ; cstr "jbuild_version" (Jbuild_version.t @> nil) (fun _ -> []) ; cstr_loc "include" (relative_file @> nil) (fun loc fn -> - let dir = Filename.dirname loc.start.pos_fname in - let fn = - if dir <> Filename.current_dir_name then - Filename.concat dir fn - else - fn - in - let sexps = Sexp.load ~fname:fn ~mode:Many in - parse pkgs sexps ~default_version:Jbuild_version.V1) + let include_stack = (loc, file) :: include_stack in + let dir = Path.parent file in + let file = Path.relative dir fn in + if List.exists include_stack ~f:(fun (_, f) -> f = file) then + raise (Include_loop (file, include_stack)); + let sexps = Sexp.load ~fname:(Path.to_string file) ~mode:Many in + parse pkgs sexps ~default_version:Jbuild_version.V1 ~file ~include_stack) ] - and select : Jbuild_version.t -> Scope.t -> Stanza.t list Sexp.Of_sexp.t = function + and select + : Jbuild_version.t + -> Scope.t + -> file:Path.t + -> include_stack:(Loc.t * Path.t) list + -> Stanza.t list Sexp.Of_sexp.t = function | V1 -> v1 - and parse ?(default_version=Jbuild_version.latest_stable) pkgs sexps = + and parse ~default_version ~file ~include_stack pkgs sexps = let versions, sexps = List.partition_map sexps ~f:(function | List (loc, [Atom (_, "jbuild_version"); ver]) -> @@ -997,7 +1002,30 @@ module Stanzas = struct | _ :: (_, loc) :: _ -> Loc.fail loc "jbuild_version specified too many times" in - List.concat_map sexps ~f:(select version pkgs) + List.concat_map sexps ~f:(select version pkgs ~file ~include_stack) + + let parse ?(default_version=Jbuild_version.latest_stable) ~file pkgs sexps = + try + parse pkgs sexps ~default_version ~include_stack:[] ~file + with + | Include_loop (_, []) -> assert false + | Include_loop (file, last :: rest) -> + let loc = fst (Option.value (List.last rest) ~default:last) in + let line_loc (loc, file) = + sprintf "%s:%d" + (Path.to_string_maybe_quoted file) + loc.Loc.start.pos_lnum + in + Loc.fail loc + "Recursive inclusion of jbuild files detected:\n\ + File %s is included from %s%s" + (Path.to_string_maybe_quoted file) + (line_loc last) + (String.concat ~sep:"" + (List.map rest ~f:(fun x -> + sprintf + "\n--> included from %s" + (line_loc x)))) let lib_names ts = List.fold_left ts ~init:String_set.empty ~f:(fun acc (_, _, stanzas) -> diff --git a/src/jbuild.mli b/src/jbuild.mli index f9d36d8e..d427d5e2 100644 --- a/src/jbuild.mli +++ b/src/jbuild.mli @@ -269,6 +269,7 @@ module Stanzas : sig val parse : ?default_version:Jbuild_version.t + -> file:Path.t -> Scope.t -> Sexp.Ast.t list -> t diff --git a/src/jbuild_load.ml b/src/jbuild_load.ml index cec40585..a2de2f62 100644 --- a/src/jbuild_load.ml +++ b/src/jbuild_load.ml @@ -146,7 +146,7 @@ end Did you forgot to call [Jbuild_plugin.V*.send]?" (Path.to_string file); let sexps = Sexp.load ~fname:(Path.to_string generated_jbuild) ~mode:Many in - return (dir, scope, Stanzas.parse scope sexps)) + return (dir, scope, Stanzas.parse scope sexps ~file:generated_jbuild)) |> Future.all end @@ -161,7 +161,7 @@ let load ~dir ~scope = let file = Path.relative dir "jbuild" in match Sexp.load_many_or_ocaml_script (Path.to_string file) with | Sexps sexps -> - Jbuilds.Literal (dir, scope, Stanzas.parse scope sexps) + Jbuilds.Literal (dir, scope, Stanzas.parse scope sexps ~file) | Ocaml_script -> Script { dir; scope } diff --git a/test/blackbox-tests/jbuild b/test/blackbox-tests/jbuild index d31877f2..869b4391 100644 --- a/test/blackbox-tests/jbuild +++ b/test/blackbox-tests/jbuild @@ -247,3 +247,13 @@ (progn (run ${exe:cram.exe} run.t) (diff? run.t run.t.corrected))))))) + +(alias + ((name runtest) + (deps ((files_recursively_in test-cases/include-loop))) + (action + (chdir test-cases/include-loop + (setenv JBUILDER ${bin:jbuilder} + (progn + (run ${exe:cram.exe} run.t) + (diff? run.t run.t.corrected))))))) diff --git a/test/blackbox-tests/test-cases/include-loop/a.inc b/test/blackbox-tests/test-cases/include-loop/a.inc new file mode 100644 index 00000000..dbd55d5d --- /dev/null +++ b/test/blackbox-tests/test-cases/include-loop/a.inc @@ -0,0 +1,2 @@ +(jbuild_version 1) +(include b.inc) diff --git a/test/blackbox-tests/test-cases/include-loop/b.inc b/test/blackbox-tests/test-cases/include-loop/b.inc new file mode 100644 index 00000000..46930a6c --- /dev/null +++ b/test/blackbox-tests/test-cases/include-loop/b.inc @@ -0,0 +1,2 @@ +(jbuild_version 1) +(include c.inc) diff --git a/test/blackbox-tests/test-cases/include-loop/c.inc b/test/blackbox-tests/test-cases/include-loop/c.inc new file mode 100644 index 00000000..85ef54cd --- /dev/null +++ b/test/blackbox-tests/test-cases/include-loop/c.inc @@ -0,0 +1,2 @@ +(jbuild_version 1) +(include a.inc) diff --git a/test/blackbox-tests/test-cases/include-loop/jbuild b/test/blackbox-tests/test-cases/include-loop/jbuild new file mode 100644 index 00000000..85ef54cd --- /dev/null +++ b/test/blackbox-tests/test-cases/include-loop/jbuild @@ -0,0 +1,2 @@ +(jbuild_version 1) +(include a.inc) diff --git a/test/blackbox-tests/test-cases/include-loop/run.t b/test/blackbox-tests/test-cases/include-loop/run.t new file mode 100644 index 00000000..ad78cab7 --- /dev/null +++ b/test/blackbox-tests/test-cases/include-loop/run.t @@ -0,0 +1,8 @@ + $ $JBUILDER build --root . -j 1 + File "jbuild", line 2, characters 0-15: + Error: Recursive inclusion of jbuild files detected: + File a.inc is included from c.inc:2 + --> included from b.inc:2 + --> included from a.inc:2 + --> included from jbuild:2 + [1]