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

[build] Bump minimal dune version to 3.6.1 #18359

Merged
merged 1 commit into from Dec 13, 2023
Merged

Conversation

ejgallego
Copy link
Member

@ejgallego ejgallego commented Dec 1, 2023

This brings mainly improvements into the directory target feature, so we can go ahead with packaging the docs properly.

Note some fixes in documentation, plus now Dune enables some more warnings by default that we address in the following way:

  • warning 67 [unused functor parameter]: we make the arguments anonymous as OCaml seems to recommend

  • warning 69 [unused-field]: we disable this warning, but should be maybe enabled and handled again. It seems that most of the triggers for this warning are legit, however the situation is delicate due to some parts of the code involved using Obj.magic or Marshall. Also quite a few of these warnings are in code that is scheduled for removal at some point.

Note that we couldn't bump to 3.5 due to
ocaml/dune#6545

@ejgallego ejgallego requested review from a team as code owners December 1, 2023 13:49
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 1, 2023
@ejgallego ejgallego added part: build The build system. kind: compatibility Changes allowing for compatibility between versions. labels Dec 1, 2023
@ejgallego
Copy link
Member Author

Unfortunately due to ocaml/dune#6545 , we either rework quite a few opens in the codebase, or bump to 3.6.1.

@ejgallego
Copy link
Member Author

Here's an example of pitfalls of testing vs versioning of the build language, as Coq didn't bump the Dune version, the bug wasn't detected at 3.5.0 release time.

@ejgallego ejgallego changed the title [build] Bump minimal dune version to 3.5 [build] Bump minimal dune version to 3.6 Dec 1, 2023
@gares
Copy link
Member

gares commented Dec 2, 2023

The patch says 3.6.1 here and 3.6.0 there. Which is the right one?

@ejgallego
Copy link
Member Author

@coqbot run full CI

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 2, 2023
@ejgallego ejgallego changed the title [build] Bump minimal dune version to 3.6 [build] Bump minimal dune version to 3.6.1 Dec 2, 2023
@ejgallego
Copy link
Member Author

The patch says 3.6.1 here and 3.6.0 there. Which is the right one?

Indeed 3.5.0 and 3.6.0 are buggy and cannot build Coq if dune lang >= 3.5.

We'll have to tweak this in the dune-project file manually.

@ejgallego ejgallego added needs: fixing The proposed code change is broken. kind: infrastructure CI, build tools, development tools. labels Dec 2, 2023
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 4, 2023
@ejgallego
Copy link
Member Author

Dep for coq-core fixed manually; IMHO it is ok as is.

@ejgallego ejgallego removed the needs: fixing The proposed code change is broken. label Dec 4, 2023
@ejgallego
Copy link
Member Author

@coqbot run full CI

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 4, 2023
@ejgallego
Copy link
Member Author

@coqbot run full CI

@ejgallego
Copy link
Member Author

This seems ready too, anyone willing to assign?

@SkySkimmer SkySkimmer added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Dec 8, 2023
@SkySkimmer SkySkimmer added this to the 8.20+rc1 milestone Dec 8, 2023
@SkySkimmer
Copy link
Contributor

Dune 3.6.1 is from 2022-11-24, debian testing has 3.11.1, should be fine to require 3.6.1.

@SkySkimmer SkySkimmer self-assigned this Dec 8, 2023
@coqbot-app coqbot-app bot added needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. and removed needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Dec 8, 2023
@github-actions github-actions bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Dec 11, 2023
This brings mainly improvements into the directory target feature, so
we can go ahead with packaging the docs properly.

Note some fixes in documentation, plus now Dune enables some more
warnings by default that we address in the following way:

- warning 67 [unused functor parameter]: we make the arguments
  anonymous as OCaml seems to recommend

- warning 69 [unused-field]: we disable this warning, but should be
  maybe enabled and handled again. It seems that most of the triggers
  for this warning are legit, however the situation is delicate due to
  some parts of the code involved using `Obj.magic` or
  `Marshall`. Also quite a few of these warnings are in code that is
  scheduled for removal at some point.

Note that we couldn't bump to 3.5 directly due to
ocaml/dune#6545
@ejgallego ejgallego added the request: full CI Use this label when you want your next push to trigger a full CI. label Dec 11, 2023
@coqbot-app coqbot-app bot removed needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. request: full CI Use this label when you want your next push to trigger a full CI. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. labels Dec 11, 2023
@SkySkimmer
Copy link
Contributor

@coqbot merge now

@coqbot-app coqbot-app bot merged commit efb1f56 into coq:master Dec 13, 2023
5 of 7 checks passed
@ejgallego ejgallego deleted the dune_35 branch December 13, 2023 12:12
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Dec 14, 2023
coqbot-app bot added a commit that referenced this pull request Dec 15, 2023
Reviewed-by: ppedrot
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
JasonGross added a commit to JasonGross/coq-packaging that referenced this pull request Dec 21, 2023
JasonGross added a commit to JasonGross/coq-packaging that referenced this pull request Dec 21, 2023
@@ -27,7 +27,7 @@ homepage: "https://coq.inria.fr/"
doc: "https://coq.github.io/doc/"
bug-reports: "https://github.com/coq/coq/issues"
depends: [
"dune" {>= "2.9"}
"dune" {>= "3.6" & >= "3.6.1"}
Copy link
Member

Choose a reason for hiding this comment

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

Why is it written like this instead of just >= "3.6.1"?

Copy link
Member Author

@ejgallego ejgallego Dec 23, 2023

Choose a reason for hiding this comment

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

As we declare (lang dune 3.6) dune adds automatically the dune >= 3.6 lower bound when auto-generating the opam file, then our bound is added, as 3.6.0 contains a bug fatal to Coq.

louiseddp pushed a commit to louiseddp/coq that referenced this pull request Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: compatibility Changes allowing for compatibility between versions. kind: infrastructure CI, build tools, development tools. part: build The build system.
Projects
Dune
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

4 participants