Skip to content

#1217 is broken with make install in ruby/ruby #1221

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.

Sign up for GitHub

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

Jump to bottom
Closed
hsbt opened this issue Dec 2, 2024 · 5 comments · Fixed by #1222
Closed

#1217 is broken with make install in ruby/ruby #1221

hsbt opened this issue Dec 2, 2024 · 5 comments · Fixed by #1222
Assignees
@flavorjones
Labels
bug

Comments

@hsbt
Copy link
Member

hsbt commented Dec 2, 2024

@flavorjones @st0012

#1217 is broken with make install of ruby/ruby. I reverted that commit at ruby/ruby@0fe82ae

https://github.com/ruby/actions/actions/runs/12108034481/job/33755653615#step:23:1031

/home/runner/work/actions/actions/snapshot-master/lib/rdoc/code_object.rb:322:in 'RDoc::CodeObject#parent': undefined method 'find_class_or_module' for nil (NoMethodError)
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/code_object/class_module.rb:342:in 'RDoc::ClassModule#marshal_dump'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/store.rb:878:in 'Marshal.dump'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/store.rb:878:in 'block in RDoc::Store#save_class'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/store.rb:877:in 'IO.open'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/store.rb:877:in 'RDoc::Store#save_class'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/store.rb:763:in 'block in RDoc::Store#save'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/store.rb:762:in 'Array#each'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/store.rb:762:in 'RDoc::Store#save'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/generator/ri.rb:27:in 'RDoc::Generator::RI#generate'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/rdoc.rb:528:in 'block in RDoc::RDoc#generate'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/rdoc.rb:522:in 'Dir.chdir'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/rdoc.rb:522:in 'RDoc::RDoc#generate'
	from /home/runner/work/actions/actions/snapshot-master/lib/rdoc/rdoc.rb:501:in 'RDoc::RDoc#document'
	from ./tool/rdoc-srcdir:27:in '<main>'

Can you look that?
@flavorjones
Copy link
Collaborator

Sorry for the trouble. I'll investigate in the morning.

@hsbt
Copy link
Member Author

hsbt commented Dec 2, 2024

Thanks to your work. I will sync this feature when issue is resolved.

@flavorjones
Copy link
Collaborator

flavorjones commented Dec 2, 2024 •
edited
Loading

I've diagnosed the problem, which is related to setting ClassModule#superclass to a NormalClass in #1217. At runtime, there is no problem, but when the ClassModule is saved and loaded, then this problem emerges. The superclass must be set to a String in the current version of RDoc so that a ClassModule can be marshalled correctly.

Note that this bug is showing up in the ruby/actions CI job where RDocs are generated twice in a row; the second time fails when it #marshal_loads the classes saved during the first run.

I'll create a PR shortly.

flavorjones added a commit to flavorjones/rdoc that referenced this issue Dec 2, 2024
@flavorjones
ClassModule#superclass= accepts a ClassModule as an argument
fdb825f 
It is necessary for ClassModule's instance variable @superclass to
always be a String (or nil) so that the class can be saved with
`#marshal_dump` and loaded with `#marshal_load`.

However, there's no type checking being done, which allows a bug like
the one reported in ruby#1221 (which was introduced in ruby#1217) that sets
superclass to a ClassModule. That bug requires:

- setting a superclass to a NormalClass
- marshal_save
- marshal_load (which raises an exception)

With this change, passing a ClassModule to ClassModule#superclass= is
explicitly allowed by saving the full name of the ClassModule in the
@superclass instance variable.
@flavorjones flavorjones mentioned this issue Dec 2, 2024
Merged
@flavorjones
Copy link
Collaborator

See #1222

@flavorjones flavorjones mentioned this issue Dec 2, 2024
Merged
@st0012
Copy link
Member

st0012 commented Dec 2, 2024

Note that this bug is showing up in the ruby/actions CI job where RDocs are generated twice in a row; the second time fails when it #marshal_loads the classes saved during the first run.

Ah I was wondering why I couldn't reproduce the issue locally 😭

st0012 pushed a commit that referenced this issue Dec 2, 2024
@flavorjones
ClassModule#superclass= accepts a ClassModule as an argument (#1222)
9ced6d5 
It is necessary for ClassModule's instance variable @superclass to
always be a String (or nil) so that the class can be saved with
`#marshal_dump` and loaded with `#marshal_load`.

However, there's no type checking being done, which allows a bug like
the one reported in #1221 (which was introduced in #1217) that sets
superclass to a ClassModule. That bug requires:

- setting a superclass to a NormalClass
- marshal_save
- marshal_load (which raises an exception)

With this change, passing a ClassModule to ClassModule#superclass= is
explicitly allowed by saving the full name of the ClassModule in the
@superclass instance variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@flavorjones flavorjones

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ClassModule#superclass= accepts a ClassModule as an argument flavorjones/rdoc
4 participants
@flavorjones @hsbt @colby-swandale @st0012