Bug #2176

[rubygems] $LOAD_PATH includes bin directory

Added by nobu (Nobuyoshi Nakada) over 2 years ago. Updated about 1 year ago.

[ruby-core:25936]
Status:Closed Start date:
Priority:Urgent Due date:
Assignee:nobu (Nobuyoshi Nakada) % Done:

100%

Category:lib
Target version:1.9.2
ruby -v:1.9.2

Description

Hi,

What is the reason that bin is needed in $LOAD_PATH?  AFAIK,
bin directory is for executable files, but not for libraries.


Index: gem_prelude.rb
===================================================================
--- gem_prelude.rb	(revision 25229)
+++ gem_prelude.rb	(working copy)
@@ -256,12 +256,11 @@ if defined?(Gem) then
         GemPaths.each_value do |path|
           if File.exist?(file = File.join(path, ".require_paths")) then
-            paths = File.read(file).split.map do |require_path|
+            paths = File.readlines(file).map! do |require_path|
+              require_path.chomp!
               File.join path, require_path
             end
-
             require_paths.concat paths
           else
-            require_paths << file if File.exist?(file = File.join(path, "bin"))
-            require_paths << file if File.exist?(file = File.join(path, "lib"))
+            require_paths << (File.directory?(file = File.join(path, "lib")) ? file : path)
           end
         end
Index: lib/rubygems.rb
===================================================================
--- lib/rubygems.rb	(revision 25229)
+++ lib/rubygems.rb	(working copy)
@@ -294,7 +294,4 @@ module Gem
     end

-    # bin directory must come before library directories
-    spec.require_paths.unshift spec.bindir if spec.bindir
-
     require_paths = spec.require_paths.map do |path|
       File.join spec.full_gem_path, path
Index: lib/rubygems/require_paths_builder.rb
===================================================================
--- lib/rubygems/require_paths_builder.rb	(revision 25229)
+++ lib/rubygems/require_paths_builder.rb	(working copy)
@@ -3,13 +3,10 @@ require 'rubygems'
 module Gem::RequirePathsBuilder
   def write_require_paths_file_if_needed(spec = @spec, gem_home = @gem_home)
-    return if spec.require_paths == ["lib"] &&
-              (spec.bindir.nil? || spec.bindir == "bin")
+    require_paths = spec.require_paths
+    return if require_paths == ["lib"]
     file_name = File.join(gem_home, 'gems', "#{@spec.full_name}", ".require_paths")
     file_name.untaint
-    File.open(file_name, "w") do |file|
-      spec.require_paths.each do |path|
-        file.puts path
-      end
-      file.puts spec.bindir if spec.bindir
+    File.open(file_name, "wb") do |file|
+      file.puts require_paths
     end
   end


-- 
Nobu Nakada

Associated revisions

Revision 28447
Added by nobu (Nobuyoshi Nakada) almost 2 years ago

* lib/rubygems/require_paths_builder.rb (write_require_paths_file_if_needed): no reason that bin directory should be included in $LOAD_PATH. it is for executable files, but not libraries. [ruby-core:25936]

History

Updated by RickDeNatale (Rick DeNatale) over 2 years ago

On Sun, Oct 4, 2009 at 11:47 PM, Nobuyoshi Nakada <nobu@ruby-lang.org> wrote:
> Hi,
>
> What is the reason that bin is needed in $LOAD_PATH?  AFAIK,
> bin directory is for executable files, but not for libraries.
>

I believe that it's required because of the way the commands installed
with a gem typically bootstrap via a short ruby program which requires
rubygems and then loads the real command from the bin directory in the
required version of the gem.

I wouldn't be at all surprised if removing the gems bin directory from
the load path would break most if not all such gem installed commands.


-- 
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

Updated by luislavena (Luis Lavena) over 2 years ago

On Mon, Oct 5, 2009 at 2:20 PM, Rick DeNatale <rick.denatale@gmail.com> wrote:
> On Sun, Oct 4, 2009 at 11:47 PM, Nobuyoshi Nakada <nobu@ruby-lang.org> wrote:
>> Hi,
>>
>> What is the reason that bin is needed in $LOAD_PATH?  AFAIK,
>> bin directory is for executable files, but not for libraries.
>>
>
> I believe that it's required because of the way the commands installed
> with a gem typically bootstrap via a short ruby program which requires
> rubygems and then loads the real command from the bin directory in the
> required version of the gem.
>
> I wouldn't be at all surprised if removing the gems bin directory from
> the load path would break most if not all such gem installed commands.
>

In latest RubyGems (1.3.5) this process of loading gem binaries has
been changed to use Gem::bin_path

Of course, gems installed with a previous version of RubyGems require
re-generation of stub scripts, which simply can be achieved with "gem
pristine --all"

-- 
Luis Lavena
AREA 17
-
Perfection in design is achieved not when there is nothing more to add,
but rather when there is nothing more to take away.
Antoine de Saint-Exupéry

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

Hi,

At Mon, 5 Oct 2009 21:40:08 +0900,
Luis Lavena wrote in [ruby-core:25941]:
> In latest RubyGems (1.3.5) this process of loading gem binaries has
> been changed to use Gem::bin_path
> 
> Of course, gems installed with a previous version of RubyGems require
> re-generation of stub scripts, which simply can be achieved with "gem
> pristine --all"

Then, at least, adding bindir to require_paths in
require_paths_builder.rb isn't needed anymore?

-- 
Nobu Nakada

Updated by luislavena (Luis Lavena) over 2 years ago

On Wed, Oct 7, 2009 at 9:14 AM, Nobuyoshi Nakada <nobu@ruby-lang.org> wrote:
> Hi,
>
> At Mon, 5 Oct 2009 21:40:08 +0900,
> Luis Lavena wrote in [ruby-core:25941]:
>> In latest RubyGems (1.3.5) this process of loading gem binaries has
>> been changed to use Gem::bin_path
>>
>> Of course, gems installed with a previous version of RubyGems require
>> re-generation of stub scripts, which simply can be achieved with "gem
>> pristine --all"
>
> Then, at least, adding bindir to require_paths in
> require_paths_builder.rb isn't needed anymore?
>

It shouldn't, but perhaps Eric Hodel can comment on this better.

Eric: does the usage of bin_path solve the .require_paths needs to add
'bin' folder?
-- 
Luis Lavena
AREA 17
-
Perfection in design is achieved not when there is nothing more to add,
but rather when there is nothing more to take away.
Antoine de Saint-Exupéry

Updated by naruse (Yui NARUSE) over 2 years ago

  • Category set to lib
  • Status changed from Open to Assigned
  • Assignee set to drbrain (Eric Hodel)
  • Priority changed from Low to Urgent
  • Target version set to 1.9.2
  • ruby -v set to 1.9.2

Updated by mame (Yusuke Endoh) about 2 years ago

Hi, Eric

Please answer to nobu and Luis.

-- 
Yusuke Endoh <mame@tsg.ne.jp>

Updated by mame (Yusuke Endoh) almost 2 years ago

Hi, Eric

Please please answer to nobu and Luis.

-- 
Yusuke Endoh <mame@tsg.ne.jp>

Updated by drbrain (Eric Hodel) almost 2 years ago

We can remove bin from load path, but users upgrading from a previous ruby install may need to run `gem pristine`.

Gems installed using RubyGems 1.3.1 and older do not use Gem.bin_path to locate the correct executable inside the gem dir.

I was planning on making this change in RubyGems 1.4, but I think it is ok to make it for ruby 1.9.2.

Updated by mame (Yusuke Endoh) almost 2 years ago

Hi, Eric

Thank you for your answering.


Nobu,

Unfortunately, it may be too late and dangerous to commit your patch.
Do you think the patch must be included in 1.9.2?
If you do, please commit your patch ASAP.
If you don't, let's set the target to 1.9.x.

-- 
Yusuke Endoh <mame@tsg.ne.jp>

Updated by mame (Yusuke Endoh) almost 2 years ago

  • Assignee changed from drbrain (Eric Hodel) to nobu (Nobuyoshi Nakada)

Updated by nobu (Nobuyoshi Nakada) almost 2 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100
This issue was solved with changeset r28447.
Nobuyoshi, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

Also available in: Atom PDF