From ec878b3eab8e154587a00970564b6e4a2e8af4f0 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 15 Oct 2025 16:10:14 +0100 Subject: [PATCH 1/4] dev Signed-off-by: Rudi Grinberg From cdc0273c205cc5b927e4b806c40842ac0b2ca933 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 15 Oct 2025 16:11:40 +0100 Subject: [PATCH 2/4] Reproduce issue in #12381 The version of dune inside the opam repo is overriding the version of dune that is being used. That seems wrong. At least most of the time. Signed-off-by: Rudi Grinberg --- .../test-cases/pkg/implicit-dune-constraint.t | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t b/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t index 29267818784..d6f6fca72a0 100644 --- a/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t +++ b/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t @@ -6,9 +6,6 @@ dependency. $ . ./helpers.sh $ mkrepo - $ mkpkg dune 3.11.0 < EOF - $ test() { > mkpkg foo < depends: [ "dune" {<= "$1"} ] @@ -29,6 +26,10 @@ dependency. Solution for dune.lock: - foo.0.0.1 + $ test "4.0.0" 2>&1 | sed -E 's/3.[0-9]+/3.XX/g' + Solution for dune.lock: + - foo.0.0.1 + Create a fake project and ensure `dune` can be used as a dependency: $ cat > dune-project < (lang dune 3.13) From 55db3f31ab3bb0b7636666d385e78645b63b2cfc Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 15 Oct 2025 16:49:38 +0100 Subject: [PATCH 3/4] Remove [Resolved_package.set_url] Fuse it with [git_repo] since that is the only place where it's needed Signed-off-by: Rudi Grinberg --- src/dune_pkg/opam_repo.ml | 5 +- src/dune_pkg/pinned_package.ml | 69 +++++++++---------- src/dune_pkg/resolved_package.ml | 24 +++---- src/dune_pkg/resolved_package.mli | 3 +- .../test-cases/pkg/implicit-dune-constraint.t | 26 ++++--- 5 files changed, 66 insertions(+), 61 deletions(-) diff --git a/src/dune_pkg/opam_repo.ml b/src/dune_pkg/opam_repo.ml index 565c886fdcc..8037661645c 100644 --- a/src/dune_pkg/opam_repo.ml +++ b/src/dune_pkg/opam_repo.ml @@ -134,7 +134,7 @@ let load_opam_package_from_dir ~(dir : Path.t) package = | false -> None | true -> let files_dir = Some (Paths.files_dir package) in - Some (Resolved_package.local_fs package ~dir ~opam_file_path ~files_dir) + Some (Resolved_package.local_fs package ~dir ~opam_file_path ~files_dir ~url:None) ;; let load_packages_from_git rev_store opam_packages = @@ -151,7 +151,8 @@ let load_packages_from_git rev_store opam_packages = ~opam_file:(Rev_store.File.path opam_file) ~opam_file_contents rev - ~files_dir:(Some files_dir)) + ~files_dir:(Some files_dir) + ~url:None) ;; let all_packages_versions_in_dir loc ~dir opam_package_name = diff --git a/src/dune_pkg/pinned_package.ml b/src/dune_pkg/pinned_package.ml index 4aad4296fca..5ab401ddfea 100644 --- a/src/dune_pkg/pinned_package.ml +++ b/src/dune_pkg/pinned_package.ml @@ -66,39 +66,38 @@ let resolve_package { Local_package.loc; url = loc_url, url; name; version; orig (Package_name.to_opam_package_name name) (Package_version.to_opam_package_version version) in - let+ resolved_package = - let* mount = Mount.of_opam_url loc_url url in - let* opam_file_path, files_dir = discover_layout loc name mount in - match Mount.backend mount with - | Path dir -> - Resolved_package.local_fs package ~dir ~opam_file_path ~files_dir |> Fiber.return - | Git rev -> - let+ opam_file_contents = - (* CR-rgrinberg: not efficient to make such individual calls *) - Mount.read mount opam_file_path - >>| function - | Some p -> p - | None -> - let files = - match Path.Local.parent opam_file_path with - | None -> [] - | Some parent -> - Rev_store.At_rev.directory_entries rev parent ~recursive:false - |> Rev_store.File.Set.to_list_map ~f:Rev_store.File.path - in - Code_error.raise - ~loc - "unable to find file" - [ "opam_file_path", Path.Local.to_dyn opam_file_path - ; "files", Dyn.list Path.Local.to_dyn files - ] - in - Resolved_package.git_repo - package - ~opam_file:opam_file_path - ~opam_file_contents - rev - ~files_dir - in - Resolved_package.set_url resolved_package url + let* mount = Mount.of_opam_url loc_url url in + let* opam_file_path, files_dir = discover_layout loc name mount in + match Mount.backend mount with + | Path dir -> + Resolved_package.local_fs package ~dir ~opam_file_path ~files_dir ~url:(Some url) + |> Fiber.return + | Git rev -> + let+ opam_file_contents = + (* CR-rgrinberg: not efficient to make such individual calls *) + Mount.read mount opam_file_path + >>| function + | Some p -> p + | None -> + let files = + match Path.Local.parent opam_file_path with + | None -> [] + | Some parent -> + Rev_store.At_rev.directory_entries rev parent ~recursive:false + |> Rev_store.File.Set.to_list_map ~f:Rev_store.File.path + in + Code_error.raise + ~loc + "unable to find file" + [ "opam_file_path", Path.Local.to_dyn opam_file_path + ; "files", Dyn.list Path.Local.to_dyn files + ] + in + Resolved_package.git_repo + package + ~opam_file:opam_file_path + ~opam_file_contents + rev + ~files_dir + ~url:(Some url) ;; diff --git a/src/dune_pkg/resolved_package.ml b/src/dune_pkg/resolved_package.ml index 26d39e6c660..81f01b65b97 100644 --- a/src/dune_pkg/resolved_package.ml +++ b/src/dune_pkg/resolved_package.ml @@ -17,25 +17,25 @@ let loc t = t.loc let package t = t.package let opam_file t = t.opam_file -let set_url t url = - let opam_file = OpamFile.OPAM.with_url (OpamFile.URL.create url) t.opam_file in - { t with opam_file } -;; - let add_opam_package_to_opam_file package opam_file = opam_file |> OpamFile.OPAM.with_version (OpamPackage.version package) |> OpamFile.OPAM.with_name (OpamPackage.name package) ;; -let read_opam_file package ~opam_file_path ~opam_file_contents = - Opam_file.read_from_string_exn ~contents:opam_file_contents opam_file_path - |> add_opam_package_to_opam_file package +let read_opam_file package ~opam_file_path ~opam_file_contents ~url = + let opam_file = + Opam_file.read_from_string_exn ~contents:opam_file_contents opam_file_path + |> add_opam_package_to_opam_file package + in + match url with + | None -> opam_file + | Some url -> OpamFile.OPAM.with_url (OpamFile.URL.create url) opam_file ;; -let git_repo package ~opam_file ~opam_file_contents rev ~files_dir = +let git_repo package ~opam_file ~opam_file_contents rev ~files_dir ~url = let opam_file_path = Path.of_local opam_file in - let opam_file = read_opam_file package ~opam_file_path ~opam_file_contents in + let opam_file = read_opam_file package ~opam_file_path ~opam_file_contents ~url in let loc = Loc.in_file opam_file_path in { dune_build = false ; loc @@ -45,12 +45,12 @@ let git_repo package ~opam_file ~opam_file_contents rev ~files_dir = } ;; -let local_fs package ~dir ~opam_file_path ~files_dir = +let local_fs package ~dir ~opam_file_path ~files_dir ~url = let opam_file_path = Path.append_local dir opam_file_path in let files_dir = Option.map files_dir ~f:(Path.append_local dir) in let opam_file = let opam_file_contents = Io.read_file ~binary:true opam_file_path in - read_opam_file package ~opam_file_path ~opam_file_contents + read_opam_file package ~opam_file_path ~opam_file_contents ~url in let loc = Loc.in_file opam_file_path in { dune_build = false diff --git a/src/dune_pkg/resolved_package.mli b/src/dune_pkg/resolved_package.mli index 4eaf167fba9..65f67599d09 100644 --- a/src/dune_pkg/resolved_package.mli +++ b/src/dune_pkg/resolved_package.mli @@ -5,7 +5,6 @@ type t val package : t -> OpamPackage.t val opam_file : t -> OpamFile.OPAM.t val loc : t -> Loc.t -val set_url : t -> OpamUrl.t -> t val dune_build : t -> bool val git_repo @@ -14,6 +13,7 @@ val git_repo -> opam_file_contents:string -> Rev_store.At_rev.t -> files_dir:Path.Local.t option + -> url:OpamUrl.t option -> t val local_fs @@ -21,6 +21,7 @@ val local_fs -> dir:Path.t -> opam_file_path:Path.Local.t -> files_dir:Path.Local.t option + -> url:OpamUrl.t option -> t val local_package diff --git a/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t b/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t index d6f6fca72a0..d2343e10763 100644 --- a/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t +++ b/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t @@ -17,18 +17,19 @@ dependency. Error: Unable to solve dependencies for the following lock directories: Lock directory dune.lock: Couldn't solve the package dependency formula. - Selected candidates: foo.0.0.1 x.dev - - dune -> (problem) - User requested = 3.XX - Rejected candidates: - dune.3.XX.0: Incompatible with restriction: = 3.XX + The following packages couldn't be found: dune $ test "4.0.0" - Solution for dune.lock: - - foo.0.0.1 + Error: Unable to solve dependencies for the following lock directories: + Lock directory dune.lock: + Couldn't solve the package dependency formula. + The following packages couldn't be found: dune + [1] $ test "4.0.0" 2>&1 | sed -E 's/3.[0-9]+/3.XX/g' - Solution for dune.lock: - - foo.0.0.1 + Error: Unable to solve dependencies for the following lock directories: + Lock directory dune.lock: + Couldn't solve the package dependency formula. + The following packages couldn't be found: dune Create a fake project and ensure `dune` can be used as a dependency: $ cat > dune-project < (depends dune)) > EOF $ dune pkg lock - Solution for dune.lock: - (no dependencies to lock) + Error: Unable to solve dependencies for the following lock directories: + Lock directory dune.lock: + Couldn't solve the package dependency formula. + The following packages couldn't be found: dune + [1] From 600c39a2781f87cc9f07f0ba59f11b4e97453a1a Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 15 Oct 2025 16:55:39 +0100 Subject: [PATCH 4/4] fix(pkg): make dune pin itelf Automatically make dune pin itself. We're going to assume that the dune we're using right now is the one that will be used to run the build plan. Perhaps this behavior isn't always deired. We could imagine wanting to produce build plans for older or newer dunes. For now, this is a good default though. Addresses #12381 Signed-off-by: Rudi Grinberg --- src/dune_pkg/dune_dep.ml | 3 + src/dune_pkg/dune_dep.mli | 2 + src/dune_pkg/opam_solver.ml | 14 ++++ src/dune_pkg/resolved_package.ml | 75 +++++++++++++------ src/dune_pkg/resolved_package.mli | 1 + .../test-cases/pkg/implicit-dune-constraint.t | 27 +++---- .../test-cases/pkg/pin-stanza/pin-dune.t | 34 +++++++++ .../unsatisfied-version-constraint-on-dune.t | 14 ++-- 8 files changed, 128 insertions(+), 42 deletions(-) create mode 100644 test/blackbox-tests/test-cases/pkg/pin-stanza/pin-dune.t diff --git a/src/dune_pkg/dune_dep.ml b/src/dune_pkg/dune_dep.ml index f57b3a7e579..dd44d704422 100644 --- a/src/dune_pkg/dune_dep.ml +++ b/src/dune_pkg/dune_dep.ml @@ -6,3 +6,6 @@ let version = let major, minor = Dune_lang.Stanza.latest_version in OpamPackage.Version.of_string @@ sprintf "%d.%d" major minor ;; + +let package = OpamPackage.create (Package_name.to_opam_package_name name) version +let opam_file = OpamFile.OPAM.create package diff --git a/src/dune_pkg/dune_dep.mli b/src/dune_pkg/dune_dep.mli index 6a276603ae7..b4a3b67f318 100644 --- a/src/dune_pkg/dune_dep.mli +++ b/src/dune_pkg/dune_dep.mli @@ -1,2 +1,4 @@ val name : Package_name.t val version : OpamPackage.Version.t +val package : OpamPackage.t +val opam_file : OpamFile.OPAM.t diff --git a/src/dune_pkg/opam_solver.ml b/src/dune_pkg/opam_solver.ml index 635b687d4aa..587abd9a7b4 100644 --- a/src/dune_pkg/opam_solver.ml +++ b/src/dune_pkg/opam_solver.ml @@ -348,6 +348,8 @@ module Context = struct let user_restrictions : t -> OpamPackage.Name.t -> OpamFormula.version_constraint option = fun t pkg -> + (* This isn't really needed because we already pin dune, but it seems to + help the error messages *) if Package_name.equal Dune_dep.name (Package_name.of_opam_package_name pkg) then Some (`Eq, t.dune_version) else None @@ -1658,6 +1660,18 @@ let solve_lock_dir ~selected_depopts ~portable_lock_dir = + let pinned_packages = + Package_name.Map.update pinned_packages Dune_dep.name ~f:(function + | None -> Some Resolved_package.dune + | Some p -> + let loc = Resolved_package.loc p in + User_error.raise + ~loc + [ Pp.text + "Dune cannot be pinned. The currently running version is the only one that \ + may be used" + ]) + in let pinned_package_names = Package_name.Set.of_keys pinned_packages in let stats_updater = Solver_stats.Updater.init () in let context = diff --git a/src/dune_pkg/resolved_package.ml b/src/dune_pkg/resolved_package.ml index 81f01b65b97..11a153b6dbd 100644 --- a/src/dune_pkg/resolved_package.ml +++ b/src/dune_pkg/resolved_package.ml @@ -4,7 +4,7 @@ type extra_files = | Inside_files_dir of Path.t option | Git_files of Path.Local.t option * Rev_store.At_rev.t -type nonrec t = +type rest = { opam_file : OpamFile.OPAM.t ; package : OpamPackage.t ; extra_files : extra_files @@ -12,10 +12,31 @@ type nonrec t = ; dune_build : bool } -let dune_build t = t.dune_build -let loc t = t.loc -let package t = t.package -let opam_file t = t.opam_file +type nonrec t = + | Dune + | Rest of rest + +let dune = Dune + +let dune_build = function + | Dune -> false + | Rest t -> t.dune_build +;; + +let loc = function + | Dune -> Loc.none + | Rest t -> t.loc +;; + +let package = function + | Dune -> Dune_dep.package + | Rest t -> t.package +;; + +let opam_file = function + | Dune -> Dune_dep.opam_file + | Rest t -> t.opam_file +;; let add_opam_package_to_opam_file package opam_file = opam_file @@ -37,12 +58,13 @@ let git_repo package ~opam_file ~opam_file_contents rev ~files_dir ~url = let opam_file_path = Path.of_local opam_file in let opam_file = read_opam_file package ~opam_file_path ~opam_file_contents ~url in let loc = Loc.in_file opam_file_path in - { dune_build = false - ; loc - ; package - ; opam_file - ; extra_files = Git_files (files_dir, rev) - } + Rest + { dune_build = false + ; loc + ; package + ; opam_file + ; extra_files = Git_files (files_dir, rev) + } ;; let local_fs package ~dir ~opam_file_path ~files_dir ~url = @@ -53,12 +75,13 @@ let local_fs package ~dir ~opam_file_path ~files_dir ~url = read_opam_file package ~opam_file_path ~opam_file_contents ~url in let loc = Loc.in_file opam_file_path in - { dune_build = false - ; loc - ; package - ; extra_files = Inside_files_dir files_dir - ; opam_file - } + Rest + { dune_build = false + ; loc + ; package + ; extra_files = Inside_files_dir files_dir + ; opam_file + } ;; (* Scan a path recursively down retrieving a list of all files together with their @@ -92,7 +115,7 @@ let local_package ~command_source loc opam_file opam_package = in let opam_file = add_opam_package_to_opam_file opam_package opam_file in let package = OpamFile.OPAM.package opam_file in - { dune_build; opam_file; package; loc; extra_files = Inside_files_dir None } + Rest { dune_build; opam_file; package; loc; extra_files = Inside_files_dir None } ;; open Fiber.O @@ -100,10 +123,18 @@ open Fiber.O let get_opam_package_files resolved_packages = let indexed = List.mapi resolved_packages ~f:(fun i w -> i, w) |> Int.Map.of_list_exn in let from_dirs, from_git = - Int.Map.partition_map indexed ~f:(fun (resolved_package : t) -> - match resolved_package.extra_files with - | Git_files (files_dir, rev) -> Right (files_dir, rev) - | Inside_files_dir dir -> Left dir) + let _dune, without_dune = + Int.Map.partition_map indexed ~f:(function + | Dune -> Left () + | Rest t -> Right t) + in + let dirs, git = + Int.Map.partition_map without_dune ~f:(fun (resolved_package : rest) -> + match resolved_package.extra_files with + | Git_files (files_dir, rev) -> Right (files_dir, rev) + | Inside_files_dir dir -> Left dir) + in + dirs, git in let+ from_git = if Int.Map.is_empty from_git diff --git a/src/dune_pkg/resolved_package.mli b/src/dune_pkg/resolved_package.mli index 65f67599d09..88b2c103f96 100644 --- a/src/dune_pkg/resolved_package.mli +++ b/src/dune_pkg/resolved_package.mli @@ -6,6 +6,7 @@ val package : t -> OpamPackage.t val opam_file : t -> OpamFile.OPAM.t val loc : t -> Loc.t val dune_build : t -> bool +val dune : t val git_repo : OpamPackage.t diff --git a/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t b/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t index d2343e10763..5f8eb6d0ca1 100644 --- a/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t +++ b/test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t @@ -17,19 +17,19 @@ dependency. Error: Unable to solve dependencies for the following lock directories: Lock directory dune.lock: Couldn't solve the package dependency formula. - The following packages couldn't be found: dune + Selected candidates: foo.0.0.1 x.dev + - dune -> (problem) + User requested = 3.XX + foo 0.0.1 requires <= 2.0.0 + Rejected candidates: + dune.3.XX: Incompatible with restriction: <= 2.0.0 $ test "4.0.0" - Error: Unable to solve dependencies for the following lock directories: - Lock directory dune.lock: - Couldn't solve the package dependency formula. - The following packages couldn't be found: dune - [1] + Solution for dune.lock: + - foo.0.0.1 $ test "4.0.0" 2>&1 | sed -E 's/3.[0-9]+/3.XX/g' - Error: Unable to solve dependencies for the following lock directories: - Lock directory dune.lock: - Couldn't solve the package dependency formula. - The following packages couldn't be found: dune + Solution for dune.lock: + - foo.0.0.1 Create a fake project and ensure `dune` can be used as a dependency: $ cat > dune-project < (depends dune)) > EOF $ dune pkg lock - Error: Unable to solve dependencies for the following lock directories: - Lock directory dune.lock: - Couldn't solve the package dependency formula. - The following packages couldn't be found: dune - [1] + Solution for dune.lock: + (no dependencies to lock) diff --git a/test/blackbox-tests/test-cases/pkg/pin-stanza/pin-dune.t b/test/blackbox-tests/test-cases/pkg/pin-stanza/pin-dune.t new file mode 100644 index 00000000000..9bf8b2ca7ea --- /dev/null +++ b/test/blackbox-tests/test-cases/pkg/pin-stanza/pin-dune.t @@ -0,0 +1,34 @@ +Pinning dune itself + + $ . ../helpers.sh + + $ mkrepo + $ add_mock_repo_if_needed + +# CR-someday rgrinberg: ideally, this source shouldn't be necessary and we +# should disqualify this pin without resolving any sources + + $ mkdir _extra_source + $ cat >_extra_source/dune-project < (lang dune 3.12) + > (package (name dune)) + > EOF + + $ cat >dune-project < (lang dune 3.13) + > (pin + > (url "file://$PWD/_extra_source") + > (package (name dune))) + > (package + > (name main)) + > EOF + +For now, pinning dune is not allowed: + + $ dune pkg lock + File "dune-project", line 4, characters 1-22: + 4 | (package (name dune))) + ^^^^^^^^^^^^^^^^^^^^^ + Error: Dune cannot be pinned. The currently running version is the only one + that may be used + [1] diff --git a/test/blackbox-tests/test-cases/pkg/unsatisfied-version-constraint-on-dune.t b/test/blackbox-tests/test-cases/pkg/unsatisfied-version-constraint-on-dune.t index 5594267f170..1e7be54e88c 100644 --- a/test/blackbox-tests/test-cases/pkg/unsatisfied-version-constraint-on-dune.t +++ b/test/blackbox-tests/test-cases/pkg/unsatisfied-version-constraint-on-dune.t @@ -22,8 +22,12 @@ project: Solve the dependencies: $ dune pkg lock 2>&1 | sed -E 's/"3.[0-9]+"/"3.XX"/' - Error: The current version of Dune does not satisfy the version constraints - for Dune in this project's dependencies. - Details: - Found version "3.XX" of package "dune" which doesn't satisfy the required - version constraint "< 3.0" + Error: Unable to solve dependencies for the following lock directories: + Lock directory dune.lock: + Couldn't solve the package dependency formula. + Selected candidates: foo.dev + - dune -> (problem) + User requested = 3.21 + foo dev requires < 3.0 + Rejected candidates: + dune.3.21: Incompatible with restriction: < 3.0