Nocrypto patches for compatibility with sexplib/ppx_sexp_conv > v0.11.0

[copy]

This commit submits @gasche's fixes from mirleft/ocaml-nocrypto#144, as nocrypto's maintainers are not responsive. This fixes mirleft/ocaml-nocrypto#143.

cc @hannesm

[camelus]

✅ All lint checks passed 5a3b33b

• These packages passed lint tests: nocrypto.0.5.4-1

---

✅ Installability check (8858 → 8859)

• new installable packages (1): nocrypto.0.5.4-1

[hannesm]

AFAIU, the patched META file in here will include the entire ppx_sexp_conv library (which includes compiler-libs?). I tried to incorporate all the open PRs in a branch https://github.com/hannesm/ocaml-nocrypto/tree/safely which I'm testing atm, where I ensured nocrypto for my use cases (esp. MirageOS unikernels) to work (and newer sexplib).

[copy]

@hannesm You're right, this shouldn't be merged as-is (I thought I had checked this earlier, but I can now see that I was mistaken, as -I …/compiler-libs and others are included when compiling a library with nocrypto after my changes). I will use @diml's method instead, which should select sexplib on < v0.11 and ppx_sexp_conv.runtime-lib on >= v0.11.

[copy]

Rebased with @diml's patches. I checked and this version doesn't include base or compiler-libs.

[hannesm]

another remark (I didn't have time to look in detail or test this PR): could you please increase the patch level (i.e. a 0.5.4-1) of nocrypto (see #10531), otherwise it's hard to tell which nocrypto variant (the one with your patches, or the one without) is installed and makes debugging tricky.

[copy]

Good point, moved the patches to a new release: nocrypto.0.5.4-1.

[mseri]

I think we should merge this release. Does anybody have any objection?

[hcarty]

No objection here.

[hannesm]

I didn't have time to test this PR completely (but AFAICT my main concern - adding unnecessary runtime dependencies - has been addressed). did anyone test this PR with both >0.11 and <0.11 ppx_sexp_conv/sexplib releases!?

concerns: why is it patch 0002, 0004, 0005, 0006 (why not 0001 and 0003)? also, 0004 adds ppx_sexp_conv to META, which is then adjusted by 0005 -- can we remove the chunk from 0004?

[mseri]

I am using it already with ppx_sexp_lib >0.11, did not test with older ones. I agree that the numbering is a bit weird, but maybe it is better to keep the patches as they are given that this is practically the import of the PR of a different author (and, hopefully, we will get a new nocrypto soon)

[copy]

concerns: why is it patch 0002, 0004, 0005, 0006 (why not 0001 and 0003)?

The reason for this is that 0003 reverts 0001, see gasche's branch.

also, 0004 adds ppx_sexp_conv to META, which is then adjusted by 0005 -- can we remove the chunk from 0004?

We could do that. We could also collect all changes into a single patch, but I think it's better to leave the patches directly derived from the commits (they were created using git format-patch).

did anyone test this PR with both >0.11 and <0.11 ppx_sexp_conv/sexplib releases!?

I did, by compiling a simple program and inspecting nocrypto's META files.

[gasche]

I think I reverted this commit because I pushed a fix in ocamlbuild, but then the fix will only be available in the next ocamlbuild release (which I've been slow in putting out...).

I'm afraid that I forgot why I pushed the revert on my fork. I think that it would be better if you did not include the revert commit: you would work with older ocamlbuild releases.

(I just tested to confirm that having the workaround in myocamlbuild.ml and a fixed version of ocamlbuild would keep working.)

[copy]

I think I reverted this commit because I pushed a fix in ocamlbuild, but then the fix will only be available in the next ocamlbuild release (which I've been slow in putting out...).

I'm afraid that I forgot why I pushed the revert on my fork. I think that it would be better if you did not include the revert commit: you would work with older ocamlbuild releases.

I already included the reverted commit, it's the last patch (I initially kept the reverting commit, and cherry-picked the reverted commit in the end).

[copy]

If there are no more objections, can we move on with merging this?

[mseri]

I think we should

[hcarty]

Thanks!