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

Restore some of the JS build #1

Merged
merged 8 commits into from
Feb 1, 2024
Merged

Restore some of the JS build #1

merged 8 commits into from
Feb 1, 2024

Conversation

protz
Copy link
Collaborator

@protz protz commented Jan 30, 2024

@TWal this is the first PR of possibly many attempting to restore the JS build of MLS*

This needs FStarLang/FStar#3202 to be merged first.

What this PR does is:

  • fix the tests so that they are split between a test driver (MLS_Test) and a test library -- the JS build has a hand-written test that needs the ability to reuse the internal test files
  • fixup a couple things in the JS bindings

Can you please add a dune build target to the Makefile to make sure that this remains on CI? I'm not sure if this should go under all or check.

The next steps are:

  • restore the "virtual module" for Primitives in the JS build
  • fix the JS test to be loadable either from node or from the browser
  • add a node js/test.js or equivalent to the build to make sure this doesn't regress

@TWal
Copy link
Collaborator

TWal commented Jan 31, 2024

I adapted the old make build that used to run dune build and added it to the CI, to make sure this doesn't break in the future!
Maybe this should go into the all target at some point, but for now the nix build explicitly omit tests in the all target, and the JS build depends on the tests so this would need further nix / Makefile refactor.

@protz protz merged commit 5aa9193 into main Feb 1, 2024
1 check passed
@protz
Copy link
Collaborator Author

protz commented Feb 1, 2024

Merged this before I open up the next PR. (It came back green.)

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

Successfully merging this pull request may close these issues.

3 participants