net-scp patch version increased net-ssh dependency minor version #80

Closed
mitchellh opened this Issue Feb 6, 2013 · 20 comments

7 participants

@mitchellh

net-scp 1.0.5/1.0.6 was released today, and as far as I can see the only thing it did was increase the net-ssh dependency to ">= 2.6.5" whereas 1.0.5 was ">= 1.99.1."

I maintain Vagrant which previously locked to net-ssh ~> 2.2.2 and net-scp ~> 1.0.4 for a stable series (1.0.x). I simply cannot justify risking Vagrant stability by upgrading from 2.2 to 2.6 of net-ssh. I took a look at the CHANGES.txt and the changes are not insignificant.

In my opinion, a patch version shouldn't do anything that even remotely risks breaking existing applications, and I don't feel confident in this when the minor versions are increasing.

Please restrict net-scp back to ">= 1.99.1" and release a net-scp 1.1.0 that changes minor version dependencies.

@jtimberman

I'm posting to this issue as it is related to the problem we encountered in the Chef project (re: #79), and the sub-projects (net-ssh-multi/net-ssh-gateway) do not have GitHub issues enabled.

As I posted briefly, Chef depends on net-ssh 2.2.2, however the update of net-ssh-gateway/net-ssh-multi's patch release (1.1.0 -> 1.1.1) to depend on net-ssh >= 2.6.4, causing a conflict.

We're moving forward with a relaxed constraint in Chef v11.2.0, but I'd like to request that future tiny (patch) be more cautious, a 1.99 to 2.6 is a pretty big jump, particularly with consideration for semver.

@sethvargo

👍 to everything @mitchellh and @jtimberman have said, but I'd like to add:

Please YANK 1.0.5 and 1.0.6 (when a new stable version is released)

@delano
Collaborator

net-scp-1.1.0 is now available on rubygems.org and 1.0.5, 1.0.6 are yanked.

@mitchellh

Woohoo. Thanks so much @delano :)

@mitchellh mitchellh closed this Feb 6, 2013
@mitchellh mitchellh referenced this issue in mitchellh/vagrant Feb 6, 2013
Closed

net-ssh version mismatch #1355

@jashmenn

Thanks @delano

@delano
Collaborator

You're welcome!

@jtimberman

Cheers

@waynehoover waynehoover referenced this issue in rubber/rubber Feb 12, 2013
Closed

Gem dependency issue with net-ssh #269

@nirvdrum

Please do not yank any gems, unless they do something like like "rm -rf /". Yanking the gems is sure to create more deployment issues for people than simply keeping the versions there, because yanking gems will break any gem install via bundler, even if the version is locked down.

@delano
Collaborator

Thanks for the feedback. I was debating it but decided to yank them since those versions broke many chef and vagrant installs.

Is there a canonical reference for yanking criteria? (Something akin to semver.org)

@nirvdrum

I guess the gem yanking ship has sailed. That's an extremely big hammer to use. Yanking them has now broken deploys for Rubber users.

Leaving the gems wouldn't have affected anyone using bundler until they bundle updated. Until that time, they would have just downloaded the version they were already downloading. Anyone relying on a production system shouldn't bundle update gems without knowing what changed anyway, so I'd be shocked if this really impacted people. On the other hand, once you have updated and locked down, yanking the gem prevents installation through bundler and you won't know until you update a system in production, since you'd likely have the gem installed locally already.

I don't know of any defined guidelines for yanking gems. But please understand that when you do so, you will break deploys for anyone using that version. And neither rubygems nor bundler really tells you why the gem couldn't be installed, so it's confusing at best.

@sethvargo

There are multiple reasons to yank: http://help.rubygems.org/kb/gemcutter/removing-a-published-rubygem

  1. It ensures no one else receives the broken gem
  2. It forces the Ruby gems dep API to reference the new version
  3. Breaking a deploy is better than breaking an application. I'd rather break the deploy and have the app still running that have a successful deploy but a botched gem.
@nirvdrum

This is obviously highly subjective. One could just as easily argue that your test suite should've caught the problem, and you'd roll back the version anyway. Breaking deploys suck, especially when there's been a string of Rails security updates. Again, yanking a gem gives you no indication of why the gem couldn't install, so most people don't even know how to begin debugging the problem. In the meanwhile, they can't deploy out security fixes.

The gem may have been broken for your uses. That doesn't necessarily mean it was broken for everyone's.

@sethvargo

The gem introduced dependency hell AND violated semver...

I see your point, but I still assert that yanking the gem was the best decision.

You can also download and vendor the gem (still) at the yanked version.

@nirvdrum

I know you're not advocating it, but I'm not going to tell everyone they should run a full mirror of RubyGems in case someone opts to yank a gem. Nor is vendoring every gem a sensible option. Vendoring just this one is catching the problem too late. At that point, I may as well just update the Gemfile. Yanking gems undermines the trust in published artifacts.

By the same release management logic, you can never trust everyone is going to follow semver and you can never trust transitive dependency graphs, so you should use more restrictive version matches. You also shouldn't blindly bundle update.

I guess we're just going to have to agree to disagree on best practices in release management. And they're already yanked, so the damage is done.

@delano
Collaborator

@nirvdrum I actually agree with you about release management logic. Blindly assuming everything upstream is correct and safe leaves the door open to a lot of issues. The tipping point was that I received a lot of emails and tweets about the issue within the first hour or so of the problem.

So I agree with you ideologically, but in practice I made the call to yank them to satisfy the most vocal groups. I didn't make the decision lightly. More background here:
http://solutious.com/blog/2013/02/06/so-i-made-52000-mistakes-today/

@nirvdrum

@delano Thanks for the thoughtful reply. I guess you got two waves because many people using capistrano + net-ssh weren't adversely affected until the gems were yanked. Naturally they wouldn't have been vocal prior to that, since nothing was broken for them :-P

In any event, I'm handling the issues on the rubber tracker and emailed the group to give people a head's up. I suspect most of the support requests will be resolved within the next week. It would be really nice if either rubygems or bundler informed you that a gem had been yanked when it fails to install, but that's another battle for another project.

@fred

ohh nooooo.

now, seriously, why not release net-scp 1.0.9 instead of 1.1.0 because now, hundreds of gems and apps with bundler are broken ....
since most are using net-scp (~> 1.0.4)
thus, 1.0.9 would not break them.... but 1.1.0 does.

cheer..

@delano
Collaborator

Why are they broken? Unless someone specifies one of the two yanked gem in their Gemfile, bundler will install 1.0.4.

@nirvdrum

Because rubber uses fog (~> 1.6) and fog's net-scp dependency was "~> 1.0.4". When updating to rubber 2.2.0, almost all users also ended updating fog and net-scp. Then net-scp was yanked. This is the problem with yanking transitive dependencies. Many fog users were affected.

[Edit: Sorry I got confused on the issue tracker here since I dealt with it in several places. But, fog was by far the biggest place I saw the yanked gem cause problems for people. No need to rehash that discussion though. The transitive dependency part is where a lot people got messed up though, because they don't have net-scp in their Gemfile at all. Now they had to go back and add an entry to lock down to 1.0.4.]

@delano
Collaborator

I understand. Thanks for the clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment