From b87adb03f455e2942bab171115462955be22f43b Mon Sep 17 00:00:00 2001 From: Andreas Hellwig Date: Sat, 26 Sep 2015 17:55:57 +0200 Subject: Extract class Standalone I wanted to fix some minor Rubocop issues and got confused by '#{ruby_engine}/#{ruby_version}'. I managed to confuse two other people working on bundler as well, so I decided to refactor the part around it. I wanted to encapsulate the file creation, so it's (even) more obvious from the context that another ruby file is created. So having an uninterpolated string with interpolation syntax in it isn't so unusual. To be extra sure I added a comment here as Rubocop might lead others to the same spot. I found that Standalone works quite well as its own object. While extracting the class I shortened some expressions and extracted methods that made sense as a unit to me. I ended up with a generate method that only deals with file output. What I think might be controversial: * The tendency to use methods instead of variables for small things like version_dir and bundler_path: I think it's easier to understand because methdos are "read_only", while values of variables might change during code execution. * Array(spec.require_paths) instead of "next if spec.require_paths.nil? # builtin gems". We lose the comment here, but I thought Array was more concise and that the information about the type of gems isn't crucial here. * flat_map { map { ... } } instead of "collecting" in paths My perspective is that paths is the result of a transformation of @specs and that's best expressed by using map. It's also shorter. --- lib/bundler/installer.rb | 41 ++----------------------------- lib/bundler/installer/standalone.rb | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 39 deletions(-) create mode 100644 lib/bundler/installer/standalone.rb diff --git a/lib/bundler/installer.rb b/lib/bundler/installer.rb index 923fbd1faf..1fd0b85dd9 100644 --- a/lib/bundler/installer.rb +++ b/lib/bundler/installer.rb @@ -2,6 +2,7 @@ require "erb" require "rubygems/dependency_installer" require "bundler/worker" require "bundler/installer/parallel_installer" +require "bundler/installer/standalone" module Bundler class Installer < Environment @@ -68,7 +69,7 @@ module Bundler install(options) lock unless Bundler.settings[:frozen] - generate_standalone(options[:standalone]) if options[:standalone] + Standalone.new(options[:standalone], @definition).generate if options[:standalone] end def install_gem_from_spec(spec, standalone = false, worker = 0, force = false) @@ -209,44 +210,6 @@ module Bundler end end - def generate_standalone(groups) - standalone_path = Bundler.settings[:path] - bundler_path = File.join(standalone_path, "bundler") - SharedHelpers.filesystem_access(bundler_path) do |p| - FileUtils.mkdir_p(p) - end - - paths = [] - - if groups.empty? - specs = @definition.requested_specs - else - specs = @definition.specs_for groups.map(&:to_sym) - end - - specs.each do |spec| - next if spec.name == "bundler" - next if spec.require_paths.nil? # builtin gems - - spec.require_paths.each do |path| - full_path = Pathname.new(path).absolute? ? path : File.join(spec.full_gem_path, path) - gem_path = Pathname.new(full_path).relative_path_from(Bundler.root.join(bundler_path)) - paths << gem_path.to_s.sub("#{Bundler.ruby_version.engine}/#{RbConfig::CONFIG["ruby_version"]}", '#{ruby_engine}/#{ruby_version}') - end - end - - File.open File.join(bundler_path, "setup.rb"), "w" do |file| - file.puts "require 'rbconfig'" - file.puts "# ruby 1.8.7 doesn't define RUBY_ENGINE" - file.puts "ruby_engine = defined?(RUBY_ENGINE) ? RUBY_ENGINE : 'ruby'" - file.puts "ruby_version = RbConfig::CONFIG[\"ruby_version\"]" - file.puts "path = File.expand_path('..', __FILE__)" - paths.each do |path| - file.puts %{$:.unshift "\#{path}/#{path}"} - end - end - end - def install_in_parallel(size, standalone, force = false) ParallelInstaller.call(self, specs, size, standalone, force) end diff --git a/lib/bundler/installer/standalone.rb b/lib/bundler/installer/standalone.rb new file mode 100644 index 0000000000..24a4183ea2 --- /dev/null +++ b/lib/bundler/installer/standalone.rb @@ -0,0 +1,48 @@ +module Bundler + class Standalone + def initialize(groups, definition) + @specs = groups.empty? ? definition.requested_specs : definition.specs_for(groups.map(&:to_sym)) + end + + def generate + SharedHelpers.filesystem_access(bundler_path) do |p| + FileUtils.mkdir_p(p) + end + File.open File.join(bundler_path, "setup.rb"), "w" do |file| + file.puts "require 'rbconfig'" + file.puts "# ruby 1.8.7 doesn't define RUBY_ENGINE" + file.puts "ruby_engine = defined?(RUBY_ENGINE) ? RUBY_ENGINE : 'ruby'" + file.puts "ruby_version = RbConfig::CONFIG[\"ruby_version\"]" + file.puts "path = File.expand_path('..', __FILE__)" + paths.each do |path| + file.puts %{$:.unshift "\#{path}/#{path}"} + end + end + end + + private + + def paths + @specs.flat_map do |spec| + next if spec.name == "bundler" + Array(spec.require_paths).map do |path| + gem_path(path, spec).sub(version_dir, '#{ruby_engine}/#{ruby_version}') + # This is a static string intentionally. It's interpolated at a later time. + end + end + end + + def version_dir + "#{Bundler.ruby_version.engine}/#{RbConfig::CONFIG["ruby_version"]}" + end + + def bundler_path + File.join(Bundler.settings[:path], "bundler") + end + + def gem_path(path, spec) + full_path = Pathname.new(path).absolute? ? path : File.join(spec.full_gem_path, path) + Pathname.new(full_path).relative_path_from(Bundler.root.join(bundler_path)).to_s + end + end +end -- cgit v1.2.1 From 16576f3a0a3780b09f38a78336ddfbd3fc5a9573 Mon Sep 17 00:00:00 2001 From: Andreas Hellwig Date: Tue, 29 Sep 2015 23:19:06 +0200 Subject: Rubocop adjustment Outdent access modifiers like private --- lib/bundler/installer/standalone.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler/installer/standalone.rb b/lib/bundler/installer/standalone.rb index 24a4183ea2..f9b50e1d98 100644 --- a/lib/bundler/installer/standalone.rb +++ b/lib/bundler/installer/standalone.rb @@ -20,7 +20,7 @@ module Bundler end end - private + private def paths @specs.flat_map do |spec| -- cgit v1.2.1 From b73c775240e02606479d9110efa2c9ad1503c982 Mon Sep 17 00:00:00 2001 From: Andreas Hellwig Date: Sat, 3 Oct 2015 09:32:35 +0200 Subject: There's no flat_map in 1.8.7 --- lib/bundler/installer/standalone.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/bundler/installer/standalone.rb b/lib/bundler/installer/standalone.rb index f9b50e1d98..6776dd6200 100644 --- a/lib/bundler/installer/standalone.rb +++ b/lib/bundler/installer/standalone.rb @@ -23,13 +23,13 @@ module Bundler private def paths - @specs.flat_map do |spec| + @specs.map do |spec| next if spec.name == "bundler" Array(spec.require_paths).map do |path| gem_path(path, spec).sub(version_dir, '#{ruby_engine}/#{ruby_version}') # This is a static string intentionally. It's interpolated at a later time. end - end + end.flatten end def version_dir -- cgit v1.2.1