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.

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

fix(opam): cleanly reject argumentless (and)/(or) #7730

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented May 15, 2023

Otherwise they would raise a code error later.

@Alizter
Copy link
Collaborator

Alizter commented May 15, 2023

We could give them meaning by setting an "empty or" to false and an "empty and" to true. However this would give us redundancy with saying true and false.

@emillon emillon marked this pull request as ready for review May 15, 2023 10:37
@rgrinberg
Copy link
Member

Can we guard this change behind 3.8?

@emillon
Copy link
Collaborator Author

emillon commented May 16, 2023

I'm not sure we need to:

  • if opam file generation is enabled, even if it is parsed, this crashes during translation
  • if it is not enabled, I don't think that (depends) and the like are used

So I don't think that this changes which files are accepted, it just improves the error message (and makes it a bit earlier).

@rgrinberg
Copy link
Member

But they are still parsed I think.

So this situation:

if it is not enabled, I don't think that (depends) and the like are used

Would still cause an existing project to stop building.

@emillon
Copy link
Collaborator Author

emillon commented May 16, 2023

Ah, of course 🤦🏻

@rgrinberg rgrinberg marked this pull request as draft May 25, 2023 08:03
@emillon emillon marked this pull request as ready for review June 9, 2023 14:17
@emillon emillon requested a review from rgrinberg June 9, 2023 14:17
Otherwise they would raise a code error later.

Signed-off-by: Etienne Millon <[email protected]>
@emillon emillon merged commit 6501f42 into ocaml:main Jun 12, 2023
@emillon emillon deleted the fix-opam-and-no-argument branch June 12, 2023 07:51
emillon added a commit to emillon/opam-repository that referenced this pull request Jun 23, 2023
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 added a commit to emillon/opam-repository that referenced this pull request Jun 28, 2023
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants