diff options
author | Bundlerbot <bot@bundler.io> | 2019-03-09 16:15:19 +0000 |
---|---|---|
committer | Bundlerbot <bot@bundler.io> | 2019-03-09 16:15:19 +0000 |
commit | 83f484dd826214183809c272181a0d80fa456a2e (patch) | |
tree | 69a4a4872e4074b29aa0a402d5af0bd6bd977bbc | |
parent | 14a2a071e1670d5560e3ba785f43bd94114d0368 (diff) | |
parent | 0bf51048e8ac40df5fa6bf11abc45375109e0a7f (diff) | |
download | bundler-83f484dd826214183809c272181a0d80fa456a2e.tar.gz |
Merge #6985
6985: Remove git subshelling from gemspec r=deivid-rodriguez a=deivid-rodriguez
### What was the end-user problem that led to this PR?
The initial problem was that the default bundler gemspec integrated in ruby-core was shipped nearly empty: https://bugs.ruby-lang.org/issues/15582. That caused issues in bundler: https://github.com/bundler/bundler/issues/6937
### What was your diagnosis of the problem?
I looked into ruby-core history, and still not fully sure how it happened but something I noticed is that bundler does not make this integration easy because ruby-core doesn't have a git environment available, so it needs to do a bit of juggling: https://github.com/ruby/ruby/blob/trunk/tool/rbinstall.rb#L802-L803. We could make things easier.
### What is your fix for the problem, implemented in this PR?
My fix is to stop subshelling to `git` from our own gemspec. I think we do this in the default generated gemspec so that newbie users don't accidentally ship generated files with their gems (although the current solution would still ship generated files that newbie users accidentally commit to source control :smile:). But we shouldn't consider ourselves newbies, so I think we can avoid that.
### Why did you choose this fix out of the possible options?
I chose this fix because it avoids shelling out to git inside the gemspec while still keeping some assurances about the correctness of the shipped set of files through a quality spec.
Co-authored-by: David RodrÃguez <deivid.rodriguez@riseup.net>
-rw-r--r-- | bundler.gemspec | 3 | ||||
-rw-r--r-- | spec/quality_spec.rb | 12 | ||||
-rw-r--r-- | task/release.rake | 26 |
3 files changed, 38 insertions, 3 deletions
diff --git a/bundler.gemspec b/bundler.gemspec index d849979594..d58b1d3d0c 100644 --- a/bundler.gemspec +++ b/bundler.gemspec @@ -46,8 +46,7 @@ Gem::Specification.new do |s| s.add_development_dependency "rspec", "~> 3.6" s.add_development_dependency "rubocop", "= 0.65.0" - base_dir = File.dirname(__FILE__).gsub(%r{([^A-Za-z0-9_\-.,:\/@\n])}, "\\\\\\1") - s.files = IO.popen("git -C #{base_dir} ls-files -z", &:read).split("\x0").select {|f| f.match(%r{^(lib|exe)/}) } + s.files = Dir.glob("{lib,exe}/**/*", File::FNM_DOTMATCH).reject {|f| File.directory?(f) } # we don't check in man pages, but we need to ship them because # we use them to generate the long-form help for each command. diff --git a/spec/quality_spec.rb b/spec/quality_spec.rb index 8bdef33a79..6477a03fe4 100644 --- a/spec/quality_spec.rb +++ b/spec/quality_spec.rb @@ -237,6 +237,18 @@ RSpec.describe "The library itself" do end end + it "ships the correct set of files" do + Dir.chdir(root) do + git_list = IO.popen("git ls-files -z", &:read).split("\x0").select {|f| f.match(%r{^(lib|exe)/}) } + git_list += %w[CHANGELOG.md LICENSE.md README.md bundler.gemspec] + git_list += Dir.glob("man/**/*") + + gem_list = Gem::Specification.load(gemspec.to_s).files + + expect(git_list.to_set).to eq(gem_list.to_set) + end + end + it "does not contain any warnings" do Dir.chdir(root) do exclusions = %w[ diff --git a/task/release.rake b/task/release.rake index 3a48c1d255..f22d1296cc 100644 --- a/task/release.rake +++ b/task/release.rake @@ -4,9 +4,33 @@ require "bundler/gem_tasks" task :build => ["build_metadata", "man:build", "generate_files"] do Rake::Task["build_metadata:clean"].tap(&:reenable).real_invoke end -task :release => ["man:require", "man:build", "release:verify_github", "build_metadata"] +task :release => ["man:require", "man:build", "release:verify_files", "release:verify_github", "build_metadata"] namespace :release do + task :verify_files do + git_list = IO.popen("git ls-files -z", &:read).split("\x0").select {|f| f.match(%r{^(lib|exe)/}) } + git_list += %w[CHANGELOG.md LICENSE.md README.md bundler.gemspec] + git_list += Dir.glob("man/**/*") + + gem_list = Gem::Specification.load("bundler.gemspec").files + + extra_files = gem_list.to_set - git_list.to_set + + error_msg = <<~MSG + + You intend to ship some files with the gem that are not generated man pages + nor source control files. Please review the extra list of files and try + again: + + #{extra_files.to_a.join("\n ")} + + MSG + + raise error_msg if extra_files.any? + + puts "The file list is correct for a release." + end + def gh_api_post(opts) gem "netrc", "~> 0.11.0" require "netrc" |