Detect recursive includes (#435)

Fix #432

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

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

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 }
 

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

test/blackbox-tests/test-cases/include-loop/a.inc

@@ -0,0 +1,2 @@
+(jbuild_version 1)
+(include b.inc)

test/blackbox-tests/test-cases/include-loop/b.inc

@@ -0,0 +1,2 @@
+(jbuild_version 1)
+(include c.inc)

test/blackbox-tests/test-cases/include-loop/c.inc

@@ -0,0 +1,2 @@
+(jbuild_version 1)
+(include a.inc)

test/blackbox-tests/test-cases/include-loop/jbuild

@@ -0,0 +1,2 @@
+(jbuild_version 1)
+(include a.inc)

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]