From 8c29161144e768c66332a48e4fede18aa9694d93 Mon Sep 17 00:00:00 2001
From: Hannes Mehnert <hannes@mehnert.org>
Date: Tue, 7 Jan 2025 13:17:10 +0100
Subject: [PATCH] Compute and present the opam file differences as a unified
 diff

---
 lib/views.ml          | 29 ++----------------
 opamdiff/dune         |  2 +-
 opamdiff/opamdiff.ml  | 68 +++++++++++++++++++------------------------
 opamdiff/opamdiff.mli |  6 ++--
 4 files changed, 35 insertions(+), 70 deletions(-)

diff --git a/lib/views.ml b/lib/views.ml
index eac0e80..fcc6758 100644
--- a/lib/views.ml
+++ b/lib/views.ml
@@ -828,33 +828,8 @@ let duniverse_diffs diffs =
 let opam_diffs diffs =
   List.concat_map (fun pd ->
       H.h4 [ txtf "%a" Opamdiff.pp_opam_diff pd ] ::
-      (match pd.Opamdiff.build with None -> [] | Some a ->
-          let l, r = Opamdiff.commands_to_strings a in
-          [
-            H.h5 [ H.txt "build instruction (without common prefix) \
-                          modifications, old:" ] ;
-            H.code (List.concat_map (fun s -> [ H.txt s ; H.br () ]) l) ;
-            H.h5 [ H.txt "new" ] ;
-            H.code (List.concat_map (fun s -> [ H.txt s ; H.br () ]) r)
-          ]) @
-      (match pd.Opamdiff.install with None -> [] | Some a ->
-          let l, r = Opamdiff.commands_to_strings a in
-          [
-            H.h5 [ H.txt "install instruction (without common prefix) \
-                          modifications, old:" ] ;
-            H.code (List.concat_map (fun s -> [ H.txt s ; H.br () ]) l) ;
-            H.h5 [ H.txt "new" ] ;
-            H.code (List.concat_map (fun s -> [ H.txt s ; H.br () ]) r)
-          ]) @
-      (match pd.Opamdiff.url with None -> [] | Some a ->
-          let l, r = Opamdiff.opt_url_to_string a in
-          [
-            H.h5 [ H.txt "URL" ] ;
-            txtf "old: %s" l;
-            H.br ();
-            txtf "new: %s" r
-          ]) @
-      [ H.br () ])
+      H.h5 [ H.txt "diff" ] ::
+      H.code [ H.txt pd.diff ; H.br () ] :: [])
     diffs
 
 let compare_builds
diff --git a/opamdiff/dune b/opamdiff/dune
index fc5561d..0cb8475 100644
--- a/opamdiff/dune
+++ b/opamdiff/dune
@@ -1,3 +1,3 @@
 (library
  (name opamdiff)
- (libraries opam-core opam-format yojson))
+ (libraries opam-core opam-format yojson bos))
diff --git a/opamdiff/opamdiff.ml b/opamdiff/opamdiff.ml
index 059af95..0befbd3 100644
--- a/opamdiff/opamdiff.ml
+++ b/opamdiff/opamdiff.ml
@@ -168,10 +168,8 @@ let pp_version_diff ppf { name; version_left; version_right } =
 
 type opam_diff = {
   pkg : OpamPackage.t ;
-  build : (OpamTypes.command list * OpamTypes.command list) option ;
-  install : (OpamTypes.command list * OpamTypes.command list) option ;
-  url : (OpamFile.URL.t option * OpamFile.URL.t option) option ;
-  otherwise_equal : bool ;
+  effectively_equal : bool ;
+  diff : string ;
 }
 
 let commands_to_strings (l, r) =
@@ -186,46 +184,40 @@ let opt_url_to_string (l, r) =
   in
   url_to_s l, url_to_s r
 
-let pp_opam_diff ppf { pkg ; otherwise_equal ; _ } =
+let pp_opam_diff ppf { pkg ; effectively_equal ; _ } =
   Format.fprintf ppf "%a%s"
     pp_opampackage pkg
-    (if otherwise_equal then "" else " (and additional changes)")
-
-let rec strip_common_prefix a b =
-  match a, b with
-  | hd::tl, hd'::tl' ->
-    if hd = hd' then
-      strip_common_prefix tl tl'
-    else
-      a, b
-  | a, b -> a, b
+    (if effectively_equal then "" else " (effectively equal)")
 
 let detailed_opam_diff pkg l r =
-  let no_build_install_url p =
-    OpamFile.OPAM.with_url_opt None
-      (OpamFile.OPAM.with_install []
-         (OpamFile.OPAM.with_build [] p))
+  let ( let* ) = Result.bind in
+  let opaml = OpamFile.OPAM.write_to_string l in
+  let opamr =
+    let o = OpamFile.make (OpamFilename.raw "opam") in
+    OpamFile.OPAM.to_string_with_preserved_format ~format_from_string:opaml o r
   in
-  let otherwise_equal =
+  let effectively_equal =
+    let no_build_install_url p =
+      OpamFile.OPAM.with_url_opt None
+        (OpamFile.OPAM.with_install []
+           (OpamFile.OPAM.with_build [] p))
+    in
     OpamFile.OPAM.effectively_equal
       (no_build_install_url l) (no_build_install_url r)
-  and build =
-    if OpamFile.OPAM.build l = OpamFile.OPAM.build r then
-      None
-    else
-      Some (strip_common_prefix (OpamFile.OPAM.build l) (OpamFile.OPAM.build r))
-  and install =
-    if OpamFile.OPAM.install l = OpamFile.OPAM.install r then
-      None
-    else
-      Some (strip_common_prefix (OpamFile.OPAM.install l) (OpamFile.OPAM.install r))
-  and url =
-    if OpamFile.OPAM.url l = OpamFile.OPAM.url r then
-      None
-    else
-      Some (OpamFile.OPAM.url l, OpamFile.OPAM.url r)
   in
-  { pkg ; build ; install ; url ; otherwise_equal }
+  let diff =
+    let* tmpl = Bos.OS.File.tmp "opaml_%s" in
+    let* () = Bos.OS.File.write tmpl opaml in
+    let* tmpr = Bos.OS.File.tmp "opamr_%s" in
+    let* () = Bos.OS.File.write tmpr opamr in
+    let cmd = Bos.Cmd.(v "diff" % "-u" % p tmpl % p tmpr) in
+    Bos.OS.Cmd.(run_out cmd |> out_string)
+  in
+  let diff = match diff with
+    | Ok (s, _) -> s
+    | Error `Msg m -> "error when comparing the opam files: " ^ m
+  in
+  { pkg ; effectively_equal ; diff }
 
 let detailed_opam_diffs left right pkgs =
   OpamPackage.Set.fold (fun p acc ->
@@ -299,9 +291,9 @@ let compare left right =
     let opam_diff_to_json opam_diff =
       `List (List.map (fun (diff : opam_diff) ->
         `Assoc [
-
           ("package_version", `String (OpamPackage.to_string diff.pkg));
-          ("otherwise_equal", `Bool diff.otherwise_equal)
+          ("effectively_equal", `Bool diff.effectively_equal);
+          ("diff", `String diff.diff);
         ]
 
         ) opam_diff)
diff --git a/opamdiff/opamdiff.mli b/opamdiff/opamdiff.mli
index fa1bf3d..54a4bec 100644
--- a/opamdiff/opamdiff.mli
+++ b/opamdiff/opamdiff.mli
@@ -1,9 +1,7 @@
 type opam_diff = {
   pkg : OpamPackage.t ;
-  build : (OpamTypes.command list * OpamTypes.command list) option ;
-  install : (OpamTypes.command list * OpamTypes.command list) option ;
-  url : (OpamFile.URL.t option * OpamFile.URL.t option) option ;
-  otherwise_equal : bool ;
+  effectively_equal : bool ;
+  diff : string ;
 }
 
 type version_diff = {