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

Don't remove top level node_modules from emscripten package #3

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

sbc100
Copy link
Contributor

@sbc100 sbc100 commented Nov 30, 2018

That plan is to start using this top level node_modules
directory for any/all node module dependencies.

Ideally the SDK version would with pre-downloaded node_modules
so that users don't need to download stuff on first use.

See emscripten-core/emscripten#5774
See emscripten-core/emscripten#7538

That plan is to start using this top level node_modules
directory for any/all node module dependencies.

Ideally the SDK version would with pre-downloaded node_modules
so that users don't need to download stuff on first use.

See emscripten-core/emscripten#5774
See emscripten-core/emscripten#7538
@sbc100
Copy link
Contributor Author

sbc100 commented Dec 18, 2018

Ping.

@kripken
Copy link
Contributor

kripken commented Dec 18, 2018

Ah, I'm sorry to say, but I think this code is a dead end at this point. It runs on the mozilla build machines, but no one maintains them and we can't get this fix there (no access). And in the new build machines we envision, they won't be running this code IIUC.

@sbc100
Copy link
Contributor Author

sbc100 commented Dec 18, 2018

But its not dead yet right? This code still produces the SDK for pretty much all SDK users. I'm happy to ignore it once we have the alternative running, but for now I don't see any other way to change the SDK contents today.

@kripken
Copy link
Contributor

kripken commented Dec 18, 2018

This repo is a dead end, though - even if this PR were merged, there's no way to get the actual mozilla machines to fetch and use this code. We don't have access to those machines, and I don't think mozilla has anyone that could help us with them (they just want to shut them off asap).

@juj
Copy link
Owner

juj commented Dec 21, 2018

This repo is a dead end, though - even if this PR were merged, there's no way to get the actual mozilla machines to fetch and use this code.

Actually, the build machines do git pull this repo on each build, so even if we don't have maintenance access, we could merge this and the CIs would update the code. Although agree that this is a dead end for us.

The emslave repository with the deployment scripts can still be useful as a way to compile+generate downloadable zips of Emscripten tools in a way that is emsdk-compatible, so there is still value you can find in the scripts in this repository.

@kripken
Copy link
Contributor

kripken commented Dec 21, 2018

Oh, I didn't realize that, thanks @juj. Sorry for getting it wrong before @sbc100

lgtm to me then. @juj? (I think only you have write access to this repo?)

@juj juj merged commit c4c6b03 into juj:master Jan 8, 2019
@juj
Copy link
Owner

juj commented Jan 8, 2019

Yeah, that works.

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