Write commit hashes into the lock dir when locking git repositories

src/dune_pkg/opam_solver.ml

@@ -1661,6 +1661,60 @@ let package_kind =
     else `Non_compiler
 ;;
 
+let resolve_url (file_url : OpamFile.URL.t) =
+  let url = OpamFile.URL.url file_url in
+  let+ url : OpamUrl.t =
+    match url.backend with
+    | `git ->
+      let loc = Loc.none in
+      let+ mount = Mount.of_opam_url loc url in
+      (match Mount.backend mount with
+       | Path path ->
+         Code_error.raise
+           "Attempted to resolve git URL but loading it resolved to a local path"
+           [ "url", OpamUrl.to_dyn url; "path", Path.to_dyn path ]
+       | Git at_rev ->
+         let resolved_hash = at_rev |> Rev_store.At_rev.rev |> Rev_store.Object.to_hex in
+         (match !Dune_engine.Clflags.display, url.hash with
+          | Short, Some old_hash ->
+            (match String.equal resolved_hash old_hash with
+             | true -> ()
+             | false ->
+               User_warning.emit
+                 [ Pp.textf
+                     "Locking repository URL %s to commit %s"
+                     (OpamUrl.to_string url)
+                     resolved_hash
+                 ])
+          | _ -> ());
+         { url with hash = Some resolved_hash })
+    | _ -> Fiber.return url
+  in
+  OpamFile.URL.with_url url file_url
+;;
+
+let resolve_opam_packages opam_packages_to_lock ~resolve_package =
+  opam_packages_to_lock
+  |> List.map ~f:(fun opam_package ->
+    let name = OpamPackage.name opam_package |> Package_name.of_opam_package_name in
+    let version = OpamPackage.version opam_package in
+    let resolved_package = resolve_package name version in
+    (* resolve URLs *)
+    let+ resolved_package =
+      let opam_file = Resolved_package.opam_file resolved_package in
+      let+ opam_file =
+        match OpamFile.OPAM.url opam_file with
+        | None -> Fiber.return opam_file
+        | Some url ->
+          let+ url = resolve_url url in
+          OpamFile.OPAM.with_url url opam_file
+      in
+      Resolved_package.with_opam_file opam_file resolved_package
+    in
+    name, opam_package, resolved_package)
+  |> Fiber.all
+;;
+
 let solve_lock_dir
       solver_env
       version_preference
@@ -1738,8 +1792,7 @@ let solve_lock_dir
          (Table.find_exn candidates_cache name).resolved
          |> OpamPackage.Version.Map.find version
        in
-       let pkgs_by_name =
-         let open Result.O in
+       let* pkgs_by_name =
          let+ pkgs =
            let version_by_package_name =
              Package_name.Map.of_list_map_exn
@@ -1748,13 +1801,10 @@ let solve_lock_dir
                  ( Package_name.of_opam_package_name (OpamPackage.name package)
                  , Package_version.of_opam_package_version (OpamPackage.version package) ))
            in
-           List.map opam_packages_to_lock ~f:(fun opam_package ->
-             let name =
-               OpamPackage.name opam_package |> Package_name.of_opam_package_name
-             in
-             let resolved_package =
-               resolve_package name (OpamPackage.version opam_package)
-             in
+           let+ resolved_pkgs =
+             resolve_opam_packages opam_packages_to_lock ~resolve_package
+           in
+           List.map resolved_pkgs ~f:(fun (name, opam_package, resolved_package) ->
              Lock_pkg.opam_package_to_lock_file_pkg
                solver_env
                stats_updater
@@ -1765,22 +1815,23 @@ let solve_lock_dir
                ~portable_lock_dir)
            |> Result.List.all
          in
-         match Package_name.Map.of_list_map pkgs ~f:(fun pkg -> pkg.info.name, pkg) with
-         | Error (name, _pkg1, _pkg2) ->
-           Code_error.raise
-             "Solver selected multiple versions for the same package"
-             [ "name", Package_name.to_dyn name ]
-         | Ok pkgs_by_name ->
-           let reachable =
-             reject_unreachable_packages
-               solver_env
-               ~dune_version:
-                 (Package_version.of_opam_package_version context.dune_version)
-               ~local_packages
-               ~pkgs_by_name
-           in
-           Package_name.Map.filteri pkgs_by_name ~f:(fun name _ ->
-             Package_name.Set.mem reachable name)
+         Result.map pkgs ~f:(fun pkgs ->
+           match Package_name.Map.of_list_map pkgs ~f:(fun pkg -> pkg.info.name, pkg) with
+           | Error (name, _pkg1, _pkg2) ->
+             Code_error.raise
+               "Solver selected multiple versions for the same package"
+               [ "name", Package_name.to_dyn name ]
+           | Ok pkgs_by_name ->
+             let reachable =
+               reject_unreachable_packages
+                 solver_env
+                 ~dune_version:
+                   (Package_version.of_opam_package_version context.dune_version)
+                 ~local_packages
+                 ~pkgs_by_name
+             in
+             Package_name.Map.filteri pkgs_by_name ~f:(fun name _ ->
+               Package_name.Set.mem reachable name))
        in
        let ocaml =
          let open Result.O in

src/dune_pkg/resolved_package.ml

@@ -38,6 +38,11 @@ let opam_file = function
   | Rest t -> t.opam_file
 ;;
 
+let with_opam_file opam_file = function
+  | Dune -> Dune
+  | Rest r -> Rest { r with opam_file }
+;;
+
 let extra_files = function
   | Dune -> None
   | Rest t -> Some t.extra_files

src/dune_pkg/resolved_package.mli

@@ -5,6 +5,7 @@ type t
 val package : t -> OpamPackage.t
 val opam_file : t -> OpamFile.OPAM.t
 val loc : t -> Loc.t
+val with_opam_file : OpamFile.OPAM.t -> t -> t
 
 (** Determines whether the package is to be built using Dune or not *)
 val dune_build : t -> bool

test/blackbox-tests/test-cases/pkg/dune

@@ -44,6 +44,10 @@
  (deps %{bin:awk} %{bin:cmp})
  (applies_to git-repo))
 
