pkg: copy files from opam repository to lock dir (#8648)

Signed-off-by: Ali Caglayan <[email protected]>

bin/pkg.ml

@@ -391,14 +391,16 @@ module Lock = struct
                ~local_packages:opam_file_map
            with
            | Error (`Diagnostic_message message) -> Error (context_name, message)
-           | Ok (summary, lock_dir) ->
+           | Ok (summary, lock_dir, files) ->
              let summary_message =
                Dune_pkg.Opam_solver.Summary.selected_packages_message
                  summary
                  ~lock_dir_path
                |> User_message.pp
              in
-             Ok (Lock_dir.Write_disk.prepare ~lock_dir_path lock_dir, summary_message))
+             Ok
+               ( Lock_dir.Write_disk.prepare ~lock_dir_path ~files lock_dir
+               , summary_message ))
        |> Result.List.all)
       >>| function
       | Error (context_name, message) ->

src/dune_pkg/lock_dir.ml

@@ -393,7 +393,14 @@ module Write_disk = struct
 
   type t = unit -> unit
 
-  let prepare ~lock_dir_path lock_dir =
+  module Files_entry = struct
+    type t =
+      { original_file : Path.t
+      ; local_file : Path.Local.t
+      }
+  end
+
+  let prepare ~lock_dir_path ~files lock_dir =
     let lock_dir_path = Path.source lock_dir_path in
     let remove_dir_if_exists = safely_remove_lock_dir_if_exists_thunk lock_dir_path in
     fun () ->
@@ -408,7 +415,19 @@ module Write_disk = struct
             Dune_lang.Ast.add_loc ~loc:Loc.none sexp |> Dune_sexp.Cst.concrete)
         in
         let pp = Dune_lang.Format.pp_top_sexps ~version:(3, 11) cst in
-        Format.asprintf "%a" Pp.to_fmt pp |> Io.write_file path)
+        Format.asprintf "%a" Pp.to_fmt pp |> Io.write_file path;
+        Package_name.Map.iteri files ~f:(fun package_name files ->
+          let files_dir =
+            Path.relative lock_dir_path (Package_name.to_string package_name ^ ".files")
+          in
+          Path.mkdir_p files_dir;
+          List.iter files ~f:(fun { Files_entry.original_file; local_file } ->
+            let dst = Path.append_local files_dir local_file in
+            Path.mkdir_p (Path.parent_exn dst);
+            Io.copy_file
+              ~src:original_file
+              ~dst:(Path.append_local files_dir local_file)
+              ())))
   ;;
 
   let commit t = t ()

src/dune_pkg/lock_dir.mli

@@ -71,7 +71,19 @@ module Write_disk : sig
   type lock_dir := t
   type t
 
-  val prepare : lock_dir_path:Path.Source.t -> lock_dir -> t
+  module Files_entry : sig
+    type t =
+      { original_file : Path.t
+      ; local_file : Path.Local.t
+      }
+  end
+
+  val prepare
+    :  lock_dir_path:Path.Source.t
+    -> files:Files_entry.t Package_name.Map.Multi.t
+    -> lock_dir
+    -> t
+
   val commit : t -> unit
 end
 

src/dune_pkg/opam_repo.ml

@@ -90,6 +90,12 @@ let get_opam_file_path t opam_package =
   / "opam"
 ;;
 
+let get_opam_package_files_path t opam_package =
+  get_opam_package_version_dir_path t (OpamPackage.name opam_package)
+  / OpamPackage.to_string opam_package
+  / "files"
+;;
+
 (* Returns a list containing all versions of a package with a given name *)
 let all_package_versions t opam_package_name =
   let version_dir_path = get_opam_package_version_dir_path t opam_package_name in

src/dune_pkg/opam_repo.mli

@@ -14,3 +14,5 @@ val load_all_versions
   :  t
   -> OpamPackage.Name.t
   -> (OpamFile.OPAM.t list, [ `Package_not_found ]) result
+
+val get_opam_package_files_path : t -> OpamPackage.t -> Path.t

src/dune_pkg/opam_solver.ml

@@ -455,6 +455,35 @@ let solve_package_list local_packages context =
   | Ok packages -> Ok (Solver.packages_of_result packages)
 ;;
 
+(* Scan a path recursively down retrieving a list of all files together with their
+   relative path. *)
+let scan_files_entries ~repo_id path =
+  (* TODO Add some cycle detection *)
+  let rec read acc dir =
+    let path = Path.append_local path dir in
+    match Path.readdir_unsorted_with_kinds path with
+    | Ok entries ->
+      List.fold_left entries ~init:acc ~f:(fun acc (filename, kind) ->
+        let local_path = Path.Local.relative dir filename in
+        match (kind : Unix.file_kind) with
+        | S_REG -> local_path :: acc
+        | S_DIR -> read acc local_path
+        | _ ->
+          (* TODO should be an error *)
+          acc)
+    | Error (Unix.ENOENT, _, _) -> acc
+    | Error err ->
+      User_error.raise
+        ?loc:(Option.map ~f:fst repo_id)
+        [ Pp.text "Unable to read file in opam repository:"; Unix_error.Detailed.pp err ]
+  in
+  read [] Path.Local.root
+  |> List.map ~f:(fun local_file ->
+    { Lock_dir.Write_disk.Files_entry.original_file = Path.append_local path local_file
+    ; local_file
+    })
+;;
+
 let solve_lock_dir solver_env version_preference (repo, repo_id) ~local_packages =
   let is_local_package package =
     OpamPackage.Name.Map.mem (OpamPackage.name package) local_packages
@@ -482,5 +511,14 @@ let solve_lock_dir solver_env version_preference (repo, repo_id) ~local_packages
       | Ok pkgs_by_name ->
         Lock_dir.create_latest_version pkgs_by_name ~ocaml:None ~repo_id
     in
-    summary, lock_dir)
+    let files =
+      Package_name.Map.of_list_map_exn opam_packages_to_lock ~f:(fun opam_package ->
+        let files_path = Opam_repo.get_opam_package_files_path repo opam_package in
+        ( Package_name.of_string
+            (OpamPackage.Name.to_string (OpamPackage.name opam_package))
+        , scan_files_entries ~repo_id files_path ))
+      |> Package_name.Map.filter_map ~f:(fun files ->
+        if List.is_empty files then None else Some files)
+    in
+    summary, lock_dir, files)
 ;;

src/dune_pkg/opam_solver.mli

@@ -1,4 +1,4 @@
-open! Stdune
+open Import
 
 module Summary : sig
   (** Some intermediate state from the solve exposed for logging purposes *)
@@ -13,4 +13,6 @@ val solve_lock_dir
   -> Version_preference.t
   -> Opam_repo.t * (Loc.t * Repository_id.t) option
   -> local_packages:OpamFile.OPAM.t OpamTypes.name_map
-  -> (Summary.t * Lock_dir.t, [ `Diagnostic_message of _ Pp.t ]) result
+  -> ( Summary.t * Lock_dir.t * Lock_dir.Write_disk.Files_entry.t Package_name.Map.Multi.t
+     , [ `Diagnostic_message of _ Pp.t ] )
+     result

test/blackbox-tests/test-cases/pkg/opam-package-copy-files.t

@@ -1,7 +1,6 @@
 This test checks that the files in the files/ directory inside a package in an opam
 repository are copied correctly to the dune.lock file.
 
-
   $ . ./helpers.sh
 
 Generate a mock opam repository
@@ -43,6 +42,6 @@ lock file.
 
   $ lock_dir="dune.lock/with-patch.files"
   $ [ -d $lock_dir ] && cat $lock_dir/$fname1
-  [1]
+  foo
   $ [ -d $lock_dir ] && cat $lock_dir/$fname2
-  [1]
+  bar

test/blackbox-tests/test-cases/pkg/opam-package-files-unix-error.t

@@ -0,0 +1,49 @@
+This test demonstrates the behaviour when a Unix error is encountered when copying the
+files/ directory from a package directory inside an opam repostory.
+
+  $ . ./helpers.sh
+
+Generate a mock opam repository
+  $ mkdir -p mock-opam-repository
+  $ cat >mock-opam-repository/repo <<EOF
+  > opam-version: "2.0"
+  > EOF
+
+
+Make a package with a patch
+  $ mkpkg with-patch <<EOF
+  > opam-version: "2.0"
+  > EOF
+
+  $ fname1="foo.patch"
+  $ fname2="dir/bar.patch"
+  $ opam_repo="mock-opam-repository/packages/with-patch/with-patch.0.0.1"
+  $ mkdir -p $opam_repo/files/dir
+  $ cat >$opam_repo/files/$fname1 <<EOF
+  > foo
+  > EOF
+  $ cat >$opam_repo/files/$fname2 <<EOF
+  > bar
+  > EOF
+We remove the read permissions for dir/
+
+  $ chmod -r $opam_repo/files/dir
+
+The error message should have a location for the opam repository.
+
+This does not currently seem to be the case.
+
+  $ solve_project <<EOF
+  > (lang dune 3.8)
+  > (package
+  >  (name x)
+  >  (allow_empty)
+  >  (depends with-patch))
+  > EOF
+  Error: Unable to read file in opam repository:
+  opendir($TESTCASE_ROOT/mock-opam-repository/packages/with-patch/with-patch.0.0.1/files/dir): Permission denied
+  [1]
+
+Make sure to set permissions back so the sandbox can be cleaned up.
+
+  $ chmod +r $opam_repo/files/dir

test/expect-tests/dune_pkg/dune_pkg_unit_tests.ml

@@ -170,7 +170,8 @@ let%expect_test "downloading, without any checksum" =
 
 let lock_dir_encode_decode_round_trip_test ~lock_dir_path ~lock_dir =
   let lock_dir_path = Path.Source.of_string lock_dir_path in
-  Lock_dir.Write_disk.(prepare ~lock_dir_path lock_dir |> commit);
+  Lock_dir.Write_disk.(
+    prepare ~lock_dir_path ~files:Package_name.Map.empty lock_dir |> commit);
   let lock_dir_round_tripped = Lock_dir.read_disk lock_dir_path in
   if Lock_dir.equal
        (Lock_dir.remove_locs lock_dir_round_tripped)