rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
33.9k stars 13.93k forks source link

GitHub gem(s) not found when using Gemfile.local.example #8272

Closed v-p-b closed 7 years ago

v-p-b commented 7 years ago

Steps to reproduce

  1. On master branch using rbenv, after msfupdate
  2. Copy Gemfile.local.example to Gemfile.local (as described here for example)
  3. ./msfconsole
  4. Startup fails with the following error: "The git source https://github.com/busterb/bit-struct is not yet checked out. Please run bundle install before trying to start your application"

Expected behavior

MSF console starts up successfully

Current behavior

Startup fails with the following error: "The git source https://github.com/busterb/bit-struct is not yet checked out. Please run bundle install before trying to start your application"

System stuff

$ ruby --version
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
$ rbenv --version
rbenv 1.1.0-2-g4f8925a
$ bundle --version
Bundler version 1.14.6

Metasploit version

fc3a880c0d59c3b61bd38cf22715423439459983 Land #8214, Fix ELM327 ISOTP commands

I installed Metasploit with:

OS

Ubuntu 16.04.2 LTS

busterb commented 7 years ago

It is telling you what to do - Please run bundle install - did you do that?

v-p-b commented 7 years ago

Yes I did. bundle install fixes it after I delete/move Gemfile.local.

v-p-b commented 7 years ago

I also checked my bundler directory under ~/.rbenv/ and found that the appropriate version of bit-struct is checked out there.

busterb commented 7 years ago

I have updated and corrected the instructions. Please give them a try.

v-p-b commented 7 years ago

Thank you, that solves my immediate problem! But wouldn't this interfere with future updates? I suppose Gemfile.local.example is commited as it is so that users won't have to resolve conflicts in their local copies all the time.

busterb commented 7 years ago

You're already implicitly modifying Gemfile.lock as well. If you're doing git pull, it should rarely have merge issues.

busterb commented 7 years ago

We could possibly just add something like this permanently to Gemfile, so it just does the right thing if the Oracle bits are installed

group :oracle do
  gem 'ruby-oci8' if File::exist?('/opt/oracle') && ENV::key?('SQLPATH') && ENV::key?('TNS_ADMIN') && ENV::key?('ORACLE_HOME')
end
v-p-b commented 7 years ago

Well, I personally got so frustrated by merge conflicts and outdated locks that I now have special notes in my MSF directory to remind me how to handle these reoccuring problems... I don't get why a .lock file should be version controlled, but I'm not a Ruby dev either.

As you seem to be OK with merge conflicts on update, shouldn't be the .example file removed from the repo?

I don't know what extensions other than Oracle use the local Gemfile, but if this is a reproducible issue I guess they should be notified too (or the example file should be updated so that it won't cause any problems).

busterb commented 7 years ago

Checking in Gemfile.lock dates from an earlier time - guess we'd have to ask Luke from 4 years ago what this is supposed to mean:

555a9f2559 - Refactor Msf::ModuleManager (4 years, 7 months ago)

As I can best tell, Gemfile.lock is just a note from our build system, or whoever last bumped a gem, that 'yep, these versions of these gems play nicely together', so that anyone who checks out Framework gets the same last-known-working-versions experience. If you're modifying Gemfile yourself, your first step should always be 'git checkout Gemfile.lock' when pulling. You can even delete it entirely since it's auto-generated if it's missing - it will simply then be pointing at the latest and greatest of every gem version.

Ruby gems used to be a lot more 'wild west, break the world every update'. The community has matured a bit since then.

v-p-b commented 7 years ago

Yes that matches my experience. I don't want to go into repo management details because I'm no expert and this should be your decision. This would also deserve a different Issue/PR.

I just wanted to point out that although modifying the master Gemfile can work around this particular problem, it won't help people relying on their local Gemfile (for whatever reasons) which looks like a supported use-case based on the repo contents. IMO you should either unsupport local Gemfiles or fix the provided example.

busterb commented 7 years ago

I think I'll defer to @egypt who added the local gemfile originally. Not certain it's the best path moving forward, and I don't want to maintain special cases if they can be avoided.

