fix(opam): gate dune-bound deduplication on (lang dune 3.23)#14466
Conversation
|
Why not gate at 3.23? |
There was a problem hiding this comment.
Pull request overview
This PR restores opam-file generation stability across Dune binary upgrades by gating dune-version-bound deduplication (and its associated warning) behind a (lang dune 3.24) opt-in, so projects on older language versions keep the pre-3.23 And [lang; user] constraint shape (per #14436).
Changes:
- Gate dune lower-bound deduplication (and the “user lower bound < lang” warning) on
dune_version >= (3, 24)during opam generation. - Update the existing blackbox test to cover the new behavior split: no dedup for
(lang dune 3.23), dedup/warning behavior for(lang dune 3.24), and account forGenerated_with_diffoutput location from 3.23+. - Add a changelog entry documenting the behavioral change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/blackbox-tests/test-cases/dune-project-meta/dune-dep.t | Updates test expectations to reflect dedup gated to (lang dune 3.24) and reads generated opam output from _build/default/ for lang ≥ 3.23. |
| src/dune_rules/opam_create.ml | Gates dune constraint deduplication/warning logic on dune_version >= (3, 24), restoring older-lang output stability. |
| doc/changes/fixed/14436.md | Adds release-note entry describing the gating and its rationale. |
Do you recommend that? I have no preference, but I thought that since this PR is merging to 'main', and since we are developing 3.24, that was the right version to use. 3.23 would have been the "right" version to use, had this been implemented with this guard properly the first time. And, choosing 3.23 would make back-porting cleaner. Recommendations, @Alizter ? |
|
I don't think it will make a difference. When we do cut 3.24 this change will be included but I think its better to fix 3.23.0 with 3.23.1. I don't know if the version bump is available in that branch, so this might not be able to be backported easily. I don't see any point in waiting for 3.24 which will be a few more weeks off. |
PR ocaml#14175 added deduplication of redundant `>=` bounds on the `dune` dependency in generated opam files (e.g. `{>= "3.17" & >= "3.20"}` collapsing to `{>= "3.20"}`), but did so without a lang-version guard. As a result, simply upgrading the dune binary to 3.23 changed the generated opam output for projects whose `(lang dune X.Y)` had not changed (issue ocaml#14436). Gate the new behaviour on `(lang dune 3.23)`, the lang version under which the dedup feature shipped. Projects at earlier lang versions get the pre-14175 `And [lang; user]` shape. The accompanying user warning (lower bound < lang version) is also gated, since it lives inside `merge_dune_constraints`. Fixes ocaml#14436 Signed-off-by: Robin Bate Boerop <me@robinbb.com>
c256a44 to
a3bcb76
Compare
|
@Alizter I changed the gating. Please have a look. |
Follows up ocaml#14434, which fixed the over-injection bug from ocaml#14428 by filling [>= "20180523"] only on packages that already declare a [menhir] dep. That fix only handled the *bare* declaration case; user-written constraints were preserved verbatim, including bounds below dune's required minimum and non-version filters that didn't combine with the bound. Tighten [upgrade_menhir_constraint] in [opam_create.ml] to mirror [merge_dune_constraints]: - Two [>= "v"] literals collapse to the higher of the two. When the user's bound is below dune's required minimum, emit a [User_warning.emit] and use [>= "20180523"]. - Any other shape (e.g. [{with-test}]) is ANDed with the lower bound: [(menhir :with-test)] now generates [{>= "20180523" & with-test}], not just [{with-test}]. Gate the entire menhir upgrade step on [(lang dune 3.23)] or newer. Earlier lang versions get the user's constraint verbatim (no fill, no warn), parallelling ocaml#14466 / the fix for ocaml#14436. The 3.23 gate also covers the bare-[(menhir)] fill from ocaml#14434, which previously applied unversioned. Effects on the generated opam file (under [(lang dune 3.23)] or newer): - [(menhir (>= 20100101))] -> warn, [{>= "20180523"}]. - [(menhir :with-test)] -> [{>= "20180523" & with-test}]. - [(menhir (>= 20211128))] -> [{>= "20211128"}] (unchanged; user's bound already at or above dune's). - bare [(menhir)] -> [{>= "20180523"}] (was already the behaviour from ocaml#14434, now gated). Suggestion from @Leonidas-from-XIV in ocaml#14434 (comment) and agreed direction in ocaml#14434 (comment). Lang-version gate per @Leonidas-from-XIV's review of ocaml#14453. Updates [opam-menhir-dep.t] Case 5 expected output, adds Case 6 covering the override-low warning path, and adds Case 7 covering the [(lang dune 3.22)] gate-off behaviour. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Follows up ocaml#14434, which fixed the over-injection bug from ocaml#14428 by filling [>= "20180523"] only on packages that already declare a [menhir] dep. That fix only handled the *bare* declaration case; user-written constraints were preserved verbatim, including bounds below dune's required minimum and non-version filters that didn't combine with the bound. Tighten [upgrade_menhir_constraint] in [opam_create.ml] to mirror [merge_dune_constraints]: - Two [>= "v"] literals collapse to the higher of the two. When the user's bound is below dune's required minimum, emit a [User_warning.emit] and use [>= "20180523"]. - Any other shape (e.g. [{with-test}]) is ANDed with the lower bound: [(menhir :with-test)] now generates [{>= "20180523" & with-test}], not just [{with-test}]. Gate the entire menhir upgrade step on [(lang dune 3.23)] or newer. Earlier lang versions get the user's constraint verbatim (no fill, no warn), parallelling ocaml#14466 / the fix for ocaml#14436. The 3.23 gate also covers the bare-[(menhir)] fill from ocaml#14434, which previously applied unversioned. Effects on the generated opam file (under [(lang dune 3.23)] or newer): - [(menhir (>= 20100101))] -> warn, [{>= "20180523"}]. - [(menhir :with-test)] -> [{>= "20180523" & with-test}]. - [(menhir (>= 20211128))] -> [{>= "20211128"}] (unchanged; user's bound already at or above dune's). - bare [(menhir)] -> [{>= "20180523"}] (was already the behaviour from ocaml#14434, now gated). Suggestion from @Leonidas-from-XIV in ocaml#14434 (comment) and agreed direction in ocaml#14434 (comment). Lang-version gate per @Leonidas-from-XIV's review of ocaml#14453. Updates [opam-menhir-dep.t] Case 5 expected output, adds Case 6 covering the override-low warning path, and adds Case 7 covering the [(lang dune 3.22)] gate-off behaviour. Case 6 reads the generated file directly from [_build/default/] instead of going through [--auto-promote], and uses [dune_cmd print-from | print-until] to extract just the warning text (dropping the [Promoting] line that was incidental to what the case actually asserts), addressing review feedback that the prior compound [sed] was unreadable. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Follows up ocaml#14434, which fixed the over-injection bug from ocaml#14428 by filling [>= "20180523"] only on packages that already declare a [menhir] dep. That fix only handled the *bare* declaration case; user-written constraints were preserved verbatim, including bounds below dune's required minimum and non-version filters that didn't combine with the bound. Tighten [upgrade_menhir_constraint] in [opam_create.ml] to mirror [merge_dune_constraints]: - Two [>= "v"] literals collapse to the higher of the two. When the user's bound is below dune's required minimum, emit a [User_warning.emit] and use [>= "20180523"]. - Any other shape (e.g. [{with-test}]) is ANDed with the lower bound: [(menhir :with-test)] now generates [{>= "20180523" & with-test}], not just [{with-test}]. Gate the entire menhir upgrade step on [(lang dune 3.23)] or newer. Earlier lang versions get the user's constraint verbatim (no fill, no warn), parallelling ocaml#14466 / the fix for ocaml#14436. The 3.23 gate also covers the bare-[(menhir)] fill from ocaml#14434, which previously applied unversioned. Effects on the generated opam file (under [(lang dune 3.23)] or newer): - [(menhir (>= 20100101))] -> warn, [{>= "20180523"}]. - [(menhir :with-test)] -> [{>= "20180523" & with-test}]. - [(menhir (>= 20211128))] -> [{>= "20211128"}] (unchanged; user's bound already at or above dune's). - bare [(menhir)] -> [{>= "20180523"}] (was already the behaviour from ocaml#14434, now gated). Suggestion from @Leonidas-from-XIV in ocaml#14434 (comment) and agreed direction in ocaml#14434 (comment). Lang-version gate per @Leonidas-from-XIV's review of ocaml#14453. Updates [opam-menhir-dep.t] Case 5 expected output, adds Case 6 covering the override-low warning path, and adds Case 7 covering the [(lang dune 3.22)] gate-off behaviour. Case 6 reads the generated file directly from [_build/default/] instead of going through [--auto-promote], and uses [dune_cmd print-from | print-until] to extract just the warning text (dropping the [Promoting] line that was incidental to what the case actually asserts), addressing review feedback that the prior compound [sed] was unreadable. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Follows up ocaml#14434, which fixed the over-injection bug from ocaml#14428 by filling [>= "20180523"] only on packages that already declare a [menhir] dep. That fix only handled the *bare* declaration case; user-written constraints were preserved verbatim, including bounds below dune's required minimum and non-version filters that didn't combine with the bound. Tighten [upgrade_menhir_constraint] in [opam_create.ml] to mirror [merge_dune_constraints]: - Two [>= "v"] literals collapse to the higher of the two. When the user's bound is below dune's required minimum, emit a [User_warning.emit] and use [>= "20180523"]. - Any other shape (e.g. [{with-test}]) is ANDed with the lower bound: [(menhir :with-test)] now generates [{>= "20180523" & with-test}], not just [{with-test}]. Gate the entire menhir upgrade step on [(lang dune 3.23)] or newer. Earlier lang versions get the user's constraint verbatim (no fill, no warn), parallelling ocaml#14466 / the fix for ocaml#14436. The 3.23 gate also covers the bare-[(menhir)] fill from ocaml#14434, which previously applied unversioned. Effects on the generated opam file (under [(lang dune 3.23)] or newer): - [(menhir (>= 20100101))] -> warn, [{>= "20180523"}]. - [(menhir :with-test)] -> [{>= "20180523" & with-test}]. - [(menhir (>= 20211128))] -> [{>= "20211128"}] (unchanged; user's bound already at or above dune's). - bare [(menhir)] -> [{>= "20180523"}] (was already the behaviour from ocaml#14434, now gated). Suggestion from @Leonidas-from-XIV in ocaml#14434 (comment) and agreed direction in ocaml#14434 (comment). Lang-version gate per @Leonidas-from-XIV's review of ocaml#14453. Updates [opam-menhir-dep.t] Case 5 expected output, adds Case 6 covering the override-low warning path, and adds Case 7 covering the [(lang dune 3.22)] gate-off behaviour. Case 6 reads the generated file directly from [_build/default/] instead of going through [--auto-promote], and uses [dune_cmd print-from | print-until] to extract just the warning text (dropping the [Promoting] line that was incidental to what the case actually asserts), addressing review feedback that the prior compound [sed] was unreadable. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Follows up ocaml#14434, which fixed the over-injection bug from ocaml#14428 by filling [>= "20180523"] only on packages that already declare a [menhir] dep. That fix only handled the *bare* declaration case; user-written constraints were preserved verbatim, including bounds below dune's required minimum and non-version filters that didn't combine with the bound. Tighten [upgrade_menhir_constraint] in [opam_create.ml] to mirror [merge_dune_constraints]: - Two [>= "v"] literals collapse to the higher of the two. When the user's bound is below dune's required minimum, emit a [User_warning.emit] and use [>= "20180523"]. - Any other shape (e.g. [{with-test}]) is ANDed with the lower bound: [(menhir :with-test)] now generates [{>= "20180523" & with-test}], not just [{with-test}]. Gate the entire menhir upgrade step on [(lang dune 3.23)] or newer. Earlier lang versions get the user's constraint verbatim (no fill, no warn), parallelling ocaml#14466 / the fix for ocaml#14436. The 3.23 gate also covers the bare-[(menhir)] fill from ocaml#14434, which previously applied unversioned. Effects on the generated opam file (under [(lang dune 3.23)] or newer): - [(menhir (>= 20100101))] -> warn, [{>= "20180523"}]. - [(menhir :with-test)] -> [{>= "20180523" & with-test}]. - [(menhir (>= 20211128))] -> [{>= "20211128"}] (unchanged; user's bound already at or above dune's). - bare [(menhir)] -> [{>= "20180523"}] (was already the behaviour from ocaml#14434, now gated). Suggestion from @Leonidas-from-XIV in ocaml#14434 (comment) and agreed direction in ocaml#14434 (comment). Lang-version gate per @Leonidas-from-XIV's review of ocaml#14453. Updates [opam-menhir-dep.t] Case 5 expected output, adds Case 6 covering the override-low warning path, and adds Cases 7 and 8 covering the [(lang dune 3.22)] gate-off behaviour for both the user-constrained and bare-[(menhir)] shapes. Case 6 reads the generated file directly from [_build/default/] instead of going through [--auto-promote], and uses [dune_cmd print-from | print-until] to extract just the warning text (dropping the [Promoting] line that was incidental to what the case actually asserts), addressing review feedback that the prior compound [sed] was unreadable. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Follows up ocaml#14434, which fixed the over-injection bug from ocaml#14428 by filling [>= "20180523"] only on packages that already declare a [menhir] dep. That fix only handled the *bare* declaration case; user-written constraints were preserved verbatim, including bounds below dune's required minimum and non-version filters that didn't combine with the bound. Tighten [upgrade_menhir_constraint] in [opam_create.ml] to mirror [merge_dune_constraints]: - Two [>= "v"] literals collapse to the higher of the two. When the user's bound is below dune's required minimum, emit a [User_warning.emit] and use [>= "20180523"]. - Any other shape (e.g. [{with-test}]) is ANDed with the lower bound: [(menhir :with-test)] now generates [{>= "20180523" & with-test}], not just [{with-test}]. Gate the entire menhir upgrade step on [(lang dune 3.23)] or newer. Earlier lang versions get the user's constraint verbatim (no fill, no warn), parallelling ocaml#14466 / the fix for ocaml#14436. The 3.23 gate also covers the bare-[(menhir)] fill from ocaml#14434, which previously applied unversioned. Effects on the generated opam file (under [(lang dune 3.23)] or newer): - [(menhir (>= 20100101))] -> warn, [{>= "20180523"}]. - [(menhir :with-test)] -> [{>= "20180523" & with-test}]. - [(menhir (>= 20211128))] -> [{>= "20211128"}] (unchanged; user's bound already at or above dune's). - bare [(menhir)] -> [{>= "20180523"}] (was already the behaviour from ocaml#14434, now gated). Suggestion from @Leonidas-from-XIV in ocaml#14434 (comment) and agreed direction in ocaml#14434 (comment). Lang-version gate per @Leonidas-from-XIV's review of ocaml#14453. Updates [opam-menhir-dep.t] Case 5 expected output, adds Case 6 covering the override-low warning path, and adds Cases 7 and 8 covering the [(lang dune 3.22)] gate-off behaviour for both the user-constrained and bare-[(menhir)] shapes. Case 6 reads the generated file directly from [_build/default/] instead of going through [--auto-promote], and uses [dune_cmd print-from | print-until] to extract just the warning text (dropping the [Promoting] line that was incidental to what the case actually asserts), addressing review feedback that the prior compound [sed] was unreadable. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
## Summary Gate the dune-version-bound deduplication in generated opam files (introduced in #14175 and shipped in 3.23.0) on `(lang dune 3.23)`. Projects at earlier lang versions get the pre-14175 `And [lang; user]` shape. Fixes #14436 Related: #14175 (cherry picked from commit eb4a64a) Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Summary
Gate the dune-version-bound deduplication in generated opam files (introduced in #14175 and shipped in 3.23.0) on
(lang dune 3.23). Projects at earlier lang versions get the pre-14175And [lang; user]shape.Fixes #14436
Related: #14175