Drop findlib:x syntax in dune and display an error (#887)

Signed-off-by: Nathan Rebours <nathan@cryptosense.com>

src/action.mli

@@ -83,7 +83,7 @@ module Unexpanded : sig
       :  t
       -> dir:Path.t
       -> map_exe:(Path.t -> Path.t)
-      -> f:(String_with_vars.Var.t -> Value.t list option)
+      -> f:(String_with_vars.Var.t -> Syntax.Version.t -> Value.t list option)
       -> Unresolved.t
   end
 
@@ -91,7 +91,7 @@ module Unexpanded : sig
     :  t
     -> dir:Path.t
     -> map_exe:(Path.t -> Path.t)
-    -> f:(String_with_vars.Var.t -> Value.t list option)
+    -> f:(String_with_vars.Var.t -> Syntax.Version.t -> Value.t list option)
     -> Partial.t
 end
 

src/string_with_vars.ml

@@ -2,7 +2,10 @@ open! Import
 
 open Usexp.Template
 
-type t = Usexp.Template.t
+type t =
+  { template : Usexp.Template.t
+  ; syntax_version : Syntax.Version.t
+  }
 
 let literal ~quoted ~loc s =
   { parts = [Text s]
@@ -13,7 +16,7 @@ let literal ~quoted ~loc s =
 (* This module implements the "old" template parsing that is only used in jbuild
    files *)
 module Jbuild : sig
-  val parse : string -> loc:Loc.t -> quoted:bool -> t
+  val parse : string -> loc:Loc.t -> quoted:bool -> Usexp.Template.t
 end = struct
   type var_syntax = Parens | Braces
   module Token = struct
@@ -101,30 +104,44 @@ let t =
     | Quoted_string (loc, s) -> literal ~quoted:true ~loc s
     | List (loc, _) -> Sexp.Of_sexp.of_sexp_error loc "Unexpected list"
   in
-  Syntax.get_exn Stanza.syntax >>= function
+  Syntax.get_exn Stanza.syntax >>= fun syntax_version ->
-  | (0, _) -> jbuild
+  let template =
-  | (_, _) -> dune
+    match syntax_version with
+    | (0, _) -> jbuild
+    | (_, _) -> dune
+  in
+  template >>| fun template ->
+  {template; syntax_version}
 
-let loc t = t.loc
+let loc t = t.template.loc
+
+let syntax_version t = t.syntax_version
+
+let virt_syntax = (0, 0)
 
 let virt ?(quoted=false) pos s =
-  Jbuild.parse ~quoted ~loc:(Loc.of_pos pos) s
+  let template = Jbuild.parse ~quoted ~loc:(Loc.of_pos pos) s in
+  {template; syntax_version = virt_syntax}
 
 let virt_var ?(quoted=false) pos s =
   assert (String.for_all s ~f:(function ':' -> false | _ -> true));
   let loc = Loc.of_pos pos in
-  { parts =
+  let template =
-      [Var { payload = None
+    { parts =
-           ; name = s
+        [Var { payload = None
-           ; syntax = Percent
+             ; name = s
-           ; loc
+             ; syntax = Percent
-           }]
+             ; loc
-  ; loc
+             }]
-  ; quoted
+    ; loc
-  }
+    ; quoted
+    }
+  in
+  {template; syntax_version = virt_syntax}
 
 let virt_text pos s =
-  { parts = [Text s];  loc = Loc.of_pos pos;  quoted = true }
+  let template = { parts = [Text s];  loc = Loc.of_pos pos;  quoted = true } in
+  {template; syntax_version = virt_syntax}
 
 let concat_rev = function
   | [] -> ""
@@ -188,9 +205,9 @@ let partial_expand
   : 'a.t
   -> mode:'a Mode.t
   -> dir:Path.t
-  -> f:(Var.t -> Value.t list option)
+  -> f:(Var.t -> Syntax.Version.t -> Value.t list option)
   -> 'a Partial.t
-  = fun t ~mode ~dir ~f ->
+  = fun ({template; syntax_version} as t) ~mode ~dir ~f ->
     let commit_text acc_text acc =
       let s = concat_rev acc_text in
       if s = "" then acc else Text s :: acc
@@ -202,35 +219,36 @@ let partial_expand
         | [] ->
           Partial.Expanded (Mode.string mode (concat_rev acc_text))
         | _  ->
-          Unexpanded { t with parts = List.rev (commit_text acc_text acc) }
+          let template = {template with parts = List.rev (commit_text acc_text acc)} in
+          Unexpanded {template; syntax_version}
         end
       | Text s :: items -> loop (s :: acc_text) acc items
       | Var var as it :: items ->
-        begin match f var with
+        begin match f var syntax_version with
-        | Some ([] | _::_::_ as e) when not t.quoted ->
+        | Some ([] | _::_::_ as e) when not template.quoted ->
           invalid_multivalue var e
         | Some t ->
           loop (Value.L.concat ~dir t :: acc_text) acc items
         | None -> loop [] (it :: commit_text acc_text acc) items
         end
     in
-    match t.parts with
+    match template.parts with
     | [] -> Partial.Expanded (Mode.string mode "")
     | [Text s] -> Expanded (Mode.string mode s)
-    | [Var var] when not t.quoted ->
+    | [Var var] when not template.quoted ->
-      begin match f var with
+      begin match f var syntax_version with
       | None -> Partial.Unexpanded t
       | Some e -> Expanded (
         match Mode.value mode e with
         | None -> invalid_multivalue var e
         | Some s -> s)
       end
-    | _ -> loop [] [] t.parts
+    | _ -> loop [] [] template.parts
 
 let expand t ~mode ~dir ~f =
   match
-    partial_expand t ~mode ~dir ~f:(fun var ->
+    partial_expand t ~mode ~dir ~f:(fun var syntax_version ->
-      match f var with
+      match f var syntax_version with
       | None ->
         begin match var.syntax with
         | Percent ->
@@ -248,9 +266,9 @@ let expand t ~mode ~dir ~f =
 
 let partial_expand t ~mode ~dir ~f = partial_expand t ~mode ~dir ~f
 
-let sexp_of_t t = Usexp.Template t
+let sexp_of_t t = Usexp.Template t.template
 
-let is_var { parts ; quoted = _; loc = _ } ~name =
+let is_var { template; syntax_version = _ } ~name =
-  match parts with
+  match template.parts with
   | [Var n] -> name = Var.full_name n
   | _ -> false

src/string_with_vars.mli

@@ -16,12 +16,14 @@ val t : t Sexp.Of_sexp.t
 val loc : t -> Loc.t
 (** [loc t] returns the location of [t] — typically, in the jbuild file. *)
 
+val syntax_version : t -> Syntax.Version.t
+
 val sexp_of_t : t -> Sexp.t
 
 (** [t] generated by the OCaml code. The first argument should be
     [__POS__]. The second is either a string to parse, a variable name
     or plain text.  [quoted] says whether the string is quoted ([false]
-    by default). *)
+    by default). Those functions expect jbuild syntax. *)
 val virt       : ?quoted: bool -> (string * int * int * int) -> string -> t
 val virt_var   : ?quoted: bool -> (string * int * int * int) -> string -> t
 val virt_text  : (string * int * int * int) -> string -> t
@@ -57,12 +59,12 @@ val expand
   :  t
   -> mode:'a Mode.t
   -> dir:Path.t
-  -> f:(Var.t -> Value.t list option)
+  -> f:(Var.t -> Syntax.Version.t -> Value.t list option)
   -> 'a
 
 val partial_expand
   :  t
   -> mode:'a Mode.t
   -> dir:Path.t
-  -> f:(Var.t -> Value.t list option)
+  -> f:(Var.t -> Syntax.Version.t -> Value.t list option)
   -> 'a Partial.t

src/super_context.ml

@@ -88,7 +88,7 @@ let expand_var_no_root t var = String.Map.find t.vars var
 
 let (expand_vars, expand_vars_path) =
   let expand t ~scope ~dir ?(extra_vars=String.Map.empty) s =
-    String_with_vars.expand ~mode:Single ~dir s ~f:(fun v ->
+    String_with_vars.expand ~mode:Single ~dir s ~f:(fun v _syntax_version ->
       match String_with_vars.Var.full_name v with
       | "ROOT" -> Some [Value.Path t.context.build_dir]
       | "SCOPE_ROOT" -> Some [Value.Path (Scope.root scope)]
@@ -627,7 +627,7 @@ module Action = struct
       ; ddeps     = String.Map.empty
       }
     in
-    let expand var =
+    let expand var syntax_version =
       let loc = String_with_vars.Var.loc var in
       let key = String_with_vars.Var.full_name var in
       match String_with_vars.Var.destruct var with
@@ -640,9 +640,14 @@ module Action = struct
           | Error e ->
             add_fail acc ({ fail = fun () -> Action.Prog.Not_found.raise e })
         end
-      (* "findlib" for compatibility with Jane Street packages which are not yet updated
+      | Pair ("findlib", s) when syntax_version >= (1, 0) ->
-         to convert "findlib" to "lib" *)
+        Loc.fail
-      | Pair (("lib"|"findlib"), s) -> begin
+          loc
+          "The findlib special variable is not supported anymore, please use lib instead:\n\
+           %%{lib:%s}"
+          s
+      | Pair ("findlib", s)
+      | Pair ("lib", s) -> begin
           let lib_dep, file = parse_lib_file ~loc s in
           add_lib_dep acc lib_dep dep_kind;
           match
@@ -721,7 +726,7 @@ module Action = struct
         | None -> String.Map.find extra_vars key
     in
     let t =
-      U.partial_expand t ~dir ~map_exe ~f:(fun var ->
+      U.partial_expand t ~dir ~map_exe ~f:(fun var syntax_version ->
         let var_name = String_with_vars.Var.full_name var in
         let loc = String_with_vars.Var.loc var in
         match var_name with
@@ -738,7 +743,7 @@ module Action = struct
           | Pair ("path-no-dep", s) ->
             Some (path_exp (Path.relative dir s))
           | _ ->
-            let exp = expand var in
+            let exp = expand var syntax_version in
             Option.iter exp ~f:(fun vs ->
               acc.sdeps <- Path.Set.union (Path.Set.of_list
                                              (Value.L.paths_only vs)) acc.sdeps;
@@ -748,7 +753,7 @@ module Action = struct
     (t, acc)
 
   let expand_step2 ~dir ~dynamic_expansions ~deps_written_by_user ~map_exe t =
-    U.Partial.expand t ~dir ~map_exe ~f:(fun var ->
+    U.Partial.expand t ~dir ~map_exe ~f:(fun var _syntax_version ->
       let key = String_with_vars.Var.full_name var in
       let loc = String_with_vars.Var.loc var in
       match String.Map.find dynamic_expansions key with

test/blackbox-tests/dune.inc

@@ -136,6 +136,14 @@
    test-cases/findlib
    (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))
 
+(alias
+ (name findlib-error)
+ (deps (package dune) (source_tree test-cases/findlib-error))
+ (action
+  (chdir
+   test-cases/findlib-error
+   (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))
+
 (alias
  (name force-test)
  (deps (package dune) (source_tree test-cases/force-test))
@@ -591,6 +599,7 @@
   (alias exclude-missing-module)
   (alias exec-cmd)
   (alias findlib)
+  (alias findlib-error)
   (alias force-test)
   (alias gen-opam-install-file)
   (alias github20)
@@ -661,6 +670,7 @@
   (alias exclude-missing-module)
   (alias exec-cmd)
   (alias findlib)
+  (alias findlib-error)
   (alias force-test)
   (alias gen-opam-install-file)
   (alias github20)

test/blackbox-tests/test-cases/findlib-error/in-dune/dune

@@ -0,0 +1,3 @@
+(rule
+(write-file target.txt %{findlib:pkg})
+)

test/blackbox-tests/test-cases/findlib-error/in-dune/dune-project

@@ -0,0 +1 @@
+(lang dune 1.0)

test/blackbox-tests/test-cases/findlib-error/in-jbuild/jbuild

@@ -0,0 +1,5 @@
+(jbuild_version 1)
+
+(rule
+(write-file target.txt ${findlib:pkg:file})
+)

test/blackbox-tests/test-cases/findlib-error/run.t

@@ -0,0 +1,16 @@
+We are dropping support for findlib in dune
+
+  $ dune build --root in-dune target.txt
+  Entering directory 'in-dune'
+  File "dune", line 2, characters 25-37:
+  Error: The findlib special variable is not supported anymore, please use lib instead:
+  %{lib:pkg}
+  [1]
+
+But it must still be available in jbuild files
+
+  $ dune build --root in-jbuild target.txt
+  Entering directory 'in-jbuild'
+  File "jbuild", line 4, characters 23-42:
+  Error: Public library "pkg" not found
+  [1]