codekitchen / dinghy

faster, friendlier Docker on OS X
MIT License
2.12k stars 109 forks source link

Errors with fsevents_to_vm installation on setup #255

Closed avit closed 6 years ago

avit commented 6 years ago

The FseventsToVm#install_if_necessary! method effectively runs this command:

sudo -H /System/Library/Frameworks/Ruby.framework/Versions/Current/usr/bin/gem install --no-rdoc --no-ri -n /usr/local/bin fsevents_to_vm -v ~> 1.1.3

A couple of problems here,

  1. The VERSION constant has a "~" ~which is expanded by the shell to my home directory, forming an invalid command. Changing this to a fixed version fixed this for me.~

  2. The system version of the gem install command on Mac OS Sierra is from ruby 2.0. This is too old and causes problems when there are already any installed gem executables in /usr/local/bin, because ruby 2.0 doesn't recognize the newer executable format installed by later rubies when Gem::Installer#check_executable_overwrite inspects them:

/Library/Ruby/Gems/2.0.0/gems/thor-0.19.4/thor.gemspec
Exception `NoMethodError' at /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/package/tar_reader.rb:71 - undefined method `seek' for #<Zlib::GzipReader:0x007f92dd203bc8>
Exception `TypeError' at /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/installer.rb:171 - no implicit conversion of nil into String
ERROR:  While executing gem ... (TypeError)
    no implicit conversion of nil into String
    /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/installer.rb:171:in `check_executable_overwrite'

(thor is a dependency that is already installed, and causing an error here.)

We could force-overwrite executables with gem install -f, but maybe this is not the best solution. Does this need to be installed via the system version of rubygems or can we use a preferred version?

(e.g. brew installed ruby or ruby-install/chruby/rvm)

codekitchen commented 6 years ago

We call system with individual string arguments, which means that shell expansion does not happen (see docs for the system method), so I don't understand your first point. Can you give more details on what you're seeing? Something else must be going on.

We have to install via the system ruby, since dinghy runs using the system ruby. We do this because there's just too much variation in what versions of ruby people have installed, and how they are configured. Homebrew does the same thing for similar reasons. I'm unable to reproduce the problem you're seeing, I think because I've already upgraded to High Sierra which includes ruby 2.3. But maybe one option is to use the --bindir flag to have it install the binaries somewhere else, since we don't need them to be in /usr/local/bin.

avit commented 6 years ago

You're right, there was no issue with the version constant. I probably ran into this while debugging and running the command parts manually joined in the shell. The actual issue was still with the gem executable conflict.

I worked around it by installing fsevents_to_vm with --force, which allowed overwriting /usr/local/bin/thor.

For the actual issue, I'd like to still see if we can make this installation as painless as possible, since part of my goal for using dinghy is to avoid having to walk new developers through complex setup steps to get their docker environment running. It's still not quite "one-click" but the fewer exceptions the better!

Just for background, in my own setup, I have:

I try to avoid installing anything into the system ruby when possible, so I know what's "mine" to replace. All my globally-installed ruby tools (such as thor) usually end up in the brew-installed rubygems.

dinghy runs using the system ruby. We do this because there's just too much variation in what versions of ruby people have installed, and how they are configured

Is this a hard requirement, or just found to be less problematic? Since brew is used to install dinghy, could it make sense to use the brew-installed ruby as the preferred dependency (when installed), then use Apple's system ruby as the fallback?

The option to specify the --bindir could also work, but it would still default to /usr/local/bin anyway. It might also be confusing if this option only affects rubygems and not other tools that dinghy installs.

Another option could be to provide a flag such as --rubygems_installer=/usr/local/bin/gem which would put it in the expected gem repository, thus recognizing gems that are already installed there, and not just change the bindir.

Finally, in case we can't work around this error, there should be some error handling around the installer with a more helpful error message, and let the user deal with it using those installer flags, or --force or whatever... probably not a good idea to automatically do it for them.

Let me know what you think, I'd be happy to send a pull-request.

codekitchen commented 6 years ago

I'm saying use --bindir internally to install the binaries elsewhere, invisible to the user. Not expose a new flag to Dinghy users or anything like that. That should be harmless, since we don't need the binaries to be installed anywhere user-accessible anyway.

assembledadam commented 6 years ago

FYI I've just upgraded to High Sierra, and ran 'dinghy up'. Got the same error as the OP.

