Bug 486248

This review request is for renaming the old terminus-font package according to the new font policies (bug 480482).

Releng ticket for removal of the old package: https://fedorahosted.org/rel-eng/ticket/1291

*** Bug 480482 has been marked as a duplicate of this bug. ***

Koji scratch build results:

  dist-f10 http://koji.fedoraproject.org/koji/taskinfo?taskID=1137853
  dist-f11 http://koji.fedoraproject.org/koji/taskinfo?taskID=1137855

Let me comment about the rpmlint output (available at
http://ndim.fedorapeople.org/packages/terminus-fonts/4.28-4.fc10/rpmlint.txt):


terminus-fonts.spec:9: E: hardcoded-library-path in /lib/kbd/consolefonts
A library path is hardcoded to one of the following paths: /lib, /usr/lib. It
should be replaced by something like /%{_lib} or %{_libdir}.

terminus-fonts.src:9: E: hardcoded-library-path in /lib/kbd/consolefonts
A library path is hardcoded to one of the following paths: /lib, /usr/lib. It
should be replaced by something like /%{_lib} or %{_libdir}.

The /lib/kbd/consolefonts directory belongs to the kbd package, and the kbd package itself hardcodes it as /lib/..., not /%{_lib} or %{_libdir} or anything with macros. Following rpmlint's suggestions would likely break things.


terminus-fonts.noarch: W: symlink-should-be-relative /etc/fonts/conf.d/63-terminus.conf /usr/share/fontconfig/conf.avail/63-terminus.conf
Absolute symlinks are problematic eg. when working with chroot environments.

terminus-fonts.noarch: W: symlink-should-be-relative /etc/X11/fontpath.d/terminus:unscaled /usr/share/fonts/terminus
Absolute symlinks are problematic eg. when working with chroot environments.

All the other packages putting symlinks into the /etc/X11/fontpath.d directory also use absolute symlinks, so I am following a common practise here.

1. You can safely drop this comment
# Do not trust font metadata versionning unless you've checked upstream does
# update versions on file changes. When in doubt use the timestamp of the most
# recent file as version.

2. Are you sure you want to keep this Provide?
Provides:	terminus-font-x11 = 4.28-2
http://fedoraproject.org/wiki/Shipping_fonts_in_Fedora_%28FAQ%29#Do_I_need_to_Provide_my_old_package_names.3F

3. You can probably use the common_desc trick found in
/etc/rpmdevtools/spectemplate-fonts-multi.spec

to simplify your spec

4. Group:		User Interface/X is a bit ironic for the console package

5. If you only push your package to rawhide (as you should do), I think you can assume filesystem is at the right version
Requires:	filesystem >= 2.4.11-1
http://fedoraproject.org/wiki/Shipping_fonts_in_Fedora_%28FAQ%29#Do_I_need_to_change_my_package_in_stable_releases_in_addition_to_Rawhide.2Fdevel.3F

6. FPC will ask you to use globals instead of defines soonish
http://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define

7. Do you really need to hardcode /etc in the catalog path?

8. It's a bit saner to put Provides/Obsoletes next to other rpm dependency rules (requires/buildrequires)

9. you do not need %dir %{_fontdir} anymore
https://admin.fedoraproject.org/updates/fontpackages-1.20-1.fc10

10. really adding a fontconfig file which is effectively a noop helps no one. Please take a look at the templates in /usr/share/fontconfig/templates/ they're not hard a all to fill

11. the symlinks warning of rpmlint is totally bogus and FRPC already decided to have it nuked
http://fedoraproject.org/wiki/PackagingDrafts/Symlinks

(In reply to comment #6)
> 1. You can safely drop this comment
> # Do not trust font metadata versionning unless you've checked upstream does
> # update versions on file changes. When in doubt use the timestamp of the most
> # recent file as version.

Dropped.

> 2. Are you sure you want to keep this Provide?
> Provides: terminus-font-x11 = 4.28-2

No. Dropped. (Turned out not to be useful after all, I had misread the rpm output.)

> 3. You can probably use the common_desc trick found in
> /etc/rpmdevtools/spectemplate-fonts-multi.spec
> to simplify your spec

Done.

> 4. Group:  User Interface/X is a bit ironic for the console package

It is, but I cannot see any fitting rpm group. "Application/Text" does not work either, because this is a font, not an application processing text. "System Environment/Base" is the group of the kbd package which contains the other console fonts, but terminus-fonts-console is not a required basic part of the system environment either.

So, in the absences of a "Others" group, I don't see a group where this package would really fit into.

Groups are not really used any more anyway, as far as I understand it, so it probably has no consequences anyway. If you want the terminus-fonts-console package in a specific, different group, I'm happy to change it, of course.

> 5. If you only push your package to rawhide (as you should do), I think you can
> assume filesystem is at the right version
> Requires: filesystem >= 2.4.11-1
> http://fedoraproject.org/wiki/Shipping_fonts_in_Fedora_%28FAQ%29#Do_I_need_to_change_my_package_in_stable_releases_in_addition_to_Rawhide.2Fdevel.3F

Dropped. F-10 shipped a newer filesystem package, and that is all I cared about.

> 6. FPC will ask you to use globals instead of defines soonish
> http://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define

Changed.

> 7. Do you really need to hardcode /etc in the catalog path?

No. Changed to %{_sysconfdir}.

> 8. It's a bit saner to put Provides/Obsoletes next to other rpm dependency
> rules (requires/buildrequires)

Done.

> 9. you do not need %dir %{_fontdir} anymore
> https://admin.fedoraproject.org/updates/fontpackages-1.20-1.fc10

Changed: Removed the %dir, and versioned the build requirement for fontpackages-devel.

> 10. really adding a fontconfig file which is effectively a noop helps no one.
> Please take a look at the templates in /usr/share/fontconfig/templates/ they're
> not hard a all to fill

Added a simple fontconfig file now.

http://fedoraproject.org/wiki/Fontconfig_packaging_tips read as if anything going beyond that empty noop fontconfig file was optional, and did not mention those (really useful) templates.

> 11. the symlinks warning of rpmlint is totally bogus and FRPC already decided
> to have it nuked
> http://fedoraproject.org/wiki/PackagingDrafts/Symlinks

Good. I am ignoring those now.


I have uploaded the a version as terminus-fonts-4.28-5.fc10 with all the above changes incorporated:

  http://ndim.fedorapeople.org/packages/terminus-fonts/4.28-5.fc10/terminus-fonts.spec
  http://ndim.fedorapeople.org/packages/terminus-fonts/4.28-5.fc10/terminus-fonts-4.28-5.fc10.src.rpm
  http://ndim.fedorapeople.org/packages/terminus-fonts/4.28-5.fc10/terminus-fonts-fontconfig.conf

  http://ndim.fedorapeople.org/packages/terminus-fonts/4.28-5.fc10/
  http://fedorapeople.org/gitweb?p=ndim/public_git/terminus-fonts-package.git;a=summary

Koji scratch build of terminus-fonts-4.28-5.fc10 for dist-f11:

  http://koji.fedoraproject.org/koji/taskinfo?taskID=1142218

(In reply to comment #8)
> Koji scratch build of terminus-fonts-4.28-5.fc10 for dist-f11:
> 
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=1142218

This failed due to

    No Package Found for fontpackages-devel >= 1.20

However, now that I have removed the %dir %{_fontdir}, I need to require this version, right?

Hmm. The fontpackages-1.20/changelog.txt suggests that requiring 1.18 would be more appropriate, and koji has dist-f11 builds of 1.18 and 1.19, so that should solve it.

I'll be uploading and test building terminus-fonts-4.28-6.fc10 with fontpackages-devel >= 1.18 shortly:

http://ndim.fedorapeople.org/packages/terminus-fonts/4.28-6.fc10/
http://fedorapeople.org/gitweb?p=ndim/public_git/terminus-fonts-package.git;a=summary

Successful dist-f11 terminus-fonts-4.28-6.fc10 koji scratch build:

  http://koji.fedoraproject.org/koji/taskinfo?taskID=1142253

Yes, http://fedoraproject.org/wiki/Fontconfig_packaging_tips should really be revised, but I can't find the time and no one else seems to bother.

Anyway your 4.28-6 seems fine. Thank you for taking the time to fix the problems

ͳͳͳ APPROVED ͳͳͳ

You can now continue from

http://fedoraproject.org/wiki/Shipping_fonts_in_Fedora_%28FAQ%29#What_if_the_new_naming_guidelines_require_me_to_rename_my_source_package.3F

(In reply to comment #11)
> Yes, http://fedoraproject.org/wiki/Fontconfig_packaging_tips should really be
> revised, but I can't find the time and no one else seems to bother.

FWIW, I have just added a short paragraph about the template files in /usr/share/fontconfig/templates/.

> Anyway your 4.28-6 seems fine. Thank you for taking the time to fix the
> problems
> 
> ͳͳͳ APPROVED ͳͳͳ

Thank you!

New Package CVS Request
=======================
Package Name: terminus-fonts
Short Description: Clean fixed width font
Owners: ndim
Branches:
InitialCC:

cvs done.

terminus-fonts has been successfully built in rawhide, so I am closing this as NEXTRELEASE.

Thanks for the help everyone.

Would you consider adding the package to epel7, please?
(See bug 1097424)

terminus-fonts is not included in RHEL 7 (unlike RHEL 6).

Closing the NEEDINFO as epel7 branch already exists.