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

Make emsdk a proper Python package #1043

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

martinRenou
Copy link

@martinRenou martinRenou commented Apr 29, 2022

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 the emsdk 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:

conda install -c conda-forge emsdk
emsdk install 3.1.2
emsdk activate 3.1.2

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.

This installs the emsdk script as a proper Python entry point
@@ -45,7 +45,7 @@ jobs:
- run:
name: test.py
command: |
source emsdk_env.sh
source emsdkpy/emsdk_env.sh
Copy link
Member

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?

Copy link
Author

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.

@sbc100
Copy link
Collaborator

sbc100 commented May 13, 2022

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.

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