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

remove all `{build}` directives from dune packages in opam-repo #14185

Closed
avsm opened this issue May 27, 2019 · 17 comments
Closed

remove all `{build}` directives from dune packages in opam-repo #14185

avsm opened this issue May 27, 2019 · 17 comments

Comments

@avsm
Copy link
Member

@avsm avsm commented May 27, 2019

See ocaml/opam#3850 for background

The dune-package files installed by dune lead to runtime failures if it is subsequently downgraded and the installed packages are not recompiled. Therefore we need to replace dune {build} with just dune in opam-repository.

The change to opam-repository is quite big though:

$ grep -r '{build' packages/|grep dune|wc -l
    1135

@AltGr @kit-ty-kate @rjbou is this something that could be automated via an enhancement to opam admin add-constraints perhaps?

@mseri

This comment has been minimized.

Copy link
Member

@mseri mseri commented May 27, 2019

Oh god, but that would mean that at each dune update one has to rebuild the whole world :(

@kit-ty-kate

This comment has been minimized.

Copy link
Contributor

@kit-ty-kate kit-ty-kate commented May 27, 2019

I'm not sure to understand what this issue has to do with the build attribute. A lot of dune packages forget they require a specific version of dune (1.8 or something) and they don't have this constraint in the opam file.

@avsm

This comment has been minimized.

Copy link
Member Author

@avsm avsm commented May 27, 2019

Can you show a concrete example of a package that you are referring to @kit-ty-kate ?

@kit-ty-kate

This comment has been minimized.

Copy link
Contributor

@kit-ty-kate kit-ty-kate commented May 27, 2019

oh... ok, I thought the content of the dune-package installed had the version number specified in the local dune-package but it doesn't seem to be the case. It feels to me more like a dune bug than something to do in opam (though it's already too late I suppose :/)

@avsm

This comment has been minimized.

Copy link
Member Author

@avsm avsm commented May 27, 2019

The dune-package installed does have the version number specified -- it's just that opam isn't enforcing the version constraint due to {build}. This is my point about why {build} is so dangerous -- as software evolves, older uses of {build} can just become wrong silently.

@kit-ty-kate

This comment has been minimized.

Copy link
Contributor

@kit-ty-kate kit-ty-kate commented May 27, 2019

The dune-package installed does have the version number specified

I've tried earlier with containers.2.5 and it's not what I got.
In the source code, the content of dune-package gives:

(lang dune 1.0)

but once installed the content of the dune-package is:

(lang dune 1.9)
[...]

in my case

@avsm

This comment has been minimized.

Copy link
Member Author

@avsm avsm commented May 27, 2019

Right, because the installed dune-package uses metadata from version 1.9 of dune. Dune uses dune-package to store structured metadata about the installation and adds to it on every release, so it's backwards but not forwards compatible.

@AltGr

This comment has been minimized.

Copy link
Member

@AltGr AltGr commented May 27, 2019

it's just that opam isn't enforcing the version constraint due to {build}.

Actually, these should always keep being enforced: {build} has no effect on the solver, but only skips recompilations. What happens, if I understand correctly, is that you can have a package that works with new and old versions of dune, but once installed with a newer version, won't work after a downgrade of dune (because of installed files using the newer format). In that case, of course, a full reinstall solves the issue, removing the artifacts that depend on the newer version.

In other terms, dune isn't indeed a build-dep, technically, since your package artefacts are tied to the dune runtime installed (although it's backwards-compatible).

It would be pretty easy, though, to change the semantics of the {build} flag to only skip rebuilds when the dependency is re-installed or upgraded, but do as if the flag was absent on downgrades. That sounds quite adhoc... but errs on the safe side.

@avsm

This comment has been minimized.

Copy link
Member Author

@avsm avsm commented May 27, 2019

Thanks; in the short term I guess we have no choice but to remove the dune {build} just to prevent this. A bit of reinstallation is painful, but not the end of the world. Any idea what the easiest way to rewrite these tags are mechanically? Some opam admin magic would be awesome :-)

@kit-ty-kate

This comment has been minimized.

Copy link
Contributor

@kit-ty-kate kit-ty-kate commented Jun 10, 2019

This issue just hit the CI on this PR: #14259
Some of the revdeps fail with:

#=== ERROR while compiling ppxlib.0.8.0 =======================================#
# context              2.0.4 | linux/x86_64 | ocaml-base-compiler.4.07.1 | git+file:///home/opam/opam-repository
# path                 ~/.opam/4.07/.opam-switch/build/ppxlib.0.8.0
# command              ~/.opam/4.07/bin/dune build -p ppxlib -j 71
# exit-code            1
# env-file             ~/.opam/log/ppxlib-17-25ffec.env
# output-file          ~/.opam/log/ppxlib-17-25ffec.out
### output ###
# File "src/dune", line 18, characters 14-37:
# 18 |   (replaces   ocaml-migrate-parsetree)
#                    ^^^^^^^^^^^^^^^^^^^^^^^
# Error: "ocaml-migrate-parsetree" is not a ppx driver


#=== ERROR while compiling ppx_tools_versioned.5.2.2 ==========================#
# context              2.0.4 | linux/x86_64 | ocaml-base-compiler.4.07.1 | git+file:///home/opam/opam-repository
# path                 ~/.opam/4.07/.opam-switch/build/ppx_tools_versioned.5.2.2
# command              ~/.opam/4.07/bin/dune build -p ppx_tools_versioned -j 71
# exit-code            1
# env-file             ~/.opam/log/ppx_tools_versioned-17-2cea93.env
# output-file          ~/.opam/log/ppx_tools_versioned-17-2cea93.out
### output ###
# File "_build/default/.ppx/ppx_tools_versioned.metaquot_402/ppx.exe", line 1, characters 0-0:
# Error: Failed to create on-demand ppx rewriter for
# ppx_tools_versioned.metaquot_402; no ppx driver were found.
# Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
# File "_build/default/.ppx/ppx_tools_versioned.metaquot_403/ppx.exe", line 1, characters 0-0:
# Error: Failed to create on-demand ppx rewriter for
# ppx_tools_versioned.metaquot_403; no ppx driver were found.
# Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
# File "_build/default/.ppx/ppx_tools_versioned.metaquot_404/ppx.exe", line 1, characters 0-0:
# Error: Failed to create on-demand ppx rewriter for
# ppx_tools_versioned.metaquot_404; no ppx driver were found.
# Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
# File "_build/default/.ppx/ppx_tools_versioned.metaquot_405/ppx.exe", line 1, characters 0-0:
# Error: Failed to create on-demand ppx rewriter for
# ppx_tools_versioned.metaquot_405; no ppx driver were found.
# Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
# File "_build/default/.ppx/ppx_tools_versioned.metaquot_406/ppx.exe", line 1, characters 0-0:
# Error: Failed to create on-demand ppx rewriter for
# ppx_tools_versioned.metaquot_406; no ppx driver were found.
# Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
# File "_build/default/.ppx/ppx_tools_versioned.metaquot_407/ppx.exe", line 1, characters 0-0:
# Error: Failed to create on-demand ppx rewriter for
# ppx_tools_versioned.metaquot_407; no ppx driver were found.
# Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.

due to a ocaml-migrate-parsetree being installed with dune 1.10.0 but later downgrated to 1.6.3 when building those two packages.

I'll do a PR to remove all {build} directives manually today as soon as I can.

@kit-ty-kate

This comment has been minimized.

Copy link
Contributor

@kit-ty-kate kit-ty-kate commented Jun 10, 2019

A bit of reinstallation is painful, but not the end of the world

On a side note, I think this is going to be quite painful for users with little computer power and/or huge packages to build (e.g. if coq switches to dune or something of a similar size). But now I'm not too sure how to avoid it.

@XVilka

This comment has been minimized.

Copy link
Contributor

@XVilka XVilka commented Aug 6, 2019

Wasn't this fixed already? Probably can be closed then.

@kit-ty-kate

This comment has been minimized.

Copy link
Contributor

@kit-ty-kate kit-ty-kate commented Aug 6, 2019

It is fixed indeed, thanks for noticing

@kit-ty-kate kit-ty-kate closed this Aug 6, 2019
talex5 added a commit to talex5/prometheus that referenced this issue Aug 20, 2019
@shonfeder

This comment has been minimized.

Copy link
Contributor

@shonfeder shonfeder commented Nov 27, 2019

On a side note, I think this is going to be quite painful for users with little computer power and/or huge packages to build (e.g. if coq switches to dune or something of a similar size). But now I'm not too sure how to avoid it.

This is indeed painful: if I want to change my build tool, I have to rebuild (virtually) the entire universe from scratch... That feels incredibly kludgey. I understand this was needed as a short term fix, but I hope we also have an issue open for a long-term solution that is more sane. I haven't been able to find one tho. Could someone point the way?

@mseri

This comment has been minimized.

Copy link
Member

@mseri mseri commented Nov 27, 2019

Dune 2.0 already contains as an experimental feature a build cache (disbled by default afaik), in he future they plan to have a global binary cache which could be reused across builds and should solve this issue. See https://discuss.ocaml.org/t/ann-dune-2-0-0/4758 and the what’s next section of https://dune.build/blog/dune-2-coming-soon/

@shonfeder

This comment has been minimized.

Copy link
Contributor

@shonfeder shonfeder commented Nov 27, 2019

Thanks @mseri. I can see too that ocaml/opam#3850 address the other side of the problem, namely the counterintuitive problem of having a build tool which isn't merely a build-time dependency (by arguing that the notion of build dependency in opam is poorly defined, I guess?).

I appreciate the pointer!

@avsm

This comment has been minimized.

Copy link
Member Author

@avsm avsm commented Nov 27, 2019

by arguing that the notion of build dependency in opam is poorly defined

Indeed -- you can consider the dune cache to be a precise and automatic version of the current manual {build} annotations. It happens per-rule and considers the external environment and requires no user input beyond activating the feature (which will be defaulting to one in a few releases of dune once tested more).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.