Please make iconv and node-icu-charset-detector optional

ccoenen commented 7 years ago

I know and have read through #390 and especially https://github.com/martynsmith/node-irc/issues/390#issuecomment-139427307. I know that node-irc works on windows even without iconv and node-icu... so why am I complaining?

I'm on a windows machine right now and I am preparing an exercise for a few students, half of which are on windows laptops. The project involves node-irc. Whenever npm install is invoked, a wall of errors will appear. Mind you, not on first install of node-irc but on every single future invocation as well.

C:\Users\user\workspace\chatbot\node_modules\iconv>
if not defined npm_config_node_gyp (node "C:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "" rebuild )
gyp ERR! configure error
gyp ERR! stack Error: Can't find Python executable "python", you can set the PYTHON env variable.
gyp ERR! stack     at failNoPython (C:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\configure.js:401:14)
gyp ERR! stack     at C:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\configure.js:356:11
gyp ERR! stack     at FSReqWrap.oncomplete (fs.js:123:15)
gyp ERR! System Windows_NT 10.0.14393
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd C:\Users\user\workspace\chatbot\node_modules\iconv
gyp ERR! node -v v6.4.0
gyp ERR! node-gyp -v v3.3.1
gyp ERR! not ok
npm WARN install:iconv@2.2.1 iconv@2.2.1 install: `node-gyp rebuild`
npm WARN install:iconv@2.2.1 Exit status 1

> node-icu-charset-detector@0.2.0 install C:\Users\user\workspace\chatbot\node_modules\node-icu-charset-detector
> node-gyp rebuild

C:\Users\user\workspace\chatbot\node_modules\node-icu-charset-detector>
if not defined npm_config_node_gyp (node "C:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "" rebuild )
gyp ERR! configure error
gyp ERR! stack Error: Can't find Python executable "python", you can set the PYTHON env variable.
gyp ERR! stack     at failNoPython (C:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\configure.js:401:14)
gyp ERR! stack     at C:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\configure.js:356:11
gyp ERR! stack     at FSReqWrap.oncomplete (fs.js:123:15)
gyp ERR! System Windows_NT 10.0.14393
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd C:\Users\user\workspace\chatbot\node_modules\node-icu-charset-detector
gyp ERR! node -v v6.4.0
gyp ERR! node-gyp -v v3.3.1
gyp ERR! not ok
npm WARN install:node-icu-charset-detector@0.2.0 node-icu-charset-detector@0.2.0 install: `node-gyp rebuild`
npm WARN install:node-icu-charset-detector@0.2.0 Exit status 1

Things I would like to avoid:

make all my students install linux
make all my students buy a mac
tell them "oh, just ignore the warnings"
tell them "oh, just install python and Visual Studio"
tell them "oh, you had an error in there? try going back to that wall of text and see if it wasn't three instead of two problems"

The only way I can think of to avoid that is by making the dependencies optional in some way. I'm open to other suggestions.

vBm commented 7 years ago

Both libs are already set as optional ones. Install it via npm install --no-optional

ward commented 7 years ago

Both libs are already set as optional ones. Install it via npm install --no-optional

Does that not give the following error on every incoming message?

ERROR: Error: Cannot find module 'node-icu-charset-detector'

It does so for me anyway.

EDIT: Only when debug is turned on. Still feels needlessly spammy though.

peanutbother commented 7 years ago

even though, ICONV is an optional dependency, please consider making it a peer dependency, people like me who need debug mode don't maybe still dont want to see messages like this missing package.

as peer dependency one can decide on his own to install the package and compile it, otherwise ignore it and don't have to mess around with spammed logs.

vBm commented 7 years ago

And/Or switch to using iconv-lite and jschardet per https://github.com/mooz/node-icu-charset-detector/issues/32#issuecomment-296349795

n3tman commented 7 years ago

To solve node-gyp error on Windows, see a guide here: https://github.com/nodejs/node-gyp#installation (using windows-build-tools is the easiest way)

As for node-icu-charset-detector, here's a guide based on the comment above:

Download this archive and extract it somewhere http://download.icu-project.org/files/icu4c/58.2/icu4c-58_2-Win64-MSVC2015.zip
Add ICU_DIRECTORY environment variable (expandable string) that points to extracted folder (e.g. d:\Downloads\icu)
Replace line 70 in package.json with

"node-icu-charset-detector": "git://github.com/shiftkey/node-icu-charset-detector.git#dev"

Run npm install
(optional) If you still see an error, remove node-icu-charset-detector from optionalDependencies in package.json and repeat step 4

martynsmith / node-irc

Please make iconv and node-icu-charset-detector optional #490