Skip to content

Make the project build using the committed lock dir#185

Merged
Leonidas-from-XIV merged 5 commits intoocaml-dune:mainfrom
Leonidas-from-XIV:build-with-dune-pkg
Mar 5, 2026
Merged

Make the project build using the committed lock dir#185
Leonidas-from-XIV merged 5 commits intoocaml-dune:mainfrom
Leonidas-from-XIV:build-with-dune-pkg

Conversation

@Leonidas-from-XIV
Copy link
Contributor

Since there is a lock.dir in the project and Dune can build the project with package management, this changes the Dockerfile to use dune pkg for building it.

Currently it is experiencing some kind of issue:

0.213 File "dune.lock/lock.dune", line 1, characters 0-0:                                                                                                                                                                                                                       
0.213 Error: The lock dir is not sync with your dune-project
0.213 Hint: run dune pkg lock

I do not experience the issue outside the Docker container and it works when using autolocking so there seems to be some kind of issue with the lock directory.

@Leonidas-from-XIV
Copy link
Contributor Author

While working on this I ran into a bit of a painful situation where the dependencies on my machine don't match the dependencies in the Docker container, so I've revived this PR.

This should make sure that the dependencies match. This will allow us to drop the usage of OPAM from this repo.

@Alizter
Copy link
Collaborator

Alizter commented Feb 26, 2026

Why can't we download the binary directly? Building dune here seems like a waste of time?

@Leonidas-from-XIV
Copy link
Contributor Author

@Alizter The issue is that this is the project that creates the binaries, thus downloading the binary would create a circular dependency. I could install a Dune from the system (Alpine 3.23 ships with 3.20.2 which is not too old), which would avoid this issue?

Signed-off-by: Marek Kubica <marek@tarides.com>
Signed-off-by: Marek Kubica <marek@tarides.com>
@Alizter
Copy link
Collaborator

Alizter commented Mar 3, 2026

I suppose the better solution would be to have dedicated docker images with the released version of dune in them, but that seems more trouble than its worth. I think using an older system version of dune seems fine.

Signed-off-by: Marek Kubica <marek@tarides.com>
@Leonidas-from-XIV
Copy link
Contributor Author

As discussed with @Alizter and @Sudha247 the issue at the moment is that Alpine ships 3.20.1 but dune pkg lock will create a lock file that features dune-configurator.3.21 (at the moment) which requires dune-lang 3.21. Thus the Dune in Alpine can't build it as it has no support for dune-lang 3.21.

This is a problem in general that the Dune solver might pick versions of packages that don't work with the Dune version at hand, I will create a ticket in Dune about this.

I've worked around this for now by forcing the lock dir to use dune-configurator < 3.21.

@shonfeder
Copy link
Member

the better solution would be to have dedicated docker images with the released version of dune in them

We have a docker container with dune build thanks to earlier work from Sudha: https://github.com/ocaml/dune/pkgs/container/dune

Would using this, with the latest tag, perhaps solve most of the problems discussed here?

apk add curl git musl-dev linux-headers make dune gcc
WORKDIR /home/sandworm
COPY . .
RUN sed -i /dependency_hash/d dune.lock/lock.dune
Copy link
Member

@shonfeder shonfeder Mar 3, 2026

Choose a reason for hiding this comment

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

Could you add a comment noting why this is necessary?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Leonidas-from-XIV mentioned that the dependency hash was being out of sync in the docker container and he had yet to determine the cause. I believe this line was an attempt at removing it. I'm not particularly sure what caused this issue yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's exactly right. If I leave it out I get the error that's mentioned in the opening message of the PR.

I plan to create a repro case for this issue (which seems like a Dune bug) which should help debug this issue in a separate repo.

Copy link
Member

Choose a reason for hiding this comment

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

Ok! Sounds like this is still under investigation. If ends up needing to be in the final state of the PR, I just request it be with a comment explaining why it is needed.

Copy link
Member

Choose a reason for hiding this comment

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

That makes it clear! Linking to the issue tracking this bug will make it easier to know when this workaround can be removed.

Signed-off-by: Marek Kubica <marek@tarides.com>
@Leonidas-from-XIV Leonidas-from-XIV marked this pull request as draft March 4, 2026 13:32
Signed-off-by: Marek Kubica <marek@tarides.com>
@Leonidas-from-XIV Leonidas-from-XIV marked this pull request as ready for review March 4, 2026 14:38
apk add curl git musl-dev linux-headers make dune gcc
WORKDIR /home/sandworm
COPY . .
RUN sed -i /dependency_hash/d dune.lock/lock.dune
Copy link
Member

Choose a reason for hiding this comment

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

That makes it clear! Linking to the issue tracking this bug will make it easier to know when this workaround can be removed.

@Leonidas-from-XIV Leonidas-from-XIV merged commit b423628 into ocaml-dune:main Mar 5, 2026
3 checks passed
@Leonidas-from-XIV Leonidas-from-XIV deleted the build-with-dune-pkg branch March 5, 2026 10:28
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.

4 participants