Skip to content

chore: bump dune version to 3.18 #11478

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

Merged
merged 3 commits into from
Apr 29, 2025

Conversation

maiste
Copy link
Collaborator

@maiste maiste commented Feb 11, 2025

This PR is intended to be merged before #11275. The idea is to update the version of the dune lang first, as it implies new changes in terms of formatting. Then we can merge the PR that supports the maintenance fields on top of it.

cc @art-w

@maiste
Copy link
Collaborator Author

maiste commented Feb 12, 2025

I thought it would solve the problem, but I'm not convinced because of what the CI shows. As we need < 3.17 to have access to the formatting, it means we have to upgrade the dune-project to 3.18. However, as the version is not released yet, it will break all the local switch test as they are trying to pin 3.17.2. It is like an egg-and-chicken problem. Maybe we should wait for 3.18 to be released before doing this?

@maiste maiste added the chore Something that just needs to be done. label Feb 12, 2025
@Leonidas-from-XIV
Copy link
Collaborator

Yeah, I think that's (unfortunately) probably the best way to go about it. Let's revisit this after 3.18.

@Leonidas-from-XIV
Copy link
Collaborator

@maiste I guess now that Dune 3.18 was released, we should rebase this and tackle it for 3.19.

@maiste maiste force-pushed the chore/bump-dune-version branch from c909eba to e474db8 Compare April 7, 2025 12:50
@maiste maiste marked this pull request as ready for review April 7, 2025 12:51
@maiste
Copy link
Collaborator Author

maiste commented Apr 7, 2025

@Leonidas-from-XIV, it is up for review. I have reverted the changes about the formatting as they would be part of 3.19.
(PS: I don't understand why, but it seems that the CI decided to die today.)

@Leonidas-from-XIV
Copy link
Collaborator

I have reverted the changes about the formatting as they would be part of 3.19.

You mean the changes to how dune files are formatted? That sounds ok to me as long as it is consistent and dune fmt doesn't reformat main.

Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

I'd probably make sure that CI works (by re-running it once Github figures out its IPv6 hickups) before merging but otherwise the code looks fine to me.

@maiste maiste force-pushed the chore/bump-dune-version branch 2 times, most recently from a7a4271 to 2cf80e1 Compare April 9, 2025 14:33
@maiste
Copy link
Collaborator Author

maiste commented Apr 9, 2025

I have almost rendered the CI green (modulo the timeout bugs). The docker image wasn't working because it was using the cache instead of using opam to fetch the new version. However, I'm struggling with the micro benchmark build using nix. Even locally, there is a conflict between the spawn version in vendor and the one installed by nix. I have seen that it already happens in the past and the fix was to disable it. I tried to, but it seems to still have the problem here. The problem occurs when lang dune >= 3.17. This is the only difference that makes it break, and I don't get why. The problem is not here when doing it without nix.

@Alizter, as you have a better understanding of nix than me, could you have an idea of what could generate the conflict in nix?

@Alizter
Copy link
Collaborator

Alizter commented Apr 17, 2025

I'm not sure what is going on with nix here.

I am a bit confused at the PR however. You appear to be doing two separate things at once. When we bump the dune lang version we don't typically need to bump the version of dune we require for building dune itself, i.e. the dune-project file in the repo. It should look something like this: #10459

@Alizter
Copy link
Collaborator

Alizter commented Apr 28, 2025

@gridbugs The version of dune in the monorepo image looks out of date. Do you know how to bump it?

@Leonidas-from-XIV Leonidas-from-XIV force-pushed the chore/bump-dune-version branch from 7276080 to 6aa4ff5 Compare April 28, 2025 14:40
@Leonidas-from-XIV
Copy link
Collaborator

@Alizter I think the issue is that this action

  monorepo_benchmark_test:
    name: Build monorepo benchmark docker image
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
      - uses: whoan/docker-build-with-cache-action@v6
        with:
          image_name: monorepobenchmark
          dockerfile: bench/monorepo/bench.Dockerfile
          push_image_and_stages: on:push
          username: ocamldune
          password: "${{ secrets.DOCKER_HUB_PASSWORD }}"

is caching the image at a version where the most recent version is Dune 3.16. I can add some no-op actions to overwrite the cache but it would be nicer if we just deleted the cached image and let it regenerate a new one. I tested it locally and when you build it you get 3.18.1 (at the time of writing).

So the question is: who has access to ocamldune on Docker Hub?

@Leonidas-from-XIV Leonidas-from-XIV force-pushed the chore/bump-dune-version branch from 0cebb0c to 6aa4ff5 Compare April 29, 2025 12:36
maiste added 2 commits April 29, 2025 15:26
Signed-off-by: Etienne Marais <[email protected]>
Signed-off-by: Etienne Marais <[email protected]>
@Leonidas-from-XIV Leonidas-from-XIV force-pushed the chore/bump-dune-version branch from 3d735c6 to 97859c9 Compare April 29, 2025 13:26
Signed-off-by: Marek Kubica <[email protected]>
@Leonidas-from-XIV Leonidas-from-XIV merged commit 4eb90c2 into ocaml:main Apr 29, 2025
23 checks passed
@maiste maiste deleted the chore/bump-dune-version branch April 29, 2025 15:00
maxim092001 pushed a commit to maxim092001/dune that referenced this pull request May 3, 2025
* chore: bump dune version

Signed-off-by: Etienne Marais <[email protected]>

* chore: bump to dune 3.18

Signed-off-by: Etienne Marais <[email protected]>

* Bust the cache

Signed-off-by: Marek Kubica <[email protected]>

---------

Signed-off-by: Etienne Marais <[email protected]>
Signed-off-by: Marek Kubica <[email protected]>
Co-authored-by: Marek Kubica <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Something that just needs to be done.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants