burke / zeus

Boot any rails app in under a second.
MIT License
3.33k stars 231 forks source link

Zeus doesn't reload code after change on macOS sierra #589

Closed pavel-d closed 7 years ago

pavel-d commented 8 years ago

Description of Problem

Zeus doesn't reload code after changes on macOS sierra

System details

$ uname -a
Darwin Pavels-MacBook-Pro.local 16.0.0 Darwin Kernel Version 16.0.0: Mon Aug 29 17:56:20 PDT 2016; root:xnu-3789.1.32~3/RELEASE_X86_64 x86_64

$ ruby -v 
ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]

Steps to Reproduce

1) zeus start in a new rails project It boots fine, but one error is printed to the stdout:

zeus start
Starting Zeus server v0.15.10
[ready] [crashed] [running] [connecting] [waiting]
boot
└── default_bundle
    ├── development_environment
    │   └── prerake
    └── test_environment
        └── test_helper

Available Commands: [waiting] [crashed] [ready]
zeus rake
zeus runner (alias: r)
zeus console (alias: c)
zeus server (alias: s)
zeus generate (alias: g)
zeus destroy (alias: d)
zeus dbconsole
zeus test (alias: rspec, testrb)
2016-09-09 15:12 zeus-darwin-amd64[61609] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)

2) touch config/application.rb

Observed Behavior

Nothing happens

Expected Behavior

zeus should reload the code

metcalf commented 8 years ago

I wonder if something in https://github.com/fsnotify/fsevents/blob/master/fsevents.go needs to be changed to support Sierra? Sadly that project has no tests so it's hard to isolate :-/. Zeus' test should catch this but I'd need a VM running Sierra, I'll see if I can spin one up.

pavel-d commented 7 years ago

@metcalf, seems so, I've recompiled the gem locally using go1.7.1 darwin/amd64, but the error is still there and the code reloading doesn't work :(

metcalf commented 7 years ago

@pavel-d: I got Zeus running inside a Sierra VM and can't reproduce this issue. What version of the Sierra preview are you running? Can you try creating a new, clean project with Zeus in it and see if the same issue occurs?

metcalf commented 7 years ago

@pavel-d: I upgraded my VM to the latest GM release of Sierra and still can't reproduce this. It'd be helpful to know if this repros for you on the latest Sierra release and with a new, clean project.

pavel-d commented 7 years ago

@metcalf, thanks! Sure, I'll dig a bit later this week

salamagd commented 7 years ago

Hmm, having the same problem with an existing project in Sierra, will see if I can reproduce it on a new project too.

metcalf commented 7 years ago

I just uncovered a slew of other bugs and edge cases in Zeus' file monitoring code (e.g. if you replace a watched file by moving another on top of it, it's no longer watched because the underlying inode changed). The work required to handle all these edge cases is great enough that I'm looking into ways to use Facebook's watchman in place of the current file monitoring code. I'm not sure when I'll be able to land that (or if it will work) so it'd still be great to have a repro of the Sierra issues, but if you can't repro then hopefully a solution is in the works!

metalelf0 commented 7 years ago

I'm having the same exact issues as @pavel-d, Sierra 10.12 and zeus 0.15.10. Being stuck on Rails 3 never felt so bad. :(

avegancafe commented 7 years ago

Is this still open? It seems like it's more of an issue with fsevents

mix86 commented 7 years ago

I'm having the same issue too. Sierra 10.12 (16A323), ruby 2.1.6p336, zeus 0.15.10. After start i see

2016-10-03 19:46 zeus-darwin-amd64[27141] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)

and zeus does not reload code

uname -a
Darwin mixael-osx.local 16.0.0 Darwin Kernel Version 16.0.0: Mon Aug 29 17:56:20 PDT 2016; root:xnu-3789.1.32~3/RELEASE_X86_64 x86_64
zubin commented 7 years ago

@kyleholzinger Yes it's still open. The "closed" above your comment refers to the PR on the left which mentions this PR.

avegancafe commented 7 years ago

Right, was just wondering since this seems like an issue with cgo, not zeus @zubin

antifuchs commented 7 years ago

I've been researching this (since it's a blocker for our upgrade to macOS Sierra), and it seems something changed in Sierra to cause zeus's fsevents program to not work correctly anymore - the advice at https://github.com/facebook/react-native/issues/9309#issuecomment-238966924 says to bump kernel file limits (they were very low in 10.11, and I wonder if they got bumped even lower in 10.12).

