https://bugzilla.redhat.com/show_bug.cgi?id=1375765
Randy Barlow randy@electronsweatshop.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ | |needinfo?(spacewar@gmail.co | |m)
--- Comment #6 from Randy Barlow randy@electronsweatshop.com --- (In reply to Eric Smith from comment #5)
Spec URL: https://fedorapeople.org/~brouhaha/yosys/yosys.spec SRPM URL: https://fedorapeople.org/~brouhaha/yosys/yosys-0.6.0-2.20160923git8f5bf6d. fc24.src.rpm
Note that no action was taken for the following:
Thanks for pointing these out!
- bundled viz.js - The file viz.js isn't bundled into the generated RPMs,
so the bundling policy, which is based primarily on security concerns, is not applicable. In particular, following the bundling policy would require a "Provides: bundled(viz.js)=0.0.3", but that would clearly be wrong, as the package doesn't provide it.
I'm convinced and I agree with you now.
- /usr/share/yosys/python3/smtio.py - Not moved. There is considerable
precedent for application-specific Python files to be in /usr/share/%{name}, e.g., firewalld, hplip, qtcreator, setroubleshoot, virt-manager, etc. I've reviewed FHS 3.0 and am not convinced that having application-specific Python files in /usr/share/%{name} actually contravenes any FHS 3.0 requirement.
I still think this *should* be moved, but due to your citations we can keep it the way it is.
- tarball without URL - No actual problem. Perhaps was triggered by outdated
URL for Debian pool.
OK. FYI, the current URL is also 404, but I see the problem with the disappearing files. I recommend working with upstream to get that man page included there so we don't depend on Debian's man pages (and then Debian can also get them from upstream!)
- %check tests directory - No actual problem. Tests are present and are
correctly tested in %check section.
OK
These requested changes have been made:
- license tag - Changed to include additional licenses.
I read the issue you filed upstream, and it seems that some of the licenses included in the current spec file only apply to tests and not to the distributed package. I recommend trimming the licenses down to what is in the distributed code, but I'm not sure whether the license field should be about the entire source tree, or just the bits we distribute. I'll leave that decision up to you.
- /usr/share/yosys/python3/smtio.p - Had previously marked executable to fix
rpmlint error. Removed the chmod, so now rpmlint reports that error. Which is better, having the rpmlint error, or having the file unnecessarily marked as executable? Since it contains no main program, I felt that having it marked executable was inconsequential.
It doesn't make sense for the file to be executable so I think the current state is better. If you want to silence rpmlint, you can drop the #!/usr/bin/python3 from the top of the source file so rpmlint doesn't think it's a script. It isn't a script anyway, so that line really isn't necessary.
- versioned dependency in subpackages:
- Subpackage doc dependency on main package *removed*, as docs can stand
alone.
- Subpackage devel dependency on main package made arch-specific.
- /usr/share/yosys is architecture-independent:
- Moved /usr/share/yosys to noarch subpackage.
- Main package made dependent on -share subpackage.
The share subpackage is nice! There's a new message from fedora-review with the current spec that says that there is a large amount of data (3143680 bytes) in /usr/share, and it suggests moving that data to a noarch subpackage. Since you now have a -share package, perhaps that would be a nice improvement.
One other message from fedora-review that you might consider fixing - it says that gawk isn't needed in BuildRequires. You can change that or not at your option.
There is one thing you do need to fix for sure:
[!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/doc/yosys randy: You can probably change the doc files section to just have %{_docdir}/%{name}
Once you have that directory ownership fixed let me know!