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

Fix emscripten config #6503

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Conversation

ChristophStrehle
Copy link
Contributor

Leaving the emscripten macros, returning a value, empty was leading to syntax errors

The pointer returned by EM_ASM_PTR must be freed by calling free see
https://emscripten.org/docs/api_reference/emscripten.h.html?highlight=em_asm_ptr#c.EM_ASM_PTR

@chrchr-github
Copy link
Collaborator

Maybe we should add include detection here:

include_mappings = {'boost': ['<boost/'],

@chrchr-github
Copy link
Collaborator

@firewave Should we bump the CLIENT_VERSION in donate_cpu_lib.py?

@firewave
Copy link
Collaborator

@firewave Should we bump the CLIENT_VERSION in donate_cpu_lib.py?

Yes, absolutely.

Leaving the emscripten macros returning a value empty was leading to
syntax errors

The pointer returned by `EM_ASM_PTR` must be freed by calling `free`

Add emscripten without a syntax check for now as we would need the
emscripten tool chain
@ChristophStrehle
Copy link
Contributor Author

@firewave, @chrchr-github Sorry I missed the version number in this file. I incremented the patch version now

@chrchr-github
Copy link
Collaborator

Maybe we should add include detection here:

include_mappings = {'boost': ['<boost/'],

According to the discussion in #6461, this needs to be commented out at least until the next release. Sorry for the confusion.

@ChristophStrehle
Copy link
Contributor Author

According to the discussion in #6461, this needs to be commented out at least until the next release

I guess this case is slightly different from #6461 as the emscripten.cfg was there in the previous release. I just added a new test and an implementation instead of leaving the macros empty.

@firewave
Copy link
Collaborator

I guess this case is slightly different from #6461 as the emscripten.cfg was there in the previous release. I just added a new test and an implementation instead of leaving the macros empty.

Yes. In this case it does not need to be disabled. The file was added four years ago and never was fully integrated.

@chrchr-github chrchr-github merged commit 0bc7aed into danmar:main Jun 12, 2024
63 checks passed
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