This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 202032 - Review Request: efont-unicode-bdf: Unicode font by Electronic Font Open Laboratory
Review Request: efont-unicode-bdf: Unicode font by Electronic Font Open Labor...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul F. Johnson
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 201170
  Show dependency treegraph
 
Reported: 2006-08-10 10:35 EDT by Mamoru TASAKA
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-19 23:51:00 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)
build log of efont-unicode-bdf in mock (38.55 KB, text/plain)
2006-08-10 15:15 EDT, Mamoru TASAKA
no flags Details

  None (edit)
Description Mamoru TASAKA 2006-08-10 10:35:09 EDT
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 15:15:40 EDT
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 16:27:49 EDT
%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 16:54:03 EDT
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 17:04:04 EDT
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 17:39:21 EDT
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 17:59:14 EDT
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 18:12:28 EDT
> (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 18:58:02 EDT
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 09:56:36 EDT
My newest srpm is in comment #7.
Paul, would you check it?
Comment 10 Paul F. Johnson 2006-08-14 16:20:41 EDT
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-14 22:14:32 EDT
(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 01:33:19 EDT
(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 06:10:15 EDT
Current srpm is in comment #12.

I hope I can release this srpm.
Comment 14 Paul F. Johnson 2006-08-17 09:11:17 EDT
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 17:23:20 EDT
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-17 21:22:34 EDT
(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 16:04:25 EDT
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 19:13:28 EDT
(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-19 23:51:00 EDT
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.