Bug 565830

Summary: Review Request: paktype-naskh-basic-fonts - Fonts for Arabic from PakType
Product: [Fedora] Fedora Reporter: Naveen Kumar <nkumar>
Component: Package ReviewAssignee: Naveen Kumar <nkumar>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, nicolas.mailhot, nkumar, notting, panemade, petersen, psatpute
Target Milestone: ---Flags: psatpute: fedora‑review+
tcallawa: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-04-08 02:22:30 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 182235    

Description Naveen Kumar 2010-02-16 08:22:01 EST
Spec URL: http://pravins.fedorapeople.org/paktype-nashk-basic-fonts.spec
SRPM URL: http://pravins.fedorapeople.org/paktype-nashk-basic-fonts-3.0-1.fc12.src.rpm
Description: 
Hi! I have just finished packaging paktype-nashk-basic-fonts, and I would appreciate a review so that I can get into fedora (FE-NEEDSPONSOR). Some of the informal reviews that I have done till now:
1. https://bugzilla.redhat.com/show_bug.cgi?id=559936
2. https://bugzilla.redhat.com/show_bug.cgi?id=561270
3. https://bugzilla.redhat.com/show_bug.cgi?id=561271
4. https://bugzilla.redhat.com/show_bug.cgi?id=561289
5. https://bugzilla.redhat.com/show_bug.cgi?id=561225

I have packaged one package, which may take some time to get reviewed properly: https://bugzilla.redhat.com/show_bug.cgi?id=563444

Koji Scratch Build for paktype-nashk-basic-fonts: http://koji.fedoraproject.org/koji/taskinfo?taskID=1990670
Comment 1 Nicolas Mailhot 2010-02-22 10:23:49 EST
This could probably be a nice package, unfortunately it does not pass legal checks

The fonts are derived from GPL sources and DejaVu/Vera. GPL and Vera License do not mix (the Vera license restricts the font naming, and forbids standalone selling, and the GPL forbids additional restrictions)

Therefore, those fonts can not be shipped in Fedora, unless Bitstream agrees to relicence Vera under a GPL variant, or KACST/URW agree to relicence under the Vera license (or they all agree to relicence under some other license). Short term the best solution for upstream is to remove the vera bits from the mix.

This is all very sad, and the reason why we ask new font projects to release under GPL + FE or OFL, and stop inventing new licenses (granted, OFL has been largely inspired by the Vera license, so Vera was a huge leap forward, even though it can not mix with GPL fonts)

I'll be happy to re-review this package is the licensing problems are lifted, or any other font package you may wish to submit. It looks like you invested a lot of work in this package. Unfortunately for fonts checking legal is a good idea before looking at technical issues :(

Please close this bug if you think the legal issues can not be lifted. NEEDINFO in the meanwhile.
Comment 3 Naveen Kumar 2010-03-05 01:57:23 EST
updated Spec file and SRPM.

Koji Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2032277
Comment 4 Pravin Satpute 2010-03-05 03:51:26 EST
just some comments on spec file

1) use complete link wget link for source0, no need to create macro for only line
2) dont repeat 

ln -s %{_fontconfig_templatedir}/%{fontconf}-sa.conf \
      %{buildroot}%{_fontconfig_confdir}/%{fontconf}-sa.conf

use for loop for it as per /etc/rpmdevtools/spectemplate-fonts-multi.spec
3) 

for txt in License.txt; do
   fold -s $txt > $txt.new
   sed -i 's/\r//' $txt.new
   touch -r $txt $txt.new
   mv $txt.new $txt
done

no need of this only 
%{__sed} -i 's/\r//' License.txt working fine

4) # get rid of the white space (' ')

for PakType Naskh Basic Comparison Chart.pdf
PakType\ Naskh\ Basic\ Comparison\ Chart.htm

may be add _ in place of 'space'

5) i think no need to change name of upstream file, just add _ instead of "space" PakType\ Naskh\ Basic\ Comparison\ Chart.htm


6) from Readme sed
remove 
   sed -i 's/\x95//g' $txt.new
as it doesnt have x95 character

7)no need of $cd .. much
i think its good to do things from $pwd

8) add .conf file for PakTypeNaskhBasic.ttf as well

follow /usr/share/fontconfig/templates/basic-font-template.conf

9) i think %_font_pkg PakTypeNaskhBasic.ttf, should move just before its package.

10) remove unnecessary space from 
sindhi.conf
    <string>PakType Nashk Basic Sindhi </string>

11) please add %doc under common files, presently no rpm is getting generated.

%files common
%defattr(-,root,root,-)
%doc ....
%dir %{fontdir} ...
Comment 6 Naveen Kumar 2010-03-05 04:36:55 EST
Please ignore above scratch build
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2032496
Comment 7 Pravin Satpute 2010-03-08 05:08:38 EST
- Remove  BuildRequires: fontforge >= 20080429, as we are not generating ttf from .sfd

- also remove Requires:       fontpackages-filesystem from base package, since common package is already pulling that 

- as we are using common desc. for all packages it should be little bit more generic or please append little bit more in description of each package

- Remove
rmdir NaskhBasic-3.0;
rm -r Ready\ to\ use\ fonts/
rm -r License\ files/
rm -r NaskhBasic-3.0/Project\ files/
no need to do this


- mv PakType\ Naskh\ Basic\ License.txt License.txt
no need to rename this file, keep original name just add "_" instead of " "

- .conf file
in .ttf file, style is written as Decorative, please confirm it, what we suppose to write in that case in .conf file
Comment 8 Naveen Kumar 2010-03-09 05:12:20 EST
(In reply to comment #7)

> - .conf file
> in .ttf file, style is written as Decorative, please confirm it, what we
> suppose to write in that case in .conf file    

I do not much about this. For the time being it is written as sans-serif.


Updated Spec & SRPM:

SPEC: http://nkumar.fedorapeople.org/paktype-nashk-basic-fonts/paktype-nashk-basic-fonts-3.0-4/paktype-nashk-basic-fonts.spec

SRPM: http://nkumar.fedorapeople.org/paktype-nashk-basic-fonts/paktype-nashk-basic-fonts-3.0-4/paktype-nashk-basic-fonts-3.0-4.fc12.src.rpm

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2040498
Comment 9 Pravin Satpute 2010-03-11 23:55:47 EST
- package name should be paktype-naskh-fonts not nashk
Comment 11 Pravin Satpute 2010-03-12 03:07:08 EST
- write appropriate summary in each subpackage
This is wrong --> Summary: Tehreer Fonts for Arabic from PakType
- improve common_desc or append something in each subpackage's description
Comment 12 Naveen Kumar 2010-03-12 05:33:17 EST
(In reply to comment #11)
> - write appropriate summary in each subpackage
> This is wrong --> Summary: Tehreer Fonts for Arabic from PakType
> - improve common_desc or append something in each subpackage's description    

Updated Spec & SRPM:

SPEC: http://nkumar.fedorapeople.org/paktype-naskh-basic-fonts/paktype-naskh-basic-fonts-3.0-6.fc12/paktype-naskh-basic-fonts.spec

SRPM: http://nkumar.fedorapeople.org/paktype-naskh-basic-fonts/paktype-naskh-basic-fonts-3.0-6.fc12/paktype-naskh-basic-fonts-3.0-6.fc12.src.rpm

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2048458
Comment 13 Pravin Satpute 2010-03-12 05:50:16 EST
+ package builds in mock (rawhide i686).
koji Build => http://koji.fedoraproject.org/koji/taskinfo?taskID=2048458
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream url 
407b095967ceecc4a342355295dd8c83  NaskhBasic-3.0.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
combination of "GPLv2 with Font Exceptions" and "GPLVv2+ with Font Exceptions"
so "GPLv2 with Font Exceptions" is proper license
+ License text is included in package.
+ %doc is present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage
+ no .la files.
+ no translations are available
+ Does owns the directories it creates.
+ fonts scriptlets present.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Not a GUI application

install -m 0644 -p PakTypeNaskhBasicFarsi.ttf PakTypeNaskhBasic.ttf PakTypeNaskhBasicSA.ttf PakTypeNaskhBasicUrdu.ttf PakTypeNaskhBasicSindhi.ttf $RPM_BUILD_ROOT%{_fontdir}, do check is *.ttf works for it before import.
Comment 14 Naveen Kumar 2010-03-12 08:06:06 EST
New Package CVS Request
=======================
Package Name: paktype-naskh-basic-fonts
Short Description: Fonts for Arabic, Farsi, Urdu and Sindhi from PakType
Owners: nkumar
Branches: F-12 F-13
InitialCC: fonts-sig
Comment 15 Tom "spot" Callaway 2010-03-15 17:41:35 EDT
CVS done (by process-cvs-requests.py).
Comment 16 Naveen Kumar 2010-04-09 06:39:23 EDT
Thanks to Tom "spot" Callaway, Pravin Satpute & Nicolas Mailhot. You all have been great and supportive...:)
Comment 17 Naveen Kumar 2010-04-09 06:40:06 EDT
Tom "spot" Callaway! Thanks for the CVS.