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

Obtain WASM/JS files from emscripten-forge #37

Merged
merged 4 commits into from
Aug 8, 2024
Merged

Conversation

ianthomas23
Copy link
Member

This PR removes the WASM/JS files from this repo and instead obtains them at build time from Emscripten-forge, based on the work in #25. Co-authored by @DerThorsten.

This passes the tests locally for me and works with cockle-playground. It currently doesn't work with the JupyterLite terminal extension but I think that is an existing issue in JupyterLite related to the recent upgrades and I will deal with that separately.

@ianthomas23 ianthomas23 added the enhancement New feature or request label Aug 8, 2024
@ianthomas23
Copy link
Member Author

Looks promising. Probably the package release process is broken now, but I'll deal with that at the next release.

@ianthomas23 ianthomas23 merged commit 4c2daa7 into main Aug 8, 2024
3 checks passed
@ianthomas23 ianthomas23 deleted the wasm-from-em branch August 8, 2024 10:45
"build": "tsc",
"eslint": "npm run eslint:check -- --fix",
"eslint:check": "eslint . --cache --ext .ts,.tsx",
"lint": "npm run prettier && npm run eslint",
"lint:check": "npm run prettier:check && npm run eslint:check",
"prepack": "npm install && npm run build",
"prepack": "npm install && npm run fetch:wasm && npm run build",
Copy link
Member

Choose a reason for hiding this comment

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

Looks promising. Probably the package release process is broken now, but I'll deal with that at the next release.

Isn't this change enough to pull the files at release time?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can check the files are properly included in the release artifacts built by the check_release workflow: https://github.com/jupyterlite/cockle/actions/runs/10300533031

Copy link
Member Author

Choose a reason for hiding this comment

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

The change is enough for me locally so it should it should be fine for a release provided micromamba is set up properly. It looks I started but didn't finish that.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I see the prep-release.yml workflow was also updated to install micromamba, so it should hopefully work.

ianthomas23 added a commit that referenced this pull request Aug 15, 2024
* Obtain WASM/JS files from emscripten-forge

Co-authored-by: Thorsten Beier <[email protected]>

* Update CI

* Install micromamba in check-release CI

* Install micromamba in prep-release CI run

---------

Co-authored-by: Thorsten Beier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants