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

Use bundler/setup instead of custom loading code #530

Merged

Conversation

kgrz
Copy link
Contributor

@kgrz kgrz commented Oct 4, 2015

I'm not sure if this has been discussed already. Using the require 'bundle/setup' directive made the gem activation errors a bit more
predictable for me during some preliminary runs. It's quite possible
that I might've missed something here.

Adding "require 'bundler/setup'" to a file will add the bundled gems to
the load path. This is a much safer way to load gems instead of using
gem <gemname>, <version> where version is parsed from the Gemfile.

Loading 'bundler/setup.rb' does the following things:

  1. Cleans load path; one of it's function is to removing system gems
    from the load path.
  2. Loads the gem specs from Gemfile.lock
  3. Sets up the following path variables:
    BUNDLE_BIN_PATH
    BUNDLE_GEMFILE: location of the Gemfile
    PATH: Appends the location of bundle executable for the current
    Ruby version to the PATH.
    RUBYOPT
    RUBYLIB
  4. Activate the loaded Gem file specs. This is where the command would
    throw an exception if there's a conflict in the case where a
    different version of a particular gem in the Gemfile.lock has already
    been loaded.

This makes the process more transparent. After this change, the error
notifications would be split into two cases: 1. Before the connection
between Go daemon and the Ruby process gets established and 2. After the
connection gets setup.

In the case of 1, the error messages still won't show up in the UI and
dtruss or strace is the only way to figure out the error. However,
in the case of 2, any error in the loading would cause the zeus Go
process to throw an error (prompts turning red) and running any of the
commands would result in a backtrace.

Adding "require 'bundler/setup'" to a file will add the bundled gems to
the load path. This is a much safer way to load gems instead of using
`gem <gemname>, <version>` where version is parsed from the Gemfile.

Loading 'bundler/setup.rb' does the following things:

1. Cleans load path, whose one function is to removing system gems from the
   load path.
2. Loads the gem specs from Gemfile.lock
3. Sets up the following path variables:
   BUNDLE_BIN_PATH
   BUNDLE_GEMFILE: location of the Gemfile
   PATH: Appends the location of `bundle` executable for the current
         Ruby version to the PATH.
   RUBYOPT
   RUBYLIB
3. Activate the loaded Gem file specs. This is where the command would
   throw an exception if there's a conflict in the case where a
   different version of a particular gem in the Gemfile.lock has already
   been loaded.

This makes the process more transparent. If there are any conflicts, an
exception would be raised and this can be caught and the resulting
backtrace can be handled properly.
@kgrz
Copy link
Contributor Author

kgrz commented Oct 4, 2015

Intended to somewhat mitigate issues like #237 #489

@kgrz
Copy link
Contributor Author

kgrz commented Oct 4, 2015

Any pointers on how can this be tested are appreciated. I couldn't really figure that part out. (assuming this even works in general)

@latortuga
Copy link
Collaborator

I like this approach quite a bit, will give it a try locally and see how it works out!

@kgrz
Copy link
Contributor Author

kgrz commented Oct 5, 2015

This is one of the failure situations:

A project's Gemfile specifies method_source 0.8.2; When zeus gets installed using gem install zeus, the method_source gem version in zeus' Gemfile.lock is 0.8.2. I have just the version0.8 in my global gem list for that ruby version. In this case, the exit status 1 issue crops up. But even this change doesn't seem to fix it, although I specifically remember it being solved for a particular setup when I made this commit :/

I tried to add some print statements inside the Bundler.setup method inside the bundle gem in my ruby gem list and I can see that the things mentioned above are happening; the correct Gemfile is being picked up (the project one, not the zeus' own Gemfile) but I no clue why method source 0.8 is getting activated. This happens even if I remove all the calls to require 'method_source' from all zeus' files.

Ruby version: 2.1.5
Bundler version: 1.10.6
Zeus version: 0.15.4

@kgrz
Copy link
Contributor Author

kgrz commented Oct 5, 2015

Update

TL;DR

Even if bundler/setup is used, there is a high chance that the loading of gems will fail because of the runtime_dependency on method_source. The other often seen problems with rake and json can potentially be fixed by using require 'bundler/setup'.


The longer version

The two things I had to do to really make it work:

  1. Remove dependency on method_source. The rubygem's gemspec has a runtime_dependency on method_source >= 0.6.7.
  2. Change the file requires like require 'zeus/m' to use File.expand_path just to preserve relative path loading functionality. Here's a patch for that: https://gist.github.com/5ce3247a1b7291e1d612 I will open a separate PR if this looks okay.

To test, this is what I did:

  • The project I use has rake v. 10.4.2, method_source v. 0.8.2, json v 1.8.2 in it's Gemfile. (These are usually the gems that result in the breakage.
  • I removed these version of the gems from my global gem list and installed one version previous gems. That is, rake v.10.1.0, method_source v. 0.8.
  • I then ran zeus start in the project repo, and it worked.

Now, removing the method_source dependency is probably not going to go well, so the workaround needs to be figured out. (Since I had method_source getting included thanks to pry in my Gemfile, the tests ran fine)

To figure out the issue, this is what I did:

Modified bundler code (bundler/runtime.rb) to just log some of the functions that are there inside the def setup method which is what gets called when we include require 'bundler/setup'. At this stage, if I check the output of Bundler.rubygems.loaded_specs('method_source').version, the output is 0.8 instead of 0.8.2 which should've been loaded from Gemfile. However, bundler won't do this, and will throw an error. So I think since zeus has a runtime dependency on method_source, it will load the library that exists in the global gem directory for that ruby, and this makes it fail when bundler/setup is called.

latortuga added a commit that referenced this pull request Nov 30, 2015
…icit

Use bundler/setup instead of custom loading code
@latortuga latortuga merged commit c26469a into burke:master Nov 30, 2015
@latortuga
Copy link
Collaborator

I'm happy enough with issues surrounding json and rake being mitigated by this to merge it.

@kgrz
Copy link
Contributor Author

kgrz commented Nov 30, 2015

@latortuga Thanks for checking this out!

metcalf pushed a commit that referenced this pull request Jul 18, 2016
This reverts commit c26469a, reversing
changes made to d809774.

The README currently advises running Zeus outside of bundler:
https://github.com/burke/zeus/blob/810c8f49798d4f64b16894abd94beedbddb0b37f/README.md#installation

I agree that running Zeus without bundler is probably a bad tradeoff
in terms of stability but I don't think we can change this without
breaking everyone's Zeus integrations:
#570

I believe we should revert until we're prepared to re-introduce this
with changes to the README, better error messaging to help people
transition and possibly a major version bump.
metcalf added a commit that referenced this pull request Jul 25, 2016
Revert #530 (requires using Zeus with bundler)
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

Successfully merging this pull request may close these issues.

None yet

2 participants