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

Notify multicore<->WASM compatibility issues #121

Closed
wants to merge 5 commits into from
Closed

Conversation

CPerezz
Copy link
Member

@CPerezz CPerezz commented Jan 4, 2024

This PR mainly includes 2 things:

  1. A check (which I'm still not sure about) that will force the compilation to terminate if multicore feature is active while we compile to WASM.
  2. A README section that explains the issue and leaves it clear that these two things can't work (yet) together.

This should close #120 & #116

@CPerezz CPerezz requested review from han0110 and kilic January 4, 2024 14:06
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

LGTM! Just have a small question

README.md Outdated Show resolved Hide resolved
@CPerezz
Copy link
Member Author

CPerezz commented Jan 5, 2024

@huitseeker @han0110 what about merging #122 instead of this? Feels much more clean.

@huitseeker
Copy link
Contributor

huitseeker commented Jan 5, 2024

@CPerezz it is, but the feature removal makes it a new minor version bump (to 0.6.0) whereas the present one can be issued with just a point release (0.5.1). I'll take #122 for a whirl on Arecibo and see if it works, and report here.

@CPerezz
Copy link
Member Author

CPerezz commented Jan 5, 2024

I'll take #122 for a whirl on Arecibo and see if it works, and report here.

That's lovely! Will do the same for Halo2. Also, happy to release both anyways. Such that we have both flavours for now.

@huitseeker
Copy link
Contributor

@CPerezz Tested on Arecibo: found a few warts we have to correct on our side, no issues with #122 that I could find.

@CPerezz
Copy link
Member Author

CPerezz commented Jan 5, 2024

I also tested with privacy-scaling-explorations/halo2#242 and works perfect. Just need some fixes on our side. But nothing wrong with the PR.

So I'm tempted to close this and not merge it.
Do you need it @huitseeker ?

@CPerezz
Copy link
Member Author

CPerezz commented Jan 9, 2024

Solved via #122

@CPerezz CPerezz closed this Jan 9, 2024
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.

Add more info in regards impossibility to compile the lib with multicore feature to wasm targets
3 participants