opam icon indicating copy to clipboard operation
opam copied to clipboard

deprecate {build} directive in opam 2.1?

Open avsm opened this issue 6 years ago • 10 comments

The {build} directive seems harmful in almost any use of it, since it's often unclear what version dependencies are introduced that are totally independent of the version of the software involved.

The latest casualty is dune, which installs a dune-package file per package that is versioned so that newer features (like variants) can be supported. Dune is marked as {build} in >1500 packages in opam-repository, which leads to this situation when we downgrade from dune 1.8.0 to dune 1.2.0:

# File "/home/opam/.opam/4.07/lib/containers/dune-package", line 1, characters 11-14:
# 1 | (lang dune 1.8)
#                ^^^
# Error: Version 1.8 of dune is not supported.
# Supported versions:
# - 0.0
# - 1.0 to 1.7

Part of the problem here is that the older dune packages marked as {build} might have been safe, but they subsequently become unsafe when version-specific functionality is added in later dunes.

Unless someone can come up with a compelling use case for {build} that is robust to future changes in the packages marked as such, I think we should deprecate it. Are perhaps conf-packages one such example?

avsm avatar May 27 '19 09:05 avsm

The underlying problem might be that with a constraint like:

depends: [ "dune" {build & >="1.8.0"} ]

If dune is downgraded to 1.7.0, the package here should be recompiled since it now violates the >="1.8.0" constraint even though it was just a build dependency. That might be conservative enough to let us keep {build} safely.

avsm avatar May 27 '19 13:05 avsm

See my comment on https://github.com/ocaml/opam-repository/issues/14185. In your example above, a downgrade of dune won't be allowed (although they appear in the same formula, the {build} tag and the version constraint are mostly independant). The issue appears if you put e.g. >= "1.7.0", install your package with dune 1.8.0 then downgrade dune to 1.7.0: this is valid for opam, but dune 1.8 installed files that dine 1.7 can not read.

I can see three solutions:

  • fix dune in a way such that a package that can be installed with dune version V is installed in the same way by any newer version of dune (that would make it a proper build-dep, insofar as all allowed versions of dune behave in a compatible way)
  • remove the build flags, of course. People will get angry at every release of dune, run opam upgrade less often and heat the planet. Just trolling :D , we should of course avoid the breakage if that's the cost ; but I still think that the {build} is useful in avoiding recompilations, if we can make it safer
  • tweak opam so that {build} is ignored in the case of downgrades, and still does the recompilations.

That last solution would work well in the case of dune, but requires an update of opam, so it's only a mid/long term solution. If dune can't be fixed, we could try to combine solutions 2 and 3 by renaming the field, so that current opams ignore the flag, but the new release still skips rebuilds when it can.

I am curious, do you have other examples of such a failure besides dune ?

One would need to dig deep in the tracker, but I remember that the initial idea was to mark build tools as not triggering rebuilds, and it was later decided that was not flexible enough and that should be marked at the dependency level (because you could legitimately use the same package as a build dependency or a full dependency). Allowing later removal (or change to incompatible version) of a build dep was also discussed, but I decided against it for reproducibility reasons. We did not plan that build dependency could escape their scope and we would get very little control over that, though :)

AltGr avatar May 27 '19 15:05 AltGr

Thanks for laying out all the options (cc @diml @rgrinberg as they may have Dune opinions on this matter).

My instinct here is to keep to the conservative spirit of the source-based package management that is opam and just do the rebuilds. We're going to have both a dune binary cache soon and the opam cache in the future, so the cost of this will disappear. On the other side, the subtle semantics of {build} seem to get ever more complex and ever less optimal as special cases creep in, so I just don't think I'd miss it that much :-)

Other cases of it biting us were (iirc) its use in ppx converters, since we decided that a material change in them would affect the code generation and therefore require a rebuild. I really can't find too many cases for them except for conf packages, which have minimal rebuild time anyway unless they are running heavy configure scripts.

avsm avatar May 27 '19 17:05 avsm

In the specific case of Dune, I agree that we should make Dune able to read dune-package files produced by a future version of Dune. We actually have a RFC for it: https://github.com/ocaml/dune/issues/2147.

That said, I agree with @avsm that {build} is often misunderstood and misused. The main problem is that it is too general. The safest way to interpret it is: if X is a build only dependency, then the build artifacts will be exactly the same no matter the version of X. This is clearly not true in general.

In any case, the artifact cache will indeed diminish the usefulness of {build}.

ghost avatar May 28 '19 08:05 ghost

The safest way to interpret it is: if X is a build only dependency, then the build artifacts will be exactly the same no matter the version of X. This is clearly not true in general. Indeed. We could expand a little, but this is the safest. The manual goes build dependencies are no longer needed at run-time: they won't trigger recompilations of your package., which clearly isn't explicit enough.

conf packages, which have minimal rebuild time anyway unless they are running heavy configure scripts. it's the built time of their reverse dependencies that count, though. conf packages already have a dedicated flag, so we could use that.

I get it for the case of ppx rewriters, which should typically not be build deps, at least not if they might change your interfaces or if they have a companion lib. And you probably want to recompile whenever they change, just like you would for a library anyway.

The main problem here seems to me that the flag is only controlled by the user, sometimes misunderstood, and therefore can put the maintainer of the build tool in a delicate situation. Also, it's on-upgrade nature makes it hard to detect through CI, which really doesn't help...

Build caches would help, by globally reducing the number of recompilations needed, but — as far as the opam cache is concerned — removing (proper) build deps would actually make the cache less useful and bigger, since we would need to recompile and store at least once for every version of the build dep. Of course, the dune cache will probably be able to really check if your artefacts actually are the same with the two versions of the build dep, which would indeed really help.

A much more precise way to do, if needed, would be to put back the property into the build tool package definition (not the consumers), and allow to specify exactly what versions you are compatible with. This would allow pushing updates while saying I guarantee that this new version of my build tool is binary compatible and that you won't need to recompile. Probably too heavy/verbose to be worth it though ?

AltGr avatar May 29 '19 13:05 AltGr

A much more precise way to do, if needed, would be to put back the property into the build tool package definition (not the consumers), and allow to specify exactly what versions you are compatible with. This would allow pushing updates while saying I guarantee that this new version of my build tool is binary compatible and that you won't need to recompile. Probably too heavy/verbose to be worth it though ?

Yh, it's also quite limited. For instance, we are currently distributing a library alongside dune (dune.configurator) and we are planning to distribute more in the future (build info, relocation, ...). Given that OCaml store hashes of dependencies, it is not possible to declare a library one depends on as a build only dependency. Alternatively, we could distribute these libraries as separate packages, though that's doesn't feel great :/

ghost avatar May 29 '19 17:05 ghost

For reference, the original discussion for the feature is in #1231

AltGr avatar Jun 12 '19 09:06 AltGr

We had quite a long discussion about this on Friday:

  • On one hand, there are definite cases where the {build} predicate is correctly used, and its removal would cause pain (its usage with ocamlfind is correct, and @AltGr is fairly sure it's being used in opam-bin correctly as well)
  • On the other hand, it's quite clear that users do not understand this predicate properly - including power users!

It is possible that a major version bump of opam might consider renaming this variable, but removal of the functionality it exposes seems unlikely. However, we had these suggestions for imminent features:

Documentation

The build variable is introduced in: https://github.com/ocaml/opam/blob/ff3cb8bcca3ac293d14a36aed92cd3a7ebec59b0/doc/pages/Manual.md#L529-L531 and expanded on at: https://github.com/ocaml/opam/blob/ff3cb8bcca3ac293d14a36aed92cd3a7ebec59b0/doc/pages/Manual.md#L931-L932 and covered in the Packaging guide at: https://github.com/ocaml/opam/blob/ff3cb8bcca3ac293d14a36aed92cd3a7ebec59b0/doc/pages/Packaging.md#L191-L195

This documentation should be expanded on, with examples of:

  • legitimate: if foo.1 is upgraded to foo.2 and installed artefacts of all the revdeps in the switch when built with foo.2 are identical to the artefacts already installed when it was built foo.1, then {build} is appropriate for the revdeps. An example is ocamlfind, when being used as ocamlfind install where ocamlfind install ... has the same effect in all versions of ocamlfind "ever".
  • grey: situations where a packager might not care about dependency changes, but proceed with caution. Examples here include ppx drivers or even, as in ca-certs-nss, packages distributing executables.
  • incorrect: Dune is an example, where the downgrading of Dune must recompile packages since dune-package cannot be read by an older version of dune.

Linting

At the moment, packages submitted to opam-repository are manually checked for "dune" {build}. A new flag, named something like always-rebuild-revdeps, could compatibly be added for use in dune.opam which would indicate that the "dune" package cannot be used with the build variable. This flag:

  • would not be revealed to the solver (i.e. specifying "dune" {build} would not cause the solver to try to select a version of the dune package lacking this flag)
  • after solving, when rebuilds are being computed, opam would emit a warning for any package specifying "dune" {build} since the version of dune being installed has the always-rebuild-revdeps flag and would rebuild them anyway (the warning may be displayed in the plan i.e. "uses dune, build variable ignored, since dune.2.9.0 has the always-rebuild-revdeps flag")
  • this warning could be picked up by opam-repo-ci to fail linting (it's not part of opam-lint because it depends on the solution). Alternatively, opam-lint could be updated to have a mode which looks at a package universe and so could display a lint warning for "dune" {build} if any version of "dune" in that package universe has the always-rebuild-revdeps flag set.

Global disabling of build

The grey areas are interesting, since in this case the packager has made the decision for the user that differing binary artefacts do not matter (e.g. if an upgraded ppx produces different code). We could consider adding a global setting (in .opam/config) which completely disables the build variable, allowing users to opt-in to always rebuilding on changes. Clearly we would never do this the other way around (i.e. allow users to opt-in to less rebuilding) since it would be untested, but in this case the user would be opting in to a more paranoid setting (choosing to always rebuild revdeps).

dra27 avatar Jul 19 '21 15:07 dra27

I spent a lot of time waiting for opam to rebuild basically my entire switch recently, whenever ocamlfind or dune have an update. So I don't think just removing the concept of a build dependency is a good way forward. It is not reasonable to ask users to wait for a long while before doing anything with their switch just because dune had an update. (All I wanted is install some new packages, but opam insisted I have to first rebuild everything. Maybe the ocamlfind/dune package changed without bumping the version number and those updates cannot be ignored.)

RalfJung avatar Jul 15 '22 17:07 RalfJung

Those unnecessary rebuilds have been fixed in opam master/2.2 (see #5118)

kit-ty-kate avatar Jul 15 '22 17:07 kit-ty-kate

Since the build directive was removed, this issue should probably be renamed, or replaced by a new issue if it's not solved (and I don't think it is?)... IIUC, the plan was

We're going to have both a dune binary cache soon and the opam cache in the future, so the cost of this will disappear.

but I haven't seen an opam cache, the dune cache isn't enabled by default, and I've seen conflicts between the dune cache and opam sandboxing.

Blaisorblade avatar Oct 01 '22 16:10 Blaisorblade

Since the build directive was removed, this issue should probably be renamed, or replaced by a new issue if it's not solved (and I don't think it is?)... IIUC, the plan was

I'm not sure where you saw that but that's not the case and is not planned either in the short term.

but I haven't seen an opam cache, the dune cache isn't enabled by default, and I've seen conflicts between the dune cache and opam sandboxing.

I think @emillon has some issue recently but it looks like the fix might be on dune's side.

kit-ty-kate avatar Oct 03 '22 18:10 kit-ty-kate

Ah, the issue about the cache is https://github.com/ocaml/dune/issues/6162

Blaisorblade avatar Oct 03 '22 22:10 Blaisorblade

Correct. The fix is not completely clear given that the architecture itself does not allow hardlink to work. Dune could fallback to less efficient sharing techniques (maybe loudly), or the sandboxing scripts could be modified but I doubt there'll be just a fix in either of dune or opam.

emillon avatar Oct 04 '22 07:10 emillon

A related issue: it would be nice if opam could separate dependencies that are only used to build the project versus other kinds of dependencies. Package managers like nix define "build inputs" and "propagated build inputs" (for comparison, opam only defines "propagated build inputs"). What's the advantage? Buiding opam projects with nix (using opam-nix or opam2nix) might allow to solve for build dependencies in a separate sandbox. Opam might want to support such a feature eventually as well.

rgrinberg avatar Oct 10 '22 23:10 rgrinberg

If I understand correctly, that's another facet of the "tools" issue - when you opam install ocamlformat, you don't need to have ocamlformat's dependencies installed in the switch at the end (they're actually a nuisance since they need to be coinstallable with the rest of the project).

emillon avatar Oct 11 '22 08:10 emillon

@emillon I think the last thing you mention has been solved by dev-setup dependency kind (I think it's a nice addition but I'd have prefered if they had proceeded with a seperate opam field).

dbuenzli avatar Oct 11 '22 08:10 dbuenzli

@dbuenzli yes, for ocamlformat it looks like dev-setup would work nicely because it's not needed at build time. I should have picked cppo has a better example of what I was thinking of - when you're depending on cppo as a build step, you only care about the cppo binary, but it comes with its dependencies installed as well.

emillon avatar Oct 11 '22 08:10 emillon

Yes, cppo is a good example. Some other examples: dune, ppx's without runtime libraries, cinaps. As well as all packages which only provide binaries can mark their dependencies as "non propagated" (I'll use nix lingo since the opam terminology is still changing).

rgrinberg avatar Oct 11 '22 14:10 rgrinberg

  • incorrect: Dune is an example, where the downgrading of Dune must recompile packages since dune-package cannot be read by an older version of dune.

Should there be support for indicating "upgrades of this package need not trigger recompilation, but downgrades must"?

JasonGross avatar Apr 13 '24 14:04 JasonGross

FTR, dune can only read dune-package emitted by the same version, so upgrades and downgrades are the same here.

emillon avatar Apr 16 '24 14:04 emillon