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

fails to install due to fs-ext copmilation issues #4

Closed
dzilbers opened this issue Apr 1, 2016 · 32 comments
Closed

fails to install due to fs-ext copmilation issues #4

dzilbers opened this issue Apr 1, 2016 · 32 comments

Comments

@dzilbers
Copy link

dzilbers commented Apr 1, 2016

fs-ext is a native module and there is a bug (..\fs-ext.cc(195): error C3861: 'fcntl': identifier not found) while building it.
Since your package depends on it - it might fail as well in run-time.
Is that dependency neccessary?

@davedoesdev
Copy link
Owner

qlobber-fsq needs fs-ext for flock (fcntl-based lock semantics aren't great when you may have the same process trying to lock the same file more than once).

fs-ext installs fine for me. Do you have C header files installed?

@dzilbers
Copy link
Author

dzilbers commented Apr 2, 2016

Why should I install any C header files? I install nodejs modules I need
and I do not build any native projects (at this stage at least).
Anyway fs-ext does not compile and it uses (requires) fcntl. Perhaps you
have some older global install of fs-ext.
This issue happens to everybody around me for the last couple of weeks.
When installing different modules which are dependent on qlobber-fsq. Even
Express module... And Mosca (mqtt server). and more.

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Sat, Apr 2, 2016 at 12:03 AM, David Halls notifications@github.com
wrote:

qlobber-fsq needs fs-ext for flock (fcntl-based lock semantics aren't
great when you may have the same process trying to lock the same file).

fs-ext installs fine for me. Do you have C header files installed?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@davedoesdev
Copy link
Owner

Hi Dani-
Which platform are you on?

@dzilbers
Copy link
Author

dzilbers commented Apr 2, 2016

Windows 10 Pro 64bit, latest Node and NPM and Node-Gyp

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Sat, Apr 2, 2016 at 9:32 PM, David Halls notifications@github.com
wrote:

Hi Dani-
Which platform are you on?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@dzilbers
Copy link
Author

dzilbers commented Apr 2, 2016

But it happens to my students with Win7 and Win8.1 as well.

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Sun, Apr 3, 2016 at 1:10 AM, Dan Zilberstein dzilbers@gmail.com wrote:

Windows 10 Pro 64bit, latest Node and NPM and Node-Gyp

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Sat, Apr 2, 2016 at 9:32 PM, David Halls notifications@github.com
wrote:

Hi Dani-
Which platform are you on?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@davedoesdev
Copy link
Owner

Ah - fcntl is a Linux-only thing. Thanks for the heads up on Windows, appreciate it.
I wonder what Node does for file locking on Windows - I'll check.
Unfortunately I don't have a Windows box handy right now to test.

@davedoesdev
Copy link
Owner

Interesting- fs-ext does have Windows support: https://github.com/baudehlo/node-fs-ext/blob/master/fs-ext.cc#L201

Maybe fs-ext shouldn't #include <fcntl.h> on Windows.

Ah - I see you've opened an issue already baudehlo/node-fs-ext#57

@dzilbers
Copy link
Author

dzilbers commented Apr 3, 2016

IMHO anybody developing public modules for Node should take in
consideration both linux and Windows from the very beginning...

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Sun, Apr 3, 2016 at 10:17 AM, David Halls notifications@github.com
wrote:

Interesting- fs-ext does have Windows support:
https://github.com/baudehlo/node-fs-ext/blob/master/fs-ext.cc#L201

Maybe fs-ext shouldn't #include <fcntl.h> on Windows.

Ah - I see you've opened an issue already baudehlo/node-fs-ext#57
baudehlo/node-fs-ext#57


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@davedoesdev
Copy link
Owner

Far be it from me to speak for @baudehlo but fs-ext's initial implementation definitely did take Windows into consideration:

baudehlo/node-fs-ext@58abb72

@dzilbers
Copy link
Author

dzilbers commented Apr 4, 2016

However even there it includes fcntl.h which does not exist in Windows node
environment...:
#include <fcntl.h>
This is the reason why fs-ext fails compilation.

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Sun, Apr 3, 2016 at 10:51 PM, David Halls notifications@github.com
wrote:

Far be it from me to speak for @baudehlo https://github.com/baudehlo
but fs-ext's initial implementation definitely did take Windows into
consideration:

baudehlo/node-fs-ext@58abb72
baudehlo/node-fs-ext@58abb72


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@dzilbers
Copy link
Author

dzilbers commented Apr 4, 2016

it's very strange but <fcntl.h> is a part of C standard libraries and it
exists in MS Visual Studio environment - it's very strange that it is not
found while compiling fs-ext by gyp (which uses MSVS C++ compiler).

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Mon, Apr 4, 2016 at 12:55 PM, Dan Zilberstein dzilbers@gmail.com wrote:

However even there it includes fcntl.h which does not exist in Windows
node environment...:
#include <fcntl.h>
This is the reason why fs-ext fails compilation.

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Sun, Apr 3, 2016 at 10:51 PM, David Halls notifications@github.com
wrote:

Far be it from me to speak for @baudehlo https://github.com/baudehlo
but fs-ext's initial implementation definitely did take Windows into
consideration:

baudehlo/node-fs-ext@58abb72
baudehlo/node-fs-ext@58abb72


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@davedoesdev
Copy link
Owner

Is it fcntl.h that's not found or fcntl() ?

..\fs-ext.cc(195): error C3861: 'fcntl': identifier not found [C:\Dani-R&D\nodejs\node_modules\fs-ext\build\fs-ext.vcxproj]

@dzilbers
Copy link
Author

dzilbers commented Apr 4, 2016

Since the error is by compiler rather than by linker and my assumption
(perhaps wrong) that fcntl function is declared in fcntl.h header file, I
considered them to be related.

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Mon, Apr 4, 2016 at 3:49 PM, David Halls notifications@github.com
wrote:

Is it fcntl.h that's not found or fcntl() ?

..\fs-ext.cc(195): error C3861: 'fcntl': identifier not found [C:\Dani-R&D\nodejs\node_modules\fs-ext\build\fs-ext.vcxproj]


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@davedoesdev
Copy link
Owner

I don't have a Windows licence for this project I'm afraid.
@baudehlo do you?

@dzilbers out of interest, does npm install fs-ext work on Node 0.12 or Node 4?

@dzilbers
Copy link
Author

dzilbers commented Apr 4, 2016

It does not work on Node 4. I do not have and I have not tried it with Node
0.12
I have Windows license

BR
Dani Zilberstein
home: +972-(0)9-7495890
mob: +972-(0)50-8503456
e-mail: dzilbers@gmail.com dzilbers@gmail.com

On Mon, Apr 4, 2016 at 7:24 PM, David Halls notifications@github.com
wrote:

I don't have a Windows licence for this project I'm afraid.
@baudehlo https://github.com/baudehlo do you?

@dzilbers https://github.com/dzilbers out of interest, does npm install
fs-ext work on Node 0.12 or Node 4?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#4 (comment)

@baudehlo
Copy link

baudehlo commented Apr 4, 2016

No I don't have windows at all. I just tried to copy what Perl did for
their support of these functions on Windows.

On Mon, Apr 4, 2016 at 12:24 PM, David Halls notifications@github.com
wrote:

I don't have a Windows licence for this project I'm afraid.
@baudehlo https://github.com/baudehlo do you?

@dzilbers https://github.com/dzilbers out of interest, does npm install
fs-ext work on Node 0.12 or Node 4?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#4 (comment)

@davedoesdev
Copy link
Owner

I'll see if I can find a licence from an old (broken) laptop of mine.
It won't be for a couple of weeks though.

@davedoesdev
Copy link
Owner

Per baudehlo/node-fs-ext#57 (comment)
Windows is going to be difficult to support due to MSVCRT mess.

@davedoesdev
Copy link
Owner

davedoesdev commented Apr 20, 2016

I don't really want to switch from fs-ext to a non OS-based locking module (like lockfile or node-proper-lockfile) because they have problems if the process exits without cleaning up. node-proper-lockfile uses staleness checks but that means if you set the staleness period too small then you could end up with two processes in the lock and too large means a lock may take too long to get.

However, I'm only using flock to handle single messages in qlobber-fsq. Moreover, mosca and ascoltatori don't use single messages (single messages are those given to at most one interested subscriber).

Therefore, I propose not to support single messages on Windows in qlobber-fsq. The rest of qlobber-fsq should work fine on Windows (I now have a Windows laptop resurrected so can test it).

@dzilbers @mcollina what do you think?

@mcollina
Copy link

I agree, it's a sound course of action. Make sure to cover this with relevant messages (maybe throwing an exception, or something similar).

@davedoesdev
Copy link
Owner

Giving some time to see where nodejs/node#6369 goes.

@baudehlo
Copy link

Thanks for taking this on. I wish I had more time for this project (this being fs-ext).

On Apr 27, 2016, at 6:12 PM, David Halls notifications@github.com wrote:

Giving some time to see where nodejs/node#6369 goes.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@davedoesdev
Copy link
Owner

davedoesdev commented May 2, 2016

OK, looks like the Node issue is going to take a while.

So here I'm intending to allow the user to specify whether to process single messages (i.e. work queue semantics, which requires file locking) when creating a QlobberFSQ object. This will be true by default, so existing behaviour maintained.

If it's set false then the QlobberFSQ object will ignore any single messages it encounters. This lets you have some QlobberFSQ objects to process work queues and others not (e.g. on mult-core machines or on distributed file systems). So ascoltatori will set this option to false.

Finally, I need to make installing fs-ext optional. If fs-ext is not installed but you don't disable work queue semantics when creating the QlobberFSQ object then you'll get an exception thrown.

Now, I can either specify fs-ext in optionalDependencies or just document that to get work queue (single) support, you must install fs-ext alongside qlobber-fsq.

What's everyone's opinions on optionalDependencies? I'd like to get this done soon because I want to make a new network message processor based on qlobber-fsq

@mcollina
Copy link

mcollina commented May 2, 2016

I'm 👍 on "optionalDependencies". they do not look like during install, but hey the thing work.
Just one note: I think the library should work in both cases and not rely on an a setting if the dep is missing.
You might console.log a warning, but I will not block this to work.

@davedoesdev
Copy link
Owner

@mcollina thanks. Good point, I'll do what the module currently does for warnings (emits a warning event or logs if there aren't any registered handlers).

