Skip to content

Commit

Permalink
feature(pkg): set [post = true] when solving (#10752)
Browse files Browse the repository at this point in the history
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
  • Loading branch information
rgrinberg authored Jul 19, 2024
1 parent fbd16e8 commit 035cff0
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 49 deletions.
2 changes: 1 addition & 1 deletion src/dune_pkg/opam_solver.ml
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ let opam_package_to_lock_file_pkg
version_by_package_name
opam_file.depends
with
| Ok dep_package_names -> dep_package_names
| Ok { regular; _ } -> regular
| Error (`Formula_could_not_be_satisfied hints) ->
Code_error.raise
"Dependencies of package can't be satisfied from packages in solution"
Expand Down
2 changes: 1 addition & 1 deletion src/dune_pkg/package_universe.ml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let concrete_dependencies_of_local_package t local_package_name ~with_test =
(Solver_env.to_env t.solver_env)
t.version_by_package_name
with
| Ok s -> s
| Ok { regular; post = _ } -> regular
| Error (`Formula_could_not_be_satisfied unsatisfied_formula_hints) ->
User_error.raise
?hints:(Option.some_if with_test lockdir_regenerate_hints)
Expand Down
63 changes: 62 additions & 1 deletion src/dune_pkg/resolve_opam_formula.ml
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,67 @@ let formula_to_package_names version_by_package_name opam_formula =
Ok package_names
;;

(* Override the setting of the "post" variable to a given boolean
value by passing [Some value], or force it to be unset by passing
[None]. *)
let override_post post_value env var =
match OpamVariable.Full.scope var with
| Global
when Package_variable_name.equal
(Package_variable_name.of_opam @@ OpamVariable.Full.variable var)
Package_variable_name.post -> Option.map post_value ~f:(fun b -> OpamTypes.B b)
| _ -> env var
;;

(* Check that a package version satisfies the version constraint
associated with a package dependency in an opam file. *)
let package_version_satisfies_opam_version_constraint_opt
package_version
opam_version_constraint_opt
=
match opam_version_constraint_opt with
| None -> true
| Some (constraint_ : OpamFormula.version_constraint) ->
let opam_version = Package_version.to_opam_package_version package_version in
let version_formula = OpamFormula.Atom constraint_ in
OpamFormula.check_version_formula version_formula opam_version
;;

(* TODO (steve): This does a very similar thing to
[formula_to_package_names] so the two functions should be
combined. *)
let formula_to_package_names_allow_missing version_by_package_name opam_formula =
let cnf_terms = OpamFormula.to_cnf opam_formula in
List.filter_map cnf_terms ~f:(fun disjunction ->
(* Take the first term of the disjunction that is part of the set of packages in the
solution, if any. *)
List.find_map disjunction ~f:(fun (opam_package_name, version_constraint_opt) ->
let package_name = Package_name.of_opam_package_name opam_package_name in
Package_name.Map.find version_by_package_name package_name
|> Option.bind ~f:(fun version_in_solution ->
if package_version_satisfies_opam_version_constraint_opt
version_in_solution
version_constraint_opt
then Some package_name
else None)))
;;

type deps =
{ post : Package_name.t list
; regular : Package_name.t list
}

let filtered_formula_to_package_names env ~with_test version_by_package_name formula =
formula_to_package_names version_by_package_name (apply_filter ~with_test env formula)
let open Result.O in
let+ all =
formula_to_package_names version_by_package_name (apply_filter ~with_test env formula)
in
let regular_set =
formula_to_package_names_allow_missing
version_by_package_name
(apply_filter ~with_test (override_post (Some false) env) formula)
|> Package_name.Set.of_list
in
let regular, post = List.partition all ~f:(Package_name.Set.mem regular_set) in
{ regular; post }
;;
10 changes: 8 additions & 2 deletions src/dune_pkg/resolve_opam_formula.mli
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,18 @@ val formula_to_package_names
-> OpamTypes.formula
-> (Package_name.t list, unsatisfied_formula) result

type deps =
{ post : Package_name.t list
; regular : Package_name.t list
}

(** Like [formula_to_package_name] but takes an [OpamTypes.filtered_formula]
and evaluates its filters to produce a formula which is then resolved to a
list of package names. *)
list of package names, split into post and regular (ie. non-post)
dependencies. *)
val filtered_formula_to_package_names
: OpamFilter.env
-> with_test:bool
-> Package_version.t Package_name.Map.t
-> OpamTypes.filtered_formula
-> (Package_name.t list, unsatisfied_formula) result
-> (deps, unsatisfied_formula) result
1 change: 1 addition & 0 deletions src/dune_pkg/solver_env.ml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ let with_defaults =
, OpamVersion.to_string OpamVersion.current |> Variable_value.string )
; Package_variable_name.with_doc, Variable_value.false_
; Package_variable_name.with_dev_setup, Variable_value.false_
; Package_variable_name.post, Variable_value.true_
]
|> List.fold_left ~init:empty ~f:(fun acc (name, value) -> set acc name value)
;;
Expand Down
1 change: 1 addition & 0 deletions test/blackbox-tests/test-cases/pkg/common-filters-deps.t
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ documentation-only deps are omitted from the solution.
$ solve "(test :with-test) (doc :with-doc) (dev-setup :with-dev-setup) (dev :with-dev) (build :build) (post :post)"
Solution for dune.lock:
- build.0.0.1
- post.0.0.1
- test.0.0.1
54 changes: 54 additions & 0 deletions test/blackbox-tests/test-cases/pkg/compiler-post-dependencies.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
Exercise dune resolving the post dependencies found in compiler packages.

$ . ./helpers.sh
$ mkrepo

$ cat >dune-workspace << EOF
> (lang dune 3.16)
> (lock_dir
> (path dune.lock)
> (repositories mock)
> (solver_env
> (os linux)))
> EOF

$ mkpkg host-system-other

$ mkpkg system-mingw << EOF
> available: os = "win32"
> EOF

$ mkpkg ocaml << EOF
> depends: [
> "ocaml-base-compiler"
> ]
> EOF

This package has a dependency cycle with the "ocaml" package, broken
by the use of post dependencies. It also contains a formula with no
solutions when the "post" variable is set to "false" on non-windows
systems. Opam evaluates dependency formulae with post=true and then
does further filtering to remove post dependencies from dependency
lists to prevent circular dependencies at package build time.
$ mkpkg ocaml-base-compiler << EOF
> depends: [
> "ocaml" {post}
> (("arch-x86_64" {os = "win32" & arch = "x86_64"} & "system-mingw" &
> "mingw-w64-shims" {os-distribution = "cygwin" & build}) |
> ("arch-x86_32" {os = "win32"} & "ocaml-option-bytecode-only" &
> "system-mingw" &
> "mingw-w64-shims" {os-distribution = "cygwin" & build}) |
> "host-system-other" {os != "win32" & post})
> ]
> EOF

$ solve ocaml-base-compiler
Solution for dune.lock:
- host-system-other.0.0.1
- ocaml.0.0.1
- ocaml-base-compiler.0.0.1

Ensure that packages can be resolved at build time. This checks that
the dependency cycle between the "ocaml" and "ocaml-base-compiler"
packages is successfully broken by the use of post dependencies.
$ dune build
44 changes: 0 additions & 44 deletions test/blackbox-tests/test-cases/pkg/gh10670.t

This file was deleted.

4 changes: 4 additions & 0 deletions test/blackbox-tests/test-cases/pkg/just-print-solver-env.t
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ Add some build contexts with different environments
$ dune pkg print-solver-env --all
Solver environment for lock directory change-opam-version.lock:
- opam-version = 42
- post = true
- with-dev-setup = false
- with-doc = false
Solver environment for lock directory dune.linux.lock:
- opam-version = 2.2.0~alpha-vendored
- os = linux
- post = true
- with-dev-setup = false
- with-doc = false
Solver environment for lock directory dune.linux.no-doc.lock:
Expand All @@ -54,10 +56,12 @@ Add some build contexts with different environments
- os-distribution = ubuntu
- os-family = ubuntu
- os-version = 22.04
- post = true
- sys-ocaml-version = 5.0
- with-dev-setup = false
- with-doc = false
Solver environment for lock directory dune.lock:
- opam-version = 2.2.0~alpha-vendored
- post = true
- with-dev-setup = false
- with-doc = false
86 changes: 86 additions & 0 deletions test/blackbox-tests/test-cases/pkg/post-deps-solving.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
Solving for post dependencies:

$ . ./helpers.sh
$ mkrepo

$ mkpkg bar

$ mkpkg foo <<EOF
> depends: [ "bar" {post} ]
> EOF

We don't need bar, but we still include it:

$ solve foo
Solution for dune.lock:
- bar.0.0.1
- foo.0.0.1

$ cat dune.lock/foo.pkg dune.lock/bar.pkg
(version 0.0.1)
(version 0.0.1)

Self dependency

$ mkpkg foo <<EOF
> depends: [ "foo" {post} ]
> EOF

$ solve foo
Solution for dune.lock:
- foo.0.0.1

$ cat dune.lock/foo.pkg
(version 0.0.1)

Using post to break cycle:

$ mkpkg foo <<EOF
> depends: [ "bar" {post} ]
> EOF

$ mkpkg bar <<EOF
> depends: [ "foo" ]
> EOF

$ solve bar
Solution for dune.lock:
- bar.0.0.1
- foo.0.0.1

$ cat dune.lock/foo.pkg dune.lock/bar.pkg
(version 0.0.1)
(version 0.0.1)

(depends foo)

post "cycle":

$ mkpkg foo <<EOF
> depends: [ "bar" {post} ]
> EOF

$ mkpkg bar <<EOF
> depends: [ "foo" {post} ]
> EOF

$ solve foo
Solution for dune.lock:
- bar.0.0.1
- foo.0.0.1

$ cat dune.lock/foo.pkg dune.lock/bar.pkg
(version 0.0.1)
(version 0.0.1)

In depopts:

$ mkpkg foo <<EOF
> depopts: [ "bar" {post} ]
> EOF

$ mkpkg bar

$ solve foo
Solution for dune.lock:
- foo.0.0.1

0 comments on commit 035cff0

Please sign in to comment.