+(cram
+ (deps fakegit.sh)
+ (applies_to opam-source-conversion))
+
 (cram
  (deps %{bin:make})
  (applies_to make))

test/blackbox-tests/test-cases/pkg/fakegit.sh

@@ -0,0 +1,55 @@
+#!/bin/bash
+
+# dummy implementation of just enough git to be able to create a lock file
+
+# we need access to the real git binary, so we expect the env to have a
+# REAL_GIT variable pointing to the actual git
+REAL_GIT=${REAL_GIT:-false}
+
+case $1 in
+  init)
+    # we pass this over to actual git
+    $REAL_GIT "$@"
+    exit $?
+    ;;
+  ls-remote)
+    # hardcoded output, just HEAD pointing to a revision
+    echo "058003b274f05b092a391555ffdee8b36f1897b3        HEAD"
+    ;;
+  fetch)
+    # we don't fetch from anywhere
+    exit 0
+    ;;
+  ls-tree)
+    # the revision that HEAD is pointing to, just a single empty file
+    echo "100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391       0	empty"
+    exit 0
+    ;;
+  cat-file)
+    IFS=':' read -r -a rev_and_path <<< "$3"
+    if [[ -z ${rev_and_path[1]} ]]; then
+      echo ""
+      exit 0
+    fi
+    if [ "${rev_and_path[1]}" = ".gitmodules" ]; then
+      echo "does not exist" >&2
+      exit 128
+    fi
+    echo "Unsupported cat-file command: $@" >&2
+    exit 2
+    ;;
+  rev-parse)
+    # let git answer this one
+    if [ "$2" = "--is-bare-repository" ]; then
+      $REAL_GIT "$@"
+      exit $?
+    fi
+    echo "Unsupported rev-parse command: $@" >&2
+    exit 2
+    ;;
+  *)
+    # unsupported, exit out
+    echo "Unsupported command: $@" >&2
+    exit 2
+    ;;
+esac

test/blackbox-tests/test-cases/pkg/opam-source-conversion.t

@@ -61,7 +61,7 @@ Unsupported backends:
   > }
   > EOF
 
-  $ solve testpkg 2>&1
+  $ solve testpkg
   Solution for dune.lock:
   - testpkg.0.0.1
   $ showpkg
@@ -72,6 +72,15 @@ Unsupported backends:
     (url hg+http://no-support.com/foo)
     (checksum md5=069aa55d40e548280f92af693f6c625a)))
 
+Create a local git repo to resolve to
+
+  $ mkdir _repo
+  $ git -C _repo init --initial-branch=main --quiet
+  $ touch _repo/content
+  $ git -C _repo add -A
+  $ git -C _repo commit -m "Initial commit" --quiet
+  $ expected_hash=$(git -C _repo rev-parse HEAD)
+
 git+http
 
   $ rm -rf ${default_lock_dir}
@@ -82,35 +91,41 @@ git+http
   > }
   > EOF
 
