Bug 956134

Created attachment 745253 [details]
repo-font-audit output

fontlint reports Self Intersecting Glyph, Wrong Direction and Missing Points at Extrema.

Updating: only package ttf font after discussion on fedora-devel. Same links.

Fontlint output: 
$ fontlint ./stylesheets/perkins/mnmlicons/mnmliconsv21-webfont.ttf
Copyright (c) 2000-2012 by George Williams.
 Executable based on sources from 14:57 GMT 31-Jul-2012.
 Library based on sources from 14:57 GMT 31-Jul-2012.
Validation mnmlicons ...Failed
  Self Intersecting Glyph
  Wrong Direction
  Missing Points at Extrema

I can't find any upstream handled by the original author. Since the upstream is more or less deprecated, and the new Perkins project does not contain this font it doesn't really make sense to contact author about the repo-font-audit warnings in this case IMHO.

Created attachment 745896 [details]
repo-font-audit results.

I'll review this one :)

Review:

[+] OK
[-] NA
[?] Issue

** Mandatory review guidelines: **
 [?] rpmlint output:
[asinha@localhost  SRPMS]$ rpmlint ../SPECS/mnmlicons-fonts.spec ./mnmlicons-fonts-1.1-1.fc18.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
../SPECS/mnmlicons-fonts.spec: E: specfile-error warning: bogus date in %changelog: Tue Apr 24 2013 Alec Leamas <leamas> - 1.1-1
mnmlicons-fonts.src: W: spelling-error Summary(en_US) webfonts -> web fonts, web-fonts, webfoot
mnmlicons-fonts.src: W: invalid-url URL: http://code.google.com/p/perkins-less/ <urlopen error [Errno -2] Name or service not known>
mnmlicons-fonts.src: E: specfile-error warning: bogus date in %changelog: Tue Apr 24 2013 Alec Leamas <leamas> - 1.1-1
mnmlicons-fonts.noarch: W: spelling-error Summary(en_US) webfonts -> web fonts, web-fonts, webfoot
mnmlicons-fonts.src: W: spelling-error Summary(en_US) webfonts -> web fonts, web-fonts, webfoot
mnmlicons-fonts.src: W: invalid-url URL: http://code.google.com/p/perkins-less/ <urlopen error [Errno -2] Name or service not known>
mnmlicons-fonts.src: E: specfile-error warning: bogus date in %changelog: Tue Apr 24 2013 Alec Leamas <leamas> - 1.1-1
3 packages and 1 specfiles checked; 3 errors, 5 warnings.
[asinha@localhost  SRPMS]$

The spec date needs to be corrected. I think it was a Wednesday ;)
The spelling is a cosemtic change, can be made too.

 [+] License is acceptable (...)
[asinha@localhost  perkins]$ find . -name "*" -exec licensecheck '{}' \; | sed "/UNKNOWN/ d"
./stylesheets/perkins/mnmlicons/mnmliconsv21-webfont.svg: GENERATED FILE
./LICENSE: MIT/X11 (BSD like)

 [+] License field in spec is correct
 [+] License files included in package %docs if included in source package
 [+] License files installed when any subpackage combination is installed
 [+] Spec written in American English
 [+] Spec is legible
 [+] Sources match upstream unless altered to fix permissibility issues
[asinha@localhost  perkins]$ review-md5check.sh
../../SPECS/mnmlicons-fonts.spec Getting http://perkins-less.googlecode.com/files/perkins-1.1.zip to
/tmp/review/perkins-1.1.zip % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  405k  100  405k    0     0   250k      0  0:00:01  0:00:01 --:--:--  250k
445eeb9ca365769f2802997e5dae857a  /tmp/review/perkins-1.1.zip
445eeb9ca365769f2802997e5dae857a  /home/asinha/rpmbuild/SOURCES/perkins-1.1.zip
removed ‘/tmp/review/perkins-1.1.zip’
removed directory: ‘/tmp/review’
[asinha@localhost  perkins]$

 [+] Build succeeds on at least one primary arch
 [+] Build succeeds on all primary arches or has ExcludeArch + bugs filed
 [+] BuildRequires correct, justified where necessary
 [-] Locales handled with %find_lang, not %_datadir/locale/*
 [-] %post, %postun call ldconfig if package contains shared .so files
 [+] No bundled libs
 [-] Relocatability is justified
 [+] Package owns all directories it creates
 [+] Package requires others for directories it uses but does not own
 [+] No duplication in %files unless necessary for license files
 [+] File permissions are sane
 [+] Package contains permissible code or content
 [+] Large docs go in -doc subpackage
 [+] %doc files not required at runtime
 [-] Static libs go in -static package/virtual Provides
 [-] Development files go in -devel package
 [-] -devel packages Require base with fully-versioned dependency, %_isa
 [-] No .la files
 [-] GUI app uses .desktop file, installs it with desktop-file-install
 [-] File list does not conflict with other packages' without justification
 [+] File names are valid UTF-8

** Optional review guidelines: **
 [-] Query upstream about including license files
 [-] Translations of description, summary
 [+] Builds in mock
 [+] Builds on all arches
 [+] Functions as described (e.g. no crashes)
 [+] Scriptlets are sane
 [-] Subpackages require base with fully-versioned dependency if sensible
 [-] .pc file subpackage placement is sensible
 [-] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
 [-] Include man pages if available

Naming guidelines:
 [+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+
 [+] Package names are sane
 [+] No naming conflicts
 [+] Spec file name matches base package name
 [+] Version is sane
 [+] Version does not contain ~
 [+] Release is sane
 [+] %dist tag
 [+] Case used only when necessary
 [-] Renaming handled correctly

Packaging guidelines:
 [+] Useful without external bits
 [-] No kmods
 [-] Pre-built binaries, libs removed in %prep
 [+] Sources contain only redistributable code or content
 [+] Spec format is sane
 [+] Package obeys FHS, except libexecdir, /run, /usr/target
 [+] No files in /bin, /sbin, /lib* on >= F17
 [-] Programs run before FS mounting use /run instead of /var/run
 [-] Binaries in /bin, /sbin do not depend on files in /usr on < F17
 [+] No files under /srv, /opt, /usr/local
 [+] Changelog in prescribed format
 [+] No Packager, Vendor, Copyright, PreReq tags [+] Summary does not end in a period
 [-] Correct BuildRoot tag on < EL6
 [-] Correct %clean section on < EL6
 [-] Requires correct, justified where necessary
 [+] Summary, description do not use trademarks incorrectly
 [+] All relevant documentation is packaged, appropriately marked with %doc
 [+] Doc files do not drag in extra dependencies (e.g. due to +x)
 [-] Code compilable with gcc is compiled with gcc
 [-] Build honors applicable compiler flags or justifies otherwise
 [-] PIE used for long-running/root daemons, setuid/filecap programs
 [-] Useful -debuginfo package or disabled and justified
 [-] Package with .pc files Requires pkgconfig on < EL6
 [-] No static executables
 [-] Rpath absent or only used for internal libs
 [-] Config files marked with %config(noreplace) or justified %config
 [-] No config files under /usr
 [-] Third party package manager configs acceptable, in %_docdir
 [-] .desktop files are sane
 [-] Spec uses macros consistently
 [-] Spec uses macros instead of hard-coded names where appropriate
 [-] Spec uses macros for executables only when configurability is needed
 [-] %makeinstall used only when alternatives don't work
 [-] Macros in Summary, description are expandable at srpm build time
 [+] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
 [-] No software collections (scl)
 [-] Macro files named /etc/rpm/macros.%name
 [-] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
 [-] %global, not %define
 [-] Package translating with gettext BuildRequires it
 [-] Package translating with Linguist BuildRequires qt-devel
 [-] File ops preserve timestamps
 [-] Parallel make
 [-] No Requires(pre,post) notation
 [-] User, group creation handled correctly (See Packaging:UsersAndGroups)
 [-] Web apps go in /usr/share/%name, not /var/www
 [-] Conflicts are justified
 [+] One project per package
 [-] No bundled fonts
 [-] Patches have appropriate commentary
 [-] Available test suites executed in %check
 [-] tmpfiles.d used for /run, /run/lock on >= F15


The packaging looks good to me. The fontconfig file may need a little more work though. I looked at the fontforge output and it seems to be a Serif font.

The files in /usr/share/fontconfig/templates/ should be a good place to start. The "basic-font-template" should be sufficient for this font package.

Thanks,
Warm regards,
Ankur

Hm... I'm still quite new to fonts, and things are not really clear to me.

That said, the mnmlicons font is just a set of icons and not a traditional character font (and it's definitely not a Serif character font). All this means, still as I understand it, that the only meaningful usecase is when the app requests the mnmlicons font. It can't substitute for anything else, and can't be substituted. That's why I left  the fontconfig file empty, right or wrong.

I should have mentioned that this is packaged as a dependency of bug 957339, a web application which bundles this font upstream.

Ping?

(In reply to Alec Leamas from comment #7)
> Hm... I'm still quite new to fonts, and things are not really clear to me.
> 
> That said, the mnmlicons font is just a set of icons and not a traditional
> character font (and it's definitely not a Serif character font). All this
> means, still as I understand it, that the only meaningful usecase is when
> the app requests the mnmlicons font. It can't substitute for anything else,
> and can't be substituted. That's why I left  the fontconfig file empty,
> right or wrong.
> 
> I should have mentioned that this is packaged as a dependency of bug 957339,
> a web application which bundles this font upstream.

Hi Alec,

I confirmed with folks over the IRC. The font should be marked as a "fantasy" font as described in /usr/share/fontconfig/templates/fontconfig-generics.txt. Please update the fontconfig file using the basic-font-template.conf file with "fantasy" as the generic name. I think the priority should be 69 too. The fontconfig-priorities.txt file only goes up to 69, and the /etc/fonts/conf.d/README file says "70 through 79 select font (adjust which fonts are available)" and I'm really unsure about what that actually implies. It only lists 60-69 for generic aliases. Much safer to just stay in that range IMO.

That's all. The rest looks good. 

Thanks,
Warm regards,
Ankur

I read the IRC chat. Many thanks for the lesson for how to get some info on this! I feel somewhat enlightened, and will update also the sibling request bug #956127.

Updated the .conf file, changed priority to 69. Same links, no version bump (but a changelog entry).

Hi Alec,

[asinha@localhost  SRPMS]$ rpmlint ../SPECS/mnmlicons-fonts.spec ./mnmlicons-fonts-1.1-1.fc18.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
../SPECS/mnmlicons-fonts.spec: E: specfile-error warning: bogus date in %changelog: Wed May 19 2013 Alec Leamas <leamas> - 1.1-1
../SPECS/mnmlicons-fonts.spec: E: specfile-error warning: bogus date in %changelog: Tue Apr 24 2013 Alec Leamas <leamas> - 1.1-1
mnmlicons-fonts.src: W: spelling-error Summary(en_US) webfonts -> web fonts, web-fonts, webfoot
mnmlicons-fonts.src: E: specfile-error warning: bogus date in %changelog: Wed May 19 2013 Alec Leamas <leamas> - 1.1-1
mnmlicons-fonts.src: E: specfile-error warning: bogus date in %changelog: Tue Apr 24 2013 Alec Leamas <leamas> - 1.1-1
mnmlicons-fonts.noarch: W: spelling-error Summary(en_US) webfonts -> web fonts, web-fonts, webfoot
mnmlicons-fonts.src: W: spelling-error Summary(en_US) webfonts -> web fonts, web-fonts, webfoot
mnmlicons-fonts.src: E: specfile-error warning: bogus date in %changelog: Wed May 19 2013 Alec Leamas <leamas> - 1.1-1
mnmlicons-fonts.src: E: specfile-error warning: bogus date in %changelog: Tue Apr 24 2013 Alec Leamas <leamas> - 1.1-1
3 packages and 1 specfiles checked; 6 errors, 3 warnings.
[asinha@localhost  SRPMS]$

The package looks good. A few things you could correct before committing to SCM though:

1. The spelling errors (optional)
2. The changelog entries (MUST): I think you mean June instead of May in the latest entry ;). Not sure about the April one (it was a Wednesday). Please rectify this before you commit. 

*** APPROVED ***

Thanks,
Warm regards,
Ankur

I'll flush the changelog before committing anyway, but those dates... "blushes" Will fix spelling.

Many thanks for the review, which was so much more work than I anticipated.

Warm regards to you as well,

--alec

Package Change Request
======================
Package Name: mnmlicons-fonts
New Branches: f18 f19
Owners: leamas
InitialCC:

New Package SCM Request
=======================
Package Name: mnmlicons-fonts
Short Description: Perkins Less Web Framework webfonts
Owners: leamas
Branches: f18 f19
InitialCC: fonts-sig

New try, let's forget the previous one :(

Git done (by process-git-requests).

f18, f19 and rawhide built. f18 and f19 in updates-testing. Closing.