Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Opam constraints operator precedence is not honored. #3431

Closed
mefyl opened this issue Apr 28, 2020 · 0 comments · Fixed by #7682 or ocaml/opam-repository#24013
Closed

Opam constraints operator precedence is not honored. #3431

mefyl opened this issue Apr 28, 2020 · 0 comments · Fixed by #7682 or ocaml/opam-repository#24013
Labels
bug
Milestone
3.9.0

Comments

@mefyl
Copy link
Collaborator

mefyl commented Apr 28, 2020 •
edited
Loading

A depends stanza such as the following one in the dune-project file will output a wrong OPAM constraint expression because logical operator precedence seems to be ignored.

  (alcotest
   (and
    (or :with-test :dev)
    (>= 1.1.0)))

Expected Behavior

"alcotest" { (with-test | dev) & >= "1.1.0"}

Actual Behavior

"alcotest" {with-test | dev & >= "1.1.0"}

Reproduction

dune build any project with a dune-project looking like

(lang dune 2.2)

(name coin)

(generate_opam_files true)

(package
 (name coin)
 (synopsis "COIN")
 (depends
  (alcotest
   (or
    (and (>= 1.1.0) :with-test)
    (and (>= 1.1.0) :dev))))

and look at the generated coin.opam.

Specifications

  • Version of dune: 2.5.1
  • Version of ocaml: 4.06.0
  • Operating system : Linux 5.4.28-gentoo
@mefyl mefyl added the bug label Apr 28, 2020
@emillon emillon mentioned this issue Mar 16, 2023
Open
emillon added a commit to emillon/dune that referenced this issue Mar 16, 2023
@emillon
test: add repro case for ocaml#3431
ce70e6c 
When we generate an opam formula that would require parentheses, they
are not present.
This is actually a bug in opam-file-format:
ocaml/opam-file-format/issues#56.

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit that referenced this issue Mar 21, 2023
@emillon
test: add repro case for #3431 (#7338)
88ee173 
When we generate an opam formula that would require parentheses, they
are not present.
This is actually a bug in opam-file-format:
ocaml/opam-file-format/issues#56.

Signed-off-by: Etienne Millon <[email protected]>
Lucccyo added a commit to Lucccyo/dune that referenced this issue May 4, 2023
@Lucccyo
fix ocaml#3431 with brute force
bccfb2e
Lucccyo added a commit to Lucccyo/dune that referenced this issue May 5, 2023
@Lucccyo
fix ocaml#3431
abf52f8
Lucccyo added a commit to Lucccyo/dune that referenced this issue May 5, 2023
@Lucccyo
fix ocaml#3431 with brute force
2964afb 
Signed-off-by: Lucccyo <[email protected]>
Lucccyo added a commit to Lucccyo/dune that referenced this issue May 5, 2023
@Lucccyo
fix ocaml#3431
aad6cb4 
Signed-off-by: Lucccyo <[email protected]>
Lucccyo added a commit to Lucccyo/dune that referenced this issue May 5, 2023
@Lucccyo
fix ocaml#3431 opam_constraint add parenthesis to respect precedenc…
b6f72e3 
…e of

operations

Signed-off-by: Lucccyo <[email protected]>
@Lucccyo Lucccyo mentioned this issue May 5, 2023
Merged
@emillon emillon mentioned this issue May 15, 2023
Merged
emillon pushed a commit to emillon/dune that referenced this issue May 15, 2023
@Lucccyo @emillon
test: opam constraint generation
68d54fb 
This generalizes the test for ocaml#3431 and actually reveals that nested ors
are incorrectly translated at the moment: in `p_or3`, `(or :a :b :c)`
gets converted to `{a | b & c}`.

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue May 15, 2023
@emillon
test: opam constraint generation
97b88bf 
This generalizes the test for ocaml#3431 and actually reveals that nested ors
are incorrectly translated at the moment: in `p_or3`, `(or :a :b :c)`
gets converted to `{a | b & c}`.

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue May 15, 2023
@emillon
test: opam constraint generation
01e417e 
This generalizes the test for ocaml#3431 and actually reveals that nested ors
are incorrectly translated at the moment: in `p_or3`, `(or :a :b :c)`
gets converted to `{a | b & c}`.

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue May 15, 2023
@emillon
test: opam constraint generation
11129a7 
This generalizes the test for ocaml#3431 and actually reveals that nested ors
are incorrectly translated at the moment: in `p_or3`, `(or :a :b :c)`
gets converted to `{a | b & c}`.

It also adds a test for `(and)` and `(or)` with no argument, which
should fail cleanly.

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit that referenced this issue May 15, 2023
@emillon
test: opam constraint generation (#7729)
e3d346a 
This generalizes the test for #3431 and actually reveals that nested ors
are incorrectly translated at the moment: in `p_or3`, `(or :a :b :c)`
gets converted to `{a | b & c}`.

It also adds a test for `(and)` and `(or)` with no argument, which
should fail cleanly.

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to Lucccyo/dune that referenced this issue May 15, 2023
@emillon
fix ocaml#3431 opam_constraint add parenthesis to respect precedenc…
a8a67d1 
…e of operations

Signed-off-by: Lucccyo <[email protected]>
emillon pushed a commit to Lucccyo/dune that referenced this issue May 15, 2023
@Lucccyo @emillon
fix ocaml#3431 opam_constraint add parenthesis to respect precedenc…
5638c08 
…e of operations

Signed-off-by: Lucccyo <[email protected]>
emillon pushed a commit to Lucccyo/dune that referenced this issue May 16, 2023
@Lucccyo @emillon
fix ocaml#3431 opam_constraint add parenthesis to respect precedenc…
4302c8b 
…e of operations

Signed-off-by: Lucccyo <[email protected]>
emillon added a commit to Lucccyo/dune that referenced this issue Jun 20, 2023
@emillon @Lucccyo
fix(opam): honor precedence in constraints
253e831 
Fixes ocaml#3431

The printer in opam-file-format does not insert parentheses on its own,
but it is possible to use the Group constructor with a singleton to
force insertion of parentheses.

Signed-off-by: Etienne Millon <[email protected]>
Co-authored-by: Charlène Gros <[email protected]>
emillon added a commit to Lucccyo/dune that referenced this issue Jun 20, 2023
@emillon @Lucccyo
fix(opam): honor precedence in constraints
8df9e6c 
Fixes ocaml#3431

The printer in opam-file-format does not insert parentheses on its own,
but it is possible to use the Group constructor with a singleton to
force insertion of parentheses.

Signed-off-by: Etienne Millon <[email protected]>
Co-authored-by: Charlène Gros <[email protected]>
emillon added a commit to Lucccyo/dune that referenced this issue Jun 21, 2023
@emillon @Lucccyo
fix(opam): honor precedence in constraints
45b96e4 
Fixes ocaml#3431

The printer in opam-file-format does not insert parentheses on its own,
but it is possible to use the Group constructor with a singleton to
force insertion of parentheses.

Signed-off-by: Etienne Millon <[email protected]>
Co-authored-by: Charlène Gros <[email protected]>
emillon pushed a commit that referenced this issue Jun 21, 2023
@Lucccyo
fix(opam): honor precedence in constraints (#7682)
85543ec 
Fixes #3431

The printer in opam-file-format does not insert parentheses on its own,
but it is possible to use the Group constructor with a singleton to
force insertion of parentheses.

Signed-off-by: Etienne Millon <[email protected]>
Co-authored-by: Charlène Gros <[email protected]>
@rgrinberg rgrinberg added this to the 3.9.0 milestone Jun 21, 2023
@emillon emillon mentioned this issue Jun 23, 2023
Closed
emillon added a commit to emillon/opam-repository that referenced this issue Jun 23, 2023
@emillon
[new release] dune (15 packages) (3.9.0~alpha1)
e84760b 
CHANGES:

- Validate file extension for `$ dune ocaml top-module`. (ocaml/dune#8005, fixes ocaml/dune#8004, @3Rafal)

- Include the time it takes to read/write state files when `--trace-file` is
  enabled (ocaml/dune#7960, @rgrinberg)

- Add `dune show` command group which is an alias of `dune describe`. (ocaml/dune#7946,
  @Alizter)

- Include source tree scans in the traces produced by `--trace-file` (ocaml/dune#7937,
  @rgrinberg)

- Cinaps: The promotion rules for cinaps would only offer one file at a time no
  matter how many promotions were available. Now we offer all the promotions at
  once (ocaml/dune#7901, @rgrinberg)

- Do not re-run OCaml syntax files on every iteration of the watch mode. This
  is too memory consuming. (ocaml/dune#7894, fix ocaml/dune#6900, @rgrinberg)

- Remove some compatibility code for old version of dune that generated
  `.merlin` files. Now dune will never remove `.merlin` files automatically
  (ocaml/dune#7562)

- Add `dune show env` command and make `dune printenv` an alias of it. (ocaml/dune#7985,
  @Alizter)

- Add additional metadata to the traces provided by `--trace-file` whenever
  `--trace-extended` is passed (ocaml/dune#7778, @rleshchinskiy)

- Extensions used in `(dialect)` can contain periods (e.g., `cppo.ml`). (ocaml/dune#7782,
  fixes ocaml/dune#7777, @nojb)

- Allow `(include_subdirs qualified)` to be used when libraries define a
  `(modules ...)` field (ocaml/dune#7797, fixes ocaml/dune#7597, @anmonteiro)

- `$ dune describe` is now a command group, so arguments to subcommands must be
  passed after subcommand itself. (ocaml/dune#7919, @Alizter)

- The `interface` and `implementation` fields of a `(dialect)` are now optional
  (ocaml/dune#7757, @gpetiot)

- Add commands `dune show targets` and `dune show aliases` that display all the
  available targets and aliases in a given directory respectively. (ocaml/dune#7770,
  grants ocaml/dune#265, @Alizter)

- Allow multiple globs in library's `(stdlib (internal_modules ..))`
  (@anmonteiro, ocaml/dune#7878)

- Attach melange rules to the default alias (ocaml/dune#7926, @haochenx)

- In opam constraints, reject `(and)` and `(or)` with no arguments at parse
  time (ocaml/dune#7730, @emillon)

- Compute digests and manage sandboxes in background threads (ocaml/dune#7947,
  @rgrinberg)

- Add `(build_if)` to the `(test)` stanza. When it evaluates to false, the
  executable is not built. (ocaml/dune#7899, fixes ocaml/dune#6938, @emillon)

- Add necessary parentheses in generated opam constraints (ocaml/dune#7682, fixes ocaml/dune#3431,
  @Lucccyo)
@emillon emillon mentioned this issue Jun 28, 2023
Merged
emillon added a commit to emillon/opam-repository that referenced this issue Jun 28, 2023
@emillon
[new release] dune (15 packages) (3.9.0)
2e2aa14 
CHANGES:

- Validate file extension for `$ dune ocaml top-module`. (ocaml/dune#8005, fixes ocaml/dune#8004, @3Rafal)

- Include the time it takes to read/write state files when `--trace-file` is
  enabled (ocaml/dune#7960, @rgrinberg)

- Add `dune show` command group which is an alias of `dune describe`. (ocaml/dune#7946,
  @Alizter)

- Include source tree scans in the traces produced by `--trace-file` (ocaml/dune#7937,
  @rgrinberg)

- Cinaps: The promotion rules for cinaps would only offer one file at a time no
  matter how many promotions were available. Now we offer all the promotions at
  once (ocaml/dune#7901, @rgrinberg)

- Do not re-run OCaml syntax files on every iteration of the watch mode. This
  is too memory consuming. (ocaml/dune#7894, fix ocaml/dune#6900, @rgrinberg)

- Add `--all` option to `dune rpc status` to show all Dune RPC servers running.
  (ocaml/dune#8011, fix ocaml/dune#7902, @Alizter)

- Remove some compatibility code for old version of dune that generated
  `.merlin` files. Now dune will never remove `.merlin` files automatically
  (ocaml/dune#7562)

- Add `dune show env` command and make `dune printenv` an alias of it. (ocaml/dune#7985,
  @Alizter)

- Add additional metadata to the traces provided by `--trace-file` whenever
  `--trace-extended` is passed (ocaml/dune#7778, @rleshchinskiy)

- Extensions used in `(dialect)` can contain periods (e.g., `cppo.ml`). (ocaml/dune#7782,
  fixes ocaml/dune#7777, @nojb)

- Allow `(include_subdirs qualified)` to be used when libraries define a
  `(modules ...)` field (ocaml/dune#7797, fixes ocaml/dune#7597, @anmonteiro)

- `$ dune describe` is now a command group, so arguments to subcommands must be
  passed after subcommand itself. (ocaml/dune#7919, @Alizter)

- The `interface` and `implementation` fields of a `(dialect)` are now optional
  (ocaml/dune#7757, @gpetiot)

- Add commands `dune show targets` and `dune show aliases` that display all the
  available targets and aliases in a given directory respectively. (ocaml/dune#7770,
  grants ocaml/dune#265, @Alizter)

- Allow multiple globs in library's `(stdlib (internal_modules ..))`
  (@anmonteiro, ocaml/dune#7878)

- Attach melange rules to the default alias (ocaml/dune#7926, @haochenx)

- In opam constraints, reject `(and)` and `(or)` with no arguments at parse
  time (ocaml/dune#7730, @emillon)

- Compute digests and manage sandboxes in background threads (ocaml/dune#7947,
  @rgrinberg)

- Add `(build_if)` to the `(test)` stanza. When it evaluates to false, the
  executable is not built. (ocaml/dune#7899, fixes ocaml/dune#6938, @emillon)

- Add necessary parentheses in generated opam constraints (ocaml/dune#7682, fixes ocaml/dune#3431,
  @Lucccyo)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
3.9.0
2 participants
@rgrinberg @mefyl