Bug 454148 - Review Request: monafont - Japanese font for text arts
Review Request: monafont - Japanese font for text arts
Status: CLOSED DUPLICATE of bug 457062
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-05 08:16 EDT by Mamoru TASAKA
Modified: 2008-07-29 10:23 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-06 12:25:35 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Mamoru TASAKA 2008-07-05 08:16:49 EDT
Spec URL: http://mtasaka.fedorapeople.org/Review_request/monafont/monafont.spec
SRPM URL: http://mtasaka.fedorapeople.org/Review_request/monafont/monafont-2.90-1.fc9p.src.rpm
Description: 
Mona Font is a Japanese proportional font which allows you to view
Japanese text arts correctly.

Scratch build
for dist-f10:
http://koji.fedoraproject.org/koji/taskinfo?taskID=697709
for dist-f9-updates-candidate:
http://koji.fedoraproject.org/koji/taskinfo?taskID=697708
Comment 1 Nicolas Mailhot 2008-07-06 07:22:06 EDT
Thank you for packaging a new font! This is not a formal review, just initial
advice to make sure the actual review can proceed faster later.

1. Please make sure you've followed all the steps outlined in steps 1 and 2.a of
http://fedoraproject.org/wiki/Font_package_lifecycle

2. Please read attentively 
http://fedoraproject.org/wiki/Category:Fonts_packaging

3. Please try to stay as close as possible to
http://fedoraproject.org/wiki/Annotated_fonts_spec_template

If you feel you need to deviate (umasks, defattrs, etc) please explain in this
review why the deviation is justified.

5. Some comments on your spec after a quick overview

A. Please do not create a metapackage
B. Please adhere to our font naming conventions (mona-fonts-bitmap,
mona-fonts-sazanami etc). Unlike other distributions we do not use the ttf affix
C. please ask on fedora-devel if it's not possible to detect font package
versions automatically. Your hardcoded versions are going to bite you someday.
D. you do not need to BR gzip

6. I seem to remember other core fonts packages used to execute mkfontdir in
%post with ghosting. However we do not have any official core fonts packaging
guidelines right now. If you feel you've fully researched the subject, and found
the best solution, do not hesitate to submit a core fonts packaging guideline
draft later.

I realise this kind of package represents a lot of work (congratulations), and I
hope those comments will help you complete it.
Comment 2 Nicolas Mailhot 2008-07-06 07:23:17 EDT
There is not 4. I suck :p
Comment 3 Mamoru TASAKA 2008-07-06 08:15:36 EDT
Thank you for initial comments:

For 1:
I did not know the wiki package, however as far as I check the wiki pacakage
it seems fine.

For 2:
Perhaps I've read them

For 3:
3-1: Some fedora contributor says that unless umask is set correctly fc-cache
creates
     font cache in a wrong permission: ref:
     http://www.redhat.com/archives/rhl-devel-list/2007-March/msg00151.html
     For example, fonts-japanese package has umask scriptlet.
     I don't know if this issue has been correctly fixed. If you are sure this
     is not needed now please let me know it.
3-2: %defattr(0644,-,-,0755) = %defattr(-,root,root,-) and actually I don't think
     fonts packages must write a explicit umask values which differs from gerenal
     packaging guidelines. Are there any rationale?
5-AB: Will fix
5-C: Very intentional. Actually I don't want to have this rpm _automatically_ find
     the version.
     i.e. If the dependent fonts make update version, then this srpm won't build and
          someone (or Matt using automated massbuild) complains to me 
          "your srpm won't build, please fix it", which is actually what I am
intended.
6. Calling mkfontdir on scriptlet pulls X dependent packages, which is _banned_
   on current Fedora packaging: related to
   http://fedoraproject.org/wiki/Releases/FeatureNoMoreXFS
   Actually one day (before the release of F-8) all packages including core
fonts are
   made to not call mkfontdir or chkfontpath on scriptlet.
   
     