➜  ~ dinghy up
Starting the dinghy VM...
Starting NFS daemon, this will require sudo
Password:
Waiting for NFS daemon...
Mounting NFS /Users/adam
Installing fsevents_to_vm, this will require sudo
Fetching: rb-fsevent-0.9.8.gem (100%)
Successfully installed rb-fsevent-0.9.8
Fetching: thor-0.19.4.gem (100%)
Successfully installed thor-0.19.4
Fetching: net-ssh-2.9.4.gem (100%)
Successfully installed net-ssh-2.9.4
Fetching: fsevents_to_vm-1.1.3.gem (100%)
ERROR:  While executing gem ... (TypeError)
    no implicit conversion of nil into String
/usr/local/Cellar/dinghy/4.5.0/cli/dinghy/daemon.rb:60:in `system!': Error with the FsEvents daemon during installing (RuntimeError)
    from /usr/local/Cellar/dinghy/4.5.0/cli/dinghy/fsevents_to_vm.rb:35:in `install_if_necessary!'
    from /usr/local/Cellar/dinghy/4.5.0/cli/dinghy/fsevents_to_vm.rb:16:in `up'
    from /usr/local/Cellar/dinghy/4.5.0/cli/cli.rb:217:in `start_services'
    from /usr/local/Cellar/dinghy/4.5.0/cli/cli.rb:79:in `up'
    from /usr/local/Cellar/dinghy/4.5.0/cli/thor/lib/thor/command.rb:27:in `run'
    from /usr/local/Cellar/dinghy/4.5.0/cli/thor/lib/thor/invocation.rb:126:in `invoke_command'
    from /usr/local/Cellar/dinghy/4.5.0/cli/thor/lib/thor.rb:359:in `dispatch'
    from /usr/local/Cellar/dinghy/4.5.0/cli/thor/lib/thor/base.rb:440:in `start'
    from /usr/local/bin/_dinghy_command:12:in `<main>'
codekitchen commented 6 years ago

Darn. I'm not sure what's different between our environments, but it sounds like we better try the --bindir approach or something else. Unfortunately since I haven't been able to repro the problem, I'll need some help from others to confirm whether it fixes the issue or not. I'll give it a shot.

My original plan was to rewrite fsevents_to_vm as a self-contained binary in Go or something, the Ruby version was only intended to be a prototype. That would fix it too of course, but I probably will not have time to do that anytime soon.

assembledadam commented 6 years ago

What fixed it for me was the same as @avit - using --force when manually installing fsevents_to_vm.

Pretty sure it was to do with the gem/ruby version, which must have changed with the upgrade to High Sierra (was working perfectly on Sierra).

FYI system Ruby version: ruby 2.3.3p22 System gem version: 2.6.9

avit commented 6 years ago

Specifically the error is with using an older version of ruby to do the gem installation over top of gems installed by a newer gem installer. MacOS Sierra has ruby 2.0, but I'm surprised this is still an issue with Apple's system ruby in High Sierra.

When there are gems installed in /usr/local/bin, the old gem installer can't recognize this:

$ tail -7 /usr/local/bin/thor

if Gem.respond_to?(:activate_bin_path)
load Gem.activate_bin_path('thor', 'thor', version)
else
gem "thor", version
load Gem.bin_path("thor", "thor", version)
end

The above is what a modern rubygem executable uses to load itself. It checks for both the old and new methods of activating the gem, depending on the version of rubygems so that it can support both. Compare to what it looks like when installed with Apple's ruby:

$ tail -3 /usr/local/bin/fsevents_to_vm

gem 'fsevents_to_vm', version
load Gem.bin_path('fsevents_to_vm', 'fsevents_to_vm', version)

Gem::Installer looks for these lines to check if the file is safe for overwriting (since was created by rubygems), but Apple's version of rubygems is too old to recognize these lines in "thor", and it throws an error instead of allowing to overwrite the file.

Can someone please verify what the above gems look like on High Sierra? With just system ruby? With brew's ruby installed?

My thinking & suggestion on this would be that:

  1. dinghy is installed using homebrew, so expecting brew as a dependency manager for ruby makes sense.
  2. The brew-installed ruby will be more current/recent than Apple's, and more compatible.
  3. There will likely be installed gem executables from brew ruby already in /usr/local/bin that will conflict with overwriting by Apple's older ruby gem installer (especially common ones like rake & thor).
  4. If there is a brew-installed ruby, then we should prefer that to run the installation and avoid overwriting the gem executables with an older format.
  5. If there is no brew-installed ruby then go ahead and use Apple's, since there should not be anything in /usr/local/bin to conflict. Gems from other ruby managers (rvm, chruby etc.) are not installed there.

