Skip to content
This repository

make validator optional #28

Closed
crobi opened this Issue December 23, 2011 · 19 comments

5 participants

Robert Carnecky Brian McKelvey TeeTeeHaa mgmchenry Einar Otto Stangvik
Robert Carnecky

The native validator is built using node-waf, which is not available on Windows or OSX prebuilt nodejs installations. It is therefore not possible to install this package using NPM on these systems.

Would it be possible to make the validator optional, or to skip its installation if there is no node-waf?

Brian McKelvey
Owner

I'll work on doing what you suggest. In the mean time, version 1.0.3 includes the recent speed enhancements but not the native code utf-8 validator. It works just fine, so you should be able to successfully use WebSocket-Node with NPM by specifying version 1.0.3 in your package.json file.

TeeTeeHaa

I'm on Windows Vista 64 Bit and the following command installed the package without errors:
npm install websocket@1.0.3

mgmchenry

I was able to get the pre-built validator.js from the ws npm package. On windows, the contents of validator.js are the following:

module.exports.Validation = {
isValidUTF8: function(buffer) {
return true;
}
};

Also, I added MinGW and GnuWin32 to my sytsem in the hope of being able to build. No luck.

TeeTeeHaa's suggestion does work.

Einar Otto Stangvik

Yep, that's how I deal with it on Windows in ws. Native module support in Windows is being discussed widely at the moment, so I won't be doing more with it until that has been worked out.

Brian McKelvey
Owner

I'm going to agree with einaros' approach to this. Look for an update in the next couple days that will bypass the validator on Windows, and when native module details are more or less figured out in the Windows world, we'll revisit it then.

Alternatively, I could write a pure JS version of the UTF-8 validator if people feel that it would be an important thing to have even at the expense of performance (optional, enabled by a config option, disabled by default)

Brian McKelvey
Owner

Sorry for the delay in dealing with this issue.. I just got back from CES a couple days ago.

Brian McKelvey
Owner

Apologies again, I've been super busy preparing to present Worlize during NodeJam at Node Summit in SF next week. I'll get back to this after I get home next Friday.

mgmchenry

Just a note - it's not just that windows users need the replaced validator.js, it's that you can't install through NPM because build fails on typical windows systems that aren't set up to do that. Will you be able to fix that as well?

Brian McKelvey
Owner
Brian McKelvey
Owner

I just released version 1.0.5 which fixes the build issues on Windows. Windows users will need to have Visual C++ and Python 2.7 installed in order for the native module to compile.

How important do you think it is to have a fallback JavaScript validator for users who can't (or don't want to) build the native module?

mgmchenry

First, My NPM install is still failing:
cmd "/c" "node-gyp configure" failed with 1

I'm not sure what node-gyp is, but "node-gyp is a cross-platform command-line tool written in Node.js for compiling native addon modules for Node.js, which takes away the pain..." sounds pretty good to me.

So, "npm install node-gyp". Success! "npm install websocket" still fails with the same message.

mgmchenry

Second, yes, I think a failback is important. I don't think it's a big deal for a developer adventurous enough to be using node to also have python. I happen to have 4 versions of Visual C installed because I'm married to the Microsoft stack, but I think that's asking a lot of others.

Is it just out of the question to distribute a binary JavaScript validator for windows users?

TeeTeeHaa

My opinion: in case the native module is not essential for the websocket module the command "npm install websocket" should not fail if the native module cannot be compiled. Instead there should be a warning during installation that the websocket module will actually work but certain functionality will be missing. Ideally there should be a JavaScript based fallback of course.

I think it actually is a "big deal" to require Visual Studio and Python. One of the biggest advantages of node.js is the luxury of "npm install something" to just work - without having several other huge tools installed which all have their own requirements, dependencies, installations, updates and general knowledge about them. Personally I see a bright future for node.js and websockets and there shouldn't be obstacles for non-C++ or non-Python programmers. I say: make those requirements optional for those people needing performance (which is the main reason for the native module I assume) who will probably dig into Visual Studio and Python when the performance aspect gets important for them.

Brian McKelvey
Owner

@mgmchenry I'm looking into the build errors now.

@TeeTeeHaa and @mgmchenry I'd love to distribute a binary version of the validator but I'm not a Windows developer and @ZeroDivisi0n mentioned on Pull Request #41 that it would require users to install the MS VC++ Redistributable package to work properly. It sounded like it would be difficult to make it work easily and reliably for all users.

I'm not a C++ developer and have extremely limited exposure to the Microsoft development toolchain, so I must depend at this point on the contributions of others to make this work correctly. My experience is primarily on Linux and Mac OS X.

Brian McKelvey
Owner

@mgmchenry I just ran npm install websocket on my Windows machine (Win7, Visual Studio 2010 Professional, Python 2.7, Node v0.6.18) and it works fine:

C:\Users\Brian Mckelvey\Documents\test>dir/w
 Volume in drive C has no label.
 Volume Serial Number is 14F7-29A6

 Directory of C:\Users\Brian Mckelvey\Documents\test

[.]  [..]
               0 File(s)              0 bytes
               2 Dir(s)  25,021,947,904 bytes free

