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

add emscripten-activation #27576

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

add emscripten-activation #27576

wants to merge 7 commits into from

Conversation

h-vetinari
Copy link
Member

We'll need this to be able to use compiler-jinjas (part of the effort for conda-forge/conda-forge.github.io#2244), and more concretely, I'm running into the lack of compiler activation in conda-forge/zlib-feedstock#84.

The (de)activation scripts are taken almost verbatim from https://github.com/emscripten-forge/recipes/blob/main/recipes/recipes/emscripten_emscripten-wasm32/activate.sh (only removed some BUILD_PREFIX usage).

CC @DerThorsten @wolfv, I added you as maintainers (as in the emscripten-forge recipe already), please let me know if you're (not) OK with that.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/emscripten-activation/meta.yaml) and found some lint.

Here's what I've got...

For recipes/emscripten-activation/meta.yaml:

  • The following maintainers have not yet confirmed that they are willing to be listed here: wolfv, conda-forge/emscripten, DerThorsten. Please ask them to comment on this PR if they are.

@DerThorsten
Copy link
Contributor

@h-vetinari thanks for pushing this forward. I am fine with being a maintainer

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/emscripten-activation/meta.yaml) and found some lint.

Here's what I've got...

For recipes/emscripten-activation/meta.yaml:

  • The following maintainers have not yet confirmed that they are willing to be listed here: wolfv, conda-forge/emscripten. Please ask them to comment on this PR if they are.

@h-vetinari h-vetinari marked this pull request as ready for review September 14, 2024 08:15
@h-vetinari
Copy link
Member Author

CC @isuruf, I assume you'd like to double-check this. Would appreciate your thoughts.

@wolfv
Copy link
Member

wolfv commented Sep 14, 2024

Thanks! Happy to be a maintainer :)

@h-vetinari h-vetinari requested a review from isuruf September 14, 2024 16:37
Comment on lines +40 to +41
# useful variables
export PY_SIDE_LD_FLAGV
Copy link
Member

Choose a reason for hiding this comment

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

What is the use of this variable?

Copy link
Member Author

Choose a reason for hiding this comment

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

@DerThorsten, could you comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is very likely some leftover from some debugging in the past, should be removable

Comment on lines +40 to +41
- emcc ./testfile.c
- node a.out.js
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an example for a shared library building and use?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough. @DerThorsten, do you have a handy example we could use?

Copy link
Contributor

Choose a reason for hiding this comment

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

to compile a shared library one needs the -s SIDE_MODULE=1 flag.
For cmake, one needs and additonal

set_property(GLOBAL PROPERTY TARGET_SUPPORTS_SHARED_LIBS TRUE)

without that line above cmake will compile static libraries even when a shared library is demanded.

The -s SIDE_MODULE=1 can be passed like:

set(CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS "-s SIDE_MODULE=1")
set(CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS "-s SIDE_MODULE=1")

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the question was about actually building some concrete example of a shared library as part of the testing for the activation feedstock, rather than just testfile.c.

Copy link
Contributor

Choose a reason for hiding this comment

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

@h-vetinari
Copy link
Member Author

Thanks a lot for the quick review! 🙏

Will clean up the remnants of emscripten-forge, and I think together with @DerThorsten we'll figure out the other points as well.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/emscripten-activation/meta.yaml) and found some lint.

Here's what I've got...

For recipes/emscripten-activation/meta.yaml:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/emscripten. Please ask them to comment on this PR if they are.

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

Successfully merging this pull request may close these issues.

4 participants