windows support about broccoli HOT 42 CLOSED

Node's sync file API is brittle on windows (Source) and broccoli uses it exclusively.

Discussion about async vs sync performance: joliss/node-walk-sync#1
EPERM on windows https://github.com/joliss/broccoli/issues/11

The best way to address the issue would be to go all async and switch to sync in performance critical regions on Linux and maybe Mac. Or at least that's how I currently see it. (Replacing something async with something sync and calling the callback immediatly is easy. The other way around not so much)

from broccoli.

Node's sync file API is brittle on windows (Source) and broccoli uses it exclusively.

I don't see anything in principle stopping us from using async IO, but it seems that the issue you linked (6599) is basically a problem in Node and needs to be fixed there. Curiously, the way they're reproducing it is with concurrent read & write access, and I can't think of any place where we're doing this in Broccoli. Perhaps there's a bug in Broccoli or a plugin; or the Node issue happens in more places than 6599 suggests.

from broccoli.

For Windows support, there's also hard linking in some plugins. It seems we need a hardlink-or-copy-and-preserve-mtime (with and without overwriting) kind of package to replace it. It's curious that we haven't run into this. Does fs.linkSync not fail on Windows?

from broccoli.

By the way, in general, I consider Windows support necessary, but it's really really low on my list of priorities. Don't expect me to put much work into it over the next few months.

from broccoli.

Linking works fine because the build works. It is supported by the file system (msdn article) and node integrates it.

from broccoli.

@joliss hopefully the community will help with windows support then. It's likely a large requirement for many.

from broccoli.

Could someone put together a list of known issues with Windows in case someone stumbles upon this issue and has some time to invest in getting it to work?

from broccoli.

The only specific issue I know of is

• brittleness with EPERM errors mentioned by @MajorBreakfast above.

That said, I think what really needs to be done to get Broccoli working reliably on Windows is

• writing a test suite (including perhaps some fairly high-level integration tests).

I don't personally have a dev environment on Windows around, so to stop Windows support from breaking repeatedly, having a test suite seems like a prerequisite.

from broccoli.

Instead of totally crashing, broccoli is now still operational after such an error occurs. I think it's because of the error handling with promises. We should try using rimraf async in quick temp.

from broccoli.

I created a PR for node-quick-temp that makes it async. This is the place where the calls to rimraf happen and probably the hot spot from where most windows errors originate. If that gets merged and we notify the plugin authors that they should update their code to only use async stuff, windows should be supported.

from broccoli.

I created a PR for node-quick-temp that makes it async. This is the place where the calls to rimraf happen and probably the hot spot from where most windows errors originate.

The issue with random EPERM errors clearly needs to be fixed. I believe however that it needs to be fixed in Node - this issue you mention might be related - and Broccoli shouldn't concern itself with it. If filesystem IO has strange race conditions and is unreliable, things are fundamentally going to break, and there's nothing we can do about it. Trying to play whack-a-mole and putting in workarounds (like rimraf.js:20) as issues pop up might be convenient as a stop gap, but I believe that it's fundamentally futile. I worry that trying to maintain this kind of code is going to turn into a timesink of "just one more workaround". So I'm not willing to accept PRs to taper over such race conditions.

In my view, the proper approach to fixing this is to pop open the Node source code and figure out what's really causing the EPERM errors.

There are two exceptions:

• The related Node issue alludes to this being triggered when files are being read and written in parallel. I don't believe that this is what's happening in this case, but if it was, there might be some kind of race condition in Broccoli, and we should fix it.
• If it turns out theoretically impossible to do certain things on Windows, as opposed to being a bug in Node's Windows code, I'd be happy to revisit.

from broccoli.

Looking through the source recently and noticed a couple places with hard coded / for making paths that will likely cause problems on windows. You'll want to replace those with path.join where possible or use path.sep.

from broccoli.

Does the '/' actually cause problems on Windows?

path.sep is a possibility but hard to read. path.join is too slow for many applications.

from broccoli.

I believe it will eventually cause you problems. Interesting about path.join performance, but it's there to solve multi-platform issues like this. Your approach of getting it faster in core is probably the best tack.

from broccoli.

I never had any problems with forward slashes on windows. Node's file functions can cope with them. Or at least I didn't encouter any problems so far.

from broccoli.

@MajorBreakfast have you had any luck with advancing Windows support for Broccoli?

I'd love to see Windows support within the next couple weeks. I'd be willing to offer a bounty if necessary.

from broccoli.

@ebryn It's easy to switch to sync in an async environment. The other way around is impossible without rearchitecturing. The decision is up to @joliss. To make it happen she has to make the switch by communicating the message to all plugin authors and correcting broccoli itself.

If you ask me, then async is the way to go in run loop based systems like JavaScript. (windows bugs or not)

Windows throws errors if you're trying to alter/delete files that are currently in use (EBUSY). The async implementation of rimraf handles them through retries after timeouts. This cannot be done synchronously because timeout is async.