@mcollina
Copy link

mcollina commented May 3, 2016

Right now it's an optionalDependency, but it's throwing at runtime because it can't find the module. I will would like to release an updated version of Ascoltatori with the latest version of qlobber-fsq.

@davedoesdev
Copy link
Owner

Closed by v2.0.0, which changes fs-ext to an optional dependency, adds a single option when constructing to disable work queue semantics and auto detects when fs-ext isn't available, emitting a single_disabled event.

Also tested on Windows (without work queue semantics - tests modified to account for this case).

@davedoesdev
Copy link
Owner

FYI, qlobber-fsq is now tested on Windows: https://ci.appveyor.com/project/davedoesdev/qlobber-fsq

Moreover, since baudehlo/node-fs-ext#70 was merged, it now fully supports single messages.

@pavanIT1996
Copy link

PS C:\Users\Pavan\Desktop\TodayServer> npm install mosca --save
npm WARN deprecated mongodb@2.1.21: Please upgrade to 2.2.19 or higher
npm WARN deprecated node-uuid@1.4.8: Use uuid module instead

> fs-ext@0.5.0 install C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules\fs-ext
> node-gyp configure build


C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules\fs-ext>if not defined npm_config_node_gyp (node "C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" configure build )  else (node "C:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js" configure build )
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
  fs-ext.cc
  win_delay_load_hook.cc
..\fs-ext.cc(108): warning C4996: 'Nan::NanErrnoException': was declared deprecated [C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules
\fs-ext\build\fs-ext.vcxproj]
  C:\Users\Pavan\Desktop\TodayServer\node_modules\nan\nan.h(862): note: see declaration of 'Nan::NanErrnoException'
..\fs-ext.cc(195): error C3861: 'fcntl': identifier not found [C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules\fs-ext\build\fs-ext.v
cxproj]
..\fs-ext.cc(297): warning C4996: 'Nan::NanErrnoException': was declared deprecated [C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules
\fs-ext\build\fs-ext.vcxproj]
  C:\Users\Pavan\Desktop\TodayServer\node_modules\nan\nan.h(862): note: see declaration of 'Nan::NanErrnoException'
..\fs-ext.cc(339): warning C4996: 'Nan::NanErrnoException': was declared deprecated [C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules
\fs-ext\build\fs-ext.vcxproj]
  C:\Users\Pavan\Desktop\TodayServer\node_modules\nan\nan.h(862): note: see declaration of 'Nan::NanErrnoException'
..\fs-ext.cc(374): error C3861: 'fcntl': identifier not found [C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules\fs-ext\build\fs-ext.v
cxproj]
..\fs-ext.cc(375): warning C4996: 'Nan::NanErrnoException': was declared deprecated [C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules
\fs-ext\build\fs-ext.vcxproj]
  C:\Users\Pavan\Desktop\TodayServer\node_modules\nan\nan.h(862): note: see declaration of 'Nan::NanErrnoException'
..\fs-ext.cc(433): warning C4996: 'Nan::NanErrnoException': was declared deprecated [C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules
\fs-ext\build\fs-ext.vcxproj]
  C:\Users\Pavan\Desktop\TodayServer\node_modules\nan\nan.h(862): note: see declaration of 'Nan::NanErrnoException'
gyp ERR! build error
gyp ERR! stack Error: `C:\Program Files (x86)\MSBuild\14.0\bin\msbuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\build.js:262:23)
gyp ERR! stack     at emitTwo (events.js:126:13)
gyp ERR! stack     at ChildProcess.emit (events.js:214:7)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:198:12)
gyp ERR! System Windows_NT 10.0.17134
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "configure" "build"
gyp ERR! cwd C:\Users\Pavan\Desktop\TodayServer\node_modules\qlobber-fsq\node_modules\fs-ext
gyp ERR! node -v v8.12.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok
npm WARN todayserver@1.0.0 No description
npm WARN todayserver@1.0.0 No repository field.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fs-ext@0.5.0 (node_modules\qlobber-fsq\node_modules\fs-ext):
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fs-ext@0.5.0 install: `node-gyp configure build`
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: Exit status 1

+ mosca@2.8.3
updated 1 package and audited 2539 packages in 52.746s
found 3 vulnerabilities (2 low, 1 moderate)
  run `npm audit fix` to fix them, or `npm audit` for details

Any Ideas!!!

@davedoesdev
Copy link
Owner

It's an optional dependency so mosca has still installed OK.
I'm going to look at fs-ext again. It seems (yet again) that some native Node stuff has been deprecated.
Will take a while, I'll update this ticket with what I find.
@pavanIT1996 thanks for the report!

@davedoesdev
Copy link
Owner

Waiting for nodejs/nan#810

@davedoesdev
Copy link
Owner

@pavanIT1996 by the way I'm only seeing deprecation warnings on Windows.
For me, fs-ext installs (and qlobber-fsq) installs fine.
The problem is probably in ascoltatori (which mosca depends on), which needs to bump its qlobber-fsq dependency: https://github.com/mcollina/ascoltatori/blob/master/package.json#L78

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

No branches or pull requests

5 participants