From 97958410f453eca9332330a873947e64d4d90f8d Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 27 Feb 2019 14:02:38 +0000 Subject: [PATCH 1/2] dev-cmd/tests: improve parallel args naming. --- Library/Homebrew/dev-cmd/tests.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/dev-cmd/tests.rb b/Library/Homebrew/dev-cmd/tests.rb index dcc19badd3..4b9363c0c2 100644 --- a/Library/Homebrew/dev-cmd/tests.rb +++ b/Library/Homebrew/dev-cmd/tests.rb @@ -44,6 +44,7 @@ module Homebrew ENV.delete("HOMEBREW_NO_GITHUB_API") ENV.delete("HOMEBREW_NO_EMOJI") ENV.delete("HOMEBREW_DEVELOPER") + ENV.delete("HOMEBREW_PRY") ENV["HOMEBREW_NO_ANALYTICS_THIS_RUN"] = "1" ENV["HOMEBREW_NO_COMPAT"] = "1" if args.no_compat? ENV["HOMEBREW_TEST_GENERIC_OS"] = "1" if args.generic? @@ -85,7 +86,7 @@ module Homebrew Dir.glob("test/**/*_spec.rb") end - opts = if ENV["CI"] + parallel_args = if ENV["CI"] %w[ --combine-stderr --serialize-stdout @@ -123,7 +124,7 @@ module Homebrew puts "Randomized with seed #{seed}" if parallel - system "bundle", "exec", "parallel_rspec", *opts, "--", *bundle_args, "--", *files + system "bundle", "exec", "parallel_rspec", *parallel_args, "--", *bundle_args, "--", *files else system "bundle", "exec", "rspec", *bundle_args, "--", *files end From 17f3ee19578c5d0004070b60285ce0508728f506 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 26 Feb 2019 22:13:00 +0000 Subject: [PATCH 2/2] Improve Bundler cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than trying to be smart and doing this ourselves in `brew cleanup` let’s just installed Bundler somewhere it doesn’t try to clean itself up and use `bundle install --cleanup` when we need cleanup done. Also, use `ohai` and `odie` when possible as they look nicer. --- Library/Homebrew/.bundle/config | 1 + Library/Homebrew/cleanup.rb | 17 ------ Library/Homebrew/dev-cmd/tests.rb | 8 ++- Library/Homebrew/test/cask/cmd/style_spec.rb | 4 +- Library/Homebrew/test/cmd/style_spec.rb | 4 ++ .../test/{cmd => dev-cmd}/irb_spec.rb | 0 Library/Homebrew/test/spec_helper.rb | 8 +-- Library/Homebrew/utils/gems.rb | 61 +++++++++++++------ 8 files changed, 60 insertions(+), 43 deletions(-) rename Library/Homebrew/test/{cmd => dev-cmd}/irb_spec.rb (100%) diff --git a/Library/Homebrew/.bundle/config b/Library/Homebrew/.bundle/config index d5b0b36bc4..24c48a4c9d 100644 --- a/Library/Homebrew/.bundle/config +++ b/Library/Homebrew/.bundle/config @@ -1,5 +1,6 @@ --- BUNDLE_BIN: "false" +BUNDLE_CLEAN: "true" BUNDLE_DISABLE_SHARED_GEMS: "true" BUNDLE_JOBS: "4" BUNDLE_PATH: "vendor/bundle" diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb index ec3e00cd07..26582ae44e 100644 --- a/Library/Homebrew/cleanup.rb +++ b/Library/Homebrew/cleanup.rb @@ -199,8 +199,6 @@ module Homebrew FileUtils.touch PERIODIC_CLEAN_FILE end - cleanup_bundler - # Cleaning up Ruby needs to be done last to avoid requiring additional # files afterwards. Additionally, don't allow it on periodic cleans to # avoid having to try to do a `brew install` when we've just deleted @@ -351,21 +349,6 @@ module Homebrew end end - def cleanup_bundler - HOMEBREW_LIBRARY_PATH.cd do - Homebrew.setup_gem_environment! - bundle = "#{Gem.bindir}/bundle" - return unless File.executable?(bundle) - return if Gem::Specification.find_all_by_name("bundler").empty? - - if dry_run? - system bundle, "clean", "--dry-run" - else - system bundle, "clean" - end - end - end - def cleanup_portable_ruby system_ruby_version = Utils.popen_read("/usr/bin/ruby", "-e", "puts RUBY_VERSION") diff --git a/Library/Homebrew/dev-cmd/tests.rb b/Library/Homebrew/dev-cmd/tests.rb index 4b9363c0c2..34b63ae73f 100644 --- a/Library/Homebrew/dev-cmd/tests.rb +++ b/Library/Homebrew/dev-cmd/tests.rb @@ -33,6 +33,9 @@ module Homebrew def tests tests_args.parse + Homebrew.install_bundler_gems! + gem_user_dir = Gem.user_dir + HOMEBREW_LIBRARY_PATH.cd do ENV.delete("HOMEBREW_COLOR") ENV.delete("HOMEBREW_NO_COLOR") @@ -69,8 +72,6 @@ module Homebrew ENV["GIT_#{role}_DATE"] = "Sun Jan 22 19:59:13 2017 +0000" end - Homebrew.install_bundler_gems! - parallel = true files = if args.only @@ -123,6 +124,9 @@ module Homebrew puts "Randomized with seed #{seed}" + # Let `bundle` in PATH find its gem. + ENV["GEM_PATH"] = "#{ENV["GEM_PATH"]}:#{gem_user_dir}" + if parallel system "bundle", "exec", "parallel_rspec", *parallel_args, "--", *bundle_args, "--", *files else diff --git a/Library/Homebrew/test/cask/cmd/style_spec.rb b/Library/Homebrew/test/cask/cmd/style_spec.rb index 64c5a68bcb..336fa1140d 100644 --- a/Library/Homebrew/test/cask/cmd/style_spec.rb +++ b/Library/Homebrew/test/cask/cmd/style_spec.rb @@ -42,7 +42,7 @@ describe Cask::Cmd::Style, :cask do context "when installation succeeds" do before do - allow(Homebrew).to receive(:install_gem_setup_path!) + allow(Homebrew).to receive(:install_bundler_gems!) end it "exits successfully" do @@ -52,7 +52,7 @@ describe Cask::Cmd::Style, :cask do context "when installation fails" do before do - allow(Homebrew).to receive(:install_gem_setup_path!).and_raise(SystemExit) + allow(Homebrew).to receive(:install_bundler_gems!).and_raise(SystemExit) end it "raises an error" do diff --git a/Library/Homebrew/test/cmd/style_spec.rb b/Library/Homebrew/test/cmd/style_spec.rb index c9e1ac2afc..f105e7d208 100644 --- a/Library/Homebrew/test/cmd/style_spec.rb +++ b/Library/Homebrew/test/cmd/style_spec.rb @@ -15,6 +15,10 @@ describe "brew style" do end end + before do + allow(Homebrew).to receive(:install_bundler_gems!) + end + describe "Homebrew::check_style_json" do let(:dir) { mktmpdir } diff --git a/Library/Homebrew/test/cmd/irb_spec.rb b/Library/Homebrew/test/dev-cmd/irb_spec.rb similarity index 100% rename from Library/Homebrew/test/cmd/irb_spec.rb rename to Library/Homebrew/test/dev-cmd/irb_spec.rb diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index a9df288115..b5a601ffa3 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/Library/Homebrew/test/spec_helper.rb @@ -72,11 +72,9 @@ RSpec.configure do |config| config.silence_filter_announcements = true if ENV["TEST_ENV_NUMBER"] - # TODO: when https://github.com/rspec/rspec-expectations/pull/1056 - # makes it into a stable release: - # config.expect_with :rspec do |c| - # c.max_formatted_output_length = 200 - # end + config.expect_with :rspec do |c| + c.max_formatted_output_length = 200 + end # Never truncate output objects. RSpec::Support::ObjectFormatter.default_instance.max_formatted_output_length = nil diff --git a/Library/Homebrew/utils/gems.rb b/Library/Homebrew/utils/gems.rb index 97a68faebb..9584089343 100644 --- a/Library/Homebrew/utils/gems.rb +++ b/Library/Homebrew/utils/gems.rb @@ -8,12 +8,34 @@ module Homebrew module_function def ruby_bindir - "#{RbConfig::CONFIG["prefix"]}/bin" + "#{RbConfig::CONFIG["prefix"]}/bin".freeze end - def setup_gem_environment! + def gem_user_bindir + "#{Gem.user_dir}/bin".freeze + end + + def ohai_if_defined(message) + if defined?(ohai) + ohai message + else + puts "==> #{message}" + end + end + + def odie_if_defined(message) + if defined?(odie) + odie message + else + $stderr.puts "Error: #{message}" + exit 1 + end + end + + def setup_gem_environment!(gem_home: nil, gem_bindir: nil) # Match where our bundler gems are. - ENV["GEM_HOME"] = "#{ENV["HOMEBREW_LIBRARY"]}/Homebrew/vendor/bundle/ruby/#{RbConfig::CONFIG["ruby_version"]}" + gem_home ||= "#{ENV["HOMEBREW_LIBRARY"]}/Homebrew/vendor/bundle/ruby/#{RbConfig::CONFIG["ruby_version"]}" + ENV["GEM_HOME"] = gem_home ENV["GEM_PATH"] = ENV["GEM_HOME"] # Make RubyGems notice environment changes. @@ -21,41 +43,41 @@ module Homebrew Gem::Specification.reset # Add necessary Ruby and Gem binary directories to PATH. + gem_bindir ||= Gem.bindir paths = ENV["PATH"].split(":") paths.unshift(ruby_bindir) unless paths.include?(ruby_bindir) - paths.unshift(Gem.bindir) unless paths.include?(Gem.bindir) + paths.unshift(gem_bindir) unless paths.include?(gem_bindir) ENV["PATH"] = paths.compact.join(":") end - def install_gem!(name, version = nil) - setup_gem_environment! + def install_gem!(name, version = nil, setup_gem_environment: true) + setup_gem_environment! if setup_gem_environment return unless Gem::Specification.find_all_by_name(name, version).empty? # Shell out to `gem` to avoid RubyGems requires e.g. loading JSON. - puts "==> Installing '#{name}' gem" + ohai_if_defined "Installing '#{name}' gem" install_args = %W[--no-document #{name}] install_args << "--version" << version if version return if system "#{ruby_bindir}/gem", "install", *install_args - $stderr.puts "Error: failed to install the '#{name}' gem." - exit 1 + odie_if_defined "failed to install the '#{name}' gem." end - def install_gem_setup_path!(name, executable: name) - install_gem!(name) + def install_gem_setup_path!(name, executable: name, setup_gem_environment: true) + install_gem!(name, setup_gem_environment: setup_gem_environment) return if ENV["PATH"].split(":").any? do |path| File.executable?("#{path}/#{executable}") end - $stderr.puts <<~EOS - Error: the '#{name}' gem is installed but couldn't find '#{executable}' in the PATH: + odie_if_defined <<~EOS + the '#{name}' gem is installed but couldn't find '#{executable}' in the PATH: #{ENV["PATH"]} EOS - exit 1 end def install_bundler! - install_gem_setup_path! "bundler", executable: "bundle" + setup_gem_environment!(gem_home: Gem.user_dir, gem_bindir: gem_user_bindir) + install_gem_setup_path!("bundler", executable: "bundle", setup_gem_environment: false) end def install_bundler_gems! @@ -63,12 +85,17 @@ module Homebrew ENV["BUNDLE_GEMFILE"] = "#{ENV["HOMEBREW_LIBRARY"]}/Homebrew/Gemfile" @bundle_installed ||= begin - bundle_check_output = `#{Gem.bindir}/bundle check 2>&1` + bundle = "#{gem_user_bindir}/bundle".freeze + bundle_check_output = `#{bundle} check 2>&1` bundle_check_failed = !$CHILD_STATUS.exitstatus.zero? # for some reason sometimes the exit code lies so check the output too. if bundle_check_failed || bundle_check_output.include?("Install missing gems") - system "#{Gem.bindir}/bundle", "install" + unless system bundle, "install" + odie_if_defined <<~EOS + failed to run `#{bundle} install`! + EOS + end else true end