Skip to content

Commit f4e10f4

Browse files
mterada1228kou
andauthored
Make it loose coupling between RubyGems and RDoc (#1171)
* Make it loose coupling between RubyGems and RDoc \### Problems There are following problems because of tight coupling between RubyGems and RDoc. 1. If there are braking changes in RDoc, RubyGems is also broken. 2. When we maintain RDoc, we have to change RubyGems. The reason why they are happened is that RubyGems creates documents about a gem with installing it. Note that RubyGems uses functions of RDoc to create documents. Specifically, - Creating documents is executed by `rubygems/lib/rubygems/rdoc.rb`. - `::RDoc::RubygemsHook` which is defined by RDoc is called by the file. \### Solution RubyGems has the plugin system. If a gem includes `rubygems_plugin.rb`, RubyGems loads it. RubyGems executes a process defined in it while installing gems, uninstalling gems or other events. We can use the system to solve the problems. The root cause is RubyGems directly references the class of RDoc. We can remove the root cause by making RDoc RubyGems plugin. Alternatively `rubygems_plugin.rb` creates documents about gems. \### FAQ Q1. Do we need to change codes of RubyGems? A. No, we don't. This change keeps compatibility of API used from RubyGems. Q2. Is it better to delete existing codes related to RDoc in RubyGems? No, it isn't. If we change codes of RubyGems, we can't keep a compatibility. Example: If we delete codes that uses `RDoc::RubygemsHook` in `rubygems/lib/rubygems/rdoc.rb`, documentations are not created with old RDoc. Q3. When can we delete `rubygems/lib/rubygems/rdoc.rb`? A. We can delete it when all users use RDoc including `rubygems_plugin`. Next ruby version is 3.4. If it includes the RDoc including `rubygems_plugin`, we can delete `rubygems/lib/rubygems/rdoc.rb` after ruby 3.3 is EOL. Q4. Is it a breaking change that Rubygems creates documents with rubygems_plugin not RDoc::RubygemsHook? A. No, it isn't. If we simply implement this approach, we move the implementation from `rdoc/lib/rdoc/rubygems_hook.rb` to `rubygems_plugin.rb`. This way can be breaking change. It seems to be fine that we just need to delete `rdoc/rubygems_hook.rb` but it doesn't work. It generates multiple documents. `rubygems/lib/rubygems/rdoc.rb` has the following code. ``` begin require "rdoc/rubygems_hook" # ... rescue LoadError end ``` This code ignores RDoc related processes when `rdoc/rubygems_hook` can't be required. But, this 'require' is not failed. This is because Ruby installs Rdoc as a default gem. So, Rdoc installed as a default gem generates documents and one installed as a normal gem does it too. If you think that this behavior is accectable, we can just delete `rdoc/rubygems_hook.rb`. What do you think about this approach? In this change, we take another approach to solve the problem that creates multiple documents. If `Gem.done_installing(&Gem::RDoc.method(:generation_hook))` in `rubygems/rdoc.rb` doesn't create documents, we can solve the problem. We have some options. * We change `rubygems/rdoc.rb` and then don't execute `Gem.done_installing`. (This is a change for RubyGems.) * We change `rdoc/rubygems_hook.rb` and then make `generation_hook` a no-op method. (This is a change for RDoc.) We choose the latter to avoid changing for RubyGems. \### Test \#### Preparation Install Rdoc which including our changes by executing `rake install`. ❯ rake install We confirmed that Rdoc which including our changes was installed. ❯ gem list | grep rdoc rdoc (6.6.0, default: 6.4.0) \#### Check point We tested to check compatibility. How to chack the compatibility? We tested creating same documents by our RDoc and old RDoc with latest RubyGems. We used following versions to test. ``` ❯ ruby -v ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [arm64-darwin22] ❯ gem list | grep rdoc rdoc (default: 6.4.0) ❯ ruby -I rubygems/lib rubygems/exe/gem --version 3.5.14 ``` Here is a result of test with old RDoc. We can see that the document is created correctlly with `Parsing...` and `Done installing...`. ``` ❯ ruby -I rubygems/lib rubygems/exe/gem install pkg-config Successfully installed pkg-config-1.5.6 Parsing documentation for pkg-config-1.5.6 Done installing documentation for pkg-config after 0 seconds 1 gem installed ``` Here is a result of test with our RDoc. We can see that the document is created correctlly with `Parsing...` and `Done installing...`. ``` ❯ ruby -I rubygems/lib rubygems/exe/gem install pkg-config Successfully installed pkg-config-1.5.6 Parsing documentation for pkg-config-1.5.6 Done installing documentation for pkg-config after 0 seconds 1 gem installed ``` As you can see we got the same results, our RDoc keeps compatibility. * rename a test file * Revert "rename a test file" This reverts commit 70a144b. * revert a test class name * exclude `TestRDocRubyGemsHook` at job of ruby-core * When `rubygems_plugin.rb` is not found, `test_rdoc_rubygems_hook.rb` is skipped. * remove unnecessary whitespace * add comment * Add support for the case that RDoc is installed as a default gem * Fix problems Co-authored-by: mterada1228 <[email protected]> * Simplify * removed unused blank lines and revert test * for rerun tests * add comment for rubygems_plugin.rb --------- Co-authored-by: Sutou Kouhei <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]>
1 parent 9e2b28c commit f4e10f4

File tree

3 files changed

+83
-10
lines changed

3 files changed

+83
-10
lines changed

lib/rdoc/rubygems_hook.rb

+56-6
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,19 @@
33
require 'fileutils'
44
require_relative '../rdoc'
55

6-
##
7-
# Gem::RDoc provides methods to generate RDoc and ri data for installed gems
8-
# upon gem installation.
6+
# We define the following two similar name classes in this file:
97
#
10-
# This file is automatically required by RubyGems 1.9 and newer.
8+
# - RDoc::RubyGemsHook
9+
# - RDoc::RubygemsHook
10+
#
11+
# RDoc::RubyGemsHook is the main class that has real logic.
12+
#
13+
# RDoc::RubygemsHook is a class that is only for
14+
# compatibility. RDoc::RubygemsHook is used by RubyGems directly. We
15+
# can remove this when all maintained RubyGems remove
16+
# `rubygems/rdoc.rb`.
1117

12-
class RDoc::RubygemsHook
18+
class RDoc::RubyGemsHook
1319

1420
include Gem::UserInteraction
1521
extend Gem::UserInteraction
@@ -45,7 +51,7 @@ class << self
4551
# Post installs hook that generates documentation for each specification in
4652
# +specs+
4753

48-
def self.generation_hook installer, specs
54+
def self.generate installer, specs
4955
start = Time.now
5056
types = installer.document
5157

@@ -64,6 +70,10 @@ def self.generation_hook installer, specs
6470
say "Done installing documentation for #{names} after #{duration} seconds"
6571
end
6672

73+
def self.remove uninstaller
74+
new(uninstaller.spec).remove
75+
end
76+
6777
##
6878
# Loads the RDoc generator
6979

@@ -246,3 +256,43 @@ def setup
246256
end
247257

248258
end
259+
260+
# This class is referenced by RubyGems to create documents.
261+
# All implementations are moved to the above RubyGemsHook.
262+
#
263+
# This class does nothing when this RDoc is installed as a normal gem
264+
# or a bundled gem.
265+
#
266+
# This class does generate/remove documents for compatibility when
267+
# this RDoc is installed as a default gem.
268+
#
269+
# We can remove this when all maintained RubyGems remove
270+
# `rubygems/rdoc.rb`.
271+
module RDoc
272+
class RubygemsHook
273+
def self.default_gem?
274+
!File.exist?(File.join(__dir__, "..", "rubygems_plugin.rb"))
275+
end
276+
277+
def initialize(spec)
278+
@spec = spec
279+
end
280+
281+
def remove
282+
# Do nothing if this is NOT a default gem.
283+
return unless self.class.default_gem?
284+
285+
# Remove generated document for compatibility if this is a
286+
# default gem.
287+
RubyGemsHook.new(@spec).remove
288+
end
289+
290+
def self.generation_hook installer, specs
291+
# Do nothing if this is NOT a default gem.
292+
return unless default_gem?
293+
294+
# Generate document for compatibility if this is a default gem.
295+
RubyGemsHook.generate(installer, specs)
296+
end
297+
end
298+
end

lib/rubygems_plugin.rb

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# frozen_string_literal: true
2+
3+
# If this file is exist, RDoc generates and removes documents by rubygems plugins.
4+
#
5+
# In follwing cases,
6+
# RubyGems directly exectute RDoc::RubygemsHook.generation_hook and RDoc::RubygemsHook#remove to generate and remove documents.
7+
#
8+
# - RDoc is used as a default gem.
9+
# - RDoc is a old version that doesn't have rubygems_plugin.rb.
10+
11+
require_relative 'rdoc/rubygems_hook'
12+
13+
# To install dependency libraries of RDoc, you need to run bundle install.
14+
# At that time, rdoc/markdown is not generated.
15+
# If generate and remove are executed at that time, an error will occur.
16+
# So, we can't register generate and remove to Gem at that time.
17+
begin
18+
require_relative 'rdoc/markdown'
19+
rescue LoadError
20+
else
21+
Gem.done_installing(&RDoc::RubyGemsHook.method(:generate))
22+
Gem.pre_uninstall(&RDoc::RubyGemsHook.method(:remove))
23+
end

test/rdoc/test_rdoc_rubygems_hook.rb

+4-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
require_relative '../../lib/rdoc/rubygems_hook'
66
require 'test/unit'
77

8-
class TestRDocRubygemsHook < Test::Unit::TestCase
8+
class TestRDocRubyGemsHook < Test::Unit::TestCase
99
def setup
1010
@a = Gem::Specification.new do |s|
1111
s.platform = Gem::Platform::RUBY
@@ -40,10 +40,10 @@ def setup
4040
FileUtils.touch File.join(@tempdir, 'a-2', 'lib', 'a.rb')
4141
FileUtils.touch File.join(@tempdir, 'a-2', 'README')
4242

43-
@hook = RDoc::RubygemsHook.new @a
43+
@hook = RDoc::RubyGemsHook.new @a
4444

4545
begin
46-
RDoc::RubygemsHook.load_rdoc
46+
RDoc::RubyGemsHook.load_rdoc
4747
rescue Gem::DocumentError => e
4848
omit e.message
4949
end
@@ -63,7 +63,7 @@ def test_initialize
6363
refute @hook.generate_rdoc
6464
assert @hook.generate_ri
6565

66-
rdoc = RDoc::RubygemsHook.new @a, false, false
66+
rdoc = RDoc::RubyGemsHook.new @a, false, false
6767

6868
refute rdoc.generate_rdoc
6969
refute rdoc.generate_ri

0 commit comments

Comments
 (0)