Bug 180034 - Review Request: perl-Font-TTF (part of the dejavu-fonts toolchain)
Summary: Review Request: perl-Font-TTF (part of the dejavu-fonts toolchain)
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: noarch
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-04 22:21 UTC by Nicolas Mailhot
Modified: 2010-09-10 18:40 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-02-05 18:53:06 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)
perl-Font-TTF spec file (1.66 KB, text/plain)
2006-02-04 22:24 UTC, Nicolas Mailhot
no flags Details
proposed srpm (138.35 KB, application/x-rpm)
2006-02-05 10:44 UTC, Nicolas Mailhot
no flags Details
Cleaned-up spec (1.75 KB, text/plain)
2006-02-05 10:45 UTC, Nicolas Mailhot
no flags Details
spec file take #3 (2.16 KB, text/plain)
2006-02-05 17:40 UTC, Nicolas Mailhot
no flags Details
srpm take #3 (138.48 KB, application/x-rpm)
2006-02-05 17:41 UTC, Nicolas Mailhot
no flags Details

Description Nicolas Mailhot 2006-02-04 22:21:39 UTC
The spec file is really simple so I'm attaching it to the bug
The perl sources can be downloaded at http://search.cpan.org/CPAN/authors/id/M/MH/MHOSKEN/Font-TTF-0.37.tar.gz

Description:

This is a perl module to manipulate TTF files and do all sorts of strange things with them. It's used by the free font project dejavu (http://dejavu.sourceforge.net/wiki/index.php/FontForge)

Since I'm FE's dejavu maintainer, I'd like to get the full dejavu toolchain into FE to package dejavu from sources instead of pushing prebuild TTFs. dejavu is not GPL'd so it's not a requirement. However :

- the spirit of Fedora is to push free software, so IMHO we should follow GPL requirements even if upstream does not force them on us

- Fedora is sorely missing quality fonts and having the full font creation toolset packaged is my little attempt to get more people contributing glyphs to dejavu or starting their own project (when you force font creators to buy expensive closed software tools because the free ones are hard to get you pretty much ensure they'll never contribute to FOSS projects)

- the other big dejavu requirement has been packaged and updated to allow this, and I'd hate this effort to be wasted (bug #170177)

Comment 1 Nicolas Mailhot 2006-02-04 22:24:20 UTC
Created attachment 124187 [details]
perl-Font-TTF spec file

Very simple spec file
My rpmlint has nothing to say about it

Comment 2 Jason Tibbitts 2006-02-05 01:27:51 UTC
Generally you should supply a src.rpm; it makes the review process a bit easier
and some of the review items require having it.

My rpmlint complains:
W: perl-Font-TTF wrong-file-end-of-line-encoding
/usr/share/doc/perl-Font-TTF-0.37/README.TXT

and indeed README.TXT has CRLF endings.  However, there are several packages
which include documentation files with DOS-style line endings so I don't believe
this is a blocker.  Fix it up if you like.

Other issues:
I can't review spec file naming.
I can't review the source used to build the SRPM as none was provided.
BuildRequires: perl is not permitted.
The license does not seem to be GPL.  I see only:

The Perl TTF module is licensed under the Perl Artistic License.

I don't understand this comment in the %files section:

# For arch-specific packages: vendorarch

I can't install the resulting RPM:

error: Failed dependencies:
        perl(Win32) is needed by perl-Font-TTF-0.37-1.noarch
        perl(Win32::Registry) is needed by perl-Font-TTF-0.37-1.noarch

I believe this is the result of lib/Font/TTF/Win32.pm.  I'm not sure what would
be best to do here.  You can fix it with a quick

rm %{buildroot}/%{perl_vendorlib}/Font/TTF/Win32.pm

in the %install section but I'm not completely sure if that's acceptable. 
Another solution would be to postprocess the output of the dependency generator,
but that's rather unpalatable as well (and more complicated than just deleting
the file).

Comment 3 Roozbeh Pournader 2006-02-05 08:42:26 UTC
Nicolas, would you please provide an SRPM?

Comment 4 Nicolas Mailhot 2006-02-05 10:33:08 UTC
(In reply to comment #2)
> Generally you should supply a src.rpm; it makes the review process a bit easier
> and some of the review items require having it.

I know, it's just that my isp changed it's upload rules and I couldn't locate
the new ones yesterday

> My rpmlint complains:
> W: perl-Font-TTF wrong-file-end-of-line-encoding
> /usr/share/doc/perl-Font-TTF-0.37/README.TXT
> 
> and indeed README.TXT has CRLF endings.  However, there are several packages
> which include documentation files with DOS-style line endings so I don't believe
> this is a blocker.  Fix it up if you like.

It's as you want, the rpmlint in rawhide does not care

> Other issues:
> I can't review spec file naming.
> I can't review the source used to build the SRPM as none was provided.

> BuildRequires: perl is not permitted.
This line is straight from the fedora-rpmdevtools-1.4-2.fc5 perl template. Is
the template wrong ?

> The license does not seem to be GPL.  I see only:
> The Perl TTF module is licensed under the Perl Artistic License.

Ok, my mistake, copied the licensing in dries rpm without checking

> I don't understand this comment in the %files section: 
> # For arch-specific packages: vendorarch
This is a bit of fedora-rpmdevtools-1.4-2.fc5 perl template I should have snipped

> I can't install the resulting RPM:
> 
> error: Failed dependencies:
>         perl(Win32) is needed by perl-Font-TTF-0.37-1.noarch
>         perl(Win32::Registry) is needed by perl-Font-TTF-0.37-1.noarch
> 
> I believe this is the result of lib/Font/TTF/Win32.pm.  I'm not sure what would
> be best to do here.  You can fix it with a quick
> 
> rm %{buildroot}/%{perl_vendorlib}/Font/TTF/Win32.pm
>
> in the %install section but I'm not completely sure if that's acceptable. 
I agree on this, will do

> Another solution would be to postprocess the output of the dependency generator,
> but that's rather unpalatable as well (and more complicated than just deleting
> the file).



Comment 5 Nicolas Mailhot 2006-02-05 10:42:29 UTC
(In reply to comment #3)
> Nicolas, would you please provide an SRPM?

Since the SRPM is only 148 kiB I'll attach it too


Comment 6 Nicolas Mailhot 2006-02-05 10:44:29 UTC
Created attachment 124197 [details]
proposed srpm

Comment 7 Nicolas Mailhot 2006-02-05 10:45:16 UTC
Created attachment 124198 [details]
Cleaned-up spec

Comment 8 Ville Skyttä 2006-02-05 11:14:18 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > BuildRequires: perl is not permitted.
> This line is straight from the fedora-rpmdevtools-1.4-2.fc5 perl template. Is
> the template wrong ?

Mileages vary.  IMO not and rather the MUST in the guidelines is a bit over the
top in this case.  See bug 179426.


Comment 9 Jason Tibbitts 2006-02-05 15:26:10 UTC
I agree about the BuildRequires: perl thing, and indeed in another review I said
it was not a blocker (which was my mistake) but then I noticed the MUST.  I'm
not sure what to do here; I think the MUST is unnecessary and conflicts with
language in the the packaging guidelines:

"There is no need to include the following packages or their dependencies as
BuildRequires because they would occur too often."

which doesn't sound very MUST like.

We can't just ignore the guidelines, so I suggest removing the BuildRequires:
while this gets worked out on the mailing list.

Also, could you comment the %exclude you added, so it's obvious why this is
required.

I'll finish off the review in a few minutes.

Comment 10 Nicolas Mailhot 2006-02-05 17:33:01 UTC
(In reply to comment #9)
> We can't just ignore the guidelines, so I suggest removing the BuildRequires:
> while this gets worked out on the mailing list.

I strongly suspect most FE packages follow the official templates, so leaving it
might be the more conservative option IMHO. But I'll follow the reviewer position ;)

> Also, could you comment the %exclude you added, so it's obvious why this is
> required.
> 
> I'll finish off the review in a few minutes.

Ok, new spec/srpm in 30s


Comment 11 Nicolas Mailhot 2006-02-05 17:40:49 UTC
Created attachment 124215 [details]
spec file take #3

Comment 12 Nicolas Mailhot 2006-02-05 17:41:38 UTC
Created attachment 124216 [details]
srpm take #3

Comment 13 Jason Tibbitts 2006-02-05 18:09:13 UTC
Cool.  OK, everything's looking good:

No rpmlint blockers, just the end-of-line warning.
Package meets naming and packaging guidelines.
License is acceptable and matches License: tag.
Specfile is properly named, legible, well-written, well-commented and uses
macros consistently.
Source file matches upstream.
Package builds and installs on FC3 and FC4.
BuildRequires: is proper.

Approved.

Comment 14 Mark Chappell 2010-09-10 18:36:35 UTC
Package Change Request
======================
Package Name: perl-Font-TTF
New Branches: EL-5 EL-6
Owners: tremble

nim listed on 
https://fedoraproject.org/wiki/EPEL/ContributorStatusNo

Comment 15 Kevin Fenzi 2010-09-10 18:40:17 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.