Hi,
looks great :), I'll comment more inline.
nit regarding the spec license, "GPL-2.0" is not valid SPDX (according
to license-validate at least), however, there is "GPL-2.0-only" or
"GPL-2.0-or-later".
I am maybe coming too soon with this comment, but since we are also on
the SPDX topic in parallel ;).
On 11/23/22 17:47, Vít Ondruch wrote:
Building upon this, here is another attempt:
https://fedorapeople.org/cgit/vondruch/public_git/darkfish.git/commit/?h=rawhide&id=487234ef5f64f78291ce767a8a989649ee941c73
https://koji.fedoraproject.org/koji/taskinfo?taskID=94460961
Would you consider using COPR for these proof-of-concept builds?
It makes testing the builds a small bit easier.
I am having trouble making it work, there already is registered
generator in the "done_installing_hooks", resulting in hitting both the
raises (the second one I hit after commenting out the first one).
Secondly (if I ignore both exceptions), I am met with a trace when
trying to generate `ri` (default on my `gem install --local`):
~~~
$ gem uninstall chronic; gem install ./chronic-0.10.2.gem
--document=ri,rdoc --local
Successfully uninstalled chronic-0.10.2
Successfully installed chronic-0.10.2
FedoraRDoc.generation_hook - the `specs` could be modified to include
additional `rdoc_options`
Parsing documentation for chronic-0.10.2
ERROR: While executing gem ... (OptionParser::InvalidArgument)
invalid argument: Invalid output formatter fedora::ri
/usr/share/gems/gems/rdoc-6.4.0/lib/rdoc/options.rb:1220:in
`setup_generator'
/usr/share/gems/gems/rdoc-6.4.0/lib/rdoc/rubygems_hook.rb:121:in
`document'
/usr/share/darkfish-rdoc/rubygems_plugin.rb:30:in `document'
/usr/share/gems/gems/rdoc-6.4.0/lib/rdoc/rubygems_hook.rb:195:in
`generate'
/usr/share/gems/gems/rdoc-6.4.0/lib/rdoc/rubygems_hook.rb:56:in `block
in generation_hook'
/usr/share/gems/gems/rdoc-6.4.0/lib/rdoc/rubygems_hook.rb:55:in `each'
/usr/share/gems/gems/rdoc-6.4.0/lib/rdoc/rubygems_hook.rb:55:in
`generation_hook'
/usr/share/darkfish-rdoc/rubygems_plugin.rb:25:in `generation_hook'
/usr/share/rubygems/rubygems/request_set.rb:311:in `block in
install_hooks'
/usr/share/rubygems/rubygems/request_set.rb:310:in `each'
/usr/share/rubygems/rubygems/request_set.rb:310:in `install_hooks'
/usr/share/rubygems/rubygems/request_set.rb:209:in `install'
/usr/share/rubygems/rubygems/commands/install_command.rb:210:in
`install_gem'
/usr/share/rubygems/rubygems/commands/install_command.rb:226:in `block
in install_gems'
/usr/share/rubygems/rubygems/commands/install_command.rb:219:in `each'
/usr/share/rubygems/rubygems/commands/install_command.rb:219:in
`install_gems'
/usr/share/rubygems/rubygems/commands/install_command.rb:167:in `execute'
/usr/share/rubygems/rubygems/command.rb:323:in `invoke_with_build_args'
/usr/share/rubygems/rubygems/command_manager.rb:185:in `process_args'
/usr/share/rubygems/rubygems/command_manager.rb:149:in `run'
/usr/share/rubygems/rubygems/gem_runner.rb:51:in `run'
/usr/bin/gem:13:in `<main>'
~~~
From `invalid argument: Invalid output formatter fedora::ri` present in
the log,
I'd say that following line
https://fedorapeople.org/cgit/vondruch/public_git/darkfish.git/tree/rubygems_plugin.rb?h=rawhide&id=487234ef5f64f78291ce767a8a989649ee941c73#n29
is too aggressive. We can also provide the corresponding RDoc generator
(even if it would be empty subclass).
This succeeds with only generating rdoc, ignoring ri format:
~~~
$ gem uninstall chronic; gem install ./chronic-0.10.2.gem
--document=rdoc --local
Successfully uninstalled chronic-0.10.2
Successfully installed chronic-0.10.2
FedoraRDoc.generation_hook - the `specs` could be modified to include
additional `rdoc_options`
Parsing documentation for chronic-0.10.2
Installing fedora::darkfish documentation for chronic-0.10.2
Done installing documentation for chronic after 1 seconds
1 gem installed
~~~
In regards to tests (%check):
What we want to test is: We can generate the documentation in context of
Fedora (IOW, `gem install` passes with `--document=rdoc,ri`)
and then validate that anything that is an image, or JS file that is not
"search_index.js" is symlinked to the template and that there are no
fonts present,
as those are the places where we differ from the tested upstream behavior.
This would help us catch any major regression that I can think of, at
build time, hopefully being a step ahead of accidentally breaking
rubygem builds.
Or maybe in addition there should a new sanity test case for the general
Ruby integration tests, that we can successfully run the mentioned gem
install, maybe even some simple rubygem package build,
for the case that the RDoc darkfish template changes in some breaking
way during a Ruby update.
While I have reverted back to non gem approach, the RubyGems plugin
hook can still be utilized. I have reverted from the gem approach,
because it allows to have the template outside of the RubyGems
directory structure, while it does not prohibit to install RubyGems hook.
I have also tried different approaches, such as having full featured
RDoc extension, but the RDoc templates are loaded quite late in the
process [1], so they would not be available for the RubyGems generator.
This iteration also removes monkey patching in favor of inheritance.
Nit to the code, if it is using inheritance, maybe replace the method
aliasing with `super' in this file:
https://fedorapeople.org/cgit/vondruch/public_git/darkfish.git/tree/fedora_darkfish.rb?h=rawhide&id=487234ef5f64f78291ce767a8a989649ee941c73
?
It seems like it can be simply replaced if I didn't miss anything. That
was one of my reasons for inheritance, call to `super' seems simpler.
Last but not least, this iteration helps me a bit with clarification
of the name of the project. The generators has to have
`RDoc::Generator::` prefix, therefore the class is named
`RDoc::Generator::Fedora::Darkfish` (and there is also
`RDoc::Generator::Fedora::JsonIndex`), therefore I think the
package/project should be named fedora-darkfish. I'm still not clear
if `rdoc` suffix/prefix would help. While long, maybe
`rdoc-generator-fedora-darkfish` prefix would correlate with the class
name the best and it would hint to RDoc and Darkfish. Should there be
another generators packaged, they could possibly follow similar scheme
(while in theory the should have rubygem- prefix 🫣).
I am a fan of the "rdoc-generator-fedora-darkfish".
RDoc generators could have rubygem-rdoc-generator-* if we would want to
mandate prefixes, but that would come with new packaging guidelines, etc...
This iteration does not have rubygem-* prefix as it is not a gem, so I
would say, this is the odd case :).
PTAL and let me know what do you think about this approach. Thx
I'd say we are on the right path with this.
Regards,
Jarek
Vít
[1]
https://github.com/ruby/rdoc/blob/45259f9024b7d08ae2ba119c9a02983bf672f10c/lib/rdoc/rdoc.rb#L532-L546
Dne 01. 11. 22 v 18:53 Vít Ondruch napsal(a):
Dne 26. 10. 22 v 17:32 Vít Ondruch napsal(a):
We could also try to somehow modify the Gem.done_installing hooks
and remove the RDoc hook and replace it with ours.
Building upon this idea, I am attaching an proof of concept. This
loads the original RubyGems RDoc hook, removes it an replaces by a
custom hook. In this custom hook (which is inherited from the
original code), the generator name can be modified from "darkfish" to
something completely different and hence load the custom generator
(which can inherit from Darkfish). The options could also be
potentially modified to change the #setup_generator method.
Another option could be leave the generator as it is and have
additional hook, which would run after the default generators and
update the generated content. While tempting, I still think the
generator/template should be extracted from RDoc and therefore this
is less appealing option IMHO.
BTW we could also use the hook to remove the `--document=ri,rdoc`
option from `%gem_install` macro [1]. But that would be probably too
much magic.
Vít
[1]
https://src.fedoraproject.org/rpms/ruby/blob/588a4ae9f02928d7bedbcf46a739d36b0a76e632/f/macros.rubygems#_31
_______________________________________________
ruby-sig mailing list --ruby-sig@lists.fedoraproject.org
To unsubscribe send an email toruby-sig-le...@lists.fedoraproject.org
Fedora Code of
Conduct:https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:https://fedoraproject.org/wiki/Mailing_list_guidelines
List
Archives:https://lists.fedoraproject.org/archives/list/ruby-sig@lists.fedoraproject.org
Do not reply to spam, report
it:https://pagure.io/fedora-infrastructure/new_issue
_______________________________________________
ruby-sig mailing list -- ruby-sig@lists.fedoraproject.org
To unsubscribe send an email to ruby-sig-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/ruby-sig@lists.fedoraproject.org
Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue