Replace every use of GNU Patch by the OCaml patch library

Author: kit-ty-kate

.github/workflows/ci.ml

@@ -527,8 +527,8 @@ let main oc : unit =
     ("OPAM12CACHE", "~/.cache/opam1.2/cache");
     (* These should be identical to the values in appveyor.yml *)
     ("OPAM_REPO", "https://github.com/ocaml/opam-repository.git");
-    ("OPAM_TEST_REPO_SHA", "67e940587b8aca227f511e1943bcd31eabe6b1db");
-    ("OPAM_REPO_SHA", "67e940587b8aca227f511e1943bcd31eabe6b1db");
+    ("OPAM_TEST_REPO_SHA", "0c42e982f4cf97fc698132fb2a16b49524a26ab3");
+    ("OPAM_REPO_SHA", "0c42e982f4cf97fc698132fb2a16b49524a26ab3");
     ("SOLVER", "");
     (* Cygwin configuration *)
     ("CYGWIN_MIRROR", "http://mirrors.kernel.org/sourceware/cygwin/");

.github/workflows/main.yml

@@ -26,8 +26,8 @@ env:
   OPAMBSROOT: ~/.cache/.opam.cached
   OPAM12CACHE: ~/.cache/opam1.2/cache
   OPAM_REPO: https://github.com/ocaml/opam-repository.git
-  OPAM_TEST_REPO_SHA: 67e940587b8aca227f511e1943bcd31eabe6b1db
-  OPAM_REPO_SHA: 67e940587b8aca227f511e1943bcd31eabe6b1db
+  OPAM_TEST_REPO_SHA: 0c42e982f4cf97fc698132fb2a16b49524a26ab3
+  OPAM_REPO_SHA: 0c42e982f4cf97fc698132fb2a16b49524a26ab3
   SOLVER:
   CYGWIN_MIRROR: http://mirrors.kernel.org/sourceware/cygwin/
   CYGWIN_ROOT: D:\cygwin

configure

@@ -366,6 +366,7 @@ AC_CHECK_OCAML_PKG([jsonm])
 AC_CHECK_OCAML_PKG([uutf])
 AC_CHECK_OCAML_PKG([sha])
 AC_CHECK_OCAML_PKG([swhid_core])
+AC_CHECK_OCAML_PKG([patch])
 
 # Optional dependencies
 AC_CHECK_OCAML_PKG_AT_LEAST([mccs],[1.1+17])
@@ -414,6 +415,7 @@ AS_IF([test "x${enable_checks}" != "xno" && {
        test "x$OCAML_PKG_uutf" = "xno" ||
        test "x$OCAML_PKG_sha" = "xno" ||
        test "x$OCAML_PKG_swhid_core" = "xno" ||
+       test "x$OCAML_PKG_patch" = "xno" ||
        test "x$OCAML_PKG_mccs$MCCS_ENABLED" = "xnotrue";}],[
   AS_IF([test "x${with_vendored_deps}" != "xyes"],[
     AC_MSG_ERROR([Dependencies missing. Use --with-vendored-deps or --disable-checks])

configure.ac

@@ -29,6 +29,9 @@ users)
 ## Install
 
 ## Build (package)
+  * Patches are now applied using the `patch` OCaml library instead of GNU Patch [#5892 @kit-ty-kate - fix #6019]
+  * ✘ Patches: Context diffs are not supported anymore, only Unified diffs are (including its git extensions) [#5892 @kit-ty-kate]
+  * ✘ Patches: Stop support of file permission changes via git extension to the unified diff specification [#5892 @kit-ty-kate - fix #3782]
 
 ## Remove
 
@@ -64,6 +67,7 @@ users)
   * [BUG] Do not show the not-up-to-date message with packages tagged with avoid-version [#6273 @kit-ty-kate - fix #6271]
   * [BUG] Fix a regression on `opam upgrade <package>` upgrading unrelated packages [#6373 @AltGr]
   * [BUG] Fix a regression on `opam upgrade --all <uninstalled-pkg>` not upgrading the whole switch [#6373 @kit-ty-kate]
+  * Updates are now applied using the `patch` OCaml library instead of the system GNU Patch and diff utilities [#5892 @kit-ty-kate - fix ocaml/setup-ocaml#933]
 
 ## Tree
 
@@ -82,6 +86,7 @@ users)
   * Update SWH API request [#6036 @rjbou]
   * Rework SWH fallback to have a more correct archive retrieval and more fine grained error handling [#6036 @rjbou - fix #5721]
   * Check that the repositories given to `opam repository remove` actually exist [#5014 @kit-ty-kate - fixes #5012]
+  * ✘ Symlinks in repositories are no longer supported [#5892 @kit-ty-kate]
 
 ## Lock
   * [BUG] Fix `pin-depends` for `with-*` dependencies [#5471 @rjbou - fix #5428]
@@ -99,6 +104,7 @@ users)
   * Lookup at `gpatch` before `patch` on macOS now that both homebrew and macports expose `gpatch` as `gpatch` since Homebrew/homebrew-core#174687 [#6255 @kit-ty-kate]
   * Relax lookup on OpenBSD to consider all installed packages [#6362 @semarie]
   * Speedup the detection of available system packages with pacman and brew [#6324 @kit-ty-kate]
+  * The system GNU Patch and diff are no longer runtime dependencies of opam [#5892 @kit-ty-kate]
 
 ## Format upgrade
 

master_changes.md

@@ -30,6 +30,7 @@ depends: [
   "sha" {>= "1.13"}
   "jsonm"
   "swhid_core"
+  "patch" {>= "3.0.0~alpha1"}
   "uutf"
   (("host-system-mingw" {os = "win32" & os-distribution != "cygwinports"} &
     "conf-mingw-w64-gcc-i686" {os = "win32" & os-distribution != "cygwinports"} &

opam-core.opam

@@ -30,5 +30,6 @@ build: [
 depends: [
   "ocaml" {>= "4.08.0"}
   "opam-format" {= version}
+  "patch" {>= "3.0.0~alpha1"}
   "dune" {>= "2.8.0"}
 ]

opam-repository.opam

@@ -368,7 +368,7 @@ let prepare_package_build env opam nv dir =
   let apply_patches ?(dryrun=false) () =
     let patch base =
       if dryrun then Done None else
-        OpamFilename.patch
+        OpamFilename.patch ~allow_unclean:true
           (dir // OpamFilename.Base.to_string base) dir
     in
     let rec aux = function

src/client/opamAction.ml

@@ -52,11 +52,6 @@ let not_win32_filter =
   FOp (FIdent ([], OpamVariable.of_string "os", None), `Neq, FString "win32")
 let sandbox_filter = FOr (linux_filter, macos_filter)
 
-let gpatch_filter =
-  FOr (FOr (openbsd_filter, netbsd_filter),
-       FOr (freebsd_filter, FOr (dragonflybsd_filter, macos_filter)))
-let patch_filter = FNot gpatch_filter
-
 let gtar_filter = openbsd_filter
 let tar_filter = FNot gtar_filter
 
@@ -131,9 +126,6 @@ let recommended_tools () =
 let required_tools ~sandboxing () =
   req_dl_tools () @
   [
-    ["diff"], None, None;
-    ["patch"], None, Some patch_filter;
-    ["gpatch"], None, Some gpatch_filter;
     ["tar"], None, Some tar_filter;
     ["gtar"], None, Some gtar_filter;
     ["unzip"], None, None;
@@ -146,9 +138,7 @@ let required_tools ~sandboxing () =
 
 let required_packages_for_cygwin =
   [
-    "diffutils";
     "make";
-    "patch";
     "tar";
     "unzip";
     "rsync";

src/client/opamInitDefaults.ml

@@ -3,7 +3,7 @@
   (public_name opam-core)
   (synopsis    "OCaml Package Manager core internal stdlib")
   ; TODO: Remove (re_export ...) when CI uses the OCaml version that includes https://github.com/ocaml/ocaml/pull/11989
-  (libraries   re (re_export ocamlgraph) unix sha jsonm swhid_core uutf)
+  (libraries   re (re_export ocamlgraph) unix sha jsonm swhid_core uutf patch)
   (flags       (:standard
                (:include ../ocaml-flags-standard.sexp)
                (:include ../ocaml-flags-configure.sexp)

src/core/dune

@@ -445,8 +445,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 ?preprocess ~allow_unclean filename dirname =
+  OpamSystem.patch ?preprocess ~allow_unclean ~dir:(Dir.to_string dirname) (to_string filename)
 
 let flock flag ?dontblock file = OpamSystem.flock flag ?dontblock (to_string file)
 

src/core/opamFilename.ml

@@ -272,7 +272,7 @@ 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
+val patch: ?preprocess:bool -> allow_unclean:bool -> t -> Dir.t -> exn option OpamProcess.job
 
 (** Create an empty file *)
 val touch: t -> unit

src/core/opamFilename.mli

@@ -1588,41 +1588,67 @@ let translate_patch ~dir orig corrected =
   end;
   close_in ch
 
-let gpatch = lazy begin
-  let rec search_gpatch = function
-    | [] -> None
-    | patch_cmd::patch_cmds ->
-      match OpamProcess.run (make_command ~name:"patch" patch_cmd ["--version"]) with
-      | r ->
-        (match OpamProcess.is_success r, r.OpamProcess.r_stdout with
-         | true, full::_ when
-             OpamStd.String.is_prefix_of ~from:0 ~full "GNU patch " ->
-           Some patch_cmd
-         | _ ->
-           search_gpatch patch_cmds)
-      | exception _ -> search_gpatch patch_cmds
+exception Internal_patch_error of string
+
+let internal_patch ~allow_unclean ~patch_filename ~dir diffs =
+  let fmt = Printf.sprintf in
+  (* NOTE: It is important to keep this `concat dir ""` to ensure the
+     is_prefix_of below doesn't match another similarily named directory *)
+  let dir = Filename.concat (real_path dir) "" in
+  let get_path file =
+    let file = real_path (Filename.concat dir file) in
+    if not (OpamStd.String.is_prefix_of ~from:0 ~full:file dir) then
+      raise (Internal_patch_error (fmt "Patch %S tried to escape its scope."
+                                     patch_filename));
+    file
   in
-  let default_cmd, other_cmds =
-    match OpamStd.Sys.os () with
-    | Darwin
-    | DragonFly
-    | FreeBSD
-    | NetBSD
-    | OpenBSD -> ("gpatch", ["patch"])
-    | Cygwin
-    | Linux
-    | Unix
-    | Win32
-    | Other _ -> ("patch", ["gpatch"])
+  let patch ~file content diff =
+    match Patch.patch ~cleanly:true content diff with
+    | Some x -> x
+    | None -> assert false
+    | exception _ when not allow_unclean ->
+      raise (Internal_patch_error (fmt "Patch %S does not apply cleanly."
+                                     patch_filename))
+    | exception _ ->
+      match Patch.patch ~cleanly:false content diff with
+      | Some x ->
+        OpamStd.Option.iter (write (file^".orig")) content;
+        x
+      | None -> assert false
+      | exception _ ->
+        OpamStd.Option.iter (write (file^".orig")) content;
+        write (file^".rej") (Format.asprintf "%a" Patch.pp diff);
+        raise (Internal_patch_error (fmt "Patch %S does not apply cleanly."
+                                       patch_filename))
   in
-  match search_gpatch (default_cmd :: other_cmds) with
-  | Some gpatch -> gpatch
-  | None ->
-    OpamConsole.warning "Invalid patch utility. Please install GNU patch";
-    default_cmd
-end
+  let apply diff = match diff.Patch.operation with
+    | Patch.Edit (file1, file2) ->
+      (* That seems to be the GNU patch behaviour *)
+      let file =
+        let file1 = get_path file1 in
+        if Sys.file_exists file1 then
+          file1
+        else
+          get_path file2
+      in
+      let content = read file in
+      let content = patch ~file:file (Some content) diff in
+      write file content;
+    | Patch.Delete file ->
+      let file = get_path file in
+      Unix.unlink file
+    | Patch.Create file ->
+      let file = get_path file in
+      let content = patch ~file None diff in
+      write file content
+    | Patch.Rename_only (src, dst) ->
+      let src = get_path src in
+      let dst = get_path dst in
+      Unix.rename src dst
+  in
+  List.iter apply diffs
 
-let patch ?(preprocess=true) ~dir p =
+let patch ?(preprocess=true) ~allow_unclean ~dir p =
   if not (Sys.file_exists p) then
     (OpamConsole.error "Patch file %S not found." p;
      raise Not_found);
@@ -1634,11 +1660,12 @@ let patch ?(preprocess=true) ~dir p =
     else
       p
   in
-  let patch_cmd = Lazy.force gpatch 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))
+  let content = read p' in
+  try
+    let diffs = Patch.parse ~p:1 content in
+    internal_patch ~allow_unclean ~patch_filename:p ~dir diffs;
+    Done None
+  with exn -> Done (Some exn)
 
 let register_printer () =
   Printexc.register_printer (function

src/core/opamSystem.ml

@@ -125,6 +125,9 @@ val read: string -> string
     advisory write lock to prevent concurrent reads or writes) *)
 val write: string -> string -> unit
 
+(** [get_files dir] returns the list of files inside the directory [dir]. *)
+val get_files : string -> string list
+
 (** [remove filename] removes [filename]. Works whether [filename] is
     a file or a directory *)
 val remove: string -> unit
@@ -328,7 +331,7 @@ val get_lock_fd: lock -> Unix.file_descr
 (** 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
+val patch: ?preprocess:bool -> allow_unclean: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

src/core/opamSystem.mli

@@ -3,7 +3,7 @@
   (public_name opam-repository)
   (synopsis    "OCaml Package Manager remote repository handling library")
   ; TODO: Remove (re_export ...) when CI uses the OCaml version that includes https://github.com/ocaml/ocaml/pull/11989
-  (libraries   (re_export opam-format))
+  (libraries   (re_export opam-format) patch)
   (flags       (:standard
                (:include ../ocaml-flags-standard.sexp)
                (:include ../ocaml-flags-configure.sexp)

src/repository/dune

@@ -538,7 +538,7 @@ let apply_repo_update repo repo_root = function
       | `http | `rsync -> false
       | _ -> true
     in
-    (OpamFilename.patch ~preprocess f repo_root @@+ function
+    (OpamFilename.patch ~preprocess ~allow_unclean:false f repo_root @@+ function
       | Some e ->
         if not (OpamConsole.debug ()) then OpamFilename.remove f;
         raise e