rubygems / bundler

Manage your Ruby application's gem dependencies
https://bundler.io
MIT License
4.89k stars 2k forks source link

Incorrect handling of RubyGems versions 1.8.0..1.8.19. #2793

Closed MrJoy closed 10 years ago

MrJoy commented 10 years ago

The Modern class in lib/bundler/rubygems_integration.rb looks like this:

    # Rubygems ~> 1.8.5
    class Modern < RubygemsIntegration
      def stub_rubygems(specs)
        Gem::Specification.all = specs

        Gem.post_reset {
          Gem::Specification.all = specs
        }

        stub_source_index(specs)
      end

      def all_specs
        Gem::Specification.to_a
      end

      def find_name(name)
        Gem::Specification.find_all_by_name name
      end

      def build(spec, skip_validation = false)
        require 'rubygems/builder'
        Gem::Builder.new(spec).build(skip_validation)
      end
    end

The problem is, that until RubyGems version 1.8.20, the build method takes no parameters.

A more correct solution would be to have two classes, one for RubyGems 1.8.0..1.8.19 that calls build without parameters, and one for 1.8.20+ that calls it with the skip_validation parameter.

You can confirm the method signature issue for yourself by looking at the relevant tags of RubyGems:

https://github.com/rubygems/rubygems/blob/v1.8.19/lib/rubygems/builder.rb

  def build
    @spec.mark_version
    @spec.validate
    @signer = sign
    write_package
    say success if Gem.configuration.verbose
    File.basename @spec.cache_file
  end

https://github.com/rubygems/rubygems/blob/v1.8.20/lib/rubygems/builder.rb

  def build(skip_validation=false)
    @spec.mark_version
    @spec.validate unless skip_validation
    @signer = sign
    write_package
    say success if Gem.configuration.verbose
    File.basename @spec.cache_file
  end
MrJoy commented 10 years ago

Sorry, my apologies: The Modern class is for >= 1.8.5, and < 2.0.0. I.E. this bug shouldn't affect users of RubyGems 1.8.0..1.8.4.

zzak commented 10 years ago

More importantly is how does this effect us?

Could you write a failing test or are there any failing tests already?

MrJoy commented 10 years ago

How does it affect us? Simply put, anyone with RubyGems version >=1.8.5 and < 2.0.0 will have a failure when trying to install a gem via :path or :git.

As for writing a failing test -- I leave that to you. I don't see any infrastructure for testing against multiple versions of Rubygems, and don't feel like solving that problem for you. Our solution is to roll back to a version of Bundler that isn't brain-damaged wrt the version of Rubygems installed until we're in a good position to update to a non-stale version of Rubygems. This bug report, and the research I put into it, which gives you an exact root cause and a clear path to a robust solution is provided as a courtesy only.

In short: I am not your QA bitch.

indirect commented 10 years ago

The question was if you could write the test. You could have just said "nope, I'm too busy right now", even.

Your defensiveness was also based on incorrect assumptions: we already have infrastructure to run the entire Bundler spec suite against any version of Rubygems. You can see it on Travis, or by running rake -T. No need for you to "solve that problem for us".

Finally, excessively defensive responses that include misogynistic slurs are completely unwelcome here. Go somewhere else if your idea of "collaboration" is to be a sexist jerk.

MrJoy commented 10 years ago

You're right, my choice of words was poor. "QA lackey" or "QA grunt" lack the same impact, which is why I chose it. I'll be more mindful in the future.

That said, I'm sick and tired of doing shit-work for open source projects who can't be bothered to lift a finger. Since it's apparently so easy to add a new version of Rubygems to the test suite, what actually needs to be 'written'? Why even ask me to do so? Why not just add the line of metadata instead?

I took the time to read through the blighted mess of a spec suite that you have, and saw nothing about Rubygems -- yes, had I looked at the Travis config I might have inferred that "RGV" referred to "Rubygems Version", but well, I didn't. That speaks to the poor discoverability of your test suite.

Spare me the semantic games about what "could" means. Was I being asked about my level of proficiency with Ruby? Because your interpretation of the question only makes sense if that was the case. My interpretation -- that I was being asked to do even more work makes far more sense in context.

If you think hiding behind a critique of my choice of language makes it OK for you to ignore the issue I raised, then you've only proven me right in my assessment that actively spending even more time contributing a pull request would be wasted effort. I've contributed hundreds of patches across dozens of PRs to a variety of OSS projects with over 160 patches having been accepted upstream across half a dozen projects and as a result of that, frankly I've developed a serious lack of patience for projects that want me to spoon feed them because they can't be bothered to do basic things themselves.

Stop and consider this: I was asked to contribute what ultimately turns out to be a two-line metadata patch -- a trivial thing for people familiar with the codebase and test suite, but decidedly non-trivial for someone (like me) who is not. I was asked to do a disproportionately great amount of work after I'd already proven that an issue exists, and given you sufficient information to produce that two-line metadata addition yourselves. (To be clear: a mismatch in method arity between definition and call is an error in Ruby, and I showed that your code makes an incorrect arity assumption against a very specific list of Rubygems versions.)

The reflexive behavior of putting work off onto contributors is, frankly, repugnant. Do what you will with this report, I'm going to spend my limited time and energy contributing to projects that actually value third party contributions, instead of treating them with disdain.

indirect commented 10 years ago

Feel free to contribute wherever you like. Your entitlement to our free work while insulting us isn't appreciated.

xaviershay commented 10 years ago

hug it out, this has escalated much further than it would have in person.

@MrJoy thanks for the bug report.

MrJoy commented 10 years ago

At what point did I express any 'entitlement'?

Oh, and to be clear, my use of the word 'bitch' was not used in the sense of 'a spiteful or unpleasant woman' [Oxford American Dictionary, definition 2, variant 1], but in the sense of 'a person who is completely subservient to another' [Oxford American Dictionary, definition 2, variant 3]. I will avoid using the word in the future since clearly it's subject to misinterpretation -- and in your case, I suspect that misinterpretation was entirely intentional.

radar commented 10 years ago

That said, I'm sick and tired of doing shit-work for open source projects who can't be bothered to lift a finger. Since it's apparently so easy to add a new version of Rubygems to the test suite, what actually needs to be 'written'? Why even ask me to do so? Why not just add the line of metadata instead?

The whole point about open source is that the code is open for you to contribute back to. Be that either in the form of excellent issue reports like your initial post, or patches to fix those same issues. Why ask you to do some more work? Because the Bundler team would rather be collaborative than exclusionary. It is also because it takes the load off them from having to do all the work, and by doing that you spread the knowledge of Bundler internals outside of just a handful of people. You may even learn something too, but it seems you're pretty savvy about the RubyGems/Bundler internals already, so perhaps not. Who knows?

Travis config I might have inferred that "RGV" referred to "Rubygems Version", but well, I didn't. That speaks to the poor discoverability of your test suite.

To be fair, in .travis.yml it's pretty well commented (besides it pointing to seemingly the wrong line in Rakefile):

# Rubygems versions MUST be available as rake tasks
# see Rakefile:66 for the list of possible RGV values
env:
  # We need to know if changes to rubygems will break bundler on release
  - RGV=master
  # Test the latest rubygems release with all of our supported rubies
  - RGV=v2.1.11
  # Test the latest stable branch so we know the next minor will work
  - RGV=2.1

3 mentions of RubyGems around that section makes me think that RGV means "RubyGems version"

Spare me the semantic games about what "could" means. Was I being asked about my level of proficiency with Ruby? Because your interpretation of the question only makes sense if that was the case. My interpretation -- that I was being asked to do even more work makes far more sense in context.

We don't know your level of proficiency in Ruby, having never worked with you. The "could" in that sentence probably meant "are you able to write the test?" or "could you please spare some more time to write a test for us?". I agree with @indirect that a simple "nope, I don't have the time" would've been perfectly fine.

If you think hiding behind a critique of my choice of language makes it OK for you to ignore the issue I raised then you've only proven me right in my assessment that actively spending even more time contributing a pull request would be wasted effort.

Nobody here is ignoring your issue. In fact, it appears that from the first comment there's at least one maintainer of this project who's open to finding out more about your issue, and then your reply provides further information along with some mildly insulting language. We are only trying to help get to the bottom of this issue. The insulting language is disheartening and demotivating. Please treat people with more respect than that.

I've contributed hundreds of patches across dozens of PRs to a variety of OSS projects with over 160 patches having been accepted upstream across half a dozen projects and as a result of that, frankly I've developed a serious lack of patience for projects that want me to spoon feed them because they can't be bothered to do basic things themselves.

This feeds back into the original line I quoted at the top. imo: If you have time to write comments like that, you would have time to write the patch. If you need help developing the patch, the developers hang out on IRC at irc.freenode.net's #bundler channel and they're very receptive to new contributors.

The reflexive behavior of putting work off onto contributors is, frankly, repugnant.

In my experience, this is what open source is all about. I maintain Spree as my day job and I try as hard as I can to put work back off onto the contributors to ease my workload, and to spread the knowledge to the community as a whole. That knowledge can then be shared amongst the community rather than being locked up in the maintainers' heads.

In some cases, the contributors are able to solve their own problems and submit patches for those, which is really awesome. I've seen it happen a ton on Bundler and many other projects too. But in other cases, their problem is too hard for them to solve themselves and us maintainers have to come in and help them along. I generally talk with people in #spree about their issues and see if I can give them a nudge in the right direction to help them help us.

If I wasn't able to put that work back off onto the contributors then my workload would almost be double what it is now [citation needed].

Do what you will with this report, I'm going to spend my limited time and energy contributing to projects that actually value third party contributions, instead of treating them with disdain.

Bundler does value third party contributions. On the project page for Bundler, I can see at the moment there has been 301 contributors who have contributed back to Bundler:

bundler 2014-01-03 15-55-25

There is no disdain from the Bundler team here for you specifically, but rather for the offensive tone you have adopted in your replies.


Could you please contribute this patch back to Bundler? If you have any troubles, please come and ask the maintainers on irc.freenode.net #bundler.

Thank you.

MrJoy commented 10 years ago

The whole point about open source is that the code is open for you to contribute back to.

Then learn to take contributions in the form provided and don't reflexively demand even more from people when there's no good reason to do so. It's one thing to ask for someone to provide a patch for a feature or hard to reproduce bug, the attitude here has been like a skit I once saw where a guy sitting on the couch whinges at his friend to get him a bag of chips. Then to open it. Then to place just one chip in his mouth, because he can't be bothered to lift his own arm.

Because the Bundler team would rather be collaborative than exclusionary.

Asking for more work is exclusionary, not inclusionary. It says "how dare you contribute! I am going to penalize you for doing so!"

You may even learn something too, but it seems you're pretty savvy about the RubyGems/Bundler internals already, so perhaps not. Who knows?

I am not. I learned all of the above in the process of debugging this in our staging environment and trying to find a solution to get things working again.

To be fair, in .travis.yml it's pretty well commented (besides it pointing to seemingly the wrong line in Rakefile):

And if you are accustomed to using Travis, great! But since I do not, the request for me to come up with a patch would have involved hours of work, with me sussing out your spec suite in more detail, possibly going down a blind alley implementing my own mechanism for testing various Rubygems versions and eventually discovering the mechanism you've implemented.

Hours of work for me, minimum -- instead of seconds of work for you. That's a giant "fuck you" to would be contributors and don't pretend otherwise.

We don't know your level of proficiency in Ruby, having never worked with you. The "could" in that sentence probably meant "are you able to write the test?" or "could you please spare some more time to write a test for us?".

That I can track down the precise versions of Rubygems affected by the bug, and point to the exact point in the source of both Bundler and Rubygems where the issue occurs is sufficient proof that I understand Ruby.

However, you don't need to know what my proficiency level is here and it makes no sense to ask in this context.

So again, I call bullshit on that. Can I write a test? Yes, I absolutely can. Will I? Nope. Not when the very first thing you ask of me is to do work that amounts to adding two lines to a file because you can't be arsed to do it yourself.

I agree with @indirect that a simple "nope, I don't have the time" would've been perfectly fine.

That would be a misinterpretation of what I wrote. I never said I don't have time. I said I won't do your shit-work for you.

Nobody here is ignoring your issue.

Really? @indirect seemed quite intent on dismissing everything I had to say out of hand. But at this point, I simply don't care anymore.

This feeds back into the original line I quoted at the top. imo: If you have time to write comments like that, you would have time to write the patch. If you need help developing the patch, the developers hang out on IRC at irc.freenode.net's #bundler channel and they're very receptive to new contributors.

  1. I have a finite amount of time. I will choose where I spend it.
  2. I never said I didn't have the time, I said I wouldn't spend it on your project.
  3. My reason for refusing is the intrinsically self-absorbed attitude of reflexively pushing more work onto would-be contributors.

In my experience, this is what open source is all about.

Asking someone to contribute a patch when they are in a better position to do so is one thing. Asking people to put a chip in your mouth because your arms just feel soooo heavy is another thing entirely.

On my OSS projects, I try to actually stop and think about who's in the best position to do follow-up work. Is it easier for me or for the contributor? Is it within the scope of what I'm trying to achieve, or does it represent an expansion of that scope that I think is worthwhile?

As for the 301 contributors Bundler has -- so what? You have a project that for all intents and purposes has become a mandatory part of virtually every professional Rubyist's toolchain whether they like it or not and regardless of what issues it may have. That necessarily means that for many people the least total work is to put up with your BS and jump through your hoops. 301 contributors for a project in Bundlers position doesn't say anything about the degree to which you are mindful and respectful of the time and energy of would-be contributors.

Finally: That the proposed resolution is to mark Rubygems 1.8.5..1.8.19 as incompatible instead of making one of half a dozen simple fixes necessary to make it work with those versions is just... wow. That's some quality right there.

I mean here I thought adding an intermediate sub-class between Modern and RubygemsIntegration that handled the semantics of 1.8.5..1.8.19 would be straightforward and simple for you but I guess one chip is more than your arms can bear.

skottler commented 10 years ago

@MrJoy we get it, you don't want to contribute more than you already have and are therefore going to systematically insult each and every person who tries to move the issue forward. You don't want to spend your time working on Bundler, which is entirely fair. No one is forcing you to continue to work on this, so just don't. Continuing to insult those who do choose to spend their time working on the project is entirely unnecessary, particularly when the language and tone you've used has been thoroughly inappropriate.

MrJoy commented 10 years ago

I will rebut any point made. Especially when the maintainers of this project say one thing, and then do another. For example, stating flatly that "Nobody here is ignoring your issue.", and then having the proposed resolution be... drop support for the versions of Rubygems that you fucked up support for. It would be one thing if you hadn't set out to support those versions but you did, and now that I report a problem, your solution is... to not support them. Why? Because I won't submit a patch for you.

I was accused of being 'entitled', but the response to the very detailed and thorough issue I raised is the epitome of entitlement.

You think my language and tone are inappropriate? That's because you fail to understand just how narcissistic and self-absorbed your behavior as maintainers of this project has been.

indirect commented 10 years ago

For the record, it’s not feasible to officially support every single point release of Rubygems that has ever existed. However, this particular issue has already been fixed in 795290db246685185e62a3be848c67c24ffa8e50.

In general, please use the latest Rubygems bugfix release (meaning the highest Z version in the X.Y.Z versioning scheme) for officially supported compatibility with Bundler.

On Jan 3, 2014, at 1:37 AM, Jon Frisby notifications@github.com wrote:

Closed #2793.

— Reply to this email directly or view it on GitHub.

MrJoy commented 10 years ago

@indirect That commit link 404s. Did you mean [https://github.com/bundler/bundler/commit/c55ed6a90f96355ea0cefd73087cdaafb8b8c86a]?

indirect commented 10 years ago

Yep.

On Jan 3, 2014, at 2:18 AM, Jon Frisby notifications@github.com wrote:

@indirect That commit link 404s. Did you mean [c55ed6a]?

— Reply to this email directly or view it on GitHub.

MrJoy commented 10 years ago

Thank you.

indirect commented 10 years ago

Thanks (in part) to the content and tone of this ticket, the Bundler project now has a code of conduct! Let's stick with friendly, inclusive, and productive collaboration from now on.

MrJoy commented 10 years ago

Your own comments were neither friendly, nor productive. The sarcastic image link you used above to dismiss my complaints about the dysfunctional behavior of demanding even more of my time after I already contributed significant time to this issue is not friendly. It is not productive. It is not inclusive. Does that mean you have violated your own code of conduct?