Merge pull request #9299 from ruby/fix-gh-3340

Implement relative path handling for plugin paths

Author: hsbt

bundler/lib/bundler/plugin/index.rb

@@ -163,6 +163,8 @@ def installed_in_plugin_root?(name)
       # @param [Pathname] index file path
       # @param [Boolean] is the index file global index
       def load_index(index_file, global = false)
+        base = base_for_index(global)
+
         SharedHelpers.filesystem_access(index_file, :read) do |index_f|
           valid_file = index_f&.exist? && !index_f.size.zero?
           break unless valid_file
@@ -174,8 +176,8 @@ def load_index(index_file, global = false)
 
           @commands.merge!(index["commands"])
           @hooks.merge!(index["hooks"])
-          @load_paths.merge!(index["load_paths"])
-          @plugin_paths.merge!(index["plugin_paths"])
+          @load_paths.merge!(transform_index_paths(index["load_paths"]) {|p| absolutize_path(p, base) })
+          @plugin_paths.merge!(transform_index_paths(index["plugin_paths"]) {|p| absolutize_path(p, base) })
           @sources.merge!(index["sources"]) unless global
         end
       end
@@ -184,11 +186,13 @@ def load_index(index_file, global = false)
       # instance variables in YAML format. (The instance variables are supposed
       # to be only String key value pairs)
       def save_index
+        base = base_for_index(false)
+
         index = {
           "commands" => @commands,
           "hooks" => @hooks,
-          "load_paths" => @load_paths,
-          "plugin_paths" => @plugin_paths,
+          "load_paths" => transform_index_paths(@load_paths) {|p| relativize_path(p, base) },
+          "plugin_paths" => transform_index_paths(@plugin_paths) {|p| relativize_path(p, base) },
           "sources" => @sources,
         }
 
@@ -198,6 +202,40 @@ def save_index
           File.open(index_f, "w") {|f| f.puts YAMLSerializer.dump(index) }
         end
       end
+
+      def base_for_index(global)
+        global ? Plugin.global_root : Plugin.root
+      end
+
+      def transform_index_paths(paths)
+        return {} unless paths
+
+        paths.transform_values do |value|
+          if value.is_a?(Array)
+            value.map {|path| yield path }
+          else
+            yield value
+          end
+        end
+      end
+
+      def relativize_path(path, base)
+        pathname = Pathname.new(path)
+        return path unless pathname.absolute?
+
+        base_path = Pathname.new(base)
+        if pathname == base_path || pathname.to_s.start_with?(base_path.to_s + File::SEPARATOR)
+          pathname.relative_path_from(base_path).to_s
+        else
+          path
+        end
+      end
+
+      def absolutize_path(path, base)
+        pathname = Pathname.new(path)
+        pathname = Pathname.new(base).join(pathname) unless pathname.absolute?
+        pathname.to_s
+      end
     end
   end
 end

spec/bundler/plugin/index_spec.rb

@@ -193,4 +193,83 @@
       include_examples "it cleans up"
     end
   end
+
+  describe "relative plugin paths" do
+    let(:plugin_name) { "relative-plugin" }
+
+    before do
+      Bundler::Plugin.reset!
+      allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile)
+
+      plugin_root = Bundler::Plugin.root
+      FileUtils.mkdir_p(plugin_root)
+
+      path = plugin_root.join(plugin_name)
+      FileUtils.mkdir_p(path.join("lib"))
+
+      index.register_plugin(plugin_name, path.to_s, [path.join("lib").to_s], [], [], [])
+    end
+
+    it "stores plugin paths relative to the plugin root" do
+      require "yaml"
+      data = YAML.load_file(index.index_file)
+
+      expect(data["plugin_paths"][plugin_name]).to eq(plugin_name)
+      expect(data["load_paths"][plugin_name]).to eq([File.join(plugin_name, "lib")])
+    end
+
+    it "expands relative paths to absolute on load" do
+      require "bundler/yaml_serializer"
+
+      plugin_root = Bundler::Plugin.root
+
+      relative_index = {
+        "commands" => {},
+        "hooks" => {},
+        "load_paths" => { plugin_name => [File.join(plugin_name, "lib")] },
+        "plugin_paths" => { plugin_name => plugin_name },
+        "sources" => {},
+      }
+
+      File.open(index.index_file, "w") {|f| f.puts Bundler::YAMLSerializer.dump(relative_index) }
+
+      new_index = Index.new
+      expect(new_index.plugin_path(plugin_name)).to eq(plugin_root.join(plugin_name))
+      expect(new_index.load_paths(plugin_name)).to eq([plugin_root.join(plugin_name, "lib").to_s])
+    end
+
+    it "keeps paths outside the plugin root as absolute" do
+      outside_path = tmp.join("outside", "external-plugin")
+      FileUtils.mkdir_p(outside_path.join("lib"))
+
+      index.register_plugin("external-plugin", outside_path.to_s, [outside_path.join("lib").to_s], [], [], [])
+
+      require "yaml"
+      data = YAML.load_file(index.index_file)
+
+      expect(data["plugin_paths"]["external-plugin"]).to eq(outside_path.to_s)
+      expect(data["load_paths"]["external-plugin"]).to eq([outside_path.join("lib").to_s])
+    end
+
+    it "reads legacy index files with absolute paths" do
+      require "bundler/yaml_serializer"
+
+      plugin_root = Bundler::Plugin.root
+      absolute_path = plugin_root.join(plugin_name).to_s
+
+      legacy_index = {
+        "commands" => {},
+        "hooks" => {},
+        "load_paths" => { plugin_name => [File.join(absolute_path, "lib")] },
+        "plugin_paths" => { plugin_name => absolute_path },
+        "sources" => {},
+      }
+
+      File.open(index.index_file, "w") {|f| f.puts Bundler::YAMLSerializer.dump(legacy_index) }
+
+      new_index = Index.new
+      expect(new_index.plugin_path(plugin_name)).to eq(Pathname.new(absolute_path))
+      expect(new_index.load_paths(plugin_name)).to eq([File.join(absolute_path, "lib")])
+    end
+  end
 end