Bug 1162148 - Review Request: labiryntowy-fonts - Conscript (artifical font) letters consist of vertical and horizontal bars. No matter their thickness.
Summary: Review Request: labiryntowy-fonts - Conscript (artifical font) letters consis...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 20
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-11-10 11:40 UTC by marwin
Modified: 2014-12-06 09:59 UTC (History)
7 users (show)

Fixed In Version: labiryntowy-fonts-1.53-2.fc21
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-12-01 18:54:27 UTC
Type: ---
Embargoed:
lkundrak: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description marwin 2014-11-10 11:40:35 UTC
Spec URL: https://github.com/marwino/labiryntowe/blob/master/labiryntowy-fonts.spec
SRPM URL: https://github.com/marwino/labiryntowe/blob/master/labiryntowy-fonts-1.541-1.fc20.src.rpm
Description: Conscript (artifical font) letters consist of vertical and horizontal bars. No matter their thickness.
Fedora Account System Username: marwin

Comment 1 Lubomir Rintel 2014-11-11 10:55:34 UTC
0.) Please remove these, they're useless:

-%global shortname flabi
-# wersja 1.1

1.) The summary is too long and complicated. rpmlint complains:

-Summary:       Conscript (artifical font) letters consist of vertical and horizontal bars. No matter their thickness.
+Summary:       Artificial font consisting of vertical and horizontal bars

2.) An extra and useless argument:

-%setup -q -c %{fontname}-%{version}-ttf
+%setup -q -c

3.) Useless comments with macros in them (rpmlint complains)

-# This was wrong:
-# mkdir -p %{buildroot}-%{shortname}
-# cp -p *.ttf %{buildroot}-%{shortname}
-# _fontdir expands to /usr/share/fonts/labirintowy
...
-# This generates the files section:
...
-# This is useless -- it gets generated with _font_pkg too:
-# %files %{buildroot}-%{shortname}/*.ttf

4.) Please use a more reasonable name & address in changelog. Someone might want to contact you:

* Tue Nov 4 2014 <adres jest niedostepny pl> 1.53

Comment 2 Lubomir Rintel 2014-11-11 11:36:38 UTC
Also -- more rpmlint warnings; please address them.

labiryntowy-fonts.src: W: file-size-mismatch Labiryntowy_pl.tgz = 54121, https://alfabet-ozdobny.appspot.com/images/Labiryntowy_pl.tgz = 54166
The size of the file in the package does not match the size indicated by
peeking at its URL.  Verify that the file in the package has the intended
contents.

labiryntowy-fonts.noarch: W: incoherent-version-in-changelog 1.53 ['1.541-1.fc21', '1.541-1']
The latest entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

labiryntowy-fonts.noarch: W: invalid-url URL: http://alfabet-ozdobny.appspot.com/?str=labiryntowy HTTP Error 405: Method Not Allowed
The value should be a valid, public HTTP, HTTPS, or FTP URL.

Comment 3 marwin 2014-11-12 16:31:54 UTC
I check some trouble. But I cant know how change a .src.rpm filename and path.

Comment 6 Lubomir Rintel 2014-11-12 17:07:42 UTC
5.) SPEC is different than one in the SRPM

Contains extra garbage; please remove all of:

# % s e tup -q -c %{fontname}-%{version}-ttf

# wersja 1.2

6.) Your Summary is still wrong (rpmlint contains)

It must not contain a dot at the end and needs to be a proper English phrase.
See 0.).


7.) Your changelog version is wrong (rpmlint complains)

Should be 1.53-1, not 1.53.

Comment 7 Lubomir Rintel 2014-11-12 20:03:04 UTC
Looks good now.

APPROVED

Comment 8 marwin 2014-11-12 21:05:01 UTC
New Package SCM Request
=======================
Package Name: labiryntowy
Short Description: Artificial font consisting of vertical and horizontal bars
Upstream URL: https://github.com/marwino/labiryntowe/blob/master/labiryntowy-fonts-1.53-1.fc20.src.rpm
Owners: marwin
Branches: f19 f20 f21 el5 el6 epel7
InitialCC: fonts-sig

Comment 9 marwin 2014-11-12 21:28:05 UTC
New Package SCM Request
=======================
Package Name: labiryntowy
Short Description: Artificial font consisting of vertical and horizontal bars
Upstream URL: http://alfabet-ozdobny.appspot.com/?str=labiryntowy
Owners: marwin
Branches: f19 f20 f21 el5 el6 epel7
InitialCC: fonts-sig

Comment 10 Gwyn Ciesla 2014-11-12 22:10:33 UTC
WARNING: Requested package name labiryntowy doesn't match bug summary
labiryntowy-fonts

