https://bugzilla.redhat.com/show_bug.cgi?id=1332764
--- Comment #2 from Jerry James loganjerry@gmail.com --- Thank you for the review.
(In reply to James Hogarth from comment #1)
- License not included in tarball
- Checking the tarball reveals licensing details in factint.tex
- Please request upstream issue an update with a specific license file
in the tarball
Okay, will do.
- The PackageInfo specifies GAPDoc as required
- GAPDoc-latex a BR but no GAPDoc as a require
Same as the gap-pkg-scscp review: GAPDoc is so fundamental that gap-core pulls it in, else GAP refuses to start. The GAPDoc-latex BR pulls in all of the LaTeX packages needed to actually build the documentation.
- Some non utf-8 files found, please include these in your iconv in %prep
- Since you do use iconv please include it in your BR to prevent issues
should it be dropped from the generic build environment in future.
Actually, I believe this is not the right thing to do. The first line of doc/factint.xml is:
<?xml version="1.0" encoding="ISO-8859-1"?>
So the encoding is specified. If you look at the build log for this package, you will see a line that looks like this:
#I #I recoding input from ISO-8859-1 to UTF-8 . . .
That line comes from GAPDoc (/usr/lib/gap/pkg/GAPDoc/lib/XMLParser.gi). GAPDoc has its own builtin encoding converter, so no need for iconv. The end result is HTML pages that are UTF-8 encoded, but we keep the source xml files around because GAPDoc wants them for some functions it provides.