Comment 5 Nicolas Mailhot 2008-07-06 10:48:34 EDT
(In reply to comment #3)

> For 1:
> I did not know the wiki package, however as far as I check the wiki pacakage
> it seems fine.

I don't see any wiki page on mona in
http://fedoraproject.org/wiki/Category:In-progress_fonts
so you're definitely missing some steps in the fonts packaging workflow. Please
check. Those pages were not written for no reason.

> For 2:
> Perhaps I've read them

Can't answer for you.

> For 3:
> 3-1: Some fedora contributor says that unless umask is set correctly fc-cache
> creates font cache in a wrong permission: ref:
> http://www.redhat.com/archives/rhl-devel-list/2007-March/msg00151.html

I didn't know this link but in any case that's a lot of maybes and since almost
all our fonts do not use umask (and have been so for a long time) I doubt there
is any problem in real life.

> For example, fonts-japanese package has umask scriptlet.

This was a mistake of the fonts-japanese packager. Silently changing one package
when the same pattern is repeated in many others is wrong.

> I don't know if this issue has been correctly fixed.

If this issue is real it needs to be addressed by guideline changes so every
packager (not just the ones aware of this mail post) can fix his packages. In
the meanwhile adding umasks not in the guidelines only adds to the confusion. Thus:
— please remove umasks from your packages
— please submit a guideline change proposal to FPC arguing the need for umask,
so the issue can be discussed properly. If FPC deems the umask necessary we'll
change all packages including yours.

http://fedoraproject.org/wiki/Packaging/Committee

> 3-2: %defattr(0644,-,-,0755) = %defattr(-,root,root,-) and actually I don't think
> fonts packages must write a explicit umask values which differs from gerenal
> packaging guidelines. Are there any rationale?

The defattr mask for font packages has been approved by FPC and FESCO with the
font spec template. Official page there
http://fedoraproject.org/wiki/Packaging/FontsSpecTemplate
(unfortunately broken by recent mediawiki migration, the page I gave you is a
cleaned-up unofficial version).

FPC has generally avoided making statements on defattrs, because they can become
quite complex. It has let packagers exercise their good judgement on umasks or
not. However in the fonts case a correct deffatr is really simple which is why
the fonts guidelines include one (you can probably fish the IRC FPC minutes on
the subject if you care to).

> 5-AB: Will fix
> 5-C: Very intentional. Actually I don't want to have this rpm _automatically_ find
> the version.

Ok, good enough for me

> 6. Calling mkfontdir on scriptlet pulls X dependent packages, which is _banned_
>    on current Fedora packaging:

Ok, right, I was not thinking. The actual rule on this is
http://fedoraproject.org/wiki/Fonts_packaging_policy#Install-time_dependencies

BTW. I didn't think and didn't have any core fonts packaging guidelines to draw
on. Please write some.
Comment 6 Nicolas Mailhot 2008-07-06 11:10:12 EDT
On the spec file:

1. it's generally speaking a good idea to limit changes to the official
template, that makes rebasing after guideline changes easier. So it may be a
good idea to compare your spec with the template in rpmdevtools and try to limit
variations.

2. as stated on
http://fedoraproject.org/wiki/Annotated_fonts_spec_template
the -f flag to fc-cache should not be necessary starting from F9 and the
fontconfig packager requested its removal

3. please remove umasks

4. please add defattrs

5. it's probably a good idea to add some fontconfig rules so fontconfig knows it
can substitute mona-sazanami to sazanami (same for VLGothic). See
http://fedoraproject.org/wiki/Fontconfig_packaging_tips

6. URL could be 
URL:		http://%{archivename}.sourceforge.net/

7. Source0 could be (probably wrong right now):
Source0:
http://downloads.sourceforge.net/%{archivename}/%{archivename}-%{version}.tar.bz2

8. upstream tarball includes sfd files, and I dont see fontforge in your BR.
Please try to build from sfd sources if possible
http://fedoraproject.org/wiki/Fonts_packaging_policy#Building_from_sources

9. upstream tarball is missing a detached file with licensing statements. Please
ask upstream to add it (this would be asked during formal review)

This informal review is quickly degenerating in a full review *sigh*
Comment 7 Mamoru TASAKA 2008-07-06 11:22:23 EDT
(In reply to comment #6)
> 2. as stated on
> http://fedoraproject.org/wiki/Annotated_fonts_spec_template
> the -f flag to fc-cache should not be necessary starting from F9 and the
> fontconfig packager requested its removal

Ahh.. actually after setting "umask 000" and calling fc-cache, fontconfig cache
is created
with 0666 permission, which is wrong... So umask 133 is _definately_ needed and
FPC must
discuss this...
Comment 8 Mamoru TASAKA 2008-07-06 11:25:50 EDT
Specifyin -f option with 'umask 000' also creates 0666 permission fontconfig cache,
which is also wrong.
Comment 9 Nicolas Mailhot 2008-07-06 11:42:02 EDT
(In reply to comment #8)
> Specifyin -f option with 'umask 000' also creates 0666 permission fontconfig
cache,
> which is also wrong.

-f has nothing to do with permissions, it's a totally different setting.

As for your findings, I don't disagree with them, they just seem not to be a
problem in actual life (I suspect changing root default umask is so impacting
almost no one does this).

Nevertheless, as stated before:
— do not add an umask when the guidelines says you should not
– if you care about the problem have the guidelines changed (it's easy: write a
short wiki page on the subject, and submit it to FPC with a mail, the procedure
is on FPC's page)
Comment 10 Mamoru TASAKA 2008-07-06 11:51:23 EDT
(In reply to comment #9)
> As for your findings, I don't disagree with them, they just seem not to be a
> problem in actual life (I suspect changing root default umask is so impacting
> almost no one does this).

sudores is affected by this.

> Nevertheless, as stated before:
> — do not add an umask when the guidelines says you should not
> – if you care about the problem have the guidelines changed (it's easy: write a
> short wiki page on the subject, and submit it to FPC with a mail, the procedure
> is on FPC's page)

Are you suggesting that I have to keep a potentially broke scriptlet until
FPC discusses this? (and please keep in mind that usually I won't be able to discuss
with FPC members just because I live in Japan... At the time I am writing this
it is around 1 am)
Comment 11 Nicolas Mailhot 2008-07-06 12:16:27 EDT
(In reply to comment #10)

> Are you suggesting that I have to keep a potentially broke scriptlet until
> FPC discusses this?

Yes. Because FPC may have a different perspective from you, and the fact most of
our fonts have been shipping without this umask for years (including distro
default fonts) strongly hints the problem is limited.

If you care about it have the guidelines changed, period. Localized workarounds
are wrong. If the problem is not important enough to be fixed distro-wide, it's
not important enough to be workarounded in your package.

> (and please keep in mind that usually I won't be able to discuss
> with FPC members just because I live in Japan... At the time I am writing this
> it is around 1 am)

Get some sleep please then :). A few hours delay won't hurt anyone.

FPC discussion happens by mail and in the wiki page submitted to FPC so being in
Japan is not a huge problem.

Honestly it just takes a few week to have FPC and Fesco approve a guideline
change, so just package without the umask for now, and add it back in a few
weeks if FPC decides it's a good idea (if it decides it's a bad idea well, I
trust FPC more than any individual packager)

Comment 12 Mamoru TASAKA 2008-07-06 12:25:35 EDT
Sorry, Nicolas, another big problem is found before discussing packagins issues.....

After rechecking license issue, README.euc says that ttf font is based on *Kochi
font*,
which turned before (some years ago) to contain some legal issue and 
_cannot_ be shipped on Fedora. 
Although bitmap part is public domain, I think only shipping bitmap part is rather
of no use, so I must withdraw this review request. Very sorry for troubling you
without
checking very fundamental issue carefully...
Comment 13 Nicolas Mailhot 2008-07-06 12:32:53 EDT
np, serves me right by not starting with the licensing bit. I don't care much
about the bitmap part, but we have a few bitmap users, so if you want to restart
with just this element, it's ok with me.
Comment 14 Mamoru TASAKA 2008-07-29 10:23:22 EDT
Last my comment 12 was my misunderstanding.. resubmitting

*** This bug has been marked as a duplicate of 457062 ***

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