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

"npm install" step has suggestions #10291

Closed
kripken opened this issue Jan 28, 2020 · 4 comments · Fixed by emscripten-core/emsdk#431 or emscripten-core/emsdk#433
Closed

"npm install" step has suggestions #10291

kripken opened this issue Jan 28, 2020 · 4 comments · Fixed by emscripten-core/emsdk#431 or emscripten-core/emsdk#433

Comments

@kripken
Copy link
Member

kripken commented Jan 28, 2020

Installing the tot now, I saw

$ ./emsdk install tot
Installing SDK 'sdk-releases-upstream-856799fec303eff03345476ca987adadeb09dd40-64bit'..
[..]
Done installing tool 'releases-upstream-856799fec303eff03345476ca987adadeb09dd40-64bit'.
up to date in 0.054s


   ╭────────────────────────────────────────────────────────────────╮
   │                                                                │
   │      New minor version of npm available! 6.10.2 → 6.13.6       │
   │   Changelog: https://github.com/npm/cli/releases/tag/v6.13.6   │
   │               Run npm install -g npm to update!                │
   │                                                                │
   ╰────────────────────────────────────────────────────────────────╯

Done installing SDK 'sdk-releases-upstream-856799fec303eff03345476ca987adadeb09dd40-64bit'.

Could we hide all npm output? Or is that too dangerous?

cc @juj @sbc100

@juj
Copy link
Collaborator

juj commented Jan 28, 2020

I wonder if that goes to stdout? I suppose npm should operate as usual, and print progress to stdout and errors to stderr? In that case we could just mute stdout in the subprocess spawn and let stderr items come through.

Another way might be to pipe both streams in and grep for the output contents of stdout and stderr, and print them if process return code was nonzero, or some special keywords were found in the output?

@kripken
Copy link
Member Author

kripken commented Jan 28, 2020

Another option might be to add running npm install... in our output, and npm install succeeded after, so it's clear what's going on?

@sbc100
Copy link
Collaborator

sbc100 commented Jan 28, 2020

Interesting.. i can't repro this. Does it happen when you run npm install again in the emscripten repo?

@kripken
Copy link
Member Author

kripken commented Jan 28, 2020

I don't see that message in the emscripten repo when I try now. Odd.

(Anyhow, I think the issue of either hiding of giving context for npm output is pretty general here.)

sbc100 added a commit to emscripten-core/emsdk that referenced this issue Jan 28, 2020
sbc100 added a commit to emscripten-core/emsdk that referenced this issue Jan 28, 2020
sbc100 added a commit to emscripten-core/emsdk that referenced this issue Jan 28, 2020
Fixes: emscripten-core/emscripten#10291

Also fix issue where an old/incorrect version of node could be used
during SDK install since we were preferring activated versions of node.
sbc100 added a commit to emscripten-core/emsdk that referenced this issue Jan 28, 2020
Fixes: emscripten-core/emscripten#10291

Also fix issue where an old/incorrect version of node could be used
during SDK install since we were preferring activated versions of node.
sbc100 added a commit to emscripten-core/emsdk that referenced this issue Jan 28, 2020
sbc100 added a commit to emscripten-core/emsdk that referenced this issue Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants