Merge pull request #168 from NathanReb/fix-weird-opam-truncation

Fix a bug in opam file transformation for distrib

Author: samoht

CHANGES.md

@@ -4,6 +4,8 @@
 
 - Fix a bug where `opam submit` would fail if the opam files had no description
   (#165, @NathanReb)
+- Fix a bug where opam files could be inproperly tempered with while building
+  the distribution tarball (#168, @NathanReb)
 
 ## 1.3.2 (2019-07-12)
 

dune-project

@@ -1,2 +1,2 @@
-(lang dune 1.0)
-(name dune-release)
+(lang dune 1.2)
+(name dune-release)

dune-release.opam

@@ -15,7 +15,7 @@ build: [
 
 depends: [
   "ocaml" {>= "4.06.0"}
-  "dune" {build}
+  "dune" {>= "1.2.0"}
   "fmt"
   "bos"
   "cmdliner"

lib/pkg.ml

@@ -448,16 +448,26 @@ let v ~dry_run
 
 (* Distrib *)
 
+let version_line_re =
+  let open Re in
+  seq
+    [bos; str "version:"; rep space; char '"'; rep1 any; char '"'; rep space; eos]
+
+let prepare_opam_for_distrib ~version ~content =
+  let re = Re.compile version_line_re in
+  let is_not_version_field line = not (Re.execp re line) in
+  let without_version = List.filter is_not_version_field content in
+  Fmt.strf "version: \"%s\"" version :: without_version
+
 let distrib_version_opam_files ~dry_run ~version =
   infer_pkg_names Fpath.(v ".") [] >>= fun names ->
   List.fold_left (fun acc name ->
       acc >>= fun _acc ->
       let file = Fpath.(v name + "opam") in
       OS.File.read_lines file
-      >>= fun o ->
-      let o = List.filter (fun l -> not (String.is_prefix ~affix:"version" l)) o in
-      let o =  Fmt.strf "version: \"%s\"" version :: o in
-      Sos.write_file ~dry_run file (String.concat ~sep:"\n" o))
+      >>= fun content ->
+      let content = prepare_opam_for_distrib ~version ~content in
+      Sos.write_file ~dry_run file (String.concat ~sep:"\n" content))
     (Ok ()) names
 
 let distrib_prepare ~dry_run p ~dist_build_dir ~version =

lib/pkg.mli

@@ -154,6 +154,14 @@ val extract_tag : t -> (string, Sos.error) result
 
 val dev_repo : t -> (string option, Sos.error) result
 
+(**/**)
+
+val version_line_re : Re.t
+
+val prepare_opam_for_distrib : version: string -> content: string list -> string list
+
+(**/**)
+
 (*---------------------------------------------------------------------------
    Copyright (c) 2016 Daniel C. Bünzli
 

tests/dune

@@ -1,3 +1,4 @@
 (test
  (name tests)
- (libraries dune-release alcotest))
+ (libraries dune-release alcotest)
+ (action (run %{test} -e)))

tests/test_pkg.ml

@@ -0,0 +1,58 @@
+let test_version_line_re =
+  let make_test ~input ~expected =
+    let test_name =
+      if expected then
+        input ^ "is a valid version field line"
+      else
+        input ^ "is not a valid version field line"
+    in
+    let test_fun () =
+      let re = Re.compile Dune_release.Pkg.version_line_re in
+      let actual = Re.execp re input in
+      Alcotest.(check bool) test_name expected actual
+    in
+    (test_name, `Quick, test_fun)
+  in
+  [ make_test ~input:"" ~expected:false
+  ; make_test ~input:{|version:""|} ~expected:false
+  ; make_test ~input:{|version:"1"|} ~expected:true
+  ; make_test ~input:{|version:     "1"    |} ~expected:true
+  ; make_test ~input:{|version:"1.jfpojef.adp921709"|} ~expected:true
+  ]
+
+let test_prepare_opam_for_distrib =
+  let make_test ~name ~version ~content ~expected () =
+    let test_name = "prepare_opam_for_distrib: " ^ name in
+    let test_fun () =
+      let actual = Dune_release.Pkg.prepare_opam_for_distrib ~version ~content in
+      Alcotest.(check (list string)) test_name expected actual
+    in
+    (test_name, `Quick, test_fun)
+  in
+  [ make_test ~name:"empty" ~content:[] ~version:"1" ~expected:[{|version: "1"|}] ()
+  ; make_test
+      ~name:"replace version"
+      ~content:[{|version: "1"|}]
+      ~version:"2"
+      ~expected:[{|version: "2"|}]
+      ()
+  ; make_test
+      ~name:"only replace version field"
+      ~content:
+        [ {|version: "1"|}
+        ; {|description: """|}
+        ; {|version: "1" blablabla|}
+        ; {|"""|}
+        ]
+      ~version:"2"
+      ~expected:
+        [ {|version: "2"|}
+        ; {|description: """|}
+        ; {|version: "1" blablabla|}
+        ; {|"""|}
+        ]
+      ()
+  ]
+
+let suite =
+  ("Pkg", test_version_line_re @ test_prepare_opam_for_distrib)

tests/test_pkg.mli

@@ -0,0 +1 @@
+val suite : unit Alcotest.test

tests/tests.ml

@@ -1,5 +1,6 @@
 let () =
   Alcotest.run "dune-release" [
     Test_github.suite;
+    Test_pkg.suite;
     Test_tags.suite;
   ]