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

cppo 1.6.0 (now uses jbuilder) #10047

Merged
merged 12 commits into from
Aug 22, 2017
Merged

cppo 1.6.0 (now uses jbuilder) #10047

merged 12 commits into from
Aug 22, 2017

Conversation

mjambon
Copy link
Contributor

@mjambon mjambon commented Aug 7, 2017

No description provided.

@camelus
Copy link
Contributor

camelus commented Aug 7, 2017

❗ opam-lint warnings ff9de09
  • cconv.0.4 has some warnings:

    • warning 41: Some packages are mentionned in package scripts of features,
      but there is no dependency or depopt toward them: "doc"
  • These packages passed lint tests: bisect_ppx.1.2.0, commonjs_of_ocaml.0.1.0, cppo.1.6.0, cppo_ocamlbuild.1.6.0, hdf5.0.1.2, hdf5.0.1.3, jupyter.0.0.0, jupyter.0.1.0, lilis.0.2.1, ocp-index-top.0.4.1, opam-query.1.0, opam-query.1.1, opam-query.1.2, opam-query.1.3, pla.1.0, pla.1.1, ppx_bigarray.0.0.1, ppx_bigarray.0.1.0, ppx_bigarray.2.0.0, ppx_deriving.3.3, ppx_deriving.4.0, ppx_deriving.4.1, ppx_deriving_cmdliner.0.0.0, ppx_deriving_cmdliner.0.2.0, ppx_deriving_cmdliner.0.3.1, ppx_deriving_cmdliner.0.4.0, ppx_deriving_morphism.0.4, ppx_deriving_protobuf.2.4, ppx_deriving_protobuf.2.5, ppx_deriving_yojson.3.0, ppx_getenv.1.2, ppx_import.1.1, ppx_import.1.2, ppx_include.1.1, shcaml.0.2.0, sqlexpr.0.7.1, sqlexpr.0.8.0, touist.3.1.0, touist.3.2.0, touist.3.2.1


✅ Installability check (7288 → 7290)
  • new installable packages (2): cppo.1.6.0 cppo_ocamlbuild.1.6.0

@avsm
Copy link
Member

avsm commented Aug 9, 2017

This is the place where the revdeps start failing i think -- do we need upper bounds on bisect_ppx on cppo?

+- The following actions failed
| - build bisect_ppx 1.2.0
+- 
+- The following changes have been performed
| - install ppx_tools 5.0
+- 
[ERROR] The compilation of bisect_ppx failed at "make build".

#=== ERROR while compiling bisect_ppx.1.2.0 ===================================#
# context              2.0.0~beta3 | linux/x86_64 | ocaml-base-compiler.4.04.2 | file:///home/opam/src
# path                 ~/.opam/ocaml-base-compiler.4.04.2/.opam-switch/build/bisect_ppx.1.2.0
# command              make build
# exit-code            2
# env-file             ~/.opam/log/bisect_ppx-78-548b09.env
# output-file          ~/.opam/log/bisect_ppx-78-548b09.out
### output ###
# ocamlbuild -use-ocamlfind -no-links -byte-plugin -plugin-tag 'package(cppo_ocamlbuild)'  src/syntax/bisect_ppx.native  src/report/report.native src/bisect.cma src/bisect.cmxa  src/ocamlbuild/bisect_ppx_plugin.cma  src/ocamlbuild/bisect_ppx_plugin.cmxa
# ocamlfind ocamlc -package unix -package ocamlbuild -linkpkg -package cppo_ocamlbuild myocamlbuild.ml /home/opam/.opam/ocaml-base-compiler.4.04.2/lib/ocamlbuild/ocamlbuild.cmo -o myocamlbuild
# + ocamlfind ocamlc -package unix -package ocamlbuild -linkpkg -package cppo_ocamlbuild myocamlbuild.ml /home/opam/.opam/ocaml-base-compiler.4.04.2/lib/ocamlbuild/ocamlbuild.cmo -o myocamlbuild
# ocamlfind: Package `cppo_ocamlbuild' not found
# Command exited with code 2.
# Makefile:99: recipe for target 'build' failed
# make: *** [build] Error 10

@mjambon
Copy link
Contributor Author

mjambon commented Aug 9, 2017

@rgrinberg I'm creating an opam package for cppo_ocamlbuild.

@mjambon
Copy link
Contributor Author

mjambon commented Aug 9, 2017

Some existing packages for ocamlbuild plugins use the form ocamlbuild-* while others use *-ocamlbuild:

