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

Consider dune version #378

Open
reynir opened this issue Feb 23, 2023 · 12 comments
Open

Consider dune version #378

reynir opened this issue Feb 23, 2023 · 12 comments

Comments

@reynir
Copy link
Contributor

reynir commented Feb 23, 2023

As far as I understand mirage/mirage#1401 opam-monorepo doesn't consider version constraints on dune in packages because it is a "safe" package. It seems to me that the solver should take into account these constraints against the installed version of dune. What do you think?

@Leonidas-from-XIV
Copy link
Member

The problem with dune is that once you vendor something that requires a higher dune version than you have available, the whole thing falls apart, since it can't be built at all anymore due to older versions not supporting newer dune-lang versions. That's why it is generally safe[0] to use the highest dune version to be on the safe side.

As such I would suggest yomimono/chamelon#18 instead of the upper bound on dune.

[0]: Except for previously undetected bugs like in this case.

@reynir
Copy link
Contributor Author

reynir commented Feb 23, 2023

Yes, I understand. But are there many (any?) packages in opam-repository that put unnecessary upper bounds on dune? In the case of chamelon an upper bound on dune was recently added in opam-repository because chamelon doesn't compile with dune 3.7. Prior to that there was no upper bound on dune in chamelon.

While I think the package should be eventually fixed I don't think it is particularly useful or helpful that opam-monorepo decides on a solution that doesn't build.

See ocaml/opam-repository@7b2d98a.

@reynir
Copy link
Contributor Author

reynir commented Feb 28, 2023

Here's another thing I discovered today. May be related.

reynir@spurv:~/workspace/mirage-skeleton/tutorial/hello$ rm -r duniverse/
reynir@spurv:~/workspace/mirage-skeleton/tutorial/hello$ mirage configure -t hvt
reynir@spurv:~/workspace/mirage-skeleton/tutorial/hello$ make
-e using overlay repository mirage: [opam-overlays, mirage-overlays] 
[opam-overlays] no changes from git+https://github.com/dune-universe/opam-overlays.git
[NOTE] Repository opam-overlays has been added to the selections of switch
       /home/reynir/workspace/mirage-skeleton only.
       Run `opam repository add opam-overlays --all-switches|--set-default' to
       use it in all existing switches, or in newly created switches,
       respectively.

[mirage-overlays] no changes from git+https://github.com/dune-universe/mirage-opam-overlays.git
[NOTE] Repository mirage-overlays has been added to the selections of switch
       /home/reynir/workspace/mirage-skeleton only.
       Run `opam repository add mirage-overlays --all-switches|--set-default'
       to use it in all existing switches, or in newly created switches,
       respectively.

 ↳ generate lockfile for monorepo dependencies
==> Using 1 locally scanned package as the target.
[WARNING] Unknown variable "ocaml-system:version"
[WARNING] Unknown variable "ocaml-base-compiler:version"
[WARNING] Unknown variable "ocaml-variants:version"
[WARNING] Unknown variable "ocaml-system:version"
[WARNING] Unknown variable "ocaml-base-compiler:version"
[WARNING] Unknown variable "ocaml-variants:version"
==> Found 70 opam dependencies for the target package.
==> Querying opam database for their metadata and Dune compatibility.
==> Calculating exact pins for each of them.
==> Wrote lockfile with 34 entries to mirage/hello-hvt.opam.locked. You can now run opam monorepo pull to fetch their sources.
-e removing overlay repository [opam-overlays, mirage-overlays]
Repositories removed from the selections of switch /home/reynir/workspace/mirage-skeleton. Use '--all' to forget about them altogether.
Repositories removed from the selections of switch /home/reynir/workspace/mirage-skeleton. Use '--all' to forget about them altogether.
 ↳ opam install switch dependencies
Nothing to do.
 ↳ install external dependencies for monorepo
==> Using lockfile mirage/hello-hvt.opam.locked
 ↳ fetch monorepo rependencies in the duniverse folder
==> Using lockfile mirage/hello-hvt.opam.locked
Successfully pulled 34/34 repositories
mirage build
File "duniverse/dune_/dune-project", line 1, characters 11-14:
1 | (lang dune 3.5)
               ^^^
Error: Version 3.5 of the dune language is not supported.
Supported versions of this extension in version 3.5 of the dune language:
- 1.0 to 1.12
- 2.0 to 2.9
- 3.0 to 3.4
make[1]: *** [Makefile:56: build] Error 1
make: *** [Makefile:11: all] Error 2
reynir@spurv:~/workspace/mirage-skeleton/tutorial/hello$ dune --version
3.4.1

I wasn't aware opam-monorepo pulls in dune. Dune 3.4.1 was released 7 months ago.

@Leonidas-from-XIV
Copy link
Member

@reynir Is dune in the lockfile that was created?

@reynir
Copy link
Contributor Author

reynir commented Mar 21, 2023

It's been a few weeks, but I tried to recreate - and yes. Dune is in the lockfile (see hello-hvt.opam.locked.txt). This puzzles me a bit because I didn't think the flow was to build and run the dune in duniverse/dune_/.

@Leonidas-from-XIV
Copy link
Member

Looking at the lockfile, it isn't really dune (it's not marked as ?vendor), but dune-configurator that is being vendored, but given they are in the same repo, opam-monorepo unpacks it into the dune_ folder (to not conflict with the dune file).

@reynir
Copy link
Contributor Author

reynir commented Mar 22, 2023

Any idea why the lock file says dune 3.7.0? Is it again #331?

@Leonidas-from-XIV
Copy link
Member

Yes, it is. It has to, because if dune-configurator is vendored, it comes with a copy of dune, so the version of dune has to be the same version to match the tarball.

But even bar #331 the problem would be the similar, as to build dune-configurator 3.7.0 you need at least dune 3.5, so the system dune would fail. Of course its a bit more generous release time-wise than requiring 3.7, but conceptually it is a separate problem from #331.

The way to go as I see it, is to install the non-vendored dependencies before building. I am puzzled why in your case it says

 ↳ opam install switch dependencies
Nothing to do.

(Looking at the lockfile though, the solution it came up with is pretty bad, there's plenty of +dune versioned packages that are not to be vendored, which is weird given that that was the whole point of porting them to dune. Looks to me like there is some not-to-be-vendored package that depends on uutf, rresult, fpath etc so the solver decides to install them in the switch)

@reynir
Copy link
Contributor Author

reynir commented Mar 22, 2023

I don't understand why dune-configurator.3.7.0 got selected in the first place when dune.3.4.1 is installed. dune.3.4.1 doesn't satisfy "dune" {>= "3.5"}.

@Leonidas-from-XIV
Copy link
Member

opam-monorepo doesn't look at the system's dune version when building a solution, it assumes an empty slate.

@reynir
Copy link
Contributor Author

reynir commented Mar 22, 2023

What is the flow then? Is opam-monorepo expected to upgrade dune in the switch? Are you supposed to build dune from duniverse? If the system dune version isn't taken into account and isn't upgraded it seems broken to use it anyway as described in the usage section of the README: https://github.com/tarides/opam-monorepo/#usage

@Leonidas-from-XIV
Copy link
Member

You'll need to run opam install on the lockfile before pull to install all non-vendored packages into the switch. In the best case that's only dune but since #234 more packages can be marked as to be installed into the OPAM switch.

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

No branches or pull requests

2 participants