Bug 487913

darn: ... and bug 173897 comment 6 ...

First review pass (builds in mock, good)

1. various rpmlint warnings, of which only the following need to be fixed
culmus-fancy-fonts.src: E: invalid-spec-name

culmus-fancy-fonts.src:84: E: files-attr-not-set
A file or a directory entry in a %files section does not have attributes set
which may result in security issues in the resulting binary package depending
on the system where the package is built.  Add default attributes using
%defattr before it in the %files section, or use per line %attr's.

culmus-fancy-fonts.src:157: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

culmus-fancy-fonts.src:158: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

culmus-fancy-fonts.src:159: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

culmus-fancy-fonts.src:160: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

culmus-fancy-fonts.src:161: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

culmus-fancy-fonts.src:162: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

culmus-fancy-fonts.src:163: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

culmus-fancy-fonts.src: W: no-%build-section
The spec file does not contain a %build section.  Even if some packages don't
directly need it, section markers may be overridden in rpm's configuration to
provide additional "under the hood" functionality, such as injection of
automatic -debuginfo subpackages.  Add the section, even if empty.

fonts-hebrew-fancy-compat.noarch: W: summary-ended-with-dot Compatibility files of Culmus fancy font families.
Summary ends with a dot.

2. please use OTF over Type1 whenever possible

3. 
 a. we prefer for fonts released in different archives to be packaged separately (different rpms and srpms).
 b. also, I don't think Legal would appreciate the way you drop every licensing file but one.
 c. lastly, the different fonts actually have different timestamps so your version is misleading

However there is a tolerance for fonts that used to be packaged in a single srpm so you may avail yourself of it if you really want to. Still, I don't think that's a good idea. 7 simple packages can be easier to manage than a monster one (and are actually quicker to review)

4. It would be a good idea to contact upstream and make it add the FSF font exception to their licensing
http://fedoraproject.org/wiki/Legal_considerations_for_fonts#Good_font_licenses_allow_embedding

5. the rpm in rawhide allows you to drop the duplicate Group declarations in subpackages

6. FPC and FESCO have decided %define-s should be replaced by %global-s (cf fontpackages-devel 1.20)

7. You do not need this
Obsoletes:         %{fontname}-fonts-common < %{version}-%{release}
Obsoletes:         %{fontname}-comix-no2-fonts < %{version}-%{release}
Obsoletes:         %{fontname}-dorian-fonts < %{version}-%{release}
Obsoletes:         %{fontname}-gan-fonts < %{version}-%{release}
Obsoletes:         %{fontname}-gladia-fonts < %{version}-%{release}
Obsoletes:         %{fontname}-ktav-yad-fonts < %{version}-%{release}
Obsoletes:         %{fontname}-ozrad-fonts < %{version}-%{release}
Obsoletes:         %{fontname}-anka-fonts < %{version}-%{release}

8. You should not need this
Provides:          fonts-hebrew-fancy = %{version}-%{release}

9. Please only obsolete the last version of fonts-hebrew-fancy  built in koji + 1
https://fedoraproject.org/wiki/Upgrade_paths_%E2%80%94_renaming_or_splitting_packages

10. Your compat subpackage need not require common, it'll be pulled in by the others

11. You're supposed to add something specific to each font subpackage under  %common_desc references. Likewise your summaries are all identical, that's not user-friendly

12. You do not need the 
%dir %{_fontdir}
line anymore

13. since your fontconfig files define substitution rules, it's probably a good idea to use
/usr/share/fontconfig/templates/substitution-font-template.conf
as template

14. the spec is slightly easier to review if you keep the same line order as the template so diffs are minimal

⇒ need a little more work

thanks for the review. due to the not-so-little more work required, my other obligations, and the marginal importance of this package, I am afraid that I won't be able to fix it, certainly not on time for F11. I guess not many people depend on the package, so Fedora will survive without it for a while.

(of course if someone else around the 'net agrees to take ownership, be my guest)

Created attachment 367518 [details]
spec with fixes to several of the comments

It might be just about at the packaging standard, but certainly has several of the comments unfixed.

(In reply to comment #4)
> Created an attachment (id=367518) [details]
> spec with fixes to sever of the comments
> 
> It might be just about at the packaging standard, but certainly has several of
> the comments unfixed.  

Can you post a srpm please? It's a lot easier to review when you can build the package

Created attachment 367959 [details]
srpm with fixes to several of the problems.

This srpm does not build at all.

Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.vWyOk7
+ umask 022
+ cd /builddir/build/BUILD
+ LANG=C
+ export LANG
+ unset DISPLAY
+ cd /builddir/build/BUILD
+ rm -rf culmus-fancy-0.20051122
+ /usr/bin/gzip -dc /builddir/build/SOURCES/comix.tar.gz
+ /bin/tar -xf -
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd culmus-fancy-0.20051122
/var/tmp/rpm-tmp.vWyOk7: line 34: cd: culmus-fancy-0.20051122: No such file or directory
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.vWyOk7 (%prep)
    Bad exit status from /var/tmp/rpm-tmp.vWyOk7 (%prep)
Child returncode was: 1
EXCEPTION: Command failed. See logs for output.

It is not easy to write a multi-source srpm. It is definitely not recommended to try when you can go without.

Since in your case, upstream already publishes each font set in a separate tar.gz, why don't you just do one simple mono-source srpm per tar.gz, instead of trying to force them all to collaborate in a single monster srpm ?

Created attachment 374387 [details]
srpm with fixes to several of the problems

you are perfectly correct. I should not waste any more of your or anybody else's time about this. I just wanted to leave a useful rpm behind, but uploaded a broken old attempt instead. thanks for your help.

Please calm down, I couldn't guess you already went through the multi-source spec hackery in an unpublished srpm.

Given you did it this way I'll review the result. I still think it would have been faster to drop the multi-sourcing and propose several srpms, but there is no need to go back now if the result works

Anyway, review check:

1. (informative) I'm afraid you're going to receive "bad font naming" warnings for Comix regularly. The current font naming is acceptable, however some of the very aggressive font name munging we have to perform because of the brokeness in other Fedora fonts seems to result in a spurious warning in your case (and I don't see how to avoid this without renouncing to detect the other broken fonts)

2. (not blocking) fontlint does not like anka, please relay upstream

3. (not blocking) you do not need to repeat "Group:          User Interface/X" in Fedora releases that use a recent rpm

4. (not blocking) you do not need to require the common subpackage in your compat subpackage

5. (blocking) you don't want to obsolete fonts-hebrew-fancy < 0.20051122-7, you want to obsolete fonts-hebrew-fancy < 0.20051122-9 (assuming you'll never build anything newer as fonts-hebrew-fancy)

6. (not blocking) I'm sure users would appreciate descriptions that tell them the difference between each of those subpackages

7. (not blocking) you don't need the "%dir %{_fontdir}" in common, it will be auto-added to every font subpackage

8. (not blocking) your fontconfig rules are incomplete, you need to use the complete substitution pattern documented in /usr/share/fontconfig/templates/substitution-font-template-*

9. (not blocking) the way you package the licensing file is dangerous, if upstream ever changes the licensing file in one package but not the others you'll end up shipping the wrong legal info

10. (not blocking) some of the fonts have broken encoding in their copyright info when opened by gnome-font-viewer (no idea which one of gnome-font-viewer or the fonts is broken)

11. (not blocking) it would be nice if upstream's license included the FSF font exception that make a GPL font safe to use in a PDF

(Looking at all those clm fonts I realise we should have used clm and not culmus as foundry prefix in the naming of culmus font rpms. Bah, not worth fixing it now)

Anyway, apart from the broken obsolete, everything else can be fixed over time after Fedora import. Please fix the obsolete so I can approve this package

Hi, I'm curious what has become of this issue. The "culmus fancy" fonts (see http://culmus.sourceforge.net/fancy/index.html) are Hebrew fonts, that are fancy in the sense that they are designed for special uses (headings, signs, etc.) more than general printed text, but are nevertheless very useful for Hebrew writers, and Fedora should have them. My favorite Culmus font, for example, which I use is the "Culmus Gan" font, which is part of Culmus's "fancy fonts", not the Culmus core fonts.

Unfortunately, it appears that since Fedora 11 (which had a single "fonts-hebrew-fancy" package with all the Culmus fancy fonts), these fonts are no longer available in Fedora, which is a shame, as these are very nice and useful free fonts.

In recent Fedora releases, the large many-font packages were replaced by small single-font packages, and we have now over a dozen culmus-* packages, each with a single font. It should be relatively straightforward to add more of these packages, a package for each Culmus fancy font? I think this would be better, and more fitting current Fedora organization, to have a separate package for each font, and not a single package for all Clumus Fancy fonts like we had before Fedora 11.

Who is the current maintainer of the culmus-* packages? Can I be of any help?

Hi, Nadav. The non-fancy culmus-fonts are maintained by pravins (see https://admin.fedoraproject.org/pkgdb/acls/name/culmus-fonts ).
 
I've lost interest in maintaining the fancy fonts rpm - I had the feeling I'm their only user, and was probably not too far away.

But you or anyone else are welcome to take over, open a review request ticket for each upstream font, and push them to Fedora. I promise to use the rpms if you do this.

I don't see myself "taking over" these fonts, as I have hardly any idea how to create an RPM :( Is there any reason why Pravins would find it hard to package Culmus Gan (for example) in addition to the dozen Culmus fonts he's already packaging? I don't know if he's reading this... Should I open an issue on the "culmus-fonts" package (and ask that this issue be closed as WONTFIX)?

(In reply to comment #13)
> I don't see myself "taking over" these fonts, as I have hardly any idea how to
> create an RPM :( 

There is little technical difficulty in creating/maintaining a font package in Fedora nowadays, the process is documented and the guidelines clear (I think). Do take a look at them they're not complex

However, it does require some time investment from the packager, and this is why packages like culmus languish, if their packager is busy elsewhere

Closing per #12. Feel free to reopen if you want to continue.