Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 202032 - Review Request: efont-unicode-bdf: Unicode font by Electronic Font Open Laboratory
Summary: Review Request: efont-unicode-bdf: Unicode font by Electronic Font Open Labor...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Paul F. Johnson
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 201170
TreeView+ depends on / blocked
 
Reported: 2006-08-10 14:35 UTC by Mamoru TASAKA
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-20 03:51:00 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
build log of efont-unicode-bdf in mock (deleted)
2006-08-10 19:15 UTC, Mamoru TASAKA
no flags Details

Description Mamoru TASAKA 2006-08-10 14:35:09 UTC
Spec URL: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/efont-unicode-bdf.spec
SRPM URL:http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/efont-unicode-bdf-0.4.2-1.src.rpm
Description:  
This package provides Unicode bitmap fonts provided by
Electronic Font Open Laboratory.

Note: this package blocks bug 201170 (jfbterm)

Comment 1 Mamoru TASAKA 2006-08-10 19:15:40 UTC
Created attachment 133970 [details]
build log of efont-unicode-bdf in mock

Build log of efont-unicode-bdf-0.4.2-1.fc5 in mock on fc5 system.

rpmlint shows no errors.
efont-unicode-bdf-0.4.2-1.fc5.noarch.rpm:

efont-unicode-bdf-0.4.2-1.fc5.src.rpm:

Comment 2 Paul F. Johnson 2006-08-10 20:27:49 UTC
%post
umask 133
mkfontdir %{fontdir} && /usr/sbin/chkfontpath -q -a %{fontdir}
fc-cache 2>/dev/null

%postun
fc-cache 2>/dev/null

Okay, there needs to be some wrappers around this

you need something like

%post
if [ -x %{_bindir}/mkfontdir ]; then
  if [ - x %{_sbindir}/chkfontpath ]; then
    %{_bindir}/mkfontdir %{fontdir} && %{_sbindir}/chkfontpath -q -a %{fontdir}
  fi
fi
if [-x %{_bindir}/fc-cache ]; then
  %{_bindir}/fc-cache 2>/dev/null
fi

%postun
if [ "$1" = "0" ]; then
  if [-x %{_bindir}/fc-cache ]; then
    %{_bindir}/fc-cache 2>/dev/null
  fi
fi

Also, why are you using umask?

Comment 3 Mamoru TASAKA 2006-08-10 20:54:03 UTC
Well,

A: %Requires(post) requires xorg-x11-font-utils, /usr/sbin/chkfontpath,
fontconfig. Then, the check for existence of binary is already done by this,
I think?

B. The usage of "umask 133" was borrowed from fonts-japanese.
This is because mkfontdir creates fonts.dir in the directory in which
fonts are installed (in this case, /usr/share/fonts/japanese/misc ).
This is to ensure that fonts.dir should be 0644 permission.

(There may be the case that a people like me install rpm as a normal user
by sudo, whose umask has 0077. In this case, fonts.dir cannot be read
by another normal user.)

Comment 4 Paul F. Johnson 2006-08-10 21:04:04 UTC
A. I'd still want to see the wrapper and in any case, it needs to be %{_sbindir}

B. No. If you want to ensure the correct permission, use chmod. In anycase,
mkfontdir should be setting the permission correctly on creation. If it isn't,
you need to bugzilla it as it is a problem.

The use of umask is discouraged.

Comment 5 Mamoru TASAKA 2006-08-10 21:39:21 UTC
Okay, from that I checked for xorg-x11-fonts-base, umask treatment
seems no necesarry, removing.

The updated spec and srpm are:
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/efont-unicode-bdf-0.4.2-2.src.rpm
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/efont-unicode-bdf.spec

Comment 6 Paul F. Johnson 2006-08-10 21:59:14 UTC
mkfontdir %{fontdir} && %{_sbindir}/chkfontpath -q -a %{fontdir}

You need to be consistent. This should be

%{_bindir}/mkfontdir

%files
%{fontdir}/*

Should just be

%{fontdir}/

%build
	gzip -9 $g

To use this you need to include BR gzip

Comment 7 Mamoru TASAKA 2006-08-10 22:12:28 UTC
> (In reply to comment #6)
> mkfontdir %{fontdir} && %{_sbindir}/chkfontpath -q -a %{fontdir}
> 
> You need to be consistent. This should be

Oops!
Again updated:
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/efont-unicode-bdf.spec
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/efont-unicode-bdf-0.4.2-3.src.rpm

Comment 8 Mamoru TASAKA 2006-08-10 22:58:02 UTC
Note:

(In reply to comment #6)
> Should just be
> %{fontdir}/

> To use this you need to include BR gzip
These are also fixed in 0.4.2-3.


Comment 9 Mamoru TASAKA 2006-08-14 13:56:36 UTC
My newest srpm is in comment #7.
Paul, would you check it?

Comment 10 Paul F. Johnson 2006-08-14 20:20:41 UTC
Requires(post):	xorg-x11-font-utils, %{_sbindir}/chkfontpath, fontconfig
Requires(postun):	fontconfig

rpmlint will complain that you're using a mixture of spaces and tabs. Use one or
the other.

%define		fontdir		%{_datadir}/fonts/japanese/misc

This is a problem. This directory is already owned by
fonts-japanese-0.20050222-11.1.1.noarch

This means that you can't have

%files
%{fontdir}/

You will need to explicitly define what your package owns.

It seems to build fine, rpmlint complained about the mixed tabs and spaces

Builds cleanly in mock. I'm not going to install it until the ownership problem
is resolved as I already have the japanese fonts rpm installed. Please fix and
resubmit the spec file only.

I'll check that, rebuild and then test the other package from you.

Comment 11 Mamoru TASAKA 2006-08-15 02:14:32 UTC
(In reply to comment #10)

From your option, I came to think that this package (efont-unicode-bdf)
should own its ORIGINAL font directory.

This package doesn't require fonts-japanese, of course. However,
putting the fonts in /usr/share/fonts/japanese/misc will cause problem,
especially when fonts-japanese is removed when this package is installed
because fonts-japanese calls "chkfontpath -q -r /usr/share/fonts/japanese/misc",
which removes the entry of efont-unicode-bdf, too. We must treat
which package of this package and fonts-japanese will be removed first,
which is somewhat troublesome.

So, I moved the font directory from /usr/share/fonts/japanese/misc to
/usr/share/fonts/japanese/%{name} and added some necessary ghost
files. The updated spec file is 
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/efont-unicode-bdf.spec
(0.4.2-4) 

Note: the previous spec file is preserved as
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/efont-unicode-bdf-0.4.2-3.spec

Note: this change affects bug 201170 (jfbterm), so please check if
this change is proper.

Comment 12 Mamoru TASAKA 2006-08-15 05:33:19 UTC
(In reply to comment #10)

> rpmlint will complain that you're using a mixture of spaces and tabs. 

Ah.. rpmlint didn't complain, however, spaces and tabs mixed.
Fixed by
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/efont-unicode-bdf.spec
(0.4.2-5).

Comment 13 Mamoru TASAKA 2006-08-17 10:10:15 UTC
Current srpm is in comment #12.

I hope I can release this srpm.

Comment 14 Paul F. Johnson 2006-08-17 13:11:17 UTC
Sorry - been under the weather recently. I should have this done in the next 24
hours or so.

Comment 15 Paul F. Johnson 2006-08-17 21:23:20 UTC
Good

Builds cleanly in mock and installs fine.
rpmlint shows no problems
Directories are correctly owned
spec file is in american english
no ownership conflicts
Not showing up any duplicates
upstream corresponds with package md5sums
Same version
Correct use of scriptlets
Consistent use of macros

Bad

Should be

Requires: mkfontdir %{_sbindir}/chkfontpath

This is the only issue I can spot, so fix it and it should be good to go

Comment 16 Mamoru TASAKA 2006-08-18 01:22:34 UTC
(In reply to comment #15)
> 
> Requires: mkfontdir %{_sbindir}/chkfontpath

You refer to Requires(post)?

Well, I changed Requires(post) to  
%{_bindir}/mkfontdir, %{_sbindir}/chkfontpath, fontconfig
(fontconfig is for fc-cache) and the fixed spec file is
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/efont-unicode-bdf.spec
(0.4.2-6).

Comment 17 Paul F. Johnson 2006-08-19 20:04:25 UTC
Okay, with that fix, it's good to go. I can't spot anything which breaks any of
the guidelines.

APPROVED

Comment 18 Mamoru TASAKA 2006-08-19 23:13:28 UTC
(In reply to comment #17)
> Okay, with that fix, it's good to go. I can't spot anything which breaks any of
> the guidelines.
> 
> APPROVED

Thank you.
Then I will try to upload this package.
Please check jfbterm, too.


Comment 19 Mamoru TASAKA 2006-08-20 03:51:00 UTC
Okay.

* Build queue for devel requested as id 14365 succeeded.
http://buildsys.fedoraproject.org/logs/fedora-development-extras/14365-efont-unicode-bdf-0.4.2-6.fc6/
* SyncNeeded is requested for FE-5 branch.

Now I close this as CLOSED NEXTRELEASE. Thank you for
reviewing this package!!


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