Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install default and bundled gems into dedicated directories #8761

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

voxik
Copy link
Contributor

@voxik voxik commented Oct 25, 2023

Previously, all default/bundled gems were installed into single location such as /usr/local/lib/ruby/gems/3.3.0+0. However, the same location is used for user installed gems, which is not optimal.

With this change, the default gems goes into /usr/local/lib/ruby/default_gems/3.3.0+0 while the bundled gems goes into /usr/local/lib/ruby/bundled_gems/3.3.0+0. User installed gems continues to land in /usr/local/lib/ruby/gems/3.3.0+0.

@voxik
Copy link
Contributor Author

voxik commented Oct 26, 2023

Unfortunately, it seems that RubyGems test suite is not prepared enough for existence of lib/rubygems/defaults/ruby.rb. Not really sure how to properly detect that RubyGems test suite is running to disable the overrides ...

Previously, all default/bundled gems were installed into single location
such as `/usr/local/lib/ruby/gems/3.3.0+0`. However, the same location
is used for user installed gems, which is not optimal.

With this change, the default gems goes into
`/usr/local/lib/ruby/default_gems/3.3.0+0` while the bundled gems goes
into `/usr/local/lib/ruby/bundled_gems/3.3.0+0`. User installed gems
continues to land in `/usr/local/lib/ruby/gems/3.3.0+0`.
@voxik
Copy link
Contributor Author

voxik commented Nov 21, 2023

I have made some progress with fixing the test failures and with the recent RubyGems updates, there are 4 remaining issues:

    1) Failure:
  TestGem#test_default_path_vendor_dir [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:612]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/gemhome",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/vendor/gems/3.3.0+0"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/gemhome",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/vendor/gems/3.3.0+0",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.
  make: *** [uncommon.mk:943: yes-test-all] Error 4
  
    2) Failure:
  TestGem#test_default_path_missing_vendor [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:592]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-c41yb7/gemhome"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-c41yb7/gemhome",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.
  
    3) Failure:
  TestGem#test_default_path_user_home [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:600]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/userhome/.local/share/gem/ruby/3.3.0+0",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/gemhome"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/userhome/.local/share/gem/ruby/3.3.0+0",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/gemhome",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.
  
    4) Failure:
  TestGem#test_default_path [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:582]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-kan9o5/gemhome"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-kan9o5/gemhome",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.

These are basically checking some defaults, which are not surprisingly overridden by the custom configuration. Not sure yet how to deal with these. I don't think it would be big loss if these were disabled ...

# Path to specification files of default gems.

def default_specifications_dir
@default_specifications_dir ||= File.join(Gem.default_gems_dir, "specifications", "default")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this cached while bundled_gems_dir and default_gems_dir aren't?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cached also by RubyGems:

https://github.com/ruby/ruby/blame/209a0253f5869678d7228731605a1a5f21c76f32/lib/rubygems/defaults.rb#L63

and some parts of the test suite overrides the cached value (if I am not mistaken).

Also, I don't expect that bundled_gems_dir and default_gems_dir would be used by anything else then just by the default_specifications_dir and the default_path method, where the latter is cached here:

@paths ||= Gem::PathSupport.new(ENV)

Therefore the need for caching is minimal IMHO

RubyGems provides means to override their defaults, such as
`Gem.default_path` method. With such modified configuration, the
following errors might be observed:

~~~
    1) Failure:
  TestGem#test_default_path_vendor_dir [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:612]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/gemhome",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/vendor/gems/3.3.0+0"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/gemhome",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-qs43ch/vendor/gems/3.3.0+0",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.
  make: *** [uncommon.mk:943: yes-test-all] Error 4

    2) Failure:
  TestGem#test_default_path_missing_vendor [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:592]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-c41yb7/gemhome"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-c41yb7/gemhome",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.

    3) Failure:
  TestGem#test_default_path_user_home [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:600]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/userhome/.local/share/gem/ruby/3.3.0+0",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/gemhome"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/userhome/.local/share/gem/ruby/3.3.0+0",
   "/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-5mm6uc/gemhome",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.

    4) Failure:
  TestGem#test_default_path [/home/runner/work/ruby/ruby/src/test/rubygems/test_gem.rb:582]:
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-kan9o5/gemhome"]> expected but was
  <["/home/runner/work/ruby/ruby/build/tmp/test_rubygems_20231110-59661-kan9o5/gemhome",
   "/usr/local/lib/ruby/bundled_gems/3.3.0+0",
   "/usr/local/lib/ruby/default_gems/3.3.0+0"]>.
~~~

Of course the `Gem.default_path` could be stubbed, but testing stub
should not be the point.

Therefore, just detect if `Gem.default_path` was customized and ignore
the test cases.
@voxik
Copy link
Contributor Author

voxik commented Nov 23, 2023

I have included here patch from rubygems/rubygems#7187 which makes the test suite green

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants