search

coolaj86 / formaline

formaline is a module for handling form requests ( HTTP POSTs / PUTs ) and for fast parsing of file uploads.
Apache License 2.0
239 stars 16 forks source link

Windows compatibility fixes. #26

Closed AtliThor closed 12 years ago

AtliThor commented 13 years ago

A few updates aimed at making this work with the current Windows build (v0.5.9).

The hard-coded "/tmp/" paths have been replaced by the Windows "TEMP" variable when available; the "/" dir separators are replaced by "\" when the os.type() matches "/windows/i", and the "fs.watchFile" call is removed.

I've tested this without any issues on both Windows 7, 64 bit with Node.js v0.5.9; and Ubuntu 11.04, 32 bit w/ Node.js v0.4.12.

By the way... Correct me if I'm wrong, but isn't the "fs.watchFile" call redundant? The only thing that changes the file being watched is a call to the writeToFileStream function with a payload to be written, and such a call already sets the mtime.

rootslab commented 13 years ago

Hi @AtliThor, thanks for your pull request, I'll take a look deeper at the code as soon as possible !

About fs.watchFile(), in my opinion it is not redundant, it's true that writeToFileStream() set the mtime, but I can't rely on this asynchronous call for setting the file mtime, I need the real mtime from filesystem, so, simply I get its value with fs.watchFile().

AtliThor commented 13 years ago

Ok, I see what you mean. I figured that it would be "close enough" since mtime doesn't keep the milliseconds.

I've been playing around with the fs.watch function, trying to get it to work, but it doesn't seem to be working very well on Windows. On Linux it appears to fire pretty much instantly for every payload that is written, but it executes very infrequently on Windows and usually way to late to be usable in the same way it is on Linux.

I've got no good ideas on how to work around that, so I'm thinking it's best to just bypass the watch function on Windows but leave it in there for the rest. Hopefully there will be some fix for the watch function in future Node releases. (Unless this is just one of those Windows oddities we have to live with...)

I've added another commit to do just that.