Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

RFC: A new approach to working with gems under development #1641

Closed
wants to merge 11 commits into from

Conversation

osheroff
Copy link

This somewhat references issue #396. Note that the code is functional (and still has a couple of development-process warts), but likely still buggy and not ready for a pull. I wanted to see if the main bundler devs had any feedback on the approach I'm taking. It adds functions to the Gemfile DSL that look like:

  gem "mygem", :git => "...", :ref => "v1.0"
  local "mygem", "~/src/mygem"

At my job, we have a big monolith, and a bunch of supporting gems. Our current process for working on one of our component gems is:

  1. checkout the source locally
  2. edit Gemfile, point the gem at a :path source
  3. bundle install
  4. hack hack hack
  5. tag the gem
  6. edit Gemfile, point the gem at the new tag
  7. bundle install
  8. push

I wanted to be able to get a few of these steps. To that end, the features I wanted:

  • Be able to easily swap between the upstream source and my local copy of the gem, without having to edit Gemfile or re-bundle. This seems to be working in the current branch. It handles preferring local-source over either git or rubygems specs.
  • Have bundler detect when my local copy is too out-of-date to use, and warn me that it's going back to the original source. This works in this branch when explicitly specifying gem versions (even for git sources), but it's not currently smart enough to know when your locally checked out copy is behind the upstream ref specified in the Gemfile.

The patch is more invasive to the current source base than I'd like, but I couldn't come up with a clean way to do this at the Bundler::Source level (see my other branches for failed attempts at that).

I'm also not sure about the syntax. I was thinking one could also do:

   gem 'mygem', :git => '...', :ref => 'v1.0', :local => "~/src/mygem"

or even have a Gemfile.local with some syntax? I dunno.

Anyway, I digress. Would love any and all comments.

@indirect
Copy link
Member

I definitely like this idea (and it's something that we want to do). There's a lot of discussion around these issues in ticket #396. Since you even mentioned adding an option to :git as a possibility, what do you think about tweaking this patch to implement the consensus idea from that ticket?

@osheroff
Copy link
Author

So I went back and re-read issue #396, and I'm stil not clear on what the consensus actually was. That said;

This is easy:

# I want to override checking this out from git, if a copy exists at ~/src/foo
gem "foo", :git => "...", :path => "~/src/foo"

in which path overrides git.

However, the following would be impossible, as bundler can't decide the meaning:

# I want to override a gem that we normally pull from rubygems.org with my local development copy
gem "normally_at_rubygems.org", :path => "~/src/normally"

How do you feel about:

gem "foo", :git => "...", :local => "~/src/foo"
gem "at_ruby_gems", "2.4.6", :local => "~/src/my_copy"

@indirect
Copy link
Member

In general, I like gem 'foo', :git => '...', :path => '~/src/foo' the best. Bundler users are already familiar with :path, and by itself it does in fact mean to use a local copy.

For your earlier question about warning on old versions, I'm pretty sure that if a local :path exists it should always override a specified :ref. Gems in a :path don't have to be git repos, so it could be awkward to try to check what revision they are. :)

Can you elaborate a little bit more on "would be impossible, as bundler can't decide the meaning"? We don't support .gem files from anywhere except rubygems and ./vendor/cache. If you mean that you'd just like to have an optional local override for that rubygems gem just like you could for a :git gem, why wouldn't that work? It would probably require a conditional in the DSL parser that checks for the existence of the path before deciding whether to treat the gem like a :path gem or like a rubygems gem.

@osheroff
Copy link
Author

I guess I just meant "would be impossible while preserving the existing behavior of :path". And complicated :)

The call gem 'foo', :path => "~/src/foo" would have to mean:

On Bundler.setup:

  • if the path exists, use that
  • otherwise, check rubygems for that gem
  • otherwise, crash

On bundle install:

  • if the gem is in rubygems, use that (?)
  • otherwise, put a PATH source in Gemfile.lock

Which seems a bit tricky. One of the reasons I'm stumping for :local instead of :path is that it's clear to define the semantics: "local" always means "try this path, but it's not the canonical source for this gem. Don't store the path in Gemfile.lock." whereas "path" would still mean "this gem lives on the filesystem here. period."

As far as old versions, my only concern is that engineers would leave old copies of source lying around on their machine, and inadvertently rely on them, even when the main project got out of date (hilarity and cursing ensues). I suppose it's a gun that we could let people shoot themselves in the foot with. I forget, what's the current behavior if you say: gem 'foo', "1.1", :path => '...' but your foo.gemspec says it's only 1.0?

@indirect
Copy link
Member

Bundler says "could not find foo 1.1 in any of the sources" or something to that effect. For what it's worth, the Bundler docs have always been extremely clear that :path is not a canonical source for any gem, because Bundler is about running the same code on multiple machines, and :path doesn't automatically work if you install a bundle on another machine. That's why I thought it made sense to use :path as a "use this if it exists" option for those gems, but that would not be put into the lockfile as the canonical gem location.

On Jan 27, 2012, at 11:54 PM, osheroff wrote:

I guess I just meant "would be impossible while preserving the existing behavior of :path". And complicated :)

The call gem 'foo', :path => "~/src/foo" would have to mean:

On Bundler.setup:

  • if the path exists, use that
  • otherwise, check rubygems for that gem
  • otherwise, crash

On bundle install:

  • if the gem is in rubygems, use that (?)
  • otherwise, put a PATH source in Gemfile.lock

Which seems a bit tricky. One of the reasons I'm stumping for :local instead of :path is that it's clear to define the semantics: "local" always means "try this path, but it's not the canonical source for this gem. Don't store the path in Gemfile.lock." whereas "path" would still mean "this gem lives on the filesystem here. period."

As far as old versions, my only concern is that engineers would leave old copies of source lying around on their machine, and inadvertently rely on them, even when the main project got out of date (hilarity and cursing ensues). I suppose it's a gun that we could let people shoot themselves in the foot with. I forget, what's the current behavior if you say: gem 'foo', "1.1", :path => '...' but your foo.gemspec says it's only 1.0?


Reply to this email directly or view it on GitHub:
#1641 (comment)

@osheroff
Copy link
Author

hm, yeah. There's probably someone out there who uses :path to ship around local gems in some weird way, but I guess it's not necessary that that's supported.

I'll see how the code works if we say "we never store :path => '' options in Gemfile.lock" -- even if there's no rubygems source behind it. If that seems to work ok, I'm sold on using :path in that way.

@indirect
Copy link
Member

After thinking about this more, I think you are probably right about gems from Rubygems. We'll either have to break backwards compatibility by removing :path from the lock entirely, or need a different option name so we can tell it's an optional first source.

Do you have a strong desire to add :path/:local to regular rubygems, or is adding :path with :git fallback enough to satisfy your common use case?

On Jan 28, 2012, at 12:08 AM, osheroff wrote:

hm, yeah. There's probably someone out there who uses :path to ship around local gems in some weird way, but I guess it's not necessary that that's supported.

I'll see how the code works if we say "we never store :path => '' options in Gemfile.lock" -- even if there's no rubygems source behind it. If that seems to work ok, I'm sold on using :path in that way.


Reply to this email directly or view it on GitHub:
#1641 (comment)

@osheroff
Copy link
Author

Definitely having :git with override would satisfy 90% of the cases at my work (zendesk.com), although we do have a small handful of public gems that we'll hack on in the context of our main app.

@osheroff
Copy link
Author

maybe a straw poll on issue #396 to see if anyone else would care about local-development of a rubygems-hosted gem? i dunno.

@exviva
Copy link

exviva commented Jan 28, 2012

I'm having a quite similar problem: we're hosting a bunch of our proprietary gems on own own gem server (geminabox), and would like bundler to fetch them from there in production, but during development, we'd like to use our local git clones.