-  $ solve testpkg 2>&1
+  $ mkdir _bin
+  $ cp fakegit.sh _bin/git
+  $ export REAL_GIT=$(which git)
+
+  $ PATH=_bin:$PATH solve testpkg
   Solution for dune.lock:
   - testpkg.0.0.1
   $ showpkg
   (version 0.0.1)
 
   (source
    (fetch
-    (url git+http://github.com/foo)
+    (url git+http://github.com/foo#058003b274f05b092a391555ffdee8b36f1897b3)
     (checksum md5=069aa55d40e548280f92af693f6c625a)))
 
 git+file
 
   $ rm -rf ${default_lock_dir}
+
   $ mkpkg testpkg <<EOF
   > url {
-  >   src: "git+file://here"
+  >   src: "git+file://$PWD/_repo"
   >   checksum: "md5=069aa55d40e548280f92af693f6c625a"
   > }
   > EOF
-  $ solve testpkg 2>&1
+  $ solve testpkg
   Solution for dune.lock:
   - testpkg.0.0.1
-  $ showpkg
+  $ showpkg | dune_cmd subst "$PWD" '$PWD' | dune_cmd subst "$expected_hash" '$EXPECTED_HASH'
   (version 0.0.1)
 
   (source
    (fetch
-    (url git+file://here)
+    (url
+     git+file://$PWD/_repo#$EXPECTED_HASH)
     (checksum md5=069aa55d40e548280f92af693f6c625a)))
 
 git+foobar
@@ -122,15 +137,16 @@ git+foobar
   >   checksum: "md5=069aa55d40e548280f92af693f6c625a"
   > }
   > EOF
-  $ solve testpkg 2>&1
+  $ PATH=_bin:$PATH solve testpkg
   Solution for dune.lock:
   - testpkg.0.0.1
   $ showpkg
   (version 0.0.1)
 
   (source
    (fetch
-    (url git+foobar://random-thing-here)
+    (url
+     git+foobar://random-thing-here#058003b274f05b092a391555ffdee8b36f1897b3)
     (checksum md5=069aa55d40e548280f92af693f6c625a)))
 
 file+git
@@ -142,7 +158,7 @@ file+git
   >   checksum: "md5=069aa55d40e548280f92af693f6c625a"
   > }
   > EOF
-  $ solve testpkg 2>&1
+  $ solve testpkg
   Solution for dune.lock:
   - testpkg.0.0.1
   $ showpkg

test/blackbox-tests/test-cases/pkg/pin-depends.t

@@ -90,13 +90,14 @@ Git pinned source:
   > EOF
   $ git add -A
   $ git commit --quiet -m "Initial commit"
+  $ expected_commit=$(git rev-parse HEAD)
   $ cd ..
-  $ runtest "git+file://$PWD/$dir"
+  $ runtest "git+file://$PWD/$dir" | dune_cmd subst $expected_commit '$EXPECTED_COMMIT'
   Solution for dune.lock:
   - bar.1.0.0
   (version 1.0.0)
   (dev)
-  (source (fetch (url git+file://PWD/_bar_git)))
+  (source (fetch (url git+file://PWD/_bar_git#$EXPECTED_COMMIT)))
 
 Git pinned source with toplevel opam file:
 
@@ -109,13 +110,14 @@ Git pinned source with toplevel opam file:
   > EOF
   $ git add -A
   $ git commit --quiet -m "Initial commit"
+  $ expected_commit=$(git rev-parse HEAD)
   $ cd ..
-  $ runtest "git+file://$PWD/$dir"
+  $ runtest "git+file://$PWD/$dir" | dune_cmd subst $expected_commit '$EXPECTED_COMMIT'
   Solution for dune.lock:
   - bar.1.0.0
   (version 1.0.0)
   (dev)
-  (source (fetch (url git+file://PWD/_bar_opam_git)))
+  (source (fetch (url git+file://PWD/_bar_opam_git#$EXPECTED_COMMIT)))
 
 Git pinned source with toplevel opam dir 1
 
@@ -129,13 +131,14 @@ Git pinned source with toplevel opam dir 1
   > EOF
   $ git add -A
   $ git commit --quiet -m "Initial commit"
+  $ expected_commit=$(git rev-parse HEAD)
   $ cd ..
-  $ runtest "git+file://$PWD/$dir"
+  $ runtest "git+file://$PWD/$dir" | dune_cmd subst $expected_commit '$EXPECTED_COMMIT'
   Solution for dune.lock:
   - bar.1.0.0
   (version 1.0.0)
   (dev)
-  (source (fetch (url git+file://PWD/_bar_opam_dir_git1)))
+  (source (fetch (url git+file://PWD/_bar_opam_dir_git1#$EXPECTED_COMMIT)))
 
 Git pinned source with toplevel opam dir 2