The other piece of advice is to upgrade to watchman 4.6 (we don't use watchman in Zeus, but bear with me). It includes a bunch of fsevents-related changes over 4.5, mostly in stream setup, and the handling of the UserDropped attribute. I'm installing a Sierra VM right now, to attempt to repro this too, so maybe I can dig up something more substantial.

avegancafe commented 7 years ago

@antifuchs it's not zeus' fsevents specifically, it's cgo-- this is the line that it's breaking on, which is an external call

But either way, a switch off of fsevents would fix the issue 😂

antifuchs commented 7 years ago

Oh, interesting, I thought zeus still ships its own objc fsevents wrapper! Disregard all I said, then (:

avegancafe commented 7 years ago

@antifuchs I'm not 100% positive but I think it uses the go one

dsounded commented 7 years ago

+1

pietdaniel commented 7 years ago

I was on sierra with 0.15.4 and was not experiencing this

2016-10-11 15:56 zeus-darwin-amd64[86241] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)

After upgrading to 0.15.10 the warning comes up.

avegancafe commented 7 years ago

Interesting, I wonder if they moved from native fsevents/self-packaged to cgo fsevents @pietdaniel

artforlife commented 7 years ago

Was anyone able to resolve this? I come from React-native side and following their steps didnt help to resolve it.

artforlife commented 7 years ago

I was able to resolve it as follows: https://github.com/facebook/react-native/issues/9309#issuecomment-238966924

mikz commented 7 years ago

Changing max files as suggested does not fix it for me.

pariser commented 7 years ago

@metcalf need a computer running Sierra to test? Wanna swing by one of these days?! I'm also experiencing this error and would be happy to help :-p

andrew-stripe commented 7 years ago

@pariser (or anyone else) any chance you have a Go environment set up and can try running make test under Sierra? I'm really hoping we can repro this in tests, add a test to https://github.com/fsnotify/fsevents that also repros the problem and fix it there.

andrew-stripe commented 7 years ago

(err, sorry @andrew-stripe == @metcalf, I'm signed in from my work account)

avegancafe commented 7 years ago

@andrew-stripe running now

avegancafe commented 7 years ago

@andrew-stripe @metcalf just got this:

Error: Package "/Users/Kyle/workspace/zeus" not a go package or not in GOPATH

I tried both adding it to $GOPATH as well as moving it inside of my go directory, neither worked

andrew-stripe commented 7 years ago

@kyleholzinger: Go likes a very particular directory structure. You need to set a GOPATH environment variable and place Zeus in $GOPATH/src/github.com/burke/zeus.

mcolyer commented 7 years ago

@andrew-stripe I just had a clean run on eb88f0816fd with go version go1.7.3 darwin/amd64 on 10.12.1.

cd rubygem && bin/rspec
.............................

Finished in 0.02885 seconds (files took 0.09957 seconds to load)
29 examples, 0 failures
andrew-stripe commented 7 years ago

That snippet of output only refers to the Ruby tests, I'd have expected to see failures (if any) in the Go tests. Those look like:

ZEUS_TEST_GEMPATH="/Users/andrew/code/go/src/github.com/burke/zeus/rubygem" GO15VENDOREXPERIMENT=1 govendor test +local
?       github.com/burke/zeus/go/clienthandler  [no test files]
?       github.com/burke/zeus/go/cmd/zeus   [no test files]
?       github.com/burke/zeus/go/config [no test files]
ok      github.com/burke/zeus/go/filemonitor    1.391s
?       github.com/burke/zeus/go/messages   [no test files]
?       github.com/burke/zeus/go/processtree    [no test files]
?       github.com/burke/zeus/go/shinylog   [no test files]
?       github.com/burke/zeus/go/statuschart    [no test files]
ok      github.com/burke/zeus/go/unixsocket 0.008s
?       github.com/burke/zeus/go/zerror [no test files]
?       github.com/burke/zeus/go/zeusclient [no test files]
ok      github.com/burke/zeus/go/zeusmaster 1.954s
?       github.com/burke/zeus/go/zeusversion    [no test files]
mcolyer commented 7 years ago

Sorry about that, here's the output I have:

ZEUS_TEST_GEMPATH="/Users/mcolyer/gopath/src/github.com/burke/zeus/rubygem" GO15VENDOREXPERIMENT=1 govendor test +local
?       github.com/burke/zeus/go/clienthandler  [no test files]
?       github.com/burke/zeus/go/cmd/zeus       [no test files]
?       github.com/burke/zeus/go/config [no test files]
ok      github.com/burke/zeus/go/filemonitor    1.256s
?       github.com/burke/zeus/go/messages       [no test files]
?       github.com/burke/zeus/go/processtree    [no test files]
?       github.com/burke/zeus/go/shinylog       [no test files]
?       github.com/burke/zeus/go/statuschart    [no test files]
ok      github.com/burke/zeus/go/unixsocket     0.008s
?       github.com/burke/zeus/go/zerror [no test files]
?       github.com/burke/zeus/go/zeusclient     [no test files]
ok      github.com/burke/zeus/go/zeusmaster     1.683s
?       github.com/burke/zeus/go/zeusversion    [no test files]
andrew-stripe commented 7 years ago

Hmm, doesn't fail tests, that makes life more difficult. That would be consistent with the explanation that it has to do with max file limits since the tests watch very few files. Recent versions of Watchman encourage you to watch an entire project directory which may help avoid this.

I'm not sure whether errors from the file watching code were surfaced in the same way in 0.15.4. It may have the same issue and is just swallowing the error message. Can anyone confirm whether they observe reloading on file changes on Sierra in 0.15.4 and not 0.15.10 (regardless of any error messages)?

pariser commented 7 years ago

I'm not seeing any reloading behavior in v0.15.4 or v0.15.10. (Nor am I seeing any error messages in v0.15.4).

andrew-stripe commented 7 years ago

Sounds like this was always broken and the old file monitor just swallowed the error messages. That's not surprising given how it worked.

My barely-informed guess is that this has to do with Zeus tracking each individual file instead of setting up a watch on the project directory. The test suite doesn't track enough files to hit any kind of limit and raising the maxfiles limit works for some projects. Assuming this is correct, the fix could be local to the filemonitor package but it wouldn't be trivial. It's also possible that something in https://github.com/fsnotify/fsevents/blob/master/fsevents.go is broken for Sierra.

We've decided to move away from Zeus in favor of our own code loader with Watchman for file monitoring. I'm happy to help review changes and answer questions but I don't expect to be investing much more in Zeus.

semenovDL commented 7 years ago

@andrew-stripe Do you have some examples of replacing zeus with watchman?

andrew-stripe commented 7 years ago

Unfortunately nothing off the shelf. We're building internal tooling that's somewhat specific to our codebase. I'm hoping to open source the work someday, but given that we're only about to go into internal beta that could be quite awhile.

On Thu, Nov 10, 2016 at 5:41 AM, Alexey Kuznetsov notifications@github.com wrote:

@andrew-stripe https://github.com/andrew-stripe Do you have some examples of replacing zeus with watchman?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/burke/zeus/issues/589#issuecomment-259692295, or mute the thread https://github.com/notifications/unsubscribe-auth/AWAzbFsp6MkIAjVlON2K0z-8lGP6KcCYks5q8x7ugaJpZM4J5EKO .

leecunliffe commented 7 years ago

I've switched to using spring, seems it still works with the latest OSX sierra.

kenn commented 7 years ago

From what I read in https://github.com/facebook/react-native/issues/9309 , it looks like it's a permission issue?

sideshowcoder commented 7 years ago

I wonder if a quick fix for that would be to monitor directory instead of files on osx, it seems to work everywhere except osx. I ran into it today as well but only on a larger rails app trying to figure out the limit now.

sideshowcoder commented 7 years ago

This seems to be related to the number of monitored files as I have 2 apps 1 causes the issue (the bigger) and a smaller which does not experience it, this is on the same machine with the same version of zeus.

To resolve this the way to go seems to be to monitor the directory instead of the files for changes and then filter in the filemonitor for events we are interested in (based on the file). I took a look at the code in filemonitor_darwin and it seems to be reasonable enough to change it here without the need to touch much else. First step so is to add a test as I wonder what the amount of files needed is. I'm gonna give this a shot when I have a Mac available (aka work).

siannopollo commented 7 years ago

It would be nice to see that commit by @sideshowcoder merged into master and a new gem released. Having to constantly restart zeus is kind of antithetical to its existence.

sideshowcoder commented 7 years ago

@siannopollo sadly the commit is not a fix yet, but just a failing test and a description of the fix I'm working towards, any help is greatly appricieated and I'm happy to merge as soon as it is ready 👍

sideshowcoder commented 7 years ago

@siannopollo if you fancy testing a local built, #620 now includes a potential fix for the problem, I have to do some more local testing before merging but it seems like it fixes the issue.

sideshowcoder commented 7 years ago

Please check version 0.15.13.pre and see if it fixes the problem it seems to work for my setup.

kenn commented 7 years ago

0.15.13.pre fixed the error for me! 👍 Thank you @sideshowcoder !

sideshowcoder commented 7 years ago

I'd consider this a fix then, seem to have the same success on other machines as I can see locally here 👍

siannopollo commented 7 years ago

0.15.13.pre also fixed this issue for me. Thanks @sideshowcoder

avegancafe commented 7 years ago

I feel like from now on if this fixed anyones issue just respond with a reaction to one of the other messages haha. Plz no more emails ❤️

siannopollo commented 7 years ago

@kyleholzinger Don't you find it ironic that in order to voice that opinion, you had to leave a comment that would send emails to everyone on this thread? I mean, Github provides a Subscribe/Unsubscribe button at the top of each issue if you don't want to be notified any more... ¯_(ツ)_/¯

siannopollo commented 7 years ago

@kyleholzinger But you have no control over the actions of others. So instead of trying to dictate how others should behave to suit your preferences, why don't you use the tools that Github provides to achieve the experience you desire?