Currently, GemCheckRequiresRubygems is run on all packages from the spec file causing a lot of false positive results due to -doc sub-packages. This would be valid also for font or other sub-packages for any RubyGems.
Sub-packages should depend on the main package so the requirement of rubygems is matched anyways.
Proposed change: GemCheckRequiresRubygems should be run only on the main package and the misleading and wrong comment associated with this check should be deleted.
Wrong comment:
# it seems easier to check whether .gem is not present in rpms # than to examine %files"
Patch for devel branch 0001-Run-GemCheckRequiresRubygems-only-on-the-main-packag.patch
Generally I prefer when we have false positives then missing requires. In similar cases we used to provide a list of exceptions for subpackages (i.e. skip checking -doc subpackage).
Best solution? Why not make Ruby autorequires generator generate "Require: ruby(rubygems)" when needed? No need for manual check then
That's generally not a bad idea and would solve it. But still... I have this feeling that if a sub-package doesn't contain any Ruby code, then this shouldn't be necessary. It has its dependency on the main package and that's also what we should check, because:
1, it's always valid
2, it reflects how all the spec files looks right now in Fedora and helps us to find problems
3, would find the same issues like looking for rubygems if it's really Ruby sub-package
Your proposal feels like this sub-package really needs rubygems, which is in most cases not true. Currently it's mostly -doc sub-packages, could be font sub-packages or something else. We don't really split RubyGems into sub-packages, even rspec or rails for instance is put together as a collection of packages, just requiring each other.
Replying to [comment:3 jstribny]:
That's generally not a bad idea and would solve it. But still... I have this feeling that if a sub-package doesn't contain any Ruby code, then this shouldn't be necessary. It has its dependency on the main package and that's also what we should check, [cut]
Having -doc packages requiring the ,main package means that user can't install just the docs. This is a Bad Thing which we should not promote IMHO. And doesn't the same apply for fonts?
+1
True, I actually agree with this, but changing it for all packages at the moment is not desirable. Stating that they need rubygems is sort of the same thing.
At the very least the -doc sub-package should have this exception then.
Actually, do you know about any RubyGem in fedora that need to require rubygems in subpackages?
Here are actually a lot of options, one could inspect the content of the subpackage and/or what it actually requires. Still, the simple solution seems to be to exclude -doc and -fonts packages from the check, and the simple solution is often the best. Should we walk this path for now, and cope with whatever false positives left after implementing this?
Personally, I think this is "good enough" for now. The number of false warnings will probably drop to a level where it's not really a problem. And both -doc and -fonts packages have well-.defined semantics, there is really no risk suppressing a justified warning.
I have a special test setup here. I tried a simple "don't care about -doc and -fonts" patch and exercised it on all 395 rubygem packages. There were no false positives. So this should certainly be good enough ;)
Patch tried against all rubygem packages in rawhide (f20) Skip-doc-fonts.patch
Oops, I hadn't run it. If I really run it I get a list of 32 warnings. Some are no false but perfectly valid. Many applies to -devel packages which could be skipped - if those doesn't require the base package there are serious problems.
Bottom line: patch could be refined, but is usable as-is IMHO.
Failng rubygem packages with patch applied ruby.fails
Fixed in 5bdc2f13bc3c, closing. Thanks for reporting and also for patch. Although the solution was somewhat different, the patch pushed things forward, for sure!
Did you know that RubyGems can end with -doc or fonts? We don't have any of them in Fedora right now, but this way you potentially exclude checking in main packages of future RubyGems in Fedora...
But if you check only main package and sub-packages for requiring main package, then you cover all our RubyGems without any potential problem in future. This way you also reflect how the stack is really built right now, so the FedoraReview would reflect how things are really done (and report if something is done differently).