base-ocamlbuild
js_of_ocaml-ocamlbuild
ocamlbuild
ocamlbuild-atdgen
ocamlbuild-pkg
ocamlbuild-protoc
ocaml-migrate-parsetree-ocamlbuild

More than consistency with other packages, I think it's important to use the same name for the ocamlfind and for the opam package. Right now, the ocamlfind package is cppo_ocamlbuild and the OCaml module it provides is Ocamlbuild_cppo (flipped).

I suggest the following plan:

  1. create opam package cppo_ocamlbuild
  2. edit all opam packages that depend on both ocamlbuild and cppo, adding a dependency on cppo_ocamlbuild (bonus: ensure they use cppo_ocamlbuild)
  3. notify the maintainers of these opam packages (especially if they maintain a copy of the opam package with the sources)

I'll proceed. @rgrinberg, let me know if you think it should be done differently.

distributed with the source package `cppo`.
@mjambon
Copy link
Contributor Author

mjambon commented Aug 10, 2017

37 packages use both ocamlbuild and cppo. I need to automate this. I'll resume tomorrow.

@rgrinberg
Copy link
Member

rgrinberg commented Aug 10, 2017

Let me warn the maintainers of the packages above about this change: @akabe @mfp @vbrankov @whitequark @choeger @smondet @Armael @reynir

The summary of the upcoming change:

If you're using the cppo_ocamlbuild ocamlbuild plugin, you will need to add cppo_ocamlbuild opam package to your list of dependencies.

@mjambon
Copy link
Contributor Author

mjambon commented Aug 10, 2017

I added a "cppo_ocamlbuild" {build} line to the packages @rgrinberg listed. Let's see how that works out.

of the ocamlfind packages for cppo_ocamlbuild.
@mjambon
Copy link
Contributor Author

mjambon commented Aug 11, 2017

Now all these packages depend on cppo >= 1.6, via cppo_ocamlbuild, in order to avoid a double installation of the ocamlfind packages for cppo_ocamlbuild.

@avsm
Copy link
Member

avsm commented Aug 11, 2017

Looks like zmq failed for unrelated reasons to this PR (uint64.h not found). Not a blocker

@avsm
Copy link
Member

avsm commented Aug 11, 2017

One more package: bisect_ppx needs the dependency

#=== ERROR while compiling bisect_ppx.1.2.0 ===================================#
# context              2.0.0~beta3 | linux/x86_64 | ocaml-base-compiler.4.04.2 | file:///home/opam/src
# path                 ~/.opam/ocaml-base-compiler.4.04.2/.opam-switch/build/bisect_ppx.1.2.0
# command              make build
# exit-code            2
# env-file             ~/.opam/log/bisect_ppx-79-548b09.env
# output-file          ~/.opam/log/bisect_ppx-79-548b09.out
### output ###
# ocamlbuild -use-ocamlfind -no-links -byte-plugin -plugin-tag 'package(cppo_ocamlbuild)'  src/syntax/bisect_ppx.native  src/report/report.native src/bisect.cma src/bisect.cmxa  src/ocamlbuild/bisect_ppx_plugin.cma  src/ocamlbuild/bisect_ppx_plugin.cmxa
# ocamlfind ocamlc -package unix -package ocamlbuild -linkpkg -package cppo_ocamlbuild myocamlbuild.ml /home/opam/.opam/ocaml-base-compiler.4.04.2/lib/ocamlbuild/ocamlbuild.cmo -o myocamlbuild
# + ocamlfind ocamlc -package unix -package ocamlbuild -linkpkg -package cppo_ocamlbuild myocamlbuild.ml /home/opam/.opam/ocaml-base-compiler.4.04.2/lib/ocamlbuild/ocamlbuild.cmo -o myocamlbuild
# ocamlfind: Package `cppo_ocamlbuild' not found
# Command exited with code 2.
# Makefile:99: recipe for target 'build' failed
# make: *** [build] Error 10

@avsm
Copy link
Member

avsm commented Aug 11, 2017

and cconv and commonjs_of_ocaml

@avsm
Copy link
Member

avsm commented Aug 11, 2017

and ppx_deriving and pla

@avsm
Copy link
Member

avsm commented Aug 11, 2017

you can see the revdeps list in https://ci.ocaml.io/ocaml/opam-repository/pr/10047?test=1%20V1.2%20Build

@mjambon
Copy link
Contributor Author

mjambon commented Aug 11, 2017

Note to the opam package maintainers listed below:

I just modified opam files for the following opam packages, in order to add cppo_ocamlbuild as a dependency since it no longer comes with the cppo opam package. You may have to propagate the change, if you maintain the authoritative copy of the opam file elsewhere than in opam-repository.

The affected packages and maintainers are:

@Drup
Copy link
Contributor

Drup commented Aug 12, 2017

Looking good, thanks!

@rleonid
Copy link
Contributor

rleonid commented Aug 12, 2017

cc @aantron

@aantron
Copy link
Contributor

aantron commented Aug 12, 2017

I suspect Lwt 2.7.1 and Lwt 3.0.0 are also affected by this.

@akabe
Copy link
Contributor

akabe commented Aug 17, 2017

Thanks.

The cause of the travis-ci errors is reported at issuu/ocaml-zmq#43, and it was repaired in stdint 0.4.2 #10093. The constraint stdint {>= '0.4.2'} should be added to zmq packages, but installation of the latest stdint will fix the build error, i.e., you can fix it by merge the current master of this repository and restart the builds on travis-ci.

@avsm
Copy link
Member

avsm commented Aug 17, 2017

Does anything else need to happen with Lwt as per @aantron's comment? I can do some manual testing tomorrow and merge if noone else gets there first.

@rgrinberg
Copy link
Member

rgrinberg commented Aug 17, 2017 via email

@mjambon
Copy link
Contributor Author

mjambon commented Aug 17, 2017

Sorry guys. I just merged master into this branch. Let's see what Travis says.

@rgrinberg
Copy link
Member

@avsm this looks good. the failure in opam-query seems unrelated.

@avsm avsm merged commit 228c3a5 into ocaml:master Aug 22, 2017
@avsm
Copy link
Member

avsm commented Aug 22, 2017

Thanks! You may want to announce this on https://discuss.ocaml.org, where we have a Community category and an announce tag for this purpose.

@mjambon
Copy link
Contributor Author

mjambon commented Aug 23, 2017

Good job everyone and thank you for your patience!

@mjambon mjambon deleted the cppo.1.6.0 branch August 23, 2017 02:09
@jvillard
Copy link
Contributor

A bit of a side-note: this change broke the compilation of some other project, and breaks the only solution I currently know of for releasing source code that would keep compiling in the face of changes to opam. I'm curious what you think could be done to avoid this, if anything.

Background: Infer uses opam-lock to freeze the versions of opam packages to some "known good" versions. (There's also an "opam" file to serve as the basis for updating the opam.lock file.) The opam.lock file was introduced in reaction to several breakages we've had where a change in opam would turn out to break compilation one way or the other. Worse, these breakages could happened in released versions of infer, which then need a rerelease just to keep them compiling after opam update (for the record, this issue prompting this release)

Puzzle for future consideration: Changing the dependencies of packages without bumping their versions breaks projects in the wild because their set of constraints may become unsatisfiable with no user action save from opam update (or getting a fresh opam) (eg, cppo.1.5.0 and ppx_deriving.4.1).

@rgrinberg
Copy link
Member

rgrinberg commented Aug 23, 2017 via email

@jvillard
Copy link
Contributor

Thanks for your reply @rgrinberg. Infer is the project I was referring to.

I think I'd much prefer it if opam packages were immutable and people were forced to publish new versions of packages. But note that are disadvantages in this immutable world as well. For example, upgrades such as the one in this PR would leave a good chunk of packages that depend on the ocamlbuild plugin incompatible with the new cppo release.

Is the root cause that packages are usually optimistic about being forward-compatible with their dependencies then?

Improvised idle suggestion (apologies for going off-topic): There could be packaging versions on top of software versions, e.g. ppx_deriving.4.1-1 would have become ppx_deriving.4.1-2 here. This is more or less just an implementation detail of making packages immutable, however, so I'm not sure how much that would help on its own (and I have no idea how hard it would be to have this in opam 1 or 2). On top of this, packages could be forced to always specify bounded intervals for the versions of all their dependencies (eg, cppo >= 1.4.0 && cppo <= 1.5.0-1, so no more optimism). Then this PR would have bumped the "packaging version" of all the dependencies of cppo (which is a pain, admittedly) to bump their upper bounds to cppo <= 1.6.0-1. More generally, providing a new version of any package would bump all their reverse dependencies. Not sure how realistic this trade-off would be.

@rgrinberg
Copy link
Member

rgrinberg commented Aug 23, 2017 via email

@jvillard
Copy link
Contributor

Good to know, thanks! Being able to freeze more aggressively to a particular complete snapshot sounds like a good solution.

@dbuenzli
Copy link
Contributor

It creates a ton of churn for the maintainers of opam-repository

Somehow you could argue that this could be automated based on the results of the CI.

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

Successfully merging this pull request may close these issues.

10 participants