Declare explicit dependency on `benchmark`

[Earlopain]

Ruby plans to bundle it: https://github.com/ruby/ruby/pull/11492

That means, in the next ruby version requiring it without it appearing in the gemspec will warn, and the release after that will error.

[eregon]

I think it would be good to wait that that PR is merged at least to merge this one. It doesn't seem nice that gem install benchmark-ips will now install two gems, when benchmark-ips so far was dependency-free.

[Earlopain]

Of course, there is no rush

[evanphx]

Will this break current users though?

[Earlopain]

Will this break current users though?

If they don't update benchmark-ips for about two years but keep up with ruby, yes. I don't imagine any other breakage as a result of this change. Both ips and benchmark make no restrictions about their supported ruby version and adding past bundled gems to the gemspec of other gems has mostly been painless.

I'll come back to this later and address your PR feedback when/if the ruby PR ends up merged. Eregon has made a comment there which may have an impact.

[eregon]

benchmark doesn't test older than 2.5: https://github.com/ruby/benchmark/blob/master/.github/workflows/test.yml benchmark-ips tests from 2.3: https://github.com/evanphx/benchmark-ips/blob/master/.github/workflows/ci.yml

FWIW Process.clock_gettime exists since Ruby 2.1 (current benchmark relies on it).

[Earlopain]

Seems likenit's missing a required_ruby_version of at least 2.1. Testing against such old versions is hard and I wouldn't be surprised if there are other such instances.

For what's its worth, I agree with your points. There are barely any changes to it in the last decade and I made the same point about javascript somewhere else when the same thing happened with base64.

It's a bit sad that what basically amounts to syntactical sugar requires a dependeny. But when it comes down to it, I prefer my 50% of my downstream dependencies not having these. Same can be said for forwardable, singleton, etc. I'm sure if they were proper dependencies from the start they wouldn't have seen much adoption, and from PRs removing mutex_m, base64, observable which users mostly use for convenience expecting them to just be there, maintainers largely agree.

This all doesn't apply to gems with what I would call actual functionality, like rdoc or webrick. You can debate about some of these but overall I would 100% say it's going too far.

[eregon]

@Earlopain Maybe you could post your opinion on https://bugs.ruby-lang.org/issues/20309 too? I think it'd be good to show I'm not the only one with concerns about this change.

[Earlopain]

Interacting in the bugtracker is daunting for me. I wrote a thing there, hopefully that's adequate.

[Earlopain]

Seems like they will go on with bundling benchmark. I openend https://github.com/ruby/benchmark/pull/24 for the missing required_ruby_version but by now its already to late anyways.

I looked through the code here, and I'm wondering if there even is any dependency on benchmark. For example, https://github.com/evanphx/benchmark-ips/blob/a2c863dca36edf41681730392f2ba9d8f20dc285/lib/benchmark/timing.rb#L44-L44 handles the lack/existance of Process.clock_gettime without relying on benchmark.

This gem advertises itself as an extension to benchmark, but it seems the only thing it actually does is occupy in the same namespace. There is no require "benchmark" I can find anywhere. My bad for not checking previously.

I guess, if that is the case, I can just close this?

[eregon]

Right, seems there is no dependency so it shouldn't warn and this can be closed then.

[nateberkopec]

I played around with this last night and tried undefining benchmark at various points, etc. There's no actual dependency AFAICT 😆

[Earlopain]

Yes, I appologise. It didn't compute that there may be no dependency here and I didn't check. Thanks for the conversation anyways