from broccoli.

Just to get everyone on the same page: Windows support is, as far as I understand the issue, not blocking on making file IO in Broccoli async. That would seem to be a workaround for brokenness in Node, and my views are summarized above. As it is, I will not accept PRs to make things asynchronous, as changing the code structure to work around broken code elsewhere is clearly excessive.

I'm happy to agree to disagree with @MajorBreakfast, but wanted to be clear about what as a maintainer I will and won't merge.

(You can obviously use asynchronous code in plugins as you like - this is accomodated through promises - and this is perfectly fine. I'm not saying everything has to be synchronous. Just that using synchronous disk IO isn't bad.)

The way forward seems to be to investigate where the random EBUSY/EPERM errors are coming from, if I understand the problem correctly. It might be a problem with how Node/libuv does disk IO, or some more fundamental issue about how IO works on Windows in general.

from broccoli.

I'd like to see this get fixed within the next few days, as I'd like to use Broccoli for my Ember.js training classes.

I'm offering a $200 bounty for a solution that @joliss is satisfied with.

from broccoli.

@ebryn thoughts on a https://www.bountysource.com/ ?

from broccoli.

I kicked things off with BoutySource.com:

https://www.bountysource.com/issues/1509493-windows-support

Lets create a bounty large enough to make it happen!

from broccoli.

From my conversation with @joliss, I've decided to pursue interim solutions like the ones suggested by @MajorBreakfast.

We need Windows support ASAP for ember-cli. It's a massive blocker for my training classes, improving the Ember.js builds, and for everyone trying to use ember-cli on Windows. It's not pragmatic to block Windows support on fixing underlying issues in Node.js itself. It'll likely take months to get these fixes out into a stable build that our users will be able to use on their machines. That said, I believe we should be pursuing these fixes in parallel and remove the workarounds once a large population of users have upgraded to the appropriate version of Node in the future.

from broccoli.

If we can work around the issue then we absolutely should.

• jQuery would not be useful if it waited for browser's to fix bugs
• (I think) maintaining and depending on forks is a bigger liability than managing the work-around

I had no idea how big windows was among ember developers until ember-tools had issues in windows.

from broccoli.

@rpflorence Developping on windows has the huge advantage that you can immediatly test in IE. (and not through a VM) It has the adobe suite, sublime text and a command line - that's all you need. And since almost all computers except Macs ship with it, it's quite popular.

from broccoli.

@rpflorence totally agree

For now, I am using broccoli on windows and I didn't (for now) experienced your problem, the only one problem is with serve command raising ENOTEMPTY somtimes and I have to delete tmp folder so that it works again, my first investigations led me to think it has to do with the tmp folder not being cleaned correctly, maybe the node's exit event works differently on Windows

from broccoli.

@joliss I truly understand the frustration with what is Node's bug but I do not think waiting is the pragmatic choice. Ember-CLI and others now are going to be forced to fork broccoli in order to support Windows and this is going to cause confusion at a time when broccoli is collecting steam.

I implore you to reconsider.

from broccoli.

@joliss and WE are here to help you 👍

from broccoli.

Can someone link me to a repo that I can run under windows that fails to build properly? We are all talking about "the windows issue", but I haven't seen a set of repro steps yet....

from broccoli.

@rjackson from my understanding it is sparatic and typically manifests only under use. Maybe something we can script?

from broccoli.

Yeah, I'll throw my Ember builds at an EC2 instance sometime this week just to see what happens.

from broccoli.

We're working around this in ember-cli for now: ember-cli/ember-cli#493

Big thanks to @MajorBreakfast! He refused my bounty... what a guy! :)

from broccoli.

Can you guys explain in which case it fails ?

from broccoli.

Basically, no. I have yet to see anyone actually point to an issue that is actionable.

from broccoli.

@rjackson there's actually an issue with the serve command as I explained above, but it not "critical" for now, still it's an issue:
https://github.com/joliss/broccoli/blob/master/lib/server.js#L19
I think that the problem can be solved by cleaning on SIGINT signal in place of exit ?
see node process events

from broccoli.

@stefanpenner @rjackson @MajorBreakfast this fix is for Windows
joliss/broccoli-es6-concatenator#15

from broccoli.

🆙

from broccoli.

This should be an isolated reproduction + explanation.

https://gist.github.com/stefanpenner/2cc619b8740fe2463c2a

from broccoli.

I've added this on https://github.com/joliss/broccoli/issues/128.

Should we close this issue for now? If there are more problems with Windows support, we can open more issues for them.

from broccoli.

@joliss ya sounds good

from broccoli.

I just merged @krisselden's fix, PR #130, and released Broccoli 0.10.0. Can people please try buildling and serving broccoli-sample-app on Windows and let me know if it works now?

from broccoli.

Note: Some of the remaining issues may be due to virus scanners interfering with the files. Can people please try it, and if there's issues, report if they happen with or without virus scanners on?

from broccoli.

yeah, it might be!

from broccoli.