C:\Users\Brian Mckelvey\Documents\test>npm install websocket
npm http GET https://registry.npmjs.org/websocket
npm http 200 https://registry.npmjs.org/websocket
npm http GET https://registry.npmjs.org/websocket/-/websocket-1.0.6.tgz
npm http 200 https://registry.npmjs.org/websocket/-/websocket-1.0.6.tgz

> websocket@1.0.6 install C:\Users\Brian Mckelvey\Documents\test\node_modules\we
bsocket
> node-gyp rebuild


C:\Users\Brian Mckelvey\Documents\test\node_modules\websocket>node "C:\Program F
iles\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\n
ode-gyp.js" rebuild
info it worked if it ends with ok
spawn C:\Python27\python.exe [ 'C:\\Users\\Brian Mckelvey\\.node-gyp\\0.6.18\\to
ols\\gyp_addon',
  'binding.gyp',
  '-IC:\\Users\\Brian Mckelvey\\Documents\\test\\node_modules\\websocket\\build\
\config.gypi',
  '-f',
  'msvs',
  '-G',
  'msvs_version=2010' ]
spawn C:\Windows\Microsoft.NET\Framework\v4.0.30319\msbuild.exe [ 'build/binding
.sln',
  '/clp:Verbosity=minimal',
  '/nologo',
  '/p:Configuration=Release;Platform=Win32' ]
  validation.cc
C:\Users\Brian Mckelvey\.node-gyp\0.6.18\src\node_object_wrap.h(57): warning C4
251: 'node::ObjectWrap::handle_' : class 'v8::Persistent<T>' needs to have dll-
interface to be used by clients of class 'node::ObjectWrap' [C:\Users\Brian Mck
elvey\Documents\test\node_modules\websocket\build\validation.vcxproj]
          with
          [
              T=v8::Object
          ]
C:\Users\Brian Mckelvey\.node-gyp\0.6.18\src\node_buffer.h(68): warning C4251:
'node::Buffer::constructor_template' : class 'v8::Persistent<T>' needs to have
dll-interface to be used by clients of class 'node::Buffer' [C:\Users\Brian Mck
elvey\Documents\test\node_modules\websocket\build\validation.vcxproj]
          with
          [
              T=v8::FunctionTemplate
          ]
..\src\validation.cc(71): warning C4018: '<' : signed/unsigned mismatch [C:\Use
rs\Brian Mckelvey\Documents\test\node_modules\websocket\build\validation.vcxpro
j]
..\src\validation.cc(75): warning C4018: '>=' : signed/unsigned mismatch [C:\Us
ers\Brian Mckelvey\Documents\test\node_modules\websocket\build\validation.vcxpr
oj]
C:\Program Files\MSBuild\Microsoft.Cpp\v4.0\Microsoft.CppBuild.targets(990,5):
warning MSB8012: TargetPath(C:\Users\Brian Mckelvey\Documents\test\node_modules
\websocket\build\Release\validation.dll) does not match the Linker's OutputFile
 property value (C:\Users\Brian Mckelvey\Documents\test\node_modules\websocket\
build\Release\validation.node). This may cause your project to build incorrectl
y. To correct this, please make sure that $(OutDir), $(TargetName) and $(Target
Ext) property values match the value specified in %(Link.OutputFile). [C:\Users
\Brian Mckelvey\Documents\test\node_modules\websocket\build\validation.vcxproj]
C:\Program Files\MSBuild\Microsoft.Cpp\v4.0\Microsoft.CppBuild.targets(991,5):
warning MSB8012: TargetExt(.dll) does not match the Linker's OutputFile propert
y value (.node). This may cause your project to build incorrectly. To correct t
his, please make sure that $(OutDir), $(TargetName) and $(TargetExt) property v
alues match the value specified in %(Link.OutputFile). [C:\Users\Brian Mckelvey
\Documents\test\node_modules\websocket\build\validation.vcxproj]
     Creating library C:\Users\Brian Mckelvey\Documents\test\node_modules\webso
  cket\build\Release\validation.lib and object C:\Users\Brian Mckelvey\Document
  s\test\node_modules\websocket\build\Release\validation.exp
  Generating code
  Finished generating code
  validation.vcxproj -> C:\Users\Brian Mckelvey\Documents\test\node_modules\web
  socket\build\Release\validation.dll
info done ok
websocket@1.0.6 ./node_modules/websocket

C:\Users\Brian Mckelvey\Documents\test>
Brian McKelvey theturtle32 closed this August 16, 2012
Brian McKelvey
Owner

The validator is now optional as of version 1.0.7. If the native module cannot compile, the module will still work without the UTF-8 validation.

TeeTeeHaa

Awesome, thanks! If I have time I'll try it out, including the native module.

TeeTeeHaa

FYI: On the following platforms the native parts of the module compile and the module works, with or without native parts:

Windows 7 64 Bit
Visual Studio 2010
Python 2.7
node 0.8.11
websocket 1.0.8

CentOS 5 64 Bit
gcc 4.1.2
Python 2.6
node 0.8.17
websocket 1.0.8

Brian McKelvey
Owner

Thanks for the update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.