Use git diff instead of the system diff command

src/client/opamInitDefaults.ml

@@ -133,7 +133,7 @@ let recommended_tools () =
 let required_tools ~sandboxing () =
   req_dl_tools () @
   [
-    ["diff"], None, None;
+    ["git"], None, None;
     ["patch"], None, Some patch_filter;
     ["gpatch"], None, Some gpatch_filter;
     ["tar"], None, Some tar_filter;

src/core/opamFilename.ml

@@ -429,8 +429,8 @@ let link ?(relative=false) ~target ~link =
   OpamSystem.link target (to_string link)
 [@@ocaml.warning "-16"]
 
-let patch ?preprocess filename dirname =
-  OpamSystem.patch ?preprocess ~dir:(Dir.to_string dirname) (to_string filename)
+let patch ?delete_stragglers filename dirname =
+  OpamSystem.patch ?delete_stragglers ~dir:(Dir.to_string dirname) (to_string filename)
 
 let flock flag ?dontblock file = OpamSystem.flock flag ?dontblock (to_string file)
 

src/core/opamFilename.mli

@@ -258,9 +258,11 @@ val remove_prefix_dir: Dir.t -> Dir.t -> string
 (** Remove a suffix from a filename *)
 val remove_suffix: Base.t -> t -> string
 
-(** Apply a patch in a directory. If [preprocess] is set to false, there is no
-    CRLF translation. Returns [None] on success, the process error otherwise *)
-val patch: ?preprocess:bool -> t -> Dir.t -> exn option OpamProcess.job
+(** Apply a patch in a directory. The given patch file will be CRLF translated
+    and if [delete_stragglers] is true the undeleted files will be deleted
+    (e.g. when macOS patch is used).
+    Returns [None] on success, the process error otherwise *)
+val patch: ?delete_stragglers:bool -> t -> Dir.t -> exn option OpamProcess.job
 
 (** Create an empty file *)
 val touch: t -> unit

src/core/opamSystem.ml

@@ -1330,6 +1330,7 @@ let translate_patch ~dir orig corrected =
      encoded and also the status of individual files, so accept scanning the
      file three times instead of two. *)
   let log ?level fmt = OpamConsole.log "PATCH" ?level fmt in
+  let to_delete = ref [] in
   let strip_cr = get_eol_encoding orig = Some true in
   let ch =
     try open_in_bin orig
@@ -1433,18 +1434,19 @@ let translate_patch ~dir orig corrected =
             line
         in
         let length = String.length line in
+        let get_file () =
+          let file = String.sub line 4 (length - 4) in
+          let open OpamStd in
+          Option.map_default fst file (String.cut_at file '\t')
+        in
         let next_state =
           match state with
           | `Header ->
             begin
               match (if length > 4 then String.sub line 0 4 else "") with
               | "--- " ->
                   (* Start of a unified diff header. *)
-                  let file =
-                    let file = String.sub line 4 (length - 4) in
-                    let open OpamStd in
-                    Option.map_default fst file (String.cut_at file '\t')
-                  in
+                  let file = get_file () in
                   (* Weakness: new files are also marked with a time-stamp at
                                the start of the epoch, however it's localised,
                                making it a bit tricky to identify! New files are
@@ -1504,11 +1506,9 @@ let translate_patch ~dir orig corrected =
                 `NewChunk (neg, pos)
           | `Patching (orig, crlf) ->
               if (if length > 4 then String.sub line 0 4 else "") = "+++ " then
-                let file =
-                  let file = String.sub line 4 (length - 4) in
-                  let open OpamStd in
-                  Option.map_default fst file (String.cut_at file '\t')
-                in
+                let file = get_file () in
+                if file = "/dev/null" then
+                  to_delete := orig :: !to_delete;
                 `Processing (orig, file, crlf, None, [], `Head)
               else
                 `Header
@@ -1631,20 +1631,53 @@ let translate_patch ~dir orig corrected =
     in
     fold_lines 1 transforms
   end;
-  close_in ch
+  close_in ch;
+  !to_delete
+
+let is_empty_directory dirname =
+  let dir = Unix.opendir dirname in
+  Fun.protect ~finally:(fun () -> Unix.closedir dir) @@ fun () ->
+  let rec aux () =
+    match Unix.readdir dir with
+    | "." | ".." -> aux ()
+    | _file -> false
+    | exception End_of_file -> true
+  in
+  aux ()
+
+let remove_if_empty ~dir file =
+  let get_path file =
+    (* NOTE: Important to keep this `concat dir ""` to ensure the
+       is_prefix_of doesn't match another directory *)
+    let dir = Filename.concat (real_path dir) "" in
+    let file = real_path (Filename.concat dir file) in
+    if not (OpamStd.String.is_prefix_of ~from:0 ~full:file dir) then
+      OpamConsole.error_and_exit `Internal_error "Patch %S tried to escape its scope." file;
+    file
+  in
+  let file = get_path file in
+  let ic = Stdlib.open_in_bin file in
+  Fun.protect ~finally:(fun () -> Stdlib.close_in ic) @@ fun () ->
+  match Stdlib.input_char ic with
+  | _ ->
+    OpamConsole.warning "Patch %S was set for deletion but is not empty." file
+  | exception End_of_file ->
+    if Sys.file_exists file then begin
+      log "Deleting straggler patched file %S" file;
+      Unix.unlink file;
+      let dirname = Filename.dirname file in
+      if is_empty_directory dirname then begin
+        log "Deleting empty patched directory %S" dirname;
+        Unix.rmdir dirname;
+      end
+    end
 
-let patch ?(preprocess=true) ~dir p =
+let patch ?(delete_stragglers=false) ~dir p =
   if not (Sys.file_exists p) then
     (OpamConsole.error "Patch file %S not found." p;
      raise Not_found);
-  let p' =
-    if preprocess then
-      let p' = temp_file ~auto_clean:false "processed-patch" in
-      translate_patch ~dir p p';
-      p'
-    else
-      p
-  in
+  let p' = temp_file ~auto_clean:false "processed-patch" in
+  let files_to_delete = translate_patch ~dir p p' in
   let patch_cmd =
     match OpamStd.Sys.os () with
     | OpamStd.Sys.OpenBSD
@@ -1653,8 +1686,14 @@ let patch ?(preprocess=true) ~dir p =
   in
   make_command ~name:"patch" ~dir patch_cmd ["-p1"; "-i"; p'] @@> fun r ->
     if not (OpamConsole.debug ()) then Sys.remove p';
-    if OpamProcess.is_success r then Done None
-    else Done (Some (Process_error r))
+    if OpamProcess.is_success r then begin
+      if delete_stragglers then begin
+        (* NOTE: Non-GNU patches do not support file deletion so we need to
+           do it manually afterwards *)
+        List.iter (remove_if_empty ~dir) files_to_delete;
+      end;
+      Done None
+    end else Done (Some (Process_error r))
 
 let register_printer () =
   Printexc.register_printer (function

src/core/opamSystem.mli

@@ -304,10 +304,11 @@ val get_lock_fd: lock -> Unix.file_descr
 
 (** {2 Misc} *)
 
-(** Apply a patch file in the current directory. If [preprocess] is set to
-    false, there is no CRLF translation. Returns the error if the patch didn't
-    apply. *)
-val patch: ?preprocess:bool -> dir:string -> string -> exn option OpamProcess.job
+(** Apply a patch in a directory. The given patch file will be CRLF translated
+    and if [delete_stragglers] is true the undeleted files will be deleted
+    (e.g. when macOS patch is used).
+    Returns [None] on success, the process error otherwise *)
+val patch: ?delete_stragglers:bool -> dir:string -> string -> exn option OpamProcess.job
 
 (** Returns the end-of-line encoding style for the given file. [None] means that
     either the encoding of line endings is mixed, or the file contains no line
@@ -321,8 +322,9 @@ val get_eol_encoding : string -> bool option
     endings then the patch is transformed to patch using the encoding on disk.
     In particular, this means that patches generated against Unix checkouts of
     Git sources will correctly apply to Windows checkouts of the same sources.
+    It return a list of files to be deleted after the patch has been applied.
 *)
-val translate_patch: dir:string -> string -> string -> unit
+val translate_patch: dir:string -> string -> string -> string list
 
 (** Create a temporary file in {i ~/.opam/logs/<name>XXX}, if [dir] is not set.
     ?auto_clean controls whether the file is automatically deleted when opam

src/repository/opamRepository.ml

@@ -501,12 +501,7 @@ let apply_repo_update repo repo_root = function
     log "%a: applying patch update at %a"
       (slog OpamRepositoryName.to_string) repo.repo_name
       (slog OpamFilename.to_string) f;
-    let preprocess =
-      match repo.repo_url.OpamUrl.backend with
-      | `http | `rsync -> false
-      | _ -> true
-    in
-    (OpamFilename.patch ~preprocess f repo_root @@+ function
+    (OpamFilename.patch ~delete_stragglers:true f repo_root @@+ function
       | Some e ->
         if not (OpamConsole.debug ()) then OpamFilename.remove f;
         raise e

src/repository/opamRepositoryBackend.ml

@@ -84,8 +84,8 @@ let get_diff parent_dir dir1 dir2 =
   OpamSystem.make_command
     ~verbose:OpamCoreConfig.(!r.verbose_level >= 2)
     ~dir:(OpamFilename.Dir.to_string parent_dir) ~stdout:patch
-    "diff"
-    [ "-ruaN";
+    "git"
+    [ "diff"; "--no-index"; "-a"; "--";
       OpamFilename.Base.to_string dir1;
       OpamFilename.Base.to_string dir2; ]
   @@> function