-
Notifications
You must be signed in to change notification settings - Fork 702
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
Make emsdk a proper Python package #1043
base: main
Are you sure you want to change the base?
Conversation
419f082
to
ff6d47f
Compare
This installs the emsdk script as a proper Python entry point
ff6d47f
to
21d2c2f
Compare
d179ff8
to
3f1d44b
Compare
@@ -45,7 +45,7 @@ jobs: | |||
- run: | |||
name: test.py | |||
command: | | |||
source emsdk_env.sh | |||
source emsdkpy/emsdk_env.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I think this sounds good - having a proper python package could be useful in various ways!
I am concerned by breaking changes, though, like moving scripts out of the toplevel where users have expected them for long time. Is there a way to avoid such breaking changes? Perhaps we could create a new repo just for the package, which would include the main emsdk repo underneath it? Or maybe symlinks could help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your response. Indeed I've thought about not moving the scripts around at first, but I've not been able to do that. IIRC the issue was for installing the scripts as package_data for the Python package, which was not possible if not under a Python module. But I can give another try.
I'm not sure what conda-forge is .. but does it only support install python packages? In general I don't really see any advantage to publishing emsdk as a python package, since its basically as standalone script. That fact that its written in python is kind of on implementation detail... its not designed to interact with other parts of the python ecosystem. |
Description
This PR turns this repo into a proper Python package which installs the emsdk script as a Python entry point, meaning that
pip install emsdk
will make theemsdk
script discoverable in the path.It also allows other Python packages to depend on emsdk and use it from Python.
Motivation
The motivation behind this PR is to make a
conda-forge
package for emsdk, allowing people to install emsdk with conda:You can also publish it on PyPi if you want, but that's not mandatory.
Other comments
I am opening this PR as draft and hoping to get some feedback from the emscripten devs on this before pushing more on the PR (updating the CI so that it tests the Python installed emsdk).
I did my best to not change your usage of emsdk
I had to move some scripts, JSON and txt files under the emsdkpy Python package, otherwise setuptools would not install them as part of the Python package.