On the other hand, once we get rid of bit-struct entirely (unfortunately it stopped being maintained upstream, so we now have a local fork - which is where your problem came from downloading), this issue may also gently resolve itself. It takes a while because there are a fair number of folks in the community with out-of-tree code that also relies on bit-struct.

v-p-b commented 7 years ago

I think the one with bit-struct was just the first error that occured. Gemfile defines a couple of other gems in the same fashion:

gem 'bit-struct', git: 'https://github.com/busterb/bit-struct', branch: 'ruby-2.4'
gem 'method_source', git: 'https://github.com/banister/method_source', branch: 'master'
gem 'rubyntlm', git: 'https://github.com/WinRb/rubyntlm', branch: 'master'

If I comment out bit-struct and run bundle install the same error occurs but now with method_source upon stratup.

busterb commented 7 years ago

pretty odd - I've even tried reproing locally but no such luck. I'm using rvm but no idea if that matters.

v-p-b commented 7 years ago

I created a fresh clone from master, then:

$ cp Gemfile.local.example Gemfile.local
$ ./msfupdate
[*]
[*] Attempting to update the Metasploit Framework...
[*]

[*] Checking for updates via git
[*] Note: Updating from bleeding edge
fatal: 'upstream' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Those fatals may indicate the problem we have here. After this:

[...]
Bundle complete! 18 Gemfile dependencies, 132 gems now installed.
Use `bundle show [gemname]` to see where a bundled gem is installed.
$ ./msfconsole 
The git source https://github.com/busterb/bit-struct is not yet checked out. Please run `bundle install` before trying to start your application
$ bundle install
[...]
$ ./msfconsole 
The git source https://github.com/busterb/bit-struct is not yet checked out. Please run `bundle install` before trying to start your application
busterb commented 7 years ago

is there a reason you use 'msfupdate' here? why not just 'git pull' if you're using source checkout?

v-p-b commented 7 years ago

This has been the least painful way, especially because of dependency management. IIRC after git pull I'd also have to do at least a bundle installl to get things right, right?

egypt commented 7 years ago

@busterb If you delete Gemfile.lock, yes, you get the latest of everything that isn't locked to a specific version. That also means you get a bunch of code that we haven't run through CI.

busterb commented 7 years ago

Understood @egypt , though we do simply run rspec and bump every week. Frequency of failure is about once every 6 months (usually due to bundler or rake breaking something)

jmartin-tech commented 7 years ago

A reasonable write up containing what reads to me as good advice on when and when not to commit Gemfile.lock.

http://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/

In our case we have a combination of an Application and a Gem however the reasons to commit the file for an Application described hold true here for why Metasploit should maintain Gemfile.lock.

egypt commented 7 years ago

@v-p-b Note the comment in Gemfile.local.example --

# This file will not be used by default within the framework. As such, one
# must first install the custom Gemfile.local with bundle:
#   bundle install --gemfile Gemfile.local
#
# Note that msfupdate does not consider Gemfile.local when updating the
# framework. If it is used, it may be necessary to run the above bundle
# command after the update.

@busterb I agree with you that Gemfile.local is not a great solution, but the problem it solves kinda sucks too. It's the only way to add gems to the framework without requiring that they be in upstream master because of the way Bundler works. Bundler modifies the load path so that only gems in Gemfile.lock can be required.

egypt commented 7 years ago

@busterb When you say "bump every week" do you mean we run bundle update?

busterb commented 7 years ago

Yep, we've been doing it for over a year.

busterb commented 7 years ago

I think I figured out how to make Gemfile.local work for you. Try this:

bundle install --gemfile Gemfile.local

v-p-b commented 7 years ago

@egypt Thanks, I totally missed that note. Does this mean that not running bundle install --gemfile Gemfile.local after an update thus getting errors regarding gems of the master Gemfile is expected behavior? In this case you can close this issue (taking care of this in msfupdate sounds like a good idea to me though), I can see the changes of the Oracle wiki page is already reverted.

Thanks again for the help!

busterb commented 7 years ago

I only found a random posting on stackoverflow that suggested it, and it worked for me. Not sure if expected, but that's the reality of it :)

Yeah, if we updated msfupdate to be aware of Gemfile.local, that would solve your issue.