An ideal solution for us would be something like:

gem 'our-proprietary-gem', git: 'github/url', local: '..'

then, running bundle install --local (or having --local in .bundle/config) would:

  1. run git clone github/url .. if does not exist yet
  2. use the cloned repo just like with :path

and running bundle install (without --local anywhere) would just fetch the gem from the sources.

The caveat is handling various refs of the same repo in different Gemfiles. For instance, one project is locked to tag v1.1, while another one is locked to v1.2 (using the :tag option), both pointing to the same :local path.

Now bundle install should also git checkout tag v1.1 when executed in project 1, and tag v1.2 in project 2, which seems like quite a monster-feature.

I think it makes sense to store some kind of an option in .bundle/config so that a single Gemfile.lock can behave differently in production and in development. What do you guys think? I'm willing to work on a patch, given some directions.

@osheroff
Copy link
Author

well, I changed it from using the "local" method to having a :local option, and bug-fixed a bit.

@indirect, It does sound like exviva is looking to override a rubygems source with a local copy. So there's one vote... I think.

@exviva, I'm a little confused about your workflow, but it sounds like this patch would help with some of what you want. You would still have to manage your git repos by hand, though. You'd probably just do:

source :geminabox
gem 'foobar', '1.1', :local => "..."

having the "1.1" in there would be important, as it make it so that if you have 1.2 checked out locally bundler will throw a warning and just use the geminabox version.

@exviva
Copy link

exviva commented Jan 31, 2012

@osheroff you're right, we'd like to be able to depend on a gem from a rubygems-compatible gem server. However, this is of minor importance (we can still point to a git repository, the only negative factor being the size of the repository compared to the .gem file).

What's more important for us, it to be able to develop a hierarchy of gems locally, and then release them, all using one Gemfile per project (without having to constantly update the Gemfile.lock back and forth).

I guess this is something contradictory to what Bundler is all about: ensuring that the same Gemfile.lock activates the same dependencies across installations of the projects. On the other hand, having different development/production dependencies is such a typical pattern that I'd vote for bundler supporting it.

Regarding your example with gem 'foobar', '1.1', :local => "...", this would probably be enough for us. This means that when working on a new feature, we'd need to:

  1. go to ../foobar
  2. bump Foobar::VERSION to 1.2
  3. go to the original project
  4. bump dependency on 'foobar' to 1.2
  5. hack
  6. release foobar 1.2
  7. release the original project

right? Or am I missing something?

If we have 3 projects in a hierarchy of dependencies, we'd have to repeat steps 1-4 and 6-7 in the second and third project as well, right?

@osheroff
Copy link
Author

@exviva, yeah, you're mostly right. Although with this patch, the gem in :local is always preferred if it satisfies requirements. So let's say you have a "child" gem sitting at 1.1, and the master project references gem 'child', '1.1', :local => '' -- in that case there's no steps to take before starting to work on it, since your local copy will be preferred over the rubygems/git copy.

@exviva
Copy link

exviva commented Jan 31, 2012

Ah, ok, great! Can't wait for this to get accepted. Can I help in any way?

@indirect
Copy link
Member

indirect commented Feb 1, 2012

Testing this pull request to see if you can break it would be a great first step. :) I like the idea of :local, so provided we can add it while keeping everything working that was working before, this seems like a good addition.

@exviva
Copy link

exviva commented Feb 7, 2012

@indirect I hope to find some time this week to give it a spin.

@jasonmp85
Copy link

Don't know that I'll have any time/expertise to contribute meaningfully to getting this tested/in, but just wanted to chime in here with my +1

@osheroff
Copy link
Author

osheroff commented Feb 9, 2012

Hey @indirect -- so I want to start testing this at my work too, but it's going to be hard to get 40 devs to buy into a simultaneous upgrade.

To that end, I was thinking about adding the local 'gem', 'PATH' syntax back in so that I can add something to my gemfile like:

  gem 'foo', '1.2.3', :git => '....'
  local 'foo', "~/src/foo" if self.respond_to?(:local)

I think that the gradual upgrade + backwards compat path is maybe an argument for supporting both flavors of the syntax. Thoughts?

@indirect
Copy link
Member

indirect commented Feb 9, 2012

The problem with two separate declarations of the same gem is that... there are then two separate declarations of the same gem. Gemfile policy has been to throw an exception anytime there are two declarations of the same gem, because Bundler can't guarantee that it will use the gem that the user intends. (For example, if someone adds gem 'foo', :git ... to a Gemfile that already has a local 'foo' declaration in it, intending that the git gem be used instead of the local one...). As a result, I think that the backwards-compatible way to write your Gemfile should probably be something more like this:

local = (Bundler::VERSION >= "x.x")
gem 'foo', '1.2.3', {:git => '...'}.merge(local ? {:local=>'...'} : {})

Does that rationale make sense, at least?

@osheroff
Copy link
Author

osheroff commented Feb 9, 2012

the rationale makes sense, at least syntactically. I got permission from my CTO to use some very ugly syntax in our Gemfile, so I'll give your method a shot.

@indirect
Copy link
Member

You might be able to improve the syntax, at least...

local_foo = local ? {:local => '...'} : {}
gem 'foo', '1.2.3', {:git => '...'}.merge(local_foo)

@exviva
Copy link

exviva commented Feb 20, 2012

I'm having a hard time testing this, I have no idea how to build and use bundler from source and make my bundle executable use that version of Bundler, especially when RVM is involved :). Anyone care to give me some hints?

@ghost
Copy link

ghost commented Feb 20, 2012

You can try RUBYLIB=../bundler/lib ../bundler/bin/bundle

@indirect
Copy link
Member

I think by far the easiest way is to check out the source and then run rake install. You can uninstall it using gem when you're done.

@indirect indirect closed this Feb 20, 2012
@indirect indirect reopened this Feb 20, 2012
@exviva
Copy link

exviva commented Mar 5, 2012

Hi guys,

I've been playing around with it for a while, and I have to say, it was confusing for us to understand, which version is being used.

We're using git submodules and most submodules are under active development, so it was very rare that the version declared in the Gemfile and the version checked out in the submodule match. Most of the time, we would still go into the submodule and make sure the proper commit is checked out, or fast-forward to master.

Thus, all in all, this feature wasn't that useful considering the amount of manual work that needs to be done and cannot be delegated to Bundler.

@indirect
Copy link
Member

indirect commented Mar 7, 2012

Thanks for testing this patch. You make a really good point, and the Gemfile syntax definitely seems somewhat awkward. I think that if you are using git submodules, this feature is unlikely to be helpful to you at all. In fact, I typically use git gems via Bundler as an alternative to git submodules. Doing that means I can effectively manage the git revisions of my gem deps in the same place as I manage all the rest of my gem versions.

After some deliberation with Bundler core and alumnus, I think the better way to manage this particular feature is to allow the "local" option to be set via bundle config. That means that developers can keep their checkouts in different directories, and not have to repeatedly edit the project-wide Gemfile. The danger (of course) is that it may not be obvious that you are using a local version of some code instead of a version that is pushed to a server somewhere. If we move forward with that type of implementation, I think it makes sense to print a yellow-colored warning during deploys if local paths contain any code that is not yet checked in or not yet pushed.

As always, feedback is greatly appreciated while we try to nail down a good way to do this. :)

@marten
Copy link

marten commented Mar 26, 2012

Wouldn't that issue (Gemfile specifying older version than you're using locally, and the application code depending on the newer version) not only manifest while deploying, but also when running your project against a continuous integration server?

@indirect
Copy link
Member

indirect commented Apr 8, 2012

José implemented a version of this feature that uses bundle config on the master branch. Please try it if you're interested, and give us any feedback you have (preferably as new issues). Thanks!

@indirect indirect closed this Apr 8, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants