Dne 24. 11. 22 v 13:12 Jarek Prokop napsal(a):
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 think that it used to be valid identifier at some point in time. This should be fixed in rubygem-rdoc then:
https://src.fedoraproject.org/rpms/rubygem-rdoc/c/5e165eed4822757b49dd3b1219dd4b7a024cb336?branch=rawhide but mainly upstream: https://github.com/ruby/rdoc/issues/924
I am maybe coming too soon with this comment
Maybe just a bit ;)
, but since we are also on the SPDX topic in parallel ;).
Yeah, IC.
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=94460961Would 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 --localSuccessfully uninstalled chronic-0.10.2 Successfully installed chronic-0.10.2FedoraRDoc.generation_hook - the `specs` could be modified to include additional `rdoc_options`
Eh, have I left the message ^^ there? :)
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 --localSuccessfully uninstalled chronic-0.10.2 Successfully installed chronic-0.10.2FedoraRDoc.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 ~~~
Good catch. I was testing plain `--document=rdoc`.Hm, there should be probably done some early matching, if the `fedora::` prefixed generator is there. Otherwise just move on with whatever was specified.
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.
I have not spend too much time thinking about %check section yet, but something along these lines seems reasonable.
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.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 ?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.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.
Yes, that was probably just oversight. Will fix this.
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".
Thx 👍
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 :).
Lets leave future problems for future us. So far, there is no other RDoc generator and I don't see any other generator coming any time soon, so lets ignore the `rubygem-` prefix for now. This still might end up as a gem after all :)
PTAL and let me know what do you think about this approach. ThxI'd say we are on the right path with this.
Thx. Next thing to consider is how to enforce the dependency on the -assets. There should probably be included some generator, otherwise we would need to update gem2rpm as well as every gem.
Also, the other question is how to pull this into the build root. Should this be pulled in by rubygems-devel? Shouldn't it even be part of rubygems-devel? Or shouldn't this be opt-in, where one needs to specify this explicitly? Maybe just from the beginning unless we are sure this does not cause more troubles then it solves?
Vít
OpenPGP_signature
Description: OpenPGP digital signature
_______________________________________________ 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
