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

[coq] Allow to disable Coq language deprecation warning. #9439

Merged
merged 1 commit into from
Dec 15, 2023
Merged

[coq] Allow to disable Coq language deprecation warning. #9439

merged 1 commit into from
Dec 15, 2023

Conversation

ejgallego
Copy link
Collaborator

I had to adapt the warnings infra a little bit, IMHO that is fine, tho it would be nice to see if we can recover the vendor behavior.

I also corrected the hint message a little bit, as it was a bit confusing IMHO, but will place that in the next commit as to keep diffs tidy.

@ejgallego ejgallego added the coq label Dec 9, 2023
@ejgallego ejgallego force-pushed the warning_coq_deprecated branch 2 times, most recently from e3ea3c8 to e9f5c61 Compare December 9, 2023 09:58
@ejgallego ejgallego requested a review from rgrinberg December 9, 2023 09:58
@ejgallego
Copy link
Collaborator Author

An alternative to this is to add a Deprecated_in constructor to Syntax.Supported_versions, tho the logic would seem mainly orthogonal, as we would still need to switch the deprecation warning to the Warning_emit interface.

@ejgallego ejgallego force-pushed the warning_coq_deprecated branch from e9f5c61 to baa301d Compare December 9, 2023 10:28
@Alizter
Copy link
Collaborator

Alizter commented Dec 9, 2023 •
edited
Loading

Why do we want users to disable this warning? i.e. what's the point of the warning if it just gets ignored?

@ejgallego
Copy link
Collaborator Author

Why do we want users to disable this warning? i.e. what's the point of the warning if it just gets ignored?

I am not sure I understand your question @Alizter , doesn't what you say apply to all warnings?

There are many reasons to disable this warning, in particular for example in Coq we still require a lower bound of 3.6

@Alizter
Copy link
Collaborator

Alizter commented Dec 9, 2023

@ejgallego Then I am a bit confused why we marked it as deprecated to begin with?

@ejgallego
Copy link
Collaborator Author

We marked it as deprecated because it is going a) broken b) going to be removed in a future version.

@ejgallego ejgallego force-pushed the warning_coq_deprecated branch from baa301d to 2c040fa Compare December 13, 2023 12:41
@ejgallego
Copy link
Collaborator Author

@rgrinberg thoughts on this?

rgrinberg
rgrinberg approved these changes Dec 13, 2023
View reviewed changes
Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The code looks fine. I didn't delve into the details of what is the warning about or if it's needed, but the way it's introduced and used is correct.

@ejgallego
Copy link
Collaborator Author

Thanks! I'll ask Ali in private about his concerns with the warning and merge once he's OK.

@ejgallego ejgallego force-pushed the warning_coq_deprecated branch from 2c040fa to c5f978c Compare December 15, 2023 11:25
@ejgallego
[coq] Allow to disable Coq language deprecation warning.
bf10c0e 
I had to adapt the warnings infra a little bit, IMHO that is fine, tho
it would be nice to see if we can recover the vendor behavior.

I also corrected the hint message a little bit, as it was a bit
confusing IMHO, but will place that in the next commit as to keep
diffs tidy.

Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
@ejgallego ejgallego force-pushed the warning_coq_deprecated branch from c5f978c to bf10c0e Compare December 15, 2023 11:30
@ejgallego
Copy link
Collaborator Author

Pushed the changes to infra to other PRs, @Alizter I've sent you a message to try to understand what your concerns are.

Other than that, I think this is ready.

@ejgallego
Copy link
Collaborator Author

Ali point was "why people cannot upgrade to Coq 0.8", and indeed, most people should, but in the real world a few people is stuck due to poor reasons. So we let them opt out of the warning for now, at their own risk.

@ejgallego ejgallego merged commit 0fb3b9b into ocaml:main Dec 15, 2023
24 checks passed
@ejgallego ejgallego deleted the warning_coq_deprecated branch December 15, 2023 11:44
@emillon emillon mentioned this pull request Jan 12, 2024
Closed
emillon added a commit to emillon/opam-repository that referenced this pull request Jan 12, 2024
@emillon
[new release] dune (15 packages) (3.13.0~alpha1)
fa3e075 
CHANGES:

- Do not ignore `(formatting ..)` settings in context or workspace files
  (ocaml/dune#8447, @rgrinberg)

- Add command `dune cache clear` to completely delete all traces of the Dune
  cache. (ocaml/dune#8975, @nojb)

- Fixed a bug where Dune was incorrectly parsing the output of coqdep when it
  was escaped, as is the case on Windows. (ocaml/dune#9231, fixes ocaml/dune#9218, @Alizter)

- Copying mode for sandboxes will now follow symbolic links (ocaml/dune#9282, @rgrinberg)

- Forbid the empty `(binaries ..)` field in the `env` stanza in the workspace
  file unless language version is at least 3.2.

- [coq] Fix bug in computation of flags when composed with boot theories.
  (ocaml/dune#9347, fixes ocaml/dune#7909, @ejgallego)

- Fixed a bug where the `(select)` field of the `(libraries)` field of the
  `(test)` stanza wasn't working properly. (ocaml/dune#9387, fixes ocaml/dune#9365, @Alizter)

- Allow to disable Coq 0.8 deprecation warning (ocaml/dune#9439, @ejgallego)

- Fix handling of the `PATH` argument to `dune init proj NAME PATH`. An
  intermediate directory called `NAME` is no longer created if `PATH` is
  supplied, so `dune init proj my_project .` will now initialize a project in
  the current working directory. (ocaml/dune#9447, fixes ocaml/dune#9209, @shonfeder)

- Allow `OCAMLFIND_TOOLCHAIN` to be set per context in the workspace file
  through the `env` stanza. (ocaml/dune#9449, @rgrinberg)

- Experimental doc rules: Correctly handle the case when a package depends upon
  its own sublibraries (ocaml/dune#9461, fixes ocaml/dune#9456, @jonludlam)

- Resolve various public binaries to their build location, rather than to where
  they're copied in the `_build/install` directory (ocaml/dune#9496, fixes ocaml/dune#7908,
  @rgrinberg).

- Menhir: generate `.conflicts` file by default. Add new field to the
  `(menhir)` stanza to control the generation of this file: `(explain <blang
  expression>)`. Introduce `(menhir (flags ...) (explain ...))` field in the
  `(env)` stanza, delete `(menhir_flags)` field. All changes are guarded under
  a new version of the Menhir extension, 3.0. (ocaml/dune#9512, @nojb)

- Correctly ignore warning flags in vendored projects (ocaml/dune#9515, @rgrinberg)

- Directory targets can now be caches. (ocaml/dune#9535, @rleshchinskiy)

- Remove warning 30 from default set for projects where dune lang is at least
  3.13 (ocaml/dune#9568, @gasche)

- It is now possible to use special forms such as `(:include)` and variables
  `%{read-lines:}` in `(modules)` and similar fields. Note that the
  dependencies introduced in this way (ie the files being read) must live in a
  different directory than the stanza making use of them. (ocaml/dune#9578, @nojb)

- Use watch exclusions in watch mode on MacOS (ocaml/dune#9643, fixes ocaml/dune#9517,
  @PoorlyDefinedBehaviour)

- Fix merlin configuration for `(include_subdirs qualified)` modules (ocaml/dune#9659,
  fixes ocaml/dune#8297, @rgrinberg)

- Fix handling of `enabled_if` in binary install stanzas. Previously, we'd
  ignore the result of `enabled_if` when evaluating `%{bin:..}` (ocaml/dune#9707,
  @rgrinberg)

- Add `coqdoc_flags` field to `coq` field of `env` stanza allowing the setting
  of workspace-wide defaults for `coqdoc_flags`. (ocaml/dune#9280, fixes ocaml/dune#9139, @Alizter)

- ctypes: fix an error where `(ctypes)` with no `(function_description)` would
  cause an error trying refer to a nonexistent `_stubs.a` dependency (ocaml/dune#9302,
  fix ocaml/dune#9300, @emillon)
emillon added a commit to emillon/opam-repository that referenced this pull request Jan 16, 2024
@emillon
[new release] dune (15 packages) (3.13.0)
9fc3fa0 
CHANGES:

### Added

- Add command `dune cache clear` to completely delete all traces of the Dune
  cache. (ocaml/dune#8975, @nojb)

- Allow to disable Coq 0.8 deprecation warning (ocaml/dune#9439, @ejgallego)

- Allow `OCAMLFIND_TOOLCHAIN` to be set per context in the workspace file
  through the `env` stanza. (ocaml/dune#9449, @rgrinberg)

- Menhir: generate `.conflicts` file by default. Add new field to the
  `(menhir)` stanza to control the generation of this file: `(explain <blang
  expression>)`. Introduce `(menhir (flags ...) (explain ...))` field in the
  `(env)` stanza, delete `(menhir_flags)` field. All changes are guarded under
  a new version of the Menhir extension, 3.0. (ocaml/dune#9512, @nojb)

- Directory targets can now be cached. (ocaml/dune#9535, @rleshchinskiy)

- It is now possible to use special forms such as `(:include)` and variables
  `%{read-lines:}` in `(modules)` and similar fields. Note that the
  dependencies introduced in this way (ie the files being read) must live in a
  different directory than the stanza making use of them. (ocaml/dune#9578, @nojb)

- Remove warning 30 from default set for projects where dune lang is at least
  3.13 (ocaml/dune#9568, @gasche)

- Add `coqdoc_flags` field to `coq` field of `env` stanza allowing the setting
  of workspace-wide defaults for `coqdoc_flags`. (ocaml/dune#9280, fixes ocaml/dune#9139, @Alizter)

- ctypes: fix an error where `(ctypes)` with no `(function_description)` would
  cause an error trying refer to a nonexistent `_stubs.a` dependency (ocaml/dune#9302,
  fix ocaml/dune#9300, @emillon)

### Changed

- Check that package names in `(depends)` and related fields in `dune-project`
  are well-formed. (ocaml/dune#9472, fixes ocaml/dune#9270, @ElectreAAS)

### Fixed

- Do not ignore `(formatting ..)` settings in context or workspace files
  (ocaml/dune#8447, @rgrinberg)

- Fixed a bug where Dune was incorrectly parsing the output of coqdep when it
  was escaped, as is the case on Windows. (ocaml/dune#9231, fixes ocaml/dune#9218, @Alizter)

- Copying mode for sandboxes will now follow symbolic links (ocaml/dune#9282, @rgrinberg)

- Forbid the empty `(binaries ..)` field in the `env` stanza in the workspace
  file unless language version is at least 3.2.

- [coq] Fix bug in computation of flags when composed with boot theories.
  (ocaml/dune#9347, fixes ocaml/dune#7909, @ejgallego)

- Fixed a bug where the `(select)` field of the `(libraries)` field of the
  `(test)` stanza wasn't working properly. (ocaml/dune#9387, fixes ocaml/dune#9365, @Alizter)

- Fix handling of the `PATH` argument to `dune init proj NAME PATH`. An
  intermediate directory called `NAME` is no longer created if `PATH` is
  supplied, so `dune init proj my_project .` will now initialize a project in
  the current working directory. (ocaml/dune#9447, fixes ocaml/dune#9209, @shonfeder)

- Experimental doc rules: Correctly handle the case when a package depends upon
  its own sublibraries (ocaml/dune#9461, fixes ocaml/dune#9456, @jonludlam)

- Resolve various public binaries to their build location, rather than to where
  they're copied in the `_build/install` directory (ocaml/dune#9496, fixes ocaml/dune#7908,
  @rgrinberg).

- Correctly ignore warning flags in vendored projects (ocaml/dune#9515, @rgrinberg)

- Use watch exclusions in watch mode on MacOS (ocaml/dune#9643, fixes ocaml/dune#9517,
  @PoorlyDefinedBehaviour)

- Fix merlin configuration for `(include_subdirs qualified)` modules (ocaml/dune#9659,
  fixes ocaml/dune#8297, @rgrinberg)

- Fix handling of `enabled_if` in binary install stanzas. Previously, we'd
  ignore the result of `enabled_if` when evaluating `%{bin:..}` (ocaml/dune#9707,
  @rgrinberg)
@emillon emillon mentioned this pull request Jan 16, 2024
Merged
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
@emillon @nberth
[new release] dune (15 packages) (3.13.0)
841c213 
CHANGES:

### Added

- Add command `dune cache clear` to completely delete all traces of the Dune
  cache. (ocaml/dune#8975, @nojb)

- Allow to disable Coq 0.8 deprecation warning (ocaml/dune#9439, @ejgallego)

- Allow `OCAMLFIND_TOOLCHAIN` to be set per context in the workspace file
  through the `env` stanza. (ocaml/dune#9449, @rgrinberg)

- Menhir: generate `.conflicts` file by default. Add new field to the
  `(menhir)` stanza to control the generation of this file: `(explain <blang
  expression>)`. Introduce `(menhir (flags ...) (explain ...))` field in the
  `(env)` stanza, delete `(menhir_flags)` field. All changes are guarded under
  a new version of the Menhir extension, 3.0. (ocaml/dune#9512, @nojb)

- Directory targets can now be cached. (ocaml/dune#9535, @rleshchinskiy)

- It is now possible to use special forms such as `(:include)` and variables
  `%{read-lines:}` in `(modules)` and similar fields. Note that the
  dependencies introduced in this way (ie the files being read) must live in a
  different directory than the stanza making use of them. (ocaml/dune#9578, @nojb)

- Remove warning 30 from default set for projects where dune lang is at least
  3.13 (ocaml/dune#9568, @gasche)

- Add `coqdoc_flags` field to `coq` field of `env` stanza allowing the setting
  of workspace-wide defaults for `coqdoc_flags`. (ocaml/dune#9280, fixes ocaml/dune#9139, @Alizter)

- ctypes: fix an error where `(ctypes)` with no `(function_description)` would
  cause an error trying refer to a nonexistent `_stubs.a` dependency (ocaml/dune#9302,
  fix ocaml/dune#9300, @emillon)

### Changed

- Check that package names in `(depends)` and related fields in `dune-project`
  are well-formed. (ocaml/dune#9472, fixes ocaml/dune#9270, @ElectreAAS)

### Fixed

- Do not ignore `(formatting ..)` settings in context or workspace files
  (ocaml/dune#8447, @rgrinberg)

- Fixed a bug where Dune was incorrectly parsing the output of coqdep when it
  was escaped, as is the case on Windows. (ocaml/dune#9231, fixes ocaml/dune#9218, @Alizter)

- Copying mode for sandboxes will now follow symbolic links (ocaml/dune#9282, @rgrinberg)

- Forbid the empty `(binaries ..)` field in the `env` stanza in the workspace
  file unless language version is at least 3.2.

- [coq] Fix bug in computation of flags when composed with boot theories.
  (ocaml/dune#9347, fixes ocaml/dune#7909, @ejgallego)

- Fixed a bug where the `(select)` field of the `(libraries)` field of the
  `(test)` stanza wasn't working properly. (ocaml/dune#9387, fixes ocaml/dune#9365, @Alizter)

- Fix handling of the `PATH` argument to `dune init proj NAME PATH`. An
  intermediate directory called `NAME` is no longer created if `PATH` is
  supplied, so `dune init proj my_project .` will now initialize a project in
  the current working directory. (ocaml/dune#9447, fixes ocaml/dune#9209, @shonfeder)

- Experimental doc rules: Correctly handle the case when a package depends upon
  its own sublibraries (ocaml/dune#9461, fixes ocaml/dune#9456, @jonludlam)

- Resolve various public binaries to their build location, rather than to where
  they're copied in the `_build/install` directory (ocaml/dune#9496, fixes ocaml/dune#7908,
  @rgrinberg).

- Correctly ignore warning flags in vendored projects (ocaml/dune#9515, @rgrinberg)

- Use watch exclusions in watch mode on MacOS (ocaml/dune#9643, fixes ocaml/dune#9517,
  @PoorlyDefinedBehaviour)

- Fix merlin configuration for `(include_subdirs qualified)` modules (ocaml/dune#9659,
  fixes ocaml/dune#8297, @rgrinberg)

- Fix handling of `enabled_if` in binary install stanzas. Previously, we'd
  ignore the result of `enabled_if` when evaluating `%{bin:..}` (ocaml/dune#9707,
  @rgrinberg)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgrinberg rgrinberg rgrinberg approved these changes

Assignees
No one assigned
Labels
coq
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ejgallego @Alizter @rgrinberg