evanphx / benchmark-ips

Provides iteration per second benchmarking for Ruby
MIT License
1.72k stars 97 forks source link

Add missing stats_metric.rb to Manifest.txt #101

Closed proby closed 4 years ago

proby commented 4 years ago

Without this file gem version 2.8.0 (3dba7be) will throw a LoadError when you attempt to require 'benchmark/ips':

LoadError (cannot load such file -- benchmark/ips/stats/stats_metric)

proby commented 4 years ago

FWIW according to rake check_manifest these are the remaining files not a part of Manifest.txt:

--- Manifest.txt        2020-05-02 18:01:10.650977700 +1200
+++ Manifest.tmp        2020-05-02 18:01:22.906084700 +1200
@@ -1,8 +1,17 @@
 .autotest
+.hoeignore
+.travis.yml
+Gemfile
+Gemfile.lock
 History.txt
 Manifest.txt
 README.md
 Rakefile
+benchmark-ips.gemspec
+examples/advanced.rb
+examples/hold.rb
+examples/save.rb
+examples/simple.rb
 lib/benchmark/compare.rb
 lib/benchmark/ips.rb
 lib/benchmark/ips/job.rb

I don't believe any of these files need to be added to the manifest.

nateberkopec commented 4 years ago

@proby any reason we shouldn't just add the rest in that diff too?

proby commented 4 years ago

tl;dr: I don't think those other files should be a part of the Manifest.txt as they shouldn't be a part of the packaged gem.

NB: Having just learned about the existance of hoe what I say could be completely wrong.

Best I can tell gems for benchmark-ips are build using hoe. hoe uses the Manifest.txt to determine which files are part of the packaged gem (source). This PR will have hoe add an addional file to the package that is required upon requiring benchmark-ips as a whole. Without this, or an analogous change, anyone who tries to require 'benchmark/ips' using 2.8.0 of benchmark-ips will end up with the following:

foo@BAR$ gem install benchmark-ips
Fetching benchmark-ips-2.8.0.gem
Successfully installed benchmark-ips-2.8.0
1 gem installed
foo@BAR$ ruby -rbenchmark/ips -e 'puts "hi"'
Traceback (most recent call last):
... <snip> ...
/home/proby/.rubies/ruby-2.7.1/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require': cannot load such file -- benchmark/ips/stats/stats_metric (LoadError) 

The file being added (lib/benchmark/ips/stats/stats_metric.rb) was created in #90. 2.8.0 is the first packaged gem build since that PR was merged.

eregon commented 4 years ago

@nateberkopec or @evanphx Could you publish a 2.8.1 release with this fix? Right now the the 2.8.0 release is unfortunately unusable, and this causes e.g., some CIs to fail.

I checked locally and this works (tested on MRI 2.6.6 and truffleruby-dev):

# bump the version in lib/benchmark/ips.rb
gem i pkg/benchmark-ips-2.8.1.gem
ruby examples/simple.rb
nateberkopec commented 4 years ago

Yup going out today

nateberkopec commented 4 years ago

Ok, new version out as 2.8.2. I initially did a 2.8.1 but I forgot to commit the change I made to VERSION, so I had I to yank that, add a version change and push. Everything should be fine now.

proby commented 4 years ago

Thanks @nateberkopec

eregon commented 4 years ago

Thanks for the release!