Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
Summary: Merge Review: ghostscript-fonts Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: twaugh@redhat.com
Fedora Merge Review: ghostscript-fonts
http://cvs.fedora.redhat.com/viewcvs/devel/ghostscript-fonts/ Initial Owner: twaugh@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |twaugh@redhat.com CC| |roozbeh@farsiweb.info Flag| |fedora-review-
------- Additional Comments From roozbeh@farsiweb.info 2007-02-03 10:42 EST ------- Random notes: * URL field points to an empty page. Should perhaps be changed to http://www.gnu.org/software/ghostscript/ghostscript.html * New upstream version (6.0) is available from http://ftp.gnu.org/gnu/ghostscript/ * BuildRoot should be changed to %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * The "Requires:" on ghostscript is probably unnecessary. * May need to add some requirements for post and postun scripts.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
twaugh@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flag|fedora-review- |fedora-review?
------- Additional Comments From twaugh@redhat.com 2007-02-06 11:28 EST ------- Thanks!
- New upstream version (6.0) is available from http://ftp.gnu.org/gnu/ghostscript/
6.0 is an older release (see the timestamp).
New package built: 5.50-15.fc7
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|twaugh@redhat.com |roozbeh@farsiweb.info
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|roozbeh@farsiweb.info |twaugh@redhat.com Flag|fedora-review? |fedora-review-
------- Additional Comments From roozbeh@farsiweb.info 2007-02-07 09:13 EST ------- (In reply to comment #2)
6.0 is an older release (see the timestamp).
Worst than that, they are exactly the same thing. The bits are exactly the same! I would still recommend using 6.0, so people won't think we are not using the latest version in Fedora.
More random notes: * Your new URL is still not good. The tarball is not provided from the sourceforge servers, and not any real info either. It's a dead project. It just says go to GNU for more info. Use either http://savannah.gnu.org/projects/ghostscript/ or http://www.gnu.org/software/ghostscript/. The second is preferred, as its more user oriented. (BLOCKER)
* The URL in the Source line does not work anymore either. Use http://ftp.gnu.org/gnu/ghostscript/gnu-gs-fonts-std-%%7Bversion%7D.tar.gz
* You should change the "Requires: fontconfig" line to different lines for requirements after installation and uninstallation. Currently, one can ask rpm/yum to remove both fontconfig and ghostscript-fonts and fontconfig may get removed before ghostscript-fonts, making the post uninstallation scripts fail. A similar scenario can happen with installation. Also, you need to have mkfontscale, mkfontdir, and chkfontpath during some of these. (BLOCKER)
Suggested lines: Requires: fontconfig Requires(post): /usr/bin/mkfontscale /usr/bin/mkfontdir Requires(post): /usr/sbin/chkfontpath Requires(post): fontconfig Requires(postun): /usr/sbin/chkfontpath Requires(postun): fontconfig
* Copy the files during the %install section using the '-p' option of cp (or use install -p).
* Have an empty %build section.
* Use %{_datadir} instead of /usr/share in the %files section. Also consider adding a "/" at the end to show that we are actually including a directory and files in there.
* There is nothing in the source tarball that says the license of the files is GPL, as the license field of the spec file says. They may as well be proprietary software, as far as a random observer can tell. Since contacting upstream for including a license may not be trivial, the spec file should at least document why we have made sure this is licensed under the GPL. (BLOCKER)
* The summary ends with a dot. It shouldn't.
* I don't think the use of parenthesized "(TM)" is really necessary in the Summary line. The Fedora EULA already says that all trademarks are owned by their respective owners.
* The part of the description that says you'll need to install this for ghostscript to work is not that important to be worth a mention. That is simply a Requires line in the ghostscript package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
twaugh@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|twaugh@redhat.com |roozbeh@farsiweb.info Flag|fedora-review- |fedora-review?
------- Additional Comments From twaugh@redhat.com 2007-02-07 12:31 EST ------- Tagged and built as 5.50-16.fc7.
I changed the License tag to say 'Distributable'.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
------- Additional Comments From apodtele@ucsd.edu 2007-02-09 10:22 EST ------- Still confused. What about 8.11 with much better international support? Current offering pales in comparison. I don't know what was the reason to "upgrade" to old tiny set.
ftp://mirror.cs.wisc.edu/pub/mirrors/ghost/fonts/ghostscript-fonts-std-8.11.tar.gz
This has been raised before in bug 203369 and bug 113866. These are GPL. We are not in freeze yet. Why would you refuse them now?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|roozbeh@farsiweb.info |nobody@fedoraproject.org Flag|fedora-review? |
------- Additional Comments From roozbeh@farsiweb.info 2007-02-09 10:47 EST ------- Leaving the rest of the review to other people, as I can't say I understand all the story of the different versions and licenses.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ghostscript-fonts
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794
------- Additional Comments From apodtele@ucsd.edu 2007-02-10 14:55 EST ------- Tim just clarified [see bug 203369] that this package only contains other, miscellaneous, or additional (whatever...) fonts for ghostscript. The more important fonts for ghostscript come from urw-fonts and are shared with X.org. This is cool!
Description does mention this but can be more upfront about this in the first sentence. Maybe, the packages should be more appropriately renamed to ghostscript-fonts-other?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jussi.lehtola@iki.fi AssignedTo|nobody@fedoraproject.org |jussi.lehtola@iki.fi Flag| |fedora-review?
--- Comment #8 from Jussi Lehtola jussi.lehtola@iki.fi 2009-05-21 07:10:20 EDT --- Assigning.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
--- Comment #9 from Jussi Lehtola jussi.lehtola@iki.fi 2009-05-21 07:35:26 EDT --- - This is a legacy font package, so Font guidelines are not valid here.
- Change Requires(post): /usr/bin/mkfontscale /usr/bin/mkfontdir to Requires(post): xorg-x11-font-utils as this is the package that provides those since Fedora Core 2 (2004).
- I'm not totally sure you need Requires(post): fontconfig Requires(postun): fontconfig as you already have Requires: fontconfig. Besides, this is probably automatically picked up by rpm. Doesn't hurt having them, though.
- Change references to /etc to %{_sysconfdir}
- Setting umask is probably not necessary as this is done by rpm.
- Replace `which mkfontdir` with plain mkfontdir.
- Drop %dir %{catalogue} in %files section, as $ rpm -qf /etc/X11/fontpath.d/ filesystem-2.4.19-1.fc10.i386 is already owned on every installation.
- I can't find a single mention of a license in the tarball!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
--- Comment #10 from Tim Waugh twaugh@redhat.com 2009-06-10 10:00:33 EDT --- (In reply to comment #9)
- Change
Requires(post): /usr/bin/mkfontscale /usr/bin/mkfontdir to Requires(post): xorg-x11-font-utils as this is the package that provides those since Fedora Core 2 (2004).
Done.
- I'm not totally sure you need
Requires(post): fontconfig Requires(postun): fontconfig as you already have Requires: fontconfig. Besides, this is probably automatically picked up by rpm. Doesn't hurt having them, though.
Left alone.
- Change references to /etc to %{_sysconfdir}
Done.
- Setting umask is probably not necessary as this is done by rpm.
- Replace `which mkfontdir` with plain mkfontdir.
Not sure about these. The scriptlets were copied from urw-fonts, so if they need fixing they ought to be fixed in both places.
- Drop
%dir %{catalogue} in %files section, as $ rpm -qf /etc/X11/fontpath.d/ filesystem-2.4.19-1.fc10.i386 is already owned on every installation.
Done.
- I can't find a single mention of a license in the tarball!
No, *sigh*. Tom Callaway looked into the licenses in July last year, and there haven't been any developments since then.
5.50-21.fc12 built.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
--- Comment #11 from Jussi Lehtola jussi.lehtola@iki.fi 2009-06-10 10:10:53 EDT --- (In reply to comment #10)
- Setting umask is probably not necessary as this is done by rpm.
- Replace `which mkfontdir` with plain mkfontdir.
Not sure about these. The scriptlets were copied from urw-fonts, so if they need fixing they ought to be fixed in both places.
urw-fonts hasn't been through a merge review, so you really can't trust that the packaging is sane on all accounts.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
Tim Waugh twaugh@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
--- Comment #12 from Tim Waugh twaugh@redhat.com 2009-06-10 10:22:48 EDT --- OK, 5.50-22.fc12 built with all suggestions applied. Thanks!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Reopened Status|CLOSED |ASSIGNED Resolution|RAWHIDE |
--- Comment #13 from Jussi Lehtola jussi.lehtola@iki.fi 2009-06-10 10:50:34 EDT --- Uh, not so fast, I'll still do the review in full :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE Flag|fedora-review? |fedora-review+
--- Comment #14 from Jussi Lehtola jussi.lehtola@iki.fi 2009-06-10 11:07:16 EDT --- rpmlint output is clean.
MUST: The spec file for the package is legible and macros are used consistently. OK MUST: The package must be named according to the Package Naming Guidelines. OK MUST: The spec file name must match the base package %{name}. OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. ? - No license information available.
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK MUST: The package MUST successfully compile and build into binary rpms. OK MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. OK MUST: A package must own all directories that it creates or require the package that owns the directory. OK MUST: Files only listed once in %files listings. OK MUST: Debuginfo package is complete. N/A MUST: Permissions on files must be set properly. OK MUST: Clean section exists. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK MUST: Desktop files are installed properly. N/A MUST: No file conflicts with other packages and no general names. OK MUST: Buildroot cleaned before install. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK SHOULD: The package builds in mock. OK
I trust spot's legal opinion in the license issue, so the package is
APPROVED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225794
--- Comment #15 from Tim Waugh twaugh@redhat.com 2009-06-10 11:14:53 EDT --- Thanks!
package-review@lists.fedoraproject.org