From b241c53d1782745d694545000a7f4c251b243a1b Mon Sep 17 00:00:00 2001 From: Greg Werbin Date: Sun, 10 Sep 2017 20:30:43 -0400 Subject: [PATCH 1/6] Allow the user to set alternative to ~/.bundle Recognize new environment variables, with the following fallbacks: $BUNDLE_USER_HOME -> $HOME/.bundle $BUNDLE_USER_CACHE -> $BUNDLE_USER_HOME/cache $BUNDLE_USER_CONFIG -> $BUNDLE_USER_HOME/config $BUNDLE_USER_PLUGIN -> $BUNDLE_USER_HOME/plugin TODOs: - Error handling in Bundler.user_bundle_path when an invalid option is passed - Add tests (see https://github.com/bundler/bundler/pull/5787/commits/47fbe99387fea73fa652ad6692349b24cad6fe2e) - Draft PR with reference to GitHub issue numbers (not linked here as per contributing guidelines) + Issue: 4333 + Pull request: 5787 --- lib/bundler.rb | 21 +++++++++++++++++++-- lib/bundler/plugin.rb | 6 +++--- lib/bundler/settings.rb | 2 +- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/lib/bundler.rb b/lib/bundler.rb index 81c6a5b594a..31602dfa650 100644 --- a/lib/bundler.rb +++ b/lib/bundler.rb @@ -193,10 +193,27 @@ def tmp_home_path(login, warning) raise e.exception("#{warning}\nBundler also failed to create a temporary home directory at `#{path}':\n#{e}") end - def user_bundle_path + def user_bundle_path_default Pathname.new(user_home).join(".bundle") end + def user_bundle_path(dir="home") + # if "home", user_bundle_path_default + env_var, fallback = case dir + when "home" + ["BUNDLE_USER_HOME", user_bundle_path_default] + when "cache" + ["BUNDLE_USER_CACHE", user_bundle_path.join("cache")] + when "config" + ["BUNDLE_USER_CONFIG", user_bundle_path.join("config")] + when "plugin" + ["BUNDLE_USER_PLUGIN", user_bundle_path.join("plugin")] + else + nil + end + ENV.fetch(env_var, fallback) + end + def home bundle_path.join("bundler") end @@ -210,7 +227,7 @@ def specs_path end def user_cache - user_bundle_path.join("cache") + user_bundle_path("cache") end def root diff --git a/lib/bundler/plugin.rb b/lib/bundler/plugin.rb index 99c9a867b0c..7db159c5930 100644 --- a/lib/bundler/plugin.rb +++ b/lib/bundler/plugin.rb @@ -80,8 +80,8 @@ def index # The directory root for all plugin related data # - # Points to root in app_config_path if ran in an app else points to the one - # in user_bundle_path + # If run in an app, points to local root, in app_config_path + # Otherwise, points to global root, in Bundler.user_bundle_path("plugin") def root @root ||= if SharedHelpers.in_bundle? local_root @@ -96,7 +96,7 @@ def local_root # The global directory root for all plugin related data def global_root - Bundler.user_bundle_path.join("plugin") + Bundler.user_bundle_path("plugin") end # The cache directory for plugin stuffs diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb index c58eb3d4948..c2fe71be2ba 100644 --- a/lib/bundler/settings.rb +++ b/lib/bundler/settings.rb @@ -392,7 +392,7 @@ def global_config_file Pathname.new(ENV["BUNDLE_CONFIG"]) else begin - Bundler.user_bundle_path.join("config") + Bundler.user_bundle_path("config") rescue PermissionError, GenericSystemCallError nil end From d378a816866ce28f7fa010a4dd9095f4b2439bee Mon Sep 17 00:00:00 2001 From: Greg Werbin Date: Sun, 10 Sep 2017 21:07:47 -0400 Subject: [PATCH 2/6] Use tests from the old commit Mostly unmodified tests from PR 5787 (not linked here to respect contribution guidelines). --- spec/bundler/bundler_spec.rb | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/spec/bundler/bundler_spec.rb b/spec/bundler/bundler_spec.rb index 19e3f0336f6..34011a21a6f 100644 --- a/spec/bundler/bundler_spec.rb +++ b/spec/bundler/bundler_spec.rb @@ -227,4 +227,58 @@ expect(Bundler.tmp_home_path("USER", "")).to eq(Pathname("/TMP/bundler/home/USER")) end end + + context "user cache dir" do + let(:home_path) { ENV.fetch "HOME" } + let(:bundle_user_cache_default) { "#{home_path}/.cache" } + let(:bundle_user_cache_custom) { "#{home_path}/User/cache" } + let(:fallback_dir) { "#{bundle_user_cache_default}/bundler" } + let(:cache_dir) { "#{bundle_user_cache_custom}/bundler" } + let(:legacy_cache_dir) { "#{home_path}/.bundle/cache" } + + describe "#bundle_user_path_config" do + before do + allow(Bundler.rubygems).to receive(:user_home).and_return(home_path) + allow(File).to receive(:writable?).with(home_path).and_return(true) + allow(File).to receive(:directory?).with(home_path).and_return(true) + allow(File).to receive(:directory?).with(fallback_dir).and_return(true) + allow(File).to receive(:directory?).with(cache_dir).and_return(true) + end + + it "should use the value of BUNDLE_USER_CACHE_HOME" do + ENV["BUNDLE_USER_CACHE_HOME"] = bundle_user_cache_custom + expect(Bundler.bundle_user_home("CACHE", fallback_dir)).to eq(Pathname(cache_dir)) + end + + it "should fall back to the alternative directory" do + expect(Bundler.bundle_user_home("CACHE", bundle_user_cache_default)).to eq(Pathname(fallback_dir)) + end + end + + describe "#user_cache" do + before do + allow(Bundler.rubygems).to receive(:user_home).and_return(home_path) + allow(File).to receive(:writable?).with(home_path).and_return(true) + allow(File).to receive(:directory?).with(home_path).and_return(true) + end + + it "should use ~/.bundle/cache if it exists" do + allow(FileTest).to receive(:exist?).with(legacy_cache_dir).and_return(true) + expect(Bundler.user_cache).to eq(Pathname(legacy_cache_dir)) + end + + it "should use BUNDLE_USER_CACHE_HOME if set" do + allow(FileTest).to receive(:exist?).with(legacy_cache_dir).and_return(false) + allow(FileTest).to receive(:exist?).with(cache_dir).and_return(true) + ENV["BUNDLE_USER_CACHE_HOME"] = bundle_user_cache_custom + expect(Bundler.user_cache).to eq(Pathname(cache_dir)) + end + + it "should use ~/.cache/bundler as default cache path" do + allow(FileTest).to receive(:exist?).with(legacy_cache_dir).and_return(false) + allow(FileTest).to receive(:exist?).with(fallback_dir).and_return(true) + expect(Bundler.user_cache).to eq(Pathname(fallback_dir)) + end + end + end end From 71f13f90eaa447dd565ee6ea5d18864060d80033 Mon Sep 17 00:00:00 2001 From: Greg Werbin Date: Mon, 11 Sep 2017 11:18:50 -0400 Subject: [PATCH 3/6] Remove unnecessary method --- lib/bundler.rb | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/bundler.rb b/lib/bundler.rb index 31602dfa650..8a7296d8e7d 100644 --- a/lib/bundler.rb +++ b/lib/bundler.rb @@ -193,15 +193,10 @@ def tmp_home_path(login, warning) raise e.exception("#{warning}\nBundler also failed to create a temporary home directory at `#{path}':\n#{e}") end - def user_bundle_path_default - Pathname.new(user_home).join(".bundle") - end - def user_bundle_path(dir="home") - # if "home", user_bundle_path_default env_var, fallback = case dir when "home" - ["BUNDLE_USER_HOME", user_bundle_path_default] + ["BUNDLE_USER_HOME", Pathname.new(user_home).join(".bundle")] when "cache" ["BUNDLE_USER_CACHE", user_bundle_path.join("cache")] when "config" @@ -209,9 +204,15 @@ def user_bundle_path(dir="home") when "plugin" ["BUNDLE_USER_PLUGIN", user_bundle_path.join("plugin")] else - nil + raise BundlerError, "Unknown user path requested: #{dir}" end - ENV.fetch(env_var, fallback) + # `fallback` will already be a Pathname, but Pathname.new() is + # idempotent so it's OK + Pathname.new(ENV.fetch(env_var, fallback)) + end + + def user_cache + user_bundle_path("cache") end def home @@ -226,10 +227,6 @@ def specs_path bundle_path.join("specifications") end - def user_cache - user_bundle_path("cache") - end - def root @root ||= begin SharedHelpers.root From bea279736a06c852ef7b24e07de3c238fcb78518 Mon Sep 17 00:00:00 2001 From: Greg Werbin Date: Mon, 11 Sep 2017 11:19:08 -0400 Subject: [PATCH 4/6] Make tests green --- spec/bundler/bundler_spec.rb | 81 +++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/spec/bundler/bundler_spec.rb b/spec/bundler/bundler_spec.rb index 34011a21a6f..12db551eb3a 100644 --- a/spec/bundler/bundler_spec.rb +++ b/spec/bundler/bundler_spec.rb @@ -229,56 +229,61 @@ end context "user cache dir" do - let(:home_path) { ENV.fetch "HOME" } - let(:bundle_user_cache_default) { "#{home_path}/.cache" } - let(:bundle_user_cache_custom) { "#{home_path}/User/cache" } - let(:fallback_dir) { "#{bundle_user_cache_default}/bundler" } - let(:cache_dir) { "#{bundle_user_cache_custom}/bundler" } - let(:legacy_cache_dir) { "#{home_path}/.bundle/cache" } - - describe "#bundle_user_path_config" do - before do - allow(Bundler.rubygems).to receive(:user_home).and_return(home_path) - allow(File).to receive(:writable?).with(home_path).and_return(true) - allow(File).to receive(:directory?).with(home_path).and_return(true) - allow(File).to receive(:directory?).with(fallback_dir).and_return(true) - allow(File).to receive(:directory?).with(cache_dir).and_return(true) - end + let(:home_path) { Pathname.new(ENV["HOME"]) } - it "should use the value of BUNDLE_USER_CACHE_HOME" do - ENV["BUNDLE_USER_CACHE_HOME"] = bundle_user_cache_custom - expect(Bundler.bundle_user_home("CACHE", fallback_dir)).to eq(Pathname(cache_dir)) - end + let(:xdg_data_home) { home_path.join(".local") } + let(:xdg_cache_home) { home_path.join(".cache") } + let(:xdg_config_home) { home_path.join(".config") } - it "should fall back to the alternative directory" do - expect(Bundler.bundle_user_home("CACHE", bundle_user_cache_default)).to eq(Pathname(fallback_dir)) - end - end + let(:bundle_user_home_default) { home_path.join(".bundle") } + let(:bundle_user_home_custom) { xdg_data_home.join("bundle") } + + let(:bundle_user_cache_default) { bundle_user_home_default.join("cache") } + let(:bundle_user_cache_custom) { xdg_cache_home.join("bundle") } + + let(:bundle_user_config_default) { bundle_user_home_default.join("config") } + let(:bundle_user_config_custom) { xdg_config_home.join("bundle") } - describe "#user_cache" do + let(:bundle_user_plugin_default) { bundle_user_home_default.join("plugin") } + let(:bundle_user_plugin_custom) { xdg_data_home.join("bundle").join("plugin") } + + describe "#user_bundle_path" do before do allow(Bundler.rubygems).to receive(:user_home).and_return(home_path) - allow(File).to receive(:writable?).with(home_path).and_return(true) - allow(File).to receive(:directory?).with(home_path).and_return(true) end - it "should use ~/.bundle/cache if it exists" do - allow(FileTest).to receive(:exist?).with(legacy_cache_dir).and_return(true) - expect(Bundler.user_cache).to eq(Pathname(legacy_cache_dir)) + it "should use the default home path" do + expect(Bundler.user_bundle_path).to eq(bundle_user_home_default) + expect(Bundler.user_bundle_path("home")).to eq(bundle_user_home_default) + expect(Bundler.user_bundle_path("cache")).to eq(bundle_user_cache_default) + expect(Bundler.user_cache).to eq(bundle_user_cache_default) + expect(Bundler.user_bundle_path("config")).to eq(bundle_user_config_default) + expect(Bundler.user_bundle_path("plugin")).to eq(bundle_user_plugin_default) end - it "should use BUNDLE_USER_CACHE_HOME if set" do - allow(FileTest).to receive(:exist?).with(legacy_cache_dir).and_return(false) - allow(FileTest).to receive(:exist?).with(cache_dir).and_return(true) - ENV["BUNDLE_USER_CACHE_HOME"] = bundle_user_cache_custom - expect(Bundler.user_cache).to eq(Pathname(cache_dir)) + it "should use custom home path as root for other paths" do + ENV["BUNDLE_USER_HOME"] = bundle_user_home_custom.to_s + expect(Bundler.user_bundle_path).to eq(bundle_user_home_custom) + expect(Bundler.user_bundle_path("home")).to eq(bundle_user_home_custom) + expect(Bundler.user_bundle_path("cache")).to eq(bundle_user_home_custom.join("cache")) + expect(Bundler.user_cache).to eq(bundle_user_home_custom.join("cache")) + expect(Bundler.user_bundle_path("config")).to eq(bundle_user_home_custom.join("config")) + expect(Bundler.user_bundle_path("plugin")).to eq(bundle_user_home_custom.join("plugin")) end - it "should use ~/.cache/bundler as default cache path" do - allow(FileTest).to receive(:exist?).with(legacy_cache_dir).and_return(false) - allow(FileTest).to receive(:exist?).with(fallback_dir).and_return(true) - expect(Bundler.user_cache).to eq(Pathname(fallback_dir)) + it "should use all custom paths, except home" do + ENV.delete("BUNDLE_USER_HOME") + ENV["BUNDLE_USER_CACHE"] = bundle_user_cache_custom.to_s + ENV["BUNDLE_USER_CONFIG"] = bundle_user_config_custom.to_s + ENV["BUNDLE_USER_PLUGIN"] = bundle_user_plugin_custom.to_s + expect(Bundler.user_bundle_path).to eq(bundle_user_home_default) + expect(Bundler.user_bundle_path("home")).to eq(bundle_user_home_default) + expect(Bundler.user_bundle_path("cache")).to eq(bundle_user_cache_custom) + expect(Bundler.user_cache).to eq(bundle_user_cache_custom) + expect(Bundler.user_bundle_path("config")).to eq(bundle_user_config_custom) + expect(Bundler.user_bundle_path("plugin")).to eq(bundle_user_plugin_custom) end + end end end From abb8f976f669b8a7a1ddb80928ebd64e00c05560 Mon Sep 17 00:00:00 2001 From: Greg Werbin Date: Mon, 11 Sep 2017 15:31:33 -0400 Subject: [PATCH 5/6] Conform to code style --- lib/bundler.rb | 24 ++++++++++++------------ spec/bundler/bundler_spec.rb | 1 - 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/bundler.rb b/lib/bundler.rb index 8a7296d8e7d..1157239029e 100644 --- a/lib/bundler.rb +++ b/lib/bundler.rb @@ -193,19 +193,19 @@ def tmp_home_path(login, warning) raise e.exception("#{warning}\nBundler also failed to create a temporary home directory at `#{path}':\n#{e}") end - def user_bundle_path(dir="home") + def user_bundle_path(dir = "home") env_var, fallback = case dir - when "home" - ["BUNDLE_USER_HOME", Pathname.new(user_home).join(".bundle")] - when "cache" - ["BUNDLE_USER_CACHE", user_bundle_path.join("cache")] - when "config" - ["BUNDLE_USER_CONFIG", user_bundle_path.join("config")] - when "plugin" - ["BUNDLE_USER_PLUGIN", user_bundle_path.join("plugin")] - else - raise BundlerError, "Unknown user path requested: #{dir}" - end + when "home" + ["BUNDLE_USER_HOME", Pathname.new(user_home).join(".bundle")] + when "cache" + ["BUNDLE_USER_CACHE", user_bundle_path.join("cache")] + when "config" + ["BUNDLE_USER_CONFIG", user_bundle_path.join("config")] + when "plugin" + ["BUNDLE_USER_PLUGIN", user_bundle_path.join("plugin")] + else + raise BundlerError, "Unknown user path requested: #{dir}" + end # `fallback` will already be a Pathname, but Pathname.new() is # idempotent so it's OK Pathname.new(ENV.fetch(env_var, fallback)) diff --git a/spec/bundler/bundler_spec.rb b/spec/bundler/bundler_spec.rb index 12db551eb3a..94d4096cd33 100644 --- a/spec/bundler/bundler_spec.rb +++ b/spec/bundler/bundler_spec.rb @@ -283,7 +283,6 @@ expect(Bundler.user_bundle_path("config")).to eq(bundle_user_config_custom) expect(Bundler.user_bundle_path("plugin")).to eq(bundle_user_plugin_custom) end - end end end From b3108fac3f9ec1a85554790b9401ade370392b75 Mon Sep 17 00:00:00 2001 From: Greg Werbin Date: Mon, 11 Sep 2017 15:51:53 -0400 Subject: [PATCH 6/6] Conform to code style --- lib/bundler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler.rb b/lib/bundler.rb index 1157239029e..6c942baf59d 100644 --- a/lib/bundler.rb +++ b/lib/bundler.rb @@ -205,7 +205,7 @@ def user_bundle_path(dir = "home") ["BUNDLE_USER_PLUGIN", user_bundle_path.join("plugin")] else raise BundlerError, "Unknown user path requested: #{dir}" - end + end # `fallback` will already be a Pathname, but Pathname.new() is # idempotent so it's OK Pathname.new(ENV.fetch(env_var, fallback))