Spec URL: http://people.redhat.com/mclasen/liberation-fonts.spec SRPM URL: http://people.redhat.com/mclasen/liberation-fonts-0.1-5.fc7.src.rpm Description: The Liberation Fonts are intended to be replacements for the three most commonly used fonts on Microsoft systems: Times New Roman, Arial, and Courier New.
Just FYI, the font config configuration that we are planning for the liberation fonts is to add them to the lists of metrically equivalent fonts in /etc/fonts/conf.d/30-aliases-fedora.conf
Formal review: ☑ MUST: rpmlint must be run on every package. OK $ rpmlint /srv/rpm/liberation-fonts-0.1-5.fc7.nim.* W: liberation-fonts incoherent-version-in-changelog 0.1-5 0.1-5.fc7.nim - Just a dist/changelog clash, ignored ☑ 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 meet the Packaging Guidelines. OK ☑ MUST: The package must be licensed with an open-source compatible license… OK ☒ MUST: The License field in the package spec file must match the actual license. NOK - As the license is not vanilla GPL v2, GPL field is misleading ☒ MUST: If … text of the license(s) for the package must be included in %doc. NOK 1. Don't do that: install -m 0644 License.txt %{buildroot}%{_datadir}/fonts/liberation 2. The license text does not seem UTF-8 encoded ☑ MUST: The spec file must be written in American English. OK ☑ MUST: The spec file for the package MUST be legible. OK ☒ MUST: The sources used to build the package must match the upstream source… NOK 1. Please add a source URL to the package 2. Make sure the archive name and content match the signed archive on the RH page 3. For fonts tar.bz2 is probably a better idea than tar.gz ☑ MUST: The package must successfully compile and build into binary rpms… OK ☑ MUST: If the package does not successfully compile, build or work on an… N/A ☑ MUST: All build dependencies must be listed in BuildRequires… OK ☑ MUST: The spec file MUST handle locales properly… N/A ☑ MUST: Every binary RPM package which stores shared library files… N/A ☑ MUST: If the package is designed to be relocatable… N/A ☑ A package must own all directories that it creates… OK ☑ MUST: A package must not contain any duplicate files in the %files listing. OK ☑ MUST: Permissions on files must be set properly… OK - though a %defattr(0644,root,root,0755) would be nicer ☑ MUST: Each package must have a %clean section… OK - please move the clean section to its usual place after %install ☑ MUST: Each package must consistently use macros… OK - please define and use fontdir and fontconfdir as in other font packages (dejavu-lgc…) ☑ MUST: The package must contain code, or permissable content… OK ☑ MUST: Large documentation files should go in a -doc subpackage… N/A ☑ MUST: If a package includes something as %doc… OK ☑ MUST: Header files must be in a -devel package. N/A ☑ MUST: Static libraries must be in a -static package. N/A ☑ MUST: Packages containing pkgconfig(.pc)… N/A ☑ MUST: If a package contains library files with a suffix… N/A ☑ MUST: In the vast majority of cases, devel packages… N/A ☑ MUST: Packages containing GUI applications… N/A ☑ MUST: Packages must not own files or directories already owned by other packages… OK ☑ MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}… OK ☑ MUST: All filenames in rpm packages must be valid UTF-8. OK ☒ SHOULD: If the source package does not include license text(s) as a separate file from upstream… NOK 1. The license text is partial: it describes the exception but not the main license. A GPL text should be joined to the package ☑ SHOULD: The description and summary sections in the package spec file should contain translations… N/A ☑ SHOULD: The reviewer should test that the package builds in mock. OK ☐ SHOULD: The package should compile and build into binary rpms on all supported architectures. I pass, but that's a noarch package ☑ SHOULD: The reviewer should test that the package functions as described… OK ☒ SHOULD: If scriptlets are used, those scriptlets must be sane. NOK Please use : %post if [ -x %{_bindir}/fc-cache ]; then %{_bindir}/fc-cache %{_datadir}/fonts fi %postun if [ "$1" = "0" ]; then if [ -x %{_bindir}/fc-cache ]; then %{_bindir}/fc-cache %{_datadir}/fonts fi fi like in other font packages (or request them fixed if %{_bindir} is NOK) ☒ SHOULD: Usually, subpackages other than devel… N/A ☒ SHOULD: The placement of pkgconfig(.pc) files depends on their usecase… N/A ☒ SHOULD: If the package has file dependencies outside of /etc, /bin… N/A
Freeform review additions: - please also use -m 0755 at directory install time - it would be nice if the metadata declaration order followed the official Fedora template - please add a FAQ or at least the contact in charge of liberation fonts as the referenced site has limited info and no contact information. In particular everyone involved in FLOSS fonts would like to know the rationale behind GPL choice when the painfully achieved consensus was to go OFL for all projects - please drop a fontconfig configuration file in /etc/fonts/conf.d/ containing at least the "assign generic names" bit of the dejavu-lgc one. After distussion on IRC with behdad the right prio is probably between dejavu-lgc and other fonts (595 unless dejavu-lgc moves to 58) - relying on /etc/fonts/conf.d/30-aliases-fedora.conf means Conflicting with fontconfig packages that do not include liberation info (major PITA). Consider working with behdad to split this file in font-specific ones (/etc/fonts/conf.d/30-001-fedora-helvetica-alias-liberation.conf, /etc/fonts/conf.d/30-002-fedora-helvetica-alias-nimbus.conf etc) so next time a new font package can just drop his own file there instead of relying on a fontconfig update
→ needs more work
2. The license text does not seem UTF-8 encoded This seems a bit picky, considering we are talking about a single copyright sign. 1. Please add a source URL to the package 2. Make sure the archive name and content match the signed archive on the RH page 3. For fonts tar.bz2 is probably a better idea than tar.gz Can't really do that, since the upstream tarball is missing the License.txt file, afaics. I'll see what I can do. gz vs bz2 is an upstream choice and pretty irrelevant to this review. 1. The license text is partial: it describes the exception but not the main license. A GPL text should be joined to the package I'll pass this on. If scriptlets are used, those scriptlets must be sane. NOK The current scriptlets are copied verbatim from the guidelines. If you are not satisfied with them, lobby for a change of the guidelines. - please move the clean section to its usual place after %install - it would be nice if the metadata declaration order followed the official Fedora template Don't include irrelevant nitpicky details here, please. - please add a FAQ or at least the contact in charge of liberation fonts as the referenced site has limited info and no contact information. In particular everyone involved in FLOSS fonts would like to know the rationale behind GPL choice when the painfully achieved consensus was to go OFL for all projects Not a topic for the package review. You already brought this up on the mailing lists, which is a much better forum than this bug. - please drop a fontconfig configuration file in /etc/fonts/conf.d/ containing at least the "assign generic names" bit of the dejavu-lgc one. After distussion on IRC with behdad the right prio is probably between dejavu-lgc and other fonts (595 unless dejavu-lgc moves to 58) Thats possible - relying on /etc/fonts/conf.d/30-aliases-fedora.conf means Conflicting with fontconfig packages that do not include liberation info (major PITA). Why do you think so ? That does not follow at all. Consider working with behdad to split this file in font-specific ones (/etc/fonts/conf.d/30-001-fedora-helvetica-alias-liberation.conf, /etc/fonts/conf.d/30-002-fedora-helvetica-alias-nimbus.conf etc) so next time a new font package can just drop his own file there instead of relying on a fontconfig update Pleaes file a separate fontconfig bug if you think you have a working scheme that is sufficiently better than what we have now.
- please drop a fontconfig configuration file in /etc/fonts/conf.d/ containing at least the "assign generic names" bit of the dejavu-lgc one. After distussion on IRC with behdad the right prio is probably between dejavu-lgc and other fonts (595 unless dejavu-lgc moves to 58) 595 is not going to work though, since it sorts before 59-
(In reply to comment #5) > 2. The license text does not seem UTF-8 encoded > > This seems a bit picky, considering we are talking about a single copyright sign. I reserve the right to be picky for highly visible packages where RH is the upstream :) > 1. Please add a source URL to the package > 2. Make sure the archive name and content match the signed archive on the RH page > > Can't really do that, since the upstream tarball is missing the License.txt > file, afaics. I'll see what I can do. Please do. > 3. For fonts tar.bz2 is probably a better idea than tar.gz > gz vs bz2 is an upstream choice and pretty irrelevant to this review. RH is the upstream there > 1. The license text is partial: it describes the exception but not the main > license. A GPL text should be joined to the package > > > I'll pass this on. > > > If scriptlets are used, those scriptlets must be sane. NOK > > The current scriptlets are copied verbatim from the guidelines. > If you are not satisfied with them, lobby for a change of the > guidelines. OK. It sucks guidelines differ from what our main font packages do in practice > - please move the clean section to its usual place after %install > - it would be nice if the metadata declaration order followed the official > Fedora template > > Don't include irrelevant nitpicky details here, please. I wouldn't mind it that much if people were not likely to take this spac as a template later. But that's up to you. > - please add a FAQ or at least the contact in charge of liberation fonts as the > referenced site has limited info and no contact information. In particular > everyone involved in FLOSS fonts would like to know the rationale behind GPL > choice when the painfully achieved consensus was to go OFL for all projects > > Not a topic for the package review. You already brought this up > on the mailing lists, which is a much better forum than this bug. That's your choice. Right now you're the only identified contact so prepare for direct questions if you release the package as-is. > - relying on /etc/fonts/conf.d/30-aliases-fedora.conf means Conflicting with > fontconfig packages that do not include liberation info (major PITA). > > Why do you think so ? That does not follow at all. The main feature of the packages is fonts that alias ms core fonts. If we allow install on fontconfig-enabled system that do not have this aliasing, users won't get the expected feature (and I'm sure our marketing people will make enough noise users will expect it). Given we do not make font packages depend on fontconfig that's a Conflict > Consider working with behdad to split this file in font-specific ones > (/etc/fonts/conf.d/30-001-fedora-helvetica-alias-liberation.conf, > /etc/fonts/conf.d/30-002-fedora-helvetica-alias-nimbus.conf etc) so next time a > new font package can just drop his own file there instead of relying on a > fontconfig update > > Pleaes file a separate fontconfig bug if you think you have a working scheme > that is sufficiently better than what we have now. already done (bug #239913) but liberation fonts is the first package that clearly needs this (In reply to comment #6) > - please drop a fontconfig configuration file in /etc/fonts/conf.d/ containing > at least the "assign generic names" bit of the dejavu-lgc one. After distussion > on IRC with behdad the right prio is probably between dejavu-lgc and other fonts > (595 unless dejavu-lgc moves to 58) > > 595 is not going to work though, since it sorts before 59- Yes that's a hack and people tend to get their hacks wrong. 605 then (and moving lgc would be much cleaner)
New spec and srpm up at Spec URL: http://people.redhat.com/mclasen/liberation-fonts.spec SRPM URL: http://people.redhat.com/mclasen/liberation-fonts-0.1-6.fc7.src.rpm I ended up using just 59-liberation-fonts.conf, which sorts correctly wrt to 59-dejavu, and avoid any wierd 3digit games. We should just move dejavu to a lower number at some point. The new upstream tarball should hopefully be in place later today or tomorrow.
Upstream tarball is in place now: https://www.redhat.com/f/fonts/liberation-fonts-ttf-2.tar.gz I've updated the packages to point to that url: Spec URL: http://people.redhat.com/mclasen/liberation-fonts.spec SRPM URL: http://people.redhat.com/mclasen/liberation-fonts-0.1-7.fc7.src.rpm
n liberation-fonts-0.1-7.fc7.src.rpm: ☒ MUST: The License field in the package spec file must match the actual license. NOK I still feel declaring GPL when it's GPL v2 + additional terms is not right. I'd like the confirmation of someone closer to FESCO on this before acking ☑ MUST: If … text of the license(s) for the package must be included in %doc OK ☑ MUST: The sources used to build the package must match the upstream source… OK ☑ SHOULD: If the source package does not include license text(s) as a separate file from upstream… OK ☑ SHOULD: If scriptlets are used, those scriptlets must be sane. OK To sum up, I still see two blockers: 1. the license field bit 2. fontconfig coordination. We can't push a font package hipped as core fonts replacement if the associated required fontconfig plumbing is missing. I'm pretty sure we have a "package works as advertised" MUST rule somewhere. Long-term fix is the 30-aliases file split + a rule file provided by this package. Short-term solutions lack appeal. Though since liberation fonts should end up at the top of their respective aliases list dumping a rule file at 29 or 31 may work. I'll settle with seeing a fontconfig package with updated 30-aliases queued for F7. Also I don't like the abandonware feeling of a package with no identified contact on its web site or in %doc. Though the packager and whoever @rh goes to the next text summit is going to take the blame, and I suppose I can let it pass. It's still a bad example to give, especially for internally-produced stuff.
You are not uptodate... [mclasen@localhost ~]$ koji latest-pkg f7-final fontconfig Build Tag Built by ---------------------------------------- -------------------- ---------------- fontconfig-2.4.2-3.fc7 f7-final mclasen [mclasen@localhost ~]$ rpm -q --changelog fontconfig | head -2 * Fri May 11 2007 Matthias Clasen <mclasen> - 2.4.2-3 - Add Liberation fonts to 30-aliases-fedora.conf I have changed the license field to say "GPL + font exception" now. Check it out at the same place, release -8. > Also I don't like the abandonware feeling of a package with no identified > contact on its web site or in %doc. Thankfully package review is not about the dislikes and feelings of the reviewer, but about packaging standards.
Ok, approved
New Package CVS Request ======================= Package Name: liberation-fonts Short Description: Fonts to replace commonly used Microsoft Windows Fonts Owners: mclasen Branches: InitialCC:
Could we have branches for FC6 too ?
Package Change Request ====================== Package Name: liberation-fonts New Branches: FC-6