savonrb / wasabi

A simple WSDL parser
MIT License
90 stars 84 forks source link

Gem includes heavy test files unnecessarily #68

Closed Ferdy89 closed 4 years ago

Ferdy89 commented 7 years ago

The gemspec lists the files for the gem as

s.files = `git ls-files`.split("\n")

This includes all the files in the spec directory which add up to 3.4MB. I believe none of these files are used out of the tests, making them unnecessary when bundling the gem.

This causes the gem to take longer to install (source code is only 34KB) and it also slightly hurts applications hosted in platforms with size limit, like Heroku (300MB).

I believe a solution for this should simply be to change that line to be

s.files = `git ls-files -- lib/*`.split("\n")

Just like the s.test_files and s.executables lines in the gemspec do. I'd be happy to make a PR for this.

olleolleolle commented 4 years ago

For inspiration: Here's a comparable solution to the same problem

https://github.com/cucumber/aruba/pull/721

utkarsh2102 commented 4 years ago

Hello,

For more inspiration: https://github.com/cucumber/cucumber-rails/pull/476, https://github.com/mvz/gir_ffi/pull/175, and so many more. Using git to produce a list of files is problematic. This, I heartily believe, can be replaced via a simple Ruby alternative.

rubocop-packaging, an extension of RuboCop, was born because we faced a lot of such problems while maintaining gems in downstream (Debian, specifically).

Ideally, we should only ship files that are needed by the end-users. Not even gemspecs, no Gemfiles, no tests, no Rakefile, and so on.

I would, therefore, propose the following diff:

--- a/wasabi.gemspec
+++ b/wasabi.gemspec
@@ -20,9 +20,9 @@ Gem::Specification.new do |s|

   s.add_development_dependency "rake",  "~> 13.0"
   s.add_development_dependency "rspec", "~> 3.7.0"
+  s.add_development_dependency "rubocop-packaging", "~> 0.1.1"

-  s.files         = `git ls-files`.split("\n")
-  s.test_files    = `git ls-files -- {test,spec,features}/*`.split("\n")
-  s.executables   = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) }
+  s.files         = Dir["lib/**/*", "CHANGELOG.md", "LICENSE", "README.md"]
+  s.test_files    = Dir["spec/**/*"]
   s.require_paths = ["lib"]
 end
utkarsh2102 commented 4 years ago

@olleolleolle, would you be interested? Should I raise a PR?

olleolleolle commented 4 years ago

We would love a PR!

Wishlist: include the smallest amount of files, avoid using Rake to make the selection. A glob pattern plus explicit files is easiest, I think.

Thanks for this! It's reducing sizes in many places.

utkarsh2102 commented 4 years ago

Thanks, @olleolleolle, raised via #89! :tada: Also, thanks, @Ferdy89, for raising this. This is really an important issue! :100: