From aa82f550f36538f6e9df42502124689032c9d0ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 7 Sep 2020 13:07:05 +0200 Subject: [PATCH 01/11] Reword some specs To not mention deprecated `--path` flag. --- bundler/spec/install/path_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bundler/spec/install/path_spec.rb b/bundler/spec/install/path_spec.rb index 2239706020d4..ec19c55bed61 100644 --- a/bundler/spec/install/path_spec.rb +++ b/bundler/spec/install/path_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe "bundle install" do - describe "with --path" do + describe "with path configured" do before :each do build_gem "rack", "1.0.0", :to_system => true do |s| s.write "lib/rack.rb", "puts 'FAIL'" @@ -13,7 +13,7 @@ G end - it "does not use available system gems with bundle --path vendor/bundle", :bundler => "< 3" do + it "does not use available system gems with `vendor/bundle", :bundler => "< 3" do bundle "config --local path vendor/bundle" bundle :install expect(the_bundle).to include_gems "rack 1.0.0" @@ -30,7 +30,7 @@ dir.rmtree end - it "prints a warning to let the user know what has happened with bundle --path vendor/bundle" do + it "prints a warning to let the user know where gems where installed" do bundle "config --local path vendor/bundle" bundle :install expect(out).to include("gems are installed into `./vendor/bundle`") From fb2235406b59cd5cec8ee59b7cf9c4388a3e73fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 7 Sep 2020 13:07:49 +0200 Subject: [PATCH 02/11] Reword spec to not mention "warnings" This message is an informational piece, not a warning. --- bundler/spec/install/path_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundler/spec/install/path_spec.rb b/bundler/spec/install/path_spec.rb index ec19c55bed61..07bd0a779389 100644 --- a/bundler/spec/install/path_spec.rb +++ b/bundler/spec/install/path_spec.rb @@ -30,7 +30,7 @@ dir.rmtree end - it "prints a warning to let the user know where gems where installed" do + it "prints a message to let the user know where gems where installed" do bundle "config --local path vendor/bundle" bundle :install expect(out).to include("gems are installed into `./vendor/bundle`") From 157cd1829463a7168b88bc571b22fb36cd70d4dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 7 Sep 2020 13:11:12 +0200 Subject: [PATCH 03/11] Enable path spec on all bundler versions Since the behaviour is not specific to bundler 3. --- bundler/spec/install/path_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundler/spec/install/path_spec.rb b/bundler/spec/install/path_spec.rb index 07bd0a779389..04cf01dceaea 100644 --- a/bundler/spec/install/path_spec.rb +++ b/bundler/spec/install/path_spec.rb @@ -13,7 +13,7 @@ G end - it "does not use available system gems with `vendor/bundle", :bundler => "< 3" do + it "does not use available system gems with `vendor/bundle" do bundle "config --local path vendor/bundle" bundle :install expect(the_bundle).to include_gems "rack 1.0.0" From db53fdc8443838d7aa81e9895a6ae6f2b9a30b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 7 Sep 2020 18:51:45 +0200 Subject: [PATCH 04/11] Simplify `Settings::Path` struct --- bundler/lib/bundler/settings.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index bb70c3ff3ba4..5243e7568624 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -207,14 +207,14 @@ def path key = key_for(:path) path = ENV[key] || @global_config[key] if path && !@temporary.key?(key) && !@local_config.key?(key) - return Path.new(path, false, false) + return Path.new(path, false) end system_path = self["path.system"] || (self[:disable_shared_gems] == false) - Path.new(self[:path], system_path, Bundler.feature_flag.default_install_uses_path?) + Path.new(self[:path], system_path) end - Path = Struct.new(:explicit_path, :system_path, :default_install_uses_path) do + Path = Struct.new(:explicit_path, :system_path) do def path path = base_path path = File.join(path, Bundler.ruby_scope) unless use_system_gems? @@ -224,7 +224,7 @@ def path def use_system_gems? return true if system_path return false if explicit_path - !default_install_uses_path + !Bundler.feature_flag.default_install_uses_path? end def base_path From ba35852a84402fce80965f89cec94de37bdfc56e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 8 Sep 2020 14:46:48 +0200 Subject: [PATCH 05/11] Normalize `Settings.pretty_values_for` Always fetch the value, since we'll be using it right after. --- bundler/lib/bundler/settings.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index 5243e7568624..7260ee6f8f4e 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -182,20 +182,20 @@ def pretty_values_for(exposed_key) locations = [] - if @temporary.key?(key) - locations << "Set for the current command: #{converted_value(@temporary[key], exposed_key).inspect}" + if value = @temporary[key] + locations << "Set for the current command: #{converted_value(value, exposed_key).inspect}" end - if @local_config.key?(key) - locations << "Set for your local app (#{local_config_file}): #{converted_value(@local_config[key], exposed_key).inspect}" + if value = @local_config[key] + locations << "Set for your local app (#{local_config_file}): #{converted_value(value, exposed_key).inspect}" end if value = ENV[key] locations << "Set via #{key}: #{converted_value(value, exposed_key).inspect}" end - if @global_config.key?(key) - locations << "Set for the current user (#{global_config_file}): #{converted_value(@global_config[key], exposed_key).inspect}" + if value = @global_config[key] + locations << "Set for the current user (#{global_config_file}): #{converted_value(value, exposed_key).inspect}" end return ["You have not configured a value for `#{exposed_key}`"] if locations.empty? From 82fb6141d6205fe32ee854cd7e7c2aa4e8ee670f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 8 Sep 2020 14:08:42 +0200 Subject: [PATCH 06/11] Normalize access to `DEFAULT_CONFIG` Keys should be defined in "ENV variable format", since that's what other helpers expect, and makes it consistent with how the other configurations are accessed. This fixes the issue of the `Bundler.settings.locations` hash never returning a `:default` key. It doesn't seem to cause any issues in practice, but still. --- bundler/lib/bundler/settings.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index 7260ee6f8f4e..e29d0eda17ff 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -63,12 +63,12 @@ class Settings ].freeze DEFAULT_CONFIG = { - :silence_deprecations => false, - :disable_version_check => true, - :prefer_patch => false, - :redirect => 5, - :retry => 3, - :timeout => 10, + "BUNDLE_SILENCE_DEPRECATIONS" => false, + "BUNDLE_DISABLE_VERSION_CHECK" => true, + "BUNDLE_PREFER_PATCH" => false, + "BUNDLE_REDIRECT" => 5, + "BUNDLE_RETRY" => 3, + "BUNDLE_TIMEOUT" => 10, }.freeze def initialize(root = nil) @@ -84,7 +84,7 @@ def [](name) @local_config.fetch(key) do ENV.fetch(key) do @global_config.fetch(key) do - DEFAULT_CONFIG.fetch(name) do + DEFAULT_CONFIG.fetch(key) do nil end end end end end From 9ae5a3cc178717b7fd0f43a4e819d3a4f4ff88fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 8 Sep 2020 14:17:30 +0200 Subject: [PATCH 07/11] Normalize access to `ENV` configuration Makes code more consistent, and allows further refactoring. --- bundler/lib/bundler/settings.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index e29d0eda17ff..d520125f0892 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -74,6 +74,7 @@ class Settings def initialize(root = nil) @root = root @local_config = load_config(local_config_file) + @env_config = ENV.to_h.select {|key, _value| key =~ /\ABUNDLE_.+/ } @global_config = load_config(global_config_file) @temporary = {} end @@ -82,7 +83,7 @@ def [](name) key = key_for(name) value = @temporary.fetch(key) do @local_config.fetch(key) do - ENV.fetch(key) do + @env_config.fetch(key) do @global_config.fetch(key) do DEFAULT_CONFIG.fetch(key) do nil @@ -129,9 +130,7 @@ def set_global(key, value) end def all - env_keys = ENV.keys.grep(/\ABUNDLE_.+/) - - keys = @temporary.keys | @global_config.keys | @local_config.keys | env_keys + keys = @temporary.keys | @global_config.keys | @local_config.keys | @env_config.keys keys.map do |key| key.sub(/^BUNDLE_/, "").gsub(/__/, ".").downcase @@ -171,7 +170,7 @@ def locations(key) locations = {} locations[:temporary] = @temporary[key] if @temporary.key?(key) locations[:local] = @local_config[key] if @local_config.key?(key) - locations[:env] = ENV[key] if ENV[key] + locations[:env] = @env_config[key] if @env_config.key?(key) locations[:global] = @global_config[key] if @global_config.key?(key) locations[:default] = DEFAULT_CONFIG[key] if DEFAULT_CONFIG.key?(key) locations @@ -190,7 +189,7 @@ def pretty_values_for(exposed_key) locations << "Set for your local app (#{local_config_file}): #{converted_value(value, exposed_key).inspect}" end - if value = ENV[key] + if value = @env_config[key] locations << "Set via #{key}: #{converted_value(value, exposed_key).inspect}" end @@ -277,7 +276,7 @@ def app_cache_path def validate! all.each do |raw_key| - [@local_config, ENV, @global_config].each do |settings| + [@local_config, @env_config, @global_config].each do |settings| value = converted_value(settings[key_for(raw_key)], raw_key) Validator.validate!(raw_key, value, settings.to_hash.dup) end From a3b4a2cd208e3db37d5fa5bc8252dfd2c9824367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 8 Sep 2020 14:49:48 +0200 Subject: [PATCH 08/11] Remove unnecessary `to_hash` Since all settings are hashes now. --- bundler/lib/bundler/settings.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index d520125f0892..6909069bf250 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -278,7 +278,7 @@ def validate! all.each do |raw_key| [@local_config, @env_config, @global_config].each do |settings| value = converted_value(settings[key_for(raw_key)], raw_key) - Validator.validate!(raw_key, value, settings.to_hash.dup) + Validator.validate!(raw_key, value, settings.dup) end end end From 011e418232b43d3b81ac4d6dddc1b1ec5c7fe3bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 8 Sep 2020 14:51:23 +0200 Subject: [PATCH 09/11] Extract a `value_for` helper for latter reuse --- bundler/lib/bundler/settings.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index 6909069bf250..07551d852c40 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -277,7 +277,7 @@ def app_cache_path def validate! all.each do |raw_key| [@local_config, @env_config, @global_config].each do |settings| - value = converted_value(settings[key_for(raw_key)], raw_key) + value = value_for(raw_key, settings) Validator.validate!(raw_key, value, settings.dup) end end @@ -291,6 +291,10 @@ def key_for(key) private + def value_for(name, config) + converted_value(config[key_for(name)], name) + end + def parent_setting_for(name) split_specific_setting_for(name)[0] end From 85d8ac863cfba9af3895ab2ab1e9387947de4df1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 8 Sep 2020 14:54:02 +0200 Subject: [PATCH 10/11] Refactor prioritized access to configuration By extracting a `configs` helper which holds the different configuration levels in the right order. --- bundler/lib/bundler/settings.rb | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index 07551d852c40..c658d89e7561 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -81,13 +81,7 @@ def initialize(root = nil) def [](name) key = key_for(name) - value = @temporary.fetch(key) do - @local_config.fetch(key) do - @env_config.fetch(key) do - @global_config.fetch(key) do - DEFAULT_CONFIG.fetch(key) do - nil - end end end end end + value = configs.values.map {|config| config[key] }.compact.first converted_value(value, name) end @@ -167,13 +161,11 @@ def gem_mirrors def locations(key) key = key_for(key) - locations = {} - locations[:temporary] = @temporary[key] if @temporary.key?(key) - locations[:local] = @local_config[key] if @local_config.key?(key) - locations[:env] = @env_config[key] if @env_config.key?(key) - locations[:global] = @global_config[key] if @global_config.key?(key) - locations[:default] = DEFAULT_CONFIG[key] if DEFAULT_CONFIG.key?(key) - locations + configs.keys.inject({}) do |partial_locations, level| + value_on_level = configs[level][key] + partial_locations[level] = value_on_level unless value_on_level.nil? + partial_locations + end end def pretty_values_for(exposed_key) @@ -291,6 +283,16 @@ def key_for(key) private + def configs + { + :temporary => @temporary, + :local => @local_config, + :env => @env_config, + :global => @global_config, + :default => DEFAULT_CONFIG, + } + end + def value_for(name, config) converted_value(config[key_for(name)], name) end From 418998cab819b230352e1595d0fcfb318375f7fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 8 Sep 2020 15:07:14 +0200 Subject: [PATCH 11/11] Prioritize `path.system` over `path` when it makes sense If `path.system` is configured locally, and `path` is configured globally, gems should be installed to `path.system`. --- bundler/lib/bundler/settings.rb | 14 ++++++++------ bundler/spec/install/path_spec.rb | 8 ++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index c658d89e7561..6ce81b5006ec 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -195,14 +195,16 @@ def pretty_values_for(exposed_key) # for legacy reasons, in Bundler 2, we do not respect :disable_shared_gems def path - key = key_for(:path) - path = ENV[key] || @global_config[key] - if path && !@temporary.key?(key) && !@local_config.key?(key) - return Path.new(path, false) + configs.each do |_level, settings| + path = value_for("path", settings) + path_system = value_for("path.system", settings) + disabled_shared_gems = value_for("disable_shared_gems", settings) + next if path.nil? && path_system.nil? && disabled_shared_gems.nil? + system_path = path_system || (disabled_shared_gems == false) + return Path.new(path, system_path) end - system_path = self["path.system"] || (self[:disable_shared_gems] == false) - Path.new(self[:path], system_path) + Path.new(nil, false) end Path = Struct.new(:explicit_path, :system_path) do diff --git a/bundler/spec/install/path_spec.rb b/bundler/spec/install/path_spec.rb index 04cf01dceaea..a05467db123f 100644 --- a/bundler/spec/install/path_spec.rb +++ b/bundler/spec/install/path_spec.rb @@ -19,6 +19,14 @@ expect(the_bundle).to include_gems "rack 1.0.0" end + it "uses system gems with `path.system` configured with more priority than `path`" do + bundle "config --local path.system true" + bundle "config --global path vendor/bundle" + bundle :install + run "require 'rack'", :raise_on_error => false + expect(out).to include("FAIL") + end + it "handles paths with regex characters in them" do dir = bundled_app("bun++dle") dir.mkpath