Comment 11 Parag AN(पराग) 2014-11-13 03:26:03 UTC
Lubomir,
   First, this package is not following Fonts packaging guidelines. Second, I have not seen the package submitter done any reviews and you sponsored. How can you be so sure that package submitter knows packaging well. Third, at least you should have let package submitter submit the srpm that fixes issues raised in comment#6 and then approved this package.

I don't see package submitter satisfying any of following
http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#Submitting_quality_new_packages
OR
http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#Show_Your_Expertise_by_Commenting_on_other_Review_Requests


Please revoke this package approval. Guide the package submitter on how to follow fonts packaging guidelines.

Comment 12 Lubomir Rintel 2014-11-13 09:09:06 UTC
(In reply to Parag AN(पराग) from comment #11)
> Lubomir,
>    First, this package is not following Fonts packaging guidelines.

Could you please be more specific?

I've done my best reading through the guidelines and couldn't find a single violation. That said, none of my packages is a font package and I could be wrong.

> Second,
> I have not seen the package submitter done any reviews and you sponsored.
> How can you be so sure that package submitter knows packaging well.

I've gained a reasonable confidence by getting talking to the submitter and getting him address issues in his own package. My understanding is that doing informal reviews is a way to attract sponsor's attention while the decision is of the sponsor. I'm able to be responsible for quality of his package.

> Third,
> at least you should have let package submitter submit the srpm that fixes
> issues raised in comment#6 and then approved this package.

Eh? It might not be obvious as he didn't add a comment to Bugzilla, but the actually updated the src.rpm in place.

> Please revoke this package approval. Guide the package submitter on how to
> follow fonts packaging guidelines.

Well, it would be helpful to provide an example of a violation. While I believe you didn't make that up, I can not see any problem. The package looks all sane to me and you didn't prove that wrong.

Comment 13 marwin 2014-11-13 13:34:42 UTC
New Package SCM Request
=======================
Package Name: labiryntowy-fonts
Short Description: Artificial font consisting of vertical and horizontal bars
Upstream URL: http://alfabet-ozdobny.appspot.com/?str=labiryntowy
Owners: marwin
Branches: f19 f20 f21 el5 el6 epel7
InitialCC: fonts-sig

Comment 14 Gwyn Ciesla 2014-11-13 16:42:28 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2014-11-13 20:06:53 UTC
labiryntowy-fonts-1.53-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-1.fc20

Comment 16 Fedora Update System 2014-11-13 20:10:49 UTC
labiryntowy-fonts-1.53-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-1.fc19

Comment 17 Fedora Update System 2014-11-13 20:23:50 UTC
labiryntowy-fonts-1.53-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-1.fc21

Comment 18 Fedora Update System 2014-11-13 20:45:11 UTC
labiryntowy-fonts-1.53-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-1.el6

Comment 19 Fedora Update System 2014-11-13 21:18:55 UTC
labiryntowy-fonts-1.53-1.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-1.el7

Comment 20 Fedora Update System 2014-11-14 00:16:14 UTC
labiryntowy-fonts-1.53-1.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 21 Parag AN(पराग) 2014-11-14 01:09:13 UTC
(In reply to Lubomir Rintel from comment #12)
> (In reply to Parag AN(पराग) from comment #11)
> > Lubomir,
> >    First, this package is not following Fonts packaging guidelines.
> 
> Could you please be more specific?
> 

 1) We need to have a fontconfig file. This one only need to cover generic names rule. 

 2) I think as per guidelines this font need to fix its metadata. There is no license text, neither in source nor in font file itself. There is a example given on the guidelines page on how to do this. Copyright contains license information which looks incorrect and need to be fixed.

 3) Incorrect usage of %_font_pkg macro.

I have not checked what the package submitter has checked in git yet as there is no approved updated srpm here to verify.

Comment 22 marwin 2014-11-14 10:39:55 UTC
 
>  2) I think as per guidelines this font need to fix its metadata. There is
> no license text, neither in source nor in font file itself. There is a
> example given on the guidelines page on how to do this. Copyright contains
> license information which looks incorrect and need to be fixed.


Please open this font in fontforge Ctrl+Shft+F 
in licence:
"Licencja SIL open font. Autor Jacek Szewczyk"
You can convert ttf into fontforge text format and read it too.

Comment 23 Lubomir Rintel 2014-11-14 12:16:57 UTC
(In reply to Parag AN(पराग) from comment #21)
> (In reply to Lubomir Rintel from comment #12)
> > (In reply to Parag AN(पराग) from comment #11)
> > > Lubomir,
> > >    First, this package is not following Fonts packaging guidelines.
> > 
> > Could you please be more specific?
> > 
> 
>  1) We need to have a fontconfig file. This one only need to cover generic
> names rule. 

Please point me to the part in the guidelines that mandate this. I have not been able to find if it's mandatory however I found indication that it might be optional:

"fontconf is the optional fontconfig ruleset..." [1]

[1] http://fedoraproject.org/wiki/Simple_fonts_spec_template


>  3) Incorrect usage of %_font_pkg macro.
> 
> I have not checked what the package submitter has checked in git yet as
> there is no approved updated srpm here to verify.

There is, and it seems correct to me.

(In reply to marwin from comment #22)
>  
> >  2) I think as per guidelines this font need to fix its metadata. There is
> > no license text, neither in source nor in font file itself. There is a
> > example given on the guidelines page on how to do this. Copyright contains
> > license information which looks incorrect and need to be fixed.
> 
> 
> Please open this font in fontforge Ctrl+Shft+F 
> in licence:
> "Licencja SIL open font. Autor Jacek Szewczyk"
> You can convert ttf into fontforge text format and read it too.

It's not the full text though. Jacek, please add a file with _full_ license text (e.g. to opis.txt).

Comment 24 Parag AN(पराग) 2014-11-14 16:38:38 UTC
1) Right its optional but I recommend to have fontconfig files for fonts. Though this font is not covering any non CGL languages (only Latin) so it can be optional but I will say add generic names rule file. You can use this font style as fantasy in fontconfig.

2) Just by looking into this line
%{_font_pkg} %{_fontdir}
one can easily say its going to give duplicate dir installation warning. 

Let's check on koji for this and I can see in build.log this
warning: File listed twice: /usr/share/fonts/labiryntowy

recommended way is 
%_font_pkg *.ttf -f %{fontconf}

3) Marwin,
    I observed either something is broken in this packaged font or in ttname package itself. But I have ran ttname on some fonts in fedora and it worked well. It helped to fix copyright as well as license metadata in font files but for this font its failing. Maybe you want to report this to upstream and get the font metadata fixed as given in fonts guidelines.
   Also, When you do Ctrl+Shift+f in fontforge and click on "TTF names" in left side, you will see on right side there is no license or license url field so indeed that information is missing.

Check this -> https://fedoraproject.org/wiki/Packaging:FontsPolicy#Correcting_the_metadata_with_ttname

4) I have recently added appdata metainfo file to many fonts packages in F21 and F22. This package also need to add metainfo file. I know this is not again in Guidelines but Richard Hughes wants this. See https://lists.fedoraproject.org/pipermail/devel/2014-October/203394.html

Comment 25 Fedora Update System 2014-11-17 11:03:53 UTC
labiryntowy-fonts-1.53-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-2.fc19

Comment 26 Fedora Update System 2014-11-17 11:15:25 UTC
labiryntowy-fonts-1.53-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-2.fc20

Comment 27 Fedora Update System 2014-11-17 11:26:02 UTC
labiryntowy-fonts-1.53-2.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-2.fc21

Comment 28 Fedora Update System 2014-11-17 11:34:55 UTC
labiryntowy-fonts-1.53-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-2.el6

Comment 29 Fedora Update System 2014-11-17 11:51:20 UTC
labiryntowy-fonts-1.53-2.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/labiryntowy-fonts-1.53-2.el7

Comment 30 Parag AN(पराग) 2014-11-21 03:56:59 UTC
Lubo and marwin,
   I discussed about having fontconfig file in each font package with fontconfig maintainer and he seem to have similar opinion that it should be added. 
   In future if I get time, I will ask FPC to mandate this.
   Can we have fontconfig file added here?

Comment 31 Mamoru TASAKA 2014-11-21 15:14:24 UTC
Parag:

As Lubomir already points out and you already agreed,
your comment 30 is now just a RFE and not longer "review request"
matter.

If you wish, please open a bug against the component of 
this package (already created) and discuss there, not here.

Comment 32 Parag AN(पराग) 2014-11-21 16:30:42 UTC
Its not the problem of how this package can be improved. Its just a commit that I can do myself and fix this. Problem is this package submitter got sponsored based on just this one package submission and no package reviews done by him. I feel package submitter is still struggling to understand rpm packaging. I tried to contact him personally but the email id of this packager got bounced and no reply from Lubomir either where I also cc'ed him. 

Then somehow package submitter contacted me from his some other email id and I guided him to first fix this package by adding license text but now no further action by him.

Just see the commits by this packager which is confusing http://pkgs.fedoraproject.org/cgit/labiryntowy-fonts.git/

Through package reviewing and more package submission, contributor learns more and more about packaging.

Anyways I will stop bugging here now.

Comment 33 Fedora Update System 2014-12-01 18:54:27 UTC
labiryntowy-fonts-1.53-2.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 34 Fedora Update System 2014-12-01 19:06:57 UTC
labiryntowy-fonts-1.53-2.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 35 Fedora Update System 2014-12-06 09:59:15 UTC
labiryntowy-fonts-1.53-2.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.


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