Bug 528675

Summary: Review Request: knm-new-fixed-fonts - 12x12 JIS X 0208 Bitmap fonts
Product: [Fedora] Fedora Reporter: Akira TAGOH <tagoh>
Component: Package ReviewAssignee: Akira TAGOH <tagoh>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, fonts-bugs, ftbfs, matt_domsch, notting
Target Milestone: ---Flags: nicolas.mailhot: fedora-review+
kevin: 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: 2009-10-29 08:59:17 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 511348, 511668    

Description Akira TAGOH 2009-10-13 10:32:24 UTC
Spec URL: http://tagoh.fedorapeople.org/knm-new-fonts/knm-new-fonts.spec
SRPM URL: http://tagoh.fedorapeople.org/knm-new-fonts/knm-new-fonts-1.1-7.fc13.src.rpm
Description:
This package provides 12x12 Japanese bitmap font for JIS X 0208.
JIS X 0208 is a character set that contains Kanji characters mostly often used.

This is a package review request to be renamed from knm_new-fonts. see Bug #511668.

Comment 1 Nicolas Mailhot 2009-10-14 18:33:39 UTC
1. technically each file here is a "font" so the summary should probably be 
12x12 JIS X 0208 bitmap fonts

2.
JIS X 0208 is a character set that contains Kanji characters mostly
often used.
⇒ The JIS X 0208 character set contains the most often used Kanji glyphs.

3. your provides/obsoletes are probably too complex, rpmlint complains
knm-new-fonts.noarch: W: self-obsoletion knm_new <= 1.1-16 obsoletes knm_new = 1.1-16

IMHO you should drop Provides completely, repoquery says nothing requires  knm_new or knm-new-fonts in Fedora

4. you should put 
/etc/X11/fontpath.d/knm-new
and
/usr/share/fonts/knm-new/fonts.dir
in a separate legacy subpackage

5. The fonts declare themselves as "fixed", therefore please rename the package knm-new-fixed-fonts (we want the font name to appear in the package name). Not sure if using the same name as many other fonts is a good idea, but I guess I don't care a lot

6. fc-query complains
Can't query face 0 of font file /usr/share/fonts/knm_new/knm12pb.pcf.gz
Can't query face 0 of font file /usr/share/fonts/knm_new/knm12p.pcf.gz
Can't query face 0 of font file /usr/share/fonts/knm_new/knmzn12xb.pcf.gz
Can't query face 0 of font file /usr/share/fonts/knm_new/knmzn12x.pcf.gz

You should open a bug against fontconfig to have it investigated. This is not normal.

7. It would be nice to include a fontconfig file for the font

8. I don't think it's good style to mix macros for some commands (%{__ln_s}, %{_bindir}/mkfontdir) and not others. The current convention seems to write plain unmacroized commands with no explicit path

Anyway that's all for the review

Comment 2 Nicolas Mailhot 2009-10-14 19:00:21 UTC
(In reply to comment #1)

> 4. you should put 
> /etc/X11/fontpath.d/knm-new
> and
> /usr/share/fonts/knm-new/fonts.dir
> in a separate legacy subpackage

Or maybe not, it seems we tolerate those files right now, so I won't insist on this one, doing the rest is sufficient to make this package disappear from problem reports

Comment 3 Akira TAGOH 2009-10-19 08:23:14 UTC
(In reply to comment #1)
> 4. you should put 
> /etc/X11/fontpath.d/knm-new
> and
> /usr/share/fonts/knm-new/fonts.dir
> in a separate legacy subpackage

Is there any template or the proposed subpackage name for that? knm-new-fixed-legacy-fonts? or knm-new-fixed-fonts-legacy?

> 7. It would be nice to include a fontconfig file for the font

I'm not quite sure if this kind of the bitmap fonts only available for some pt. really helps though. hmm, I haven't tested but something like this?

<?xml version="1.0" encoding="UTF-8">
<!DOCTYPE fontconfig SYSTEM "../fonts.dtd">
<fontconfig>
  <alias>
    <family>Fixed</family>
    <default>
      <family>monospace</family>
    </default>
  </alias>
  <match>
    <test name="lang">
      <string>ja-jp</string>
    </test>
    <test name="family">
      <string>monospace</string>
    </test>
    <test name="size" compare=eq">
      <double>12</double>
    </test>
    <edit name="family" mode="prepend" binding="same">
      <string>Fixed</string>
    </edit>
  </match>
</fontconfig>

FWIW I don't think Japanese people wants to see the bitmap glyphs for the desktop.

Comment 4 Akira TAGOH 2009-10-19 08:24:33 UTC
<test name="size">, even.

Comment 5 Nicolas Mailhot 2009-10-19 21:39:14 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > 4. you should put 
> > /etc/X11/fontpath.d/knm-new
> > and
> > /usr/share/fonts/knm-new/fonts.dir
> > in a separate legacy subpackage
> 
> Is there any template or the proposed subpackage name for that?
> knm-new-fixed-legacy-fonts? or knm-new-fixed-fonts-legacy?

There are not conventions here because I try very hard to forget core fonts exist, and new packages are not supposed to register in the core fonts system. So that's only a problem for old packages.

Technically, you only have to put this stuff in a separate subpackage for multi-font srpms (because fonts.dir can only be provided by a single package), so in your case that's not absolutely needed. Sorry about this, should not have bothered you with it

> > 7. It would be nice to include a fontconfig file for the font
> 
> I'm not quite sure if this kind of the bitmap fonts only available for some pt.
> really helps though. 

The fontconfig file should not change the font visibility, and apps that can use them will be happy

> hmm, I haven't tested but something like this?
> 
> <?xml version="1.0" encoding="UTF-8">
> <!DOCTYPE fontconfig SYSTEM "../fonts.dtd">
> <fontconfig>
>   <alias>
>     <family>Fixed</family>
>     <default>
>       <family>monospace</family>
>     </default>
>   </alias>
>   <match>
>     <test name="lang">
>       <string>ja-jp</string>
>     </test>
>     <test name="family">
>       <string>monospace</string>
>     </test>
>     <test name="size" compare=eq">
>       <double>12</double>
>     </test>
>     <edit name="family" mode="prepend" binding="same">
>       <string>Fixed</string>
>     </edit>
>   </match>
> </fontconfig>

Please take /usr/share/fontconfig/templates/l10n-font-template.conf as model (adding maybe the size tweak)

> FWIW I don't think Japanese people wants to see the bitmap glyphs for the
> desktop.  

Some people say yes, others no, people have tried to write CJK rules in fontconfig for years without making everyone happy, at this stage let's not try to be creative and just use our standard template. Behdad is the only one that can hope untangling this mess.

Comment 6 Akira TAGOH 2009-10-20 05:09:22 UTC
(In reply to comment #5)
> There are not conventions here because I try very hard to forget core fonts
> exist, and new packages are not supposed to register in the core fonts system.
> So that's only a problem for old packages.

Sure. that sounds sane ;)

> > > 7. It would be nice to include a fontconfig file for the font
> > 
> > I'm not quite sure if this kind of the bitmap fonts only available for some pt.
> > really helps though. 
> 
> The fontconfig file should not change the font visibility, and apps that can
> use them will be happy

Even if there are no fontconfig file, they can find it out yes?

> Please take /usr/share/fontconfig/templates/l10n-font-template.conf as model
> (adding maybe the size tweak)

That looks similar except the size testing. does fontconfig not try to scale up/down for non-supported size but try to pick up for the exact size right? then guess I don't need to check what size is being requested.

> 
> > FWIW I don't think Japanese people wants to see the bitmap glyphs for the
> > desktop.  
> 
> Some people say yes, others no, people have tried to write CJK rules in
> fontconfig for years without making everyone happy, at this stage let's not try
> to be creative and just use our standard template. Behdad is the only one that
> can hope untangling this mess.  

Sure - I meant there is other Japanese bitmap fonts that has a good quality and the variety of the size with the same glyph design and styles, so getting the different of the looks according to the font size sounds not good to me, but anyway.

Since emacs doesn't require the X core fonts now, I personally don't need the X core fonts anymore though.

Comment 8 Nicolas Mailhot 2009-10-22 19:54:43 UTC
*** Bug 511668 has been marked as a duplicate of this bug. ***

Comment 9 Matt Domsch 2009-10-22 20:29:34 UTC
This package failed to build in Koji for Fedora 12, and at this point in the release cycle (Beta is out), we'll have to live with the F11 build of the package unless there is another bug in your package which would prevent the release of Fedora 12.

If your package is now obsolete and should be removed from Fedora 12 and future, please follow the End of Life instructions here:
https://fedoraproject.org/wiki/How_to_remove_a_package_at_end_of_life

If your package is not obsolete, and you wish it to remain in the distribution for Fedora 13, please update your package's devel branch in CVS so that it builds, build it in koji, and close this bug as "CLOSED RAWHIDE", including the link to the koji build that succeeded.  You can do this now, while final touches are being placed on Fedora 12 without affecting F12.

Shortly after F12 is out, but before the F13 Alpha release, all F12FTBFS-blocking bugs will be re-evaluated.  At that time, if progress has not been made to fix the package, it will be removed from the distribution for F13.

Comment 10 Nicolas Mailhot 2009-10-22 21:32:20 UTC
(In reply to comment #7)
> Updated.
> 
> Spec URL:
> http://tagoh.fedorapeople.org/knm-new-fixed-fonts/knm-new-fixed-fonts.spec
> SRPM URL:
> http://tagoh.fedorapeople.org/knm-new-fixed-fonts/knm-new-fixed-fonts-1.1-8.fc13.src.rpm  

The packaging is sane and the current package FTBS therefore I'm going to approve it. However that does not change the fact that fontconfig can not parse 4 of the font files. IMHO you should either fix them, or drop them

But this can be done as a maintainer of the new package, and the old knm_new-fonts package is no better in this regard, therefore I'll won't block on it

Please check the fontconfig priorities, when you're not using the l10n template, bigger prefix means lower prio, but with the l10n locale override, bigger prefix means bigger prio, so 69 will trump any ja font with a <69 prefix (IIRC) 

々々々 APPROVED 々々々

Thank you for cleaning up this historic package, I know that's not the most exciting activity :)

Comment 11 Akira TAGOH 2009-10-28 08:53:11 UTC
Thanks for the review.

(In reply to comment #10)
> Please check the fontconfig priorities, when you're not using the l10n
> template, bigger prefix means lower prio, but with the l10n locale override,
> bigger prefix means bigger prio, so 69 will trump any ja font with a <69 prefix
> (IIRC) 

Sure. will update.

New Package CVS Request
=======================
Package Name: knm-new-fixed-fonts
Short Description: 12x12 JIS X 0208 Bitmap fonts
Owners: tagoh
Branches: devel F-12 F-11 F-10
InitialCC: fonts-sig i18n-team

Comment 12 Kevin Fenzi 2009-10-29 00:07:55 UTC
cvs done.

Comment 13 Akira TAGOH 2009-10-29 08:59:17 UTC
Thanks. pushed the package.