So, that, or we can just --force install over it. I'm not too sure about changing the --bindir, since then it would have to be something else in the user's default PATH... dinghy feels like a general system utility that should be available globally without "special" configuration. (That's what we use VMs for!)

EDIT: maybe you're right about the --bindir if these are only called internally by dinghy, but it seems odd to have these installed gems normally available too.

codekitchen commented 6 years ago

There are a lot of people using something other than brew to install a newer version of ruby (rbenv, etc), so that doesn't really seem like a general solution. Using whatever ruby the user has installed, however they have installed it, will open a whole can of worms that I don't want to deal with. We tried that originally with Dinghy and ended up on the same solution as Homebrew, always using the system ruby.

The --bindir solution doesn't require adding to PATH, dinghy only needs fsevents_to_vm to use internally, it's not something the user will normally call directly.

Really using rubygems at all here is a hack that I only did because it was low effort. Another solution would be to just fold fsevents_to_vm into Dinghy rather than gem installing it. That would make more sense, but is a bit of work since we need the fsevents C extension installed.

codekitchen commented 6 years ago

Actually, it looks like the rb-fsevent gem now ships with a pre-built binary, it no longer builds the C extension on gem install. So my original reason for using rubygems no longer applies... it may be easiest to just not use rubygems anymore. I'll investigate that.

assembledadam commented 6 years ago

@avit Most likely moot if @codekitchen can remove the rubygems dep, but: I have brew installed ruby (versions in above post) on High Sierra.

$ tail -7 /usr/local/bin/thor          
  if str =~ /\A_(.*)_\z/ and Gem::Version.correct?($1) then
    version = $1
    ARGV.shift
  end
end

load Gem.bin_path('thor', 'thor', version)
$ tail -3 /usr/local/bin/fsevents_to_vm
end

load Gem.bin_path('fsevents_to_vm', 'fsevents_to_vm', version)
avit commented 6 years ago

Cool! It didn't look like the issue is anything related to C compilation since I didn't have any issues with that: this conflict is just about the gem wrapper #!ruby script.

There are a lot of people using something other than brew to install a newer version of ruby (rbenv, etc), so that doesn't really seem like a general solution.

Yes, I do that too: those don't install anything to /usr/local/bin so there is no issue there. Chruby sets up like this, for example:

$ gem env
RubyGems Environment:
  - RUBYGEMS VERSION: 2.4.5.2
  - RUBY VERSION: 2.2.6 (2016-11-15 patchlevel 396) [x86_64-darwin15]
  - INSTALLATION DIRECTORY: /Users/andrew/.gem/ruby/2.2.6
  - RUBY EXECUTABLE: /Users/andrew/.rubies/ruby-2.2.6/bin/ruby
  - EXECUTABLE DIRECTORY: /Users/andrew/.gem/ruby/2.2.6/bin

The only conflict I see is between Apple ruby and brew ruby (which may be installed as a dependency by other brew utilities).

avit commented 6 years ago

@assembledadam Interesting, those lines look like they are unchanged from Sierra's gem installer, but from your post it looks like dinghy did try to reinstall after upgrade... This is Sierra's rubygems version:

$ /System/Library/Frameworks/Ruby.framework/Versions/Current/usr/bin/gem env | head -2
RubyGems Environment:
  - RUBYGEMS VERSION: 2.0.14.1

(Just out of curiosity now.)

codekitchen commented 6 years ago

Oh I see what you mean, yeah that's true. I mentioned the C extension only because it was the original reason to use rubygems in the first place (because it handles compiling C extensions on gem install), not because it's directly causing this issue.

Now that it's not an issue, I've pulled fsevents_to_vm directly into Dinghy and everything seems to be working great. Upgrade to the latest Dinghy (v4.6.0) and it no longer uses rubygems at all, so this should be fixed. Let me know if you run into any issues and and hopefully we can close this out.

avit commented 6 years ago

https://github.com/codekitchen/dinghy/commit/005e800405fe1a78ab86eee868caebd30a06e1ad

Nice, that makes sense. There's also gem install --install-dir option if you'd still prefer to avoid the submodules and stay totally out of the way of any other local gem repositories.

I think we can close this now. Thanks!

assembledadam commented 6 years ago

Nice work guys!