From 0d27e9f90965e308875465a81a2fbd90d7b4c8bc Mon Sep 17 00:00:00 2001
From: Jeremie Dimino <jdimino@janestreet.com>
Date: Thu, 17 May 2018 15:42:40 +0100
Subject: [PATCH 1/2] Expose a bug in the S-expression record parser

---
 test/unit-tests/dune     | 10 ++++++++++
 test/unit-tests/sexp.mlt | 31 +++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 test/unit-tests/sexp.mlt

diff --git a/test/unit-tests/dune b/test/unit-tests/dune
index 9764b1c4..8fa30a6d 100644
--- a/test/unit-tests/dune
+++ b/test/unit-tests/dune
@@ -69,3 +69,13 @@
            (progn
             (run ${exe:expect_test.exe} ${<})
             (diff? ${<} ${<}.corrected))))))
+
+(alias
+ ((name runtest)
+  (deps (sexp.mlt
+         (glob_files ${SCOPE_ROOT}/src/.dune.objs/*.cmi)
+         (glob_files ${SCOPE_ROOT}/src/stdune/.stdune.objs/*.cmi)))
+  (action (chdir ${SCOPE_ROOT}
+           (progn
+            (run ${exe:expect_test.exe} ${<})
+            (diff? ${<} ${<}.corrected))))))
diff --git a/test/unit-tests/sexp.mlt b/test/unit-tests/sexp.mlt
new file mode 100644
index 00000000..114dd78b
--- /dev/null
+++ b/test/unit-tests/sexp.mlt
@@ -0,0 +1,31 @@
+(* -*- tuareg -*- *)
+open Stdune;;
+open Sexp.Of_sexp;;
+
+let pp_sexp_ast ppf sexp =
+  Sexp.pp ppf (Sexp.Ast.remove_locs sexp)
+;;
+#install_printer pp_sexp_ast;;
+[%%expect{|
+val pp_sexp_ast : Format.formatter -> Stdune.Sexp.Ast.t -> unit = <fun>
+|}]
+
+Printexc.record_backtrace false;;
+[%%expect{|
+- : unit = ()
+|}]
+
+let sexp = Sexp.parse_string ~fname:"" ~mode:Single {|
+((foo 1)
+ (foo 2))
+|}
+[%%expect{|
+val sexp : Usexp.Ast.t = ((foo 1) (foo 2))
+|}]
+
+let of_sexp = record (field "foo" int)
+let x = of_sexp sexp
+[%%expect{|
+val of_sexp : int Stdune.Sexp.Of_sexp.t = <fun>
+val x : int = 2
+|}]

From 9d3117d63ed5762771da9ed2d006ea9a70781b27 Mon Sep 17 00:00:00 2001
From: Jeremie Dimino <jdimino@janestreet.com>
Date: Thu, 17 May 2018 15:46:09 +0100
Subject: [PATCH 2/2] Fix bug exposed by previous commit

---
 CHANGES.md               | 3 +++
 src/stdune/sexp.ml       | 2 ++
 test/unit-tests/sexp.mlt | 4 +++-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/CHANGES.md b/CHANGES.md
index 711e5337..bbb87043 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -43,6 +43,9 @@ next
 - Add an `ignored_subdirs` stanza to replace `jbuild-ignore` files
   (#767, @diml)
 
+- Fix a bug where Dune ignored previous occurences of duplicated
+  fields (#779, @diml)
+
 1.0+beta20 (10/04/2018)
 -----------------------
 
diff --git a/src/stdune/sexp.ml b/src/stdune/sexp.ml
index cac9b1d4..f6f68470 100644
--- a/src/stdune/sexp.ml
+++ b/src/stdune/sexp.ml
@@ -273,6 +273,8 @@ module Of_sexp = struct
           | List (_, name_sexp :: values) -> begin
               match name_sexp with
               | Atom (_, A name) ->
+                if Name_map.mem acc name then
+                  of_sexp_errorf sexp "Field %S is present too many times" name;
                 Name_map.add acc name { values; entry = sexp }
               | List _ | Quoted_string _ ->
                 of_sexp_error name_sexp "Atom expected"
diff --git a/test/unit-tests/sexp.mlt b/test/unit-tests/sexp.mlt
index 114dd78b..826c7d42 100644
--- a/test/unit-tests/sexp.mlt
+++ b/test/unit-tests/sexp.mlt
@@ -27,5 +27,7 @@ let of_sexp = record (field "foo" int)
 let x = of_sexp sexp
 [%%expect{|
 val of_sexp : int Stdune.Sexp.Of_sexp.t = <fun>
-val x : int = 2
+Exception:
+Stdune__Sexp.Of_sexp.Of_sexp (<abstr>,
+ "Field \"foo\" is present too many times", None).
 |}]