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

5 packages from gitlab.com/dannywillems/ocaml-bls12-381/-/archive/0.4.1/ocaml-bls12-381-0.4.1.tar.bz2 #18112

Conversation

dannywillems
Copy link
Contributor

@dannywillems dannywillems commented Feb 8, 2021

This pull-request concerns:
-bls12-381.0.4.1: Virtual package for BLS12-381 primitives
-bls12-381-gen.0.4.1: Functors to generate BLS12-381 primitives based on stubs
-bls12-381-js.0.4.1: JavaScript version of BLS12-381 primitives implementing the virtual
package bls12-381
-bls12-381-js-gen.0.4.1: Functors to generate BLS12-381 JavaScript primitives based on stubs
-bls12-381-unix.0.4.1: UNIX version of BLS12-381 primitives implementing the virtual package
bls12-381

This new release brings JavaScript support using js_of_ocaml. More information can be found in the README.

For the UNIX version, nothing changes compared to before, except adding bls12-381-unix when writing an executable.
For the JavaScript version, the integration does rely on a communication with a wasm module, generated by wasm-bindgen from the same source code the UNIX package uses (rustc-bls12-381, integrated in tezos-rust-libs, see https://gitlab.com/tezos/tezos-rust-libs/-/merge_requests/1). js_of_ocaml is used on the OCaml side to generate JavaScript code which is going to call the wasm module and read/write the wasm memory.

I am not aware of any previous attempt on releasing OPAM packages using Rust code with a JavaScript support (using the same Rust dependency). This solution does work, but a bunch of improvements can be made. In particular, the JavaScript binding could be improved by writing a library « à la ctypes », see https://gitlab.com/nomadic-labs/tezos/-/issues/230 for the idea. If there is any other attempt, please share. There is an ongoing work to do the same for Sapling in Tezos, and feedbacks are welcome.

I also started (but never finished, even if the most interesting parts, like pointers are not written but used in these packages...) a bunch of README and code to write web applications using OCaml and Rust: https://gitlab.com/dannywillems/web-ocaml-rust-tuto.



🐫 Pull-request generated by opam-publish v2.0.2

@camelus
Copy link
Contributor

camelus commented Feb 8, 2021

Commit: 14c096a

@dannywillems has posted 33 contributions.

☀️ All lint checks passed 14c096a
  • These packages passed lint tests: bls12-381-gen.0.4.1, bls12-381-js-gen.0.4.1, bls12-381-js.0.4.1, bls12-381-unix.0.4.1, bls12-381.0.4.1

☀️ Installability check (+5)
  • new installable packages (5): bls12-381.0.4.1 bls12-381-gen.0.4.1 bls12-381-js.0.4.1 bls12-381-js-gen.0.4.1 bls12-381-unix.0.4.1

@dannywillems
Copy link
Contributor Author

Looks like there is an issue with ocamlformat.0.16.0 😕

@dannywillems
Copy link
Contributor Author

Also, looks like npm is missing. I am not sure using npm from the package manager is the best. Should I use anyway? Tests require it as it does run in node.

@kit-ty-kate
Copy link
Member

Looks like there is an issue with ocamlformat.0.16.0 confused

what was the issue? I couldn't see any failures related to ocamlformat

@kit-ty-kate
Copy link
Member

I pushed a fix, could you return the fix upstream?

@dannywillems
Copy link
Contributor Author

I pushed a fix, could you return the fix upstream?

Done in https://gitlab.com/dannywillems/ocaml-bls12-381/-/commit/fe9bfca313955a00e7cd11f725e7b4bac8eee1e5

@dannywillems
Copy link
Contributor Author

Looks like there is an issue with ocamlformat.0.16.0 confused

what was the issue? I couldn't see any failures related to ocamlformat

Builds failed with a cyclic deps error for bisect_ppx 2.6 and ocamlformat 0.16.0.

@dannywillems
Copy link
Contributor Author

Like here

@dannywillems
Copy link
Contributor Author

It looks like more packages are affected by the circular dependencies. The CI says it is fine, but the commands fail with an error. It is introduced by bisect_ppx 2.6.0. bisect_ppx is a dependency of ff and ff-pbt, with a version ">= 2.5". I will add a temporary fix here with bisect_ppx < "2.6.0".

@dannywillems
Copy link
Contributor Author

dannywillems commented Feb 9, 2021

Some jobs are failing because the node version is old, see - node: bad option: --experimental-wasm-modules in here. Tests require the option --experimental-wasm-modules, introduced in Node 12.3.0, see https://nodejs.org/tr/blog/uncategorized/10-lts-to-12-lts/#notable-changes-in-node-js-12-3-0.
And I suppose an old version in the tests (by using conf-npm which uses the system packages, not recommended for NPM/Node).
EDIT: it looks like this option is not required to run test tests correctly. I will update the test command. As it is not too late because it is not used yet, I will update the release tag, and include the changes we talked earlier.

For OCaml 4.12, should I add a constraint on the OCaml version as jsoo does? https://github.com/ocaml/opam-repository/blob/master/packages/js_of_ocaml-compiler/js_of_ocaml-compiler.3.8.0/opam#L17 and https://github.com/ocaml/opam-repository/blob/master/packages/js_of_ocaml-compiler/js_of_ocaml-compiler.3.7.1/opam.

On debian-unstable, it is failing because of

- npm ERR! errno -13
- npm ERR! Error: EACCES: permission denied, open '/home/opam/.opam/4.11/.opam-switch/build/bls12-381-js.0.4.1/_build/default/test/js/node/package.json'
- npm ERR!  [Error: EACCES: permission denied, open '/home/opam/.opam/4.11/.opam-switch/build/bls12-381-js.0.4.1/_build/default/test/js/node/package.json'] {
- npm ERR!   errno: -13,
- npm ERR!   code: 'EACCES',
- npm ERR!   syscall: 'open',
- npm ERR!   path: '/home/opam/.opam/4.11/.opam-switch/build/bls12-381-js.0.4.1/_build/default/test/js/node/package.json'
- npm ERR! }

@dannywillems dannywillems force-pushed the opam-publish-bls12-381-bls12-381-gen-bls12-381-js-bls12-381-js-gen-bls12-381-unix.0.4.1 branch from cff08c2 to 4ce7cb7 Compare February 9, 2021 14:07
@kit-ty-kate
Copy link
Member

Is there no way of having tests without npm? If not I'll simply disable them.

@dannywillems
Copy link
Contributor Author

Is there no way of having tests without npm? If not I'll simply disable them.

No it is not possible.

homepage: "https://gitlab.com/dannywillems/ocaml-bls12-381"
bug-reports: "https://gitlab.com/dannywillems/ocaml-bls12-381/issues"
depends: [
"ocaml" {>= "4.08" & < "4.12"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last question and it should be good to go: why is it not available on 4.12? If there is any issue I'll fix your package but if that's "just in case" I'd rather not to have that. That would add work to both of us:

  • you by making another release for 4.12
  • me by having to understand why the package was constrained in the first place and pushing the change anyway.

If it's a real incompatibility however that's totally fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is compatible with 4.12, but the CI here was failing because jsoo was not available for 4.12. I thought adding the dep was the way to go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised, was it really failing or just skipped? It's not supposed to fail.

MattWindsor91 added a commit to MattWindsor91/travesty that referenced this pull request Feb 11, 2021
This is to try to quickfix through the problem reported here:
ocaml/opam-repository#18112 (comment)

Travesty doesn't explicitly use bisect_ppx itself, so this should
be removed as soon as practical.
"dune" {>= "2.7"}
"ff-sig" {>= "0.6.1" & < "0.7.0"}
"zarith" {>= "1.10" & < "2.0"}
"bisect_ppx" {with-test & >= "2.5" & < "2.6.0"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question with that actually. I believe this is just a bug fixed by #18141

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I do confirm < "2.6.0" is just a temporary workaround because the tests were failing in the CI here.

@kit-ty-kate
Copy link
Member

Thanks!

@kit-ty-kate kit-ty-kate merged commit 218b1f7 into ocaml:master Feb 12, 2021
@dannywillems dannywillems deleted the opam-publish-bls12-381-bls12-381-gen-bls12-381-js-bls12-381-js-gen-bls12-381-unix.0.4.1 branch February 15, 2021 10:55
@dannywillems
Copy link
Contributor Author

Thanks for merging.
Looks like ocaml/dune#4195 does also happen when running opam install bls12-381-unix. I was a bit confident it passed the CI here.

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.

3 participants