Bulk-remove build tag from ppx dependencies #12013

copy · 2018-05-17T20:31:52Z

See the discussion in #11852

This PR removes the build tag from ppx dependencies in all packages in the opam universe.

It removes the build tag from these dependencies:

"bisect_ppx"
"js_of_ocaml-ppx"
"lwt_ppx"
"ppx_bitstring"
"ppx_blob"
"ppx_cstruct"
"ppx_deriving"
"ppx_deriving_yojson"
"ppx_distr_guards"
"ppx_fields_conv"
"ppx_import"
"ppx_jane"
"ppx_monadic"
"ppx_optcomp"
"ppx_sexp_conv"
"ppx_tools"
"ppx_tools_versioned"
"ppx_typerep_conv"

Which was found using:

grep -r 'ppx.*{.*build'  | grep -v ppxfind|grep -v ocamlbuild |grep -v ppx_core|grep -v ppx_driver |grep -v ppx_metaquot |grep -v ppx_type_conv |grep -v base-no-ppx |grep -v '"bisect"' |grep -o '"[^"]*"' |sort |uniq

ppx_core, ppx_driver, ppx_metaquot, ppx_type_conv and ppx_type_conv
were excluded because they are deprecated and unlikely to see further updates.

Removal of the build tag was performed using two sed scripts, which I suggest
to be reviewed:

s/\("bisect_ppx"\|"js_of_ocaml-ppx"\|"lwt_ppx"\|"ppx_bitstring"\|"ppx_blob"\|"ppx_cstruct"\|"ppx_deriving"\|"ppx_deriving_yojson"\|"ppx_distr_guards"\|"ppx_fields_conv"\|"ppx_import"\|"ppx_jane"\|"ppx_monadic"\|"ppx_optcomp"\|"ppx_sexp_conv"\|"ppx_tools"\|"ppx_tools_versioned"\|"ppx_typerep_conv"\)\s*{\s*build\s*}/\1/
s/\("bisect_ppx"\|"js_of_ocaml-ppx"\|"lwt_ppx"\|"ppx_bitstring"\|"ppx_blob"\|"ppx_cstruct"\|"ppx_deriving"\|"ppx_deriving_yojson"\|"ppx_distr_guards"\|"ppx_fields_conv"\|"ppx_import"\|"ppx_jane"\|"ppx_monadic"\|"ppx_optcomp"\|"ppx_sexp_conv"\|"ppx_tools"\|"ppx_tools_versioned"\|"ppx_typerep_conv"\)\(\s*{\s*\)build &/\1\2/

After executing these scripts the above grep command returns nothing.

This removes the {build} tag from all ppx dependencies, as ppxs often have runtime libraries and aren't true build-only dependencies, causing spurious build failures after upgrades of ppx packages. See ocaml#11852

camelus · 2018-05-17T20:47:15Z

❌ opam-lint errors `58b25cd`

goblint.1.0.0 has errors:
- error 25: Missing field 'authors'
bookaml.4.0 has some warnings:
- warning 41: Some packages are mentionned in package scripts of features, but there is no dependency or depopt toward them:
  "bookaml"
sqlexpr.0.9.0 has some warnings:
- warning 41: Some packages are mentionned in package scripts of features, but there is no dependency or depopt toward them:
  "pa_sqlexpr"
tls.0.7.1 has some warnings:
- warning 41: Some packages are mentionned in package scripts of features, but there is no dependency or depopt toward them:
  "ipaddr"
These packages passed lint tests: JsOfOCairo.1.0.1, JsOfOCairo.1.1.1, amf.0.1.0, ansi-parse.0.3.0, arp.0.1.1, arp.0.2.0, arp.0.2.1, async-zmq.0.3.0, biocaml.0.4.0, biocaml.0.5.0, biocaml.0.6.0, bisect_ppx.0.1, bisect_ppx.0.2.2, bisect_ppx.0.2.3, bisect_ppx.0.2.4, bisect_ppx.0.2.5, bisect_ppx.0.2.6, bisect_ppx.0.2, bisect_ppx.1.0.0, bisect_ppx.1.0.1, bisect_ppx.1.1.0, bisect_ppx.1.2.0, bitcoinml.0.2.4, bitcoinml.0.2, bitcoinml.0.3.0, bitcoinml.0.3.1, bitstring.3.0.0, bookaml.3.1, camlhighlight.4.0, camlhighlight.5.0, caqti-dynload.0.10.0, caqti-dynload.0.10.1, caqti-dynload.0.9.0, charrua-core.0.3, charrua-core.0.4, charrua-core.0.5, charrua-core.0.6, charrua-core.0.7, charrua-core.0.8, charrua-core.0.9, cohttp-lwt.1.0.0, cohttp.0.20.1, cohttp.0.20.2, cohttp.0.21.0, cohttp.0.21.1, cohttp.0.22.0, cohttp.1.0.0, cohttp.1.0.2, cohttp.1.1.0, conduit-async.1.0.0, conduit-async.1.0.3, conduit-async.1.1.0, conduit-lwt-unix.1.0.2, conduit-lwt-unix.1.0.3, conduit-lwt-unix.1.1.0, conduit-lwt.1.0.0, conduit-lwt.1.0.3, conduit-lwt.1.1.0, conduit.0.12.0, conduit.0.13.0, conduit.0.14.0, conduit.0.14.1, conduit.0.14.2, conduit.0.14.3, conduit.0.14.4, conduit.0.14.5, conduit.0.15.0, conduit.0.15.1, conduit.0.15.2, conduit.0.15.3, conduit.0.15.4, conduit.1.0.0, conduit.1.0.3, conduit.1.1.0, coq-serapi.8.7.1+0.4.1, coq-serapi.8.7.1+0.4.12, coq-serapi.8.7.1+0.4.2, coq-serapi.8.7.1+0.4.8, coq-serapi.8.7.1+0.4, coq-serapi.8.7.2+0.4.13, coq-serapi.8.8.0+0.5.1, datakit-ci.0.10.0, datakit-ci.0.10.1, datakit-ci.0.11.0, datakit-ci.0.12.0, datakit-ci.0.12.1, datakit-ci.0.9.0, dns-forward.0.10.0, dns-forward.0.7.2, dns-forward.0.9.0, dns.0.18.0, dns.0.18.1, dns.0.19.0, dns.0.19.1, dns.0.20.0, dns.0.20.1, dns.1.0.0, dns.1.0.1, dockerfile.1.3.0, dockerfile.1.4.0, dockerfile.1.7.0, dockerfile.1.7.1, dockerfile.1.7.2, dockerfile.2.0.0, dockerfile.2.2.0, dockerfile.2.2.1, dockerfile.2.2.2, dockerfile.2.2.3, dockerfile.3.0.0, dockerfile.3.1.0, dockerfile.4.0.0, fat-filesystem.0.11.0, fat-filesystem.0.12.0, fat-filesystem.0.12.1, fat-filesystem.0.12.2, fat-filesystem.0.12.3, gdb.0.3, github-jsoo.3.0.0, graphql_ppx.0.0.3, graphql_ppx.0.0.4, ibx.0.8.1, ipaddr.2.7.0, ipaddr.2.7.1, ipaddr.2.7.2, ipaddr.2.8.0, lens.1.1.0, lens.1.2.0, libsvm.0.9.3, mecab.0.0.0, mirage-block-unix.2.0.0, mirage-block-unix.2.1.0, mirage-block-unix.2.2.0, mirage-block-unix.2.3.0, mirage-block-unix.2.4.0, mirage-block-unix.2.5.0, mirage-block-unix.2.6.0, mirage-block-unix.2.7.0, mirage-block-xen.1.4.0, mirage-block-xen.1.5.0, mirage-block-xen.1.5.2, mirage-block-xen.1.5.3, mirage-conduit.1.0.3, mirage-conduit.3.0.0, mirage-conduit.3.0.1, mirage-nat.1.0.0, mirage-net-xen.1.6.0, mirage-net-xen.1.6.1, mirage-net-xen.1.7.0, mirage-profile.0.7.0, mirage-profile.0.8.0, mirage-profile.0.8.1, mirage-profile.0.8.2, mrt-format.0.2.0, mrt-format.0.3.0, msgpck.1.0, nbd.2.1.0, nbd.2.1.1, nbd.2.1.3, nbd.2.2.0, nbd.3.0.0, netchannel.1.7.1, netchannel.1.8.0, netml.0.1.0, nocrypto.0.5.3, nocrypto.0.5.4, ocaml-logicalform.v0.6.0, ocaml-monadic.0.3.2, ocaml-topexpect.0.3, oci.0.3, opass.1.0.6, opium.0.15.0, opium.0.15.1, otr.0.3.1, otr.0.3.2, otr.0.3.3, otr.0.3.4, partition_map.0.9.0, passmaker.1.0, pcap-format.0.4.0, pcap-format.0.5.0, planck.2.2.0, ppx_bitstring.1.0.0, ppx_bitstring.1.0.1, ppx_bitstring.1.1.0, ppx_bitstring.1.2.0, ppx_bitstring.1.3.0, ppx_bitstring.1.3.1, ppx_bitstring.1.3.2, ppx_bitstring.1.3.3, ppx_bitstring.2.0.0, ppx_bitstring.2.0.1, ppx_bitstring.2.0.2, ppx_blob.0.2, ppx_cstruct.0, ppx_cstruct.3.0.1, ppx_dryunit.0.3.1, ppx_dryunit.0.4.0, ppx_hardcaml.1.0.0, ppx_hardcaml.1.1.0, ppx_hardcaml.1.2.0, ppx_hardcaml.1.3.0, ppx_monoid.0.1, ppx_monoid.0.2, ppx_netblob.1.0, ppx_netblob.1.1, ppx_netblob.1.2.1, ppx_regexp.0.2.0, ppx_regexp.0.3.1, ppx_regexp.0.3.2, protocol-9p-unix.0.10.0, protocol-9p-unix.0.11.0, protocol-9p-unix.0.11.1, protocol-9p-unix.0.11.2, protocol-9p-unix.0.11.3, protocol-9p-unix.0.12.0, protocol-9p.0.10.0, protocol-9p.0.11.0, protocol-9p.0.11.1, protocol-9p.0.11.2, protocol-9p.0.11.3, protocol-9p.0.12.0, protocol-9p.0.6.0, protocol-9p.0.7.2, protocol-9p.0.7.3, protocol-9p.0.7.4, protocol-9p.0.8.0, protocol-9p.0.9.0, qcow-format.0.3, qcow-format.0.4.1, qcow-format.0.4.2, qcow-format.0.4, qcow-format.0.5.0, qcow.0.10.0, qcow.0.10.2, qcow.0.10.3, qcow.0.10.4, qcow.0.6.0, qcow.0.7.0, qcow.0.8.1, qcow.0.9.0, qcow.0.9.4, qcow.0.9.5, rawlink.0.4, rawlink.0.5, rawlink.0.6, rawlink.0.7, shared-block-ring.2.3.0, shared-block-ring.2.4.0, shared-memory-ring-lwt.3.0.0, shared-memory-ring.2.0.0, shared-memory-ring.2.0.1, shared-memory-ring.3.0.0, sslconf.0.8.3, systemverilog.0.0.1, tar-format.0.5.0, tar-format.0.5.1, tar-format.0.6.0, tar-format.0.6.1, tar-format.0.7.1, tar.0.8.0, tar.0.9.0, tcpip.2.7.0, tcpip.2.8.0, tcpip.2.8.1, tcpip.3.0.0, tcpip.3.1.0, tcpip.3.1.1, tcpip.3.1.2, tcpip.3.1.3, tcpip.3.1.4, tls.0.8.0, tls.0.9.0, tls.0.9.1, unmagic.0.9.0, unmagic.1.0.0, unmagic.1.0.1, unmagic.1.0.2, unmagic.1.0.3, unmagic.1.0.4, uri.1.9.2, uri.1.9.4, uri.1.9.5, uri.1.9.6, utop.1.19.1, utop.1.19.2, utop.1.19, vchan-unix.3.0.0, vchan-xen.3.0.0, vchan.2.1.0, vchan.2.2.0, vchan.2.3.0, vchan.2.3.1, vchan.3.0.0, vhd-format.0.8.0, vhd-format.0.9.1, vhd-format.0.9.2, vmnet.1.1.0, vmnet.1.2.0, vmnet.1.3.0, vmnet.1.3.1, wamp.1.0, x509.0.5.1, x509.0.5.2, x509.0.5.3, x509.0.6.0, x509.0.6.1, xenstore.1.3.0, xenstore.1.4.0, xenstore.2.0.0, yaml.0.1.0

✅ Installability check (8835 → 8835)

hannesm · 2018-05-18T10:41:50Z

I'm not sure about the reasoning in the referenced issue. Applying the same argument to build systems (version x may lead to invalid code, version (x+1) fixes this behaviour), should all the build dependencies for build systems be dropped in opam-repositories? //cc @avsm @AltGr @samoht

I can see the value, being able to derive from opam list output (plus system packages) a reproducible binary. I'm worried about too many recompilations just because ocamlbuild or dune got released.

copy · 2018-05-19T17:39:14Z

I'm not sure about the reasoning in the referenced issue. Applying the same argument to build systems (version x may lead to invalid code, version (x+1) fixes this behaviour), should all the build dependencies for build systems be dropped in opam-repositories? //cc @avsm @AltGr @samoht

I tend to agree, but this PR only changes ppx dependencies, which need to trigger rebuilds after upgrades as they usually contain runtime libraries.

mseri · 2018-05-19T19:06:46Z

@copy I see your point, but I am not sure about this. For example bisect_ppx is generally used for coverage builds and not opam builds, I think its presence in those opam files has been an oversight and should not trigger a rebuild if the library is updated. Similarly I think ppx_cstruct shouldn't necessarily trigger a rebuild unless cstruct is updated, and probably cstruct should always be listed as a dependency if ppx_cstruct is used.

copy · 2018-05-19T19:32:44Z

@mseri Indeed, I also included ppxs that don't have runtime libraries, as per this comment:

BTW, if I understand the meaning of {build} correctly, then ppx rewriters should never be flagged as build only dependencies, even when they don't have runtime dependencies. The reason is that if the code they generate changes, then all their reverse dependencies must be rebuilt as well.

However, I'd glady revert to the less controversial change of only changing ppxs that have runtime libraries.

mseri · 2018-05-19T19:47:02Z

👍 Let’s get more opinions before reverting anything

rgrinberg · 2018-05-20T06:38:50Z

I agree with this change. That said, this does not solve the issue in the long term as people will still continue submitting packages with {build} dependencies. How about a discuss thread to warn people to stop using this feature for ppx.

avsm · 2018-05-20T13:11:49Z

I also agree with this change; we should err on the side of being conservative with recompiles and doing more if there's a risk of missing a material change.

Instead of or as well as a discuss thread, I think we should get into the habit of adding checks to the Camelus bot to automate this as part of the lint step.

copy · 2018-05-26T17:46:03Z

Is there anything missing from this PR to be merged? Any interjections?

I had a quick look over the failures and they mostly come from failing old packages (that were touched by this PR), network errors and already broken packages.

samoht · 2018-05-27T09:42:32Z

Merging as this seems to be a safe thing to do.

Bulk-remove build tag from ppx dependencies

58b25cd

This removes the {build} tag from all ppx dependencies, as ppxs often have runtime libraries and aren't true build-only dependencies, causing spurious build failures after upgrades of ppx packages. See ocaml#11852

samoht mentioned this pull request May 25, 2018

tcpip>=3.2.0 uses ppx_cstruct #12055

Merged

samoht merged commit 7801656 into ocaml:master May 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bulk-remove build tag from ppx dependencies #12013

Bulk-remove build tag from ppx dependencies #12013

copy commented May 17, 2018 •

edited

Loading

camelus commented May 17, 2018

hannesm commented May 18, 2018

copy commented May 19, 2018

mseri commented May 19, 2018 •

edited

Loading

copy commented May 19, 2018

mseri commented May 19, 2018

rgrinberg commented May 20, 2018

avsm commented May 20, 2018

copy commented May 26, 2018

samoht commented May 27, 2018

Bulk-remove build tag from ppx dependencies #12013

Bulk-remove build tag from ppx dependencies #12013

Conversation

copy commented May 17, 2018 • edited Loading

camelus commented May 17, 2018

❌ opam-lint errors 58b25cd

✅ Installability check (8835 → 8835)

hannesm commented May 18, 2018

copy commented May 19, 2018

mseri commented May 19, 2018 • edited Loading

copy commented May 19, 2018

mseri commented May 19, 2018

rgrinberg commented May 20, 2018

avsm commented May 20, 2018

copy commented May 26, 2018

samoht commented May 27, 2018

copy commented May 17, 2018 •

edited

Loading

❌ opam-lint errors `58b25cd`

mseri commented May 19, 2018 •

edited

Loading