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 201170

Summary: Review Request: jfbterm - Japanese Console for Linux Frame Buffer Device
Product: [Fedora] Fedora Reporter: Mamoru TASAKA <mtasaka>
Component: Package ReviewAssignee: Paul F. Johnson <paul>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhide   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-08-21 02:58:41 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: 202032    
Bug Blocks: 163779    
Attachments:
Description Flags
log of rebuilding in mock on i386
none
build log of jfbterm in mock none

Description Mamoru TASAKA 2006-08-03 11:49:59 UTC
Spec URL: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/jfbterm.spec
SRPM URL: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/jfbterm-0.4.7-0.15.fc6.src.rpm
Description: 
JFBTERM/ME takes advantages of framebuffer device that is 
supported since linux kernel 2.2.x (at least on ix86 architecture) 
and make it enable to display multilingual text on console. 
It is developed on ix86 architecture, and it will works on 
other architectures such as linux/ppc.

Features:
   * It works with framebuffer device instead of VGA.
   * It supports pcf format font
   * It is not so fast because it doesn't take any advantages 
     of accelaration.
   * It also support coding systems other than ISO-2022, 
     such as SHIFT-JIS by using iconv(3).
   * It is userland program.

Comment 1 Mamoru TASAKA 2006-08-03 11:52:43 UTC
Created attachment 133548 [details]
log of rebuilding in mock on i386

Log of rebuilding src.rpm in mock on i386.

Result of rpmlint:
jfbterm-0.4.7-0.15.fc6.i386.rpm:

jfbterm-0.4.7-0.15.fc6.src.rpm:
W: jfbterm strange-permission jfbterm-0.4.7-configure-header.patch 0600
W: jfbterm strange-permission jfbterm-0.4.7-hang-onexit.patch 0600
W: jfbterm strange-permission jfbterm-0.4.7-mmap-newkernel.patch 0600
W: jfbterm strange-permission jfbterm-0.4.7-remove-warning.patch 0600
W: jfbterm strange-permission jfbterm-0.4.7-userspace.patch 0600
W: jfbterm strange-permission jfbterm-0.4.7.tar.gz 0600
W: jfbterm strange-permission efont-unicode-bdf-0.4.2.tar.bz2 0600
W: jfbterm mixed-use-of-spaces-and-tabs

jfbterm-debuginfo-0.4.7-0.15.fc6.i386.rpm:

Comment 2 Mamoru TASAKA 2006-08-03 11:58:56 UTC
Note:

I will bump the release number when this src.rpm is approved.

Comment 3 Paul F. Johnson 2006-08-10 12:43:07 UTC
You will need to fix the permissions and mixed use of spaces and tabs

Not sure about all the stuff at the top for determining the distro used - there
seems to be alot in there for pre FC4. As FC4 is now officially in legacy, are
you sure you need to go back that far?

This is release 1, not 0

efont is another package and should be submitted as such. You then use
R:<package_name> (or BR if you need to use it to build).

[ %{buildroot} != '/' ] || exit 1 isn't required

You will also need BR : iconv

What you need to do before a full review occurs is this

1. Decide how far back you want this package to go. If you really want it to go
back prior to FC4, I would recommend either seeking advice on the Fedora Extras
list or seeing what problems you will encounter as the distros get older and
updates don't happen.

2. If you only support FC5 and FC6, you'll need to ditch a lot of the spec file

3. Separate out the fonts and source package and make this package drag in the
fonts. Remember, other packages can make use of the fonts. You will need to
submit that as another package for review and put a blocker on this package as
it relies on the fonts package.

4. Fix the permissions - they won't be let through unless they're correctly set

5. Add BR iconv

6. Add BR whatever provides tic

Comment 4 Mamoru TASAKA 2006-08-10 12:57:26 UTC
Paul, thank you for (pre-?) reviewing.

I will try to fix as requested by you quickly as I want to have this
package released soon.

However:
for 4: Of what files do you refer the permissions are incorrect?
for 5: iconv is in glibc-common so BR for iconv is not needed, I think
for 6: BR tor tic (ncurses) is already.
NOTE: this package is built cleanly in mock. 

Comment 5 Paul F. Johnson 2006-08-10 13:08:04 UTC
For 4 : The patches - rpmlint showed them. They need to be 0644

5 and 6 - not a problem. For some reason, iconv always stuck in my head as being
a package by itself!

Comment 6 Mamoru TASAKA 2006-08-10 13:16:25 UTC
Okay, now I am trying separate fonts from jfbterm quickly.
When it is done, I will report it and block this bug.

Comment 7 Mamoru TASAKA 2006-08-10 14:09:12 UTC
Oh!!
Now font package is split:
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-1.src.rpm

http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/jfbterm.spec
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/jfbterm-0.4.7-1.src.rpm

Currently, I cannot submit new review request: I encounter:
Software error:

DBD::Pg::st execute failed: ERROR:  column "everconfirmed" specified more than once
 [for Statement "INSERT INTO bugs
(version,rep_platform,bug_severity,priority,op_sys,assigned_to,bug_status,bug_file_loc,short_desc,target_milestone,resolution,qa_contact,everconfirmed,product_id,component_id,
reporter, creation_ts, everconfirmed, estimated_time, remaining_time)VALUES
('devel','All','normal','normal','Linux','193505','NEW','','Review Request:
efont-unicode-bdf: Unicode font by Electronic Font Open
Laboratory','---','','197703','1','59','16465',166855, NOW(), 1, 0, 0)"] at
Bugzilla/DB.pm line 71
	Bugzilla::DB::SendSQL('INSERT INTO bugs
(version,rep_platform,bug_severity,priority,...') called at Bugzilla/Bug.pm line
1154
	Bugzilla::Bug::Add('Bugzilla::Bug', 'HASH(0x1376c80)') called at
/var/www/html/bugzilla/post_bug.cgi line 109

For help, please send mail to the webmaster (bugzilla-owner), giving
this error message and the time and date of the error. 


Comment 8 Mamoru TASAKA 2006-08-10 14:12:05 UTC
Note:
I am continuing to submit new package review request (efont-unicode-bdf)

Both (efont-unicode-bdf and jfbterm) now bear no rpmlint complaint
(for both src.rpm and binary).

Comment 9 Paul F. Johnson 2006-08-10 14:15:19 UTC
Remember to add a blocker on this bug with the bugzilla reference for the new
package (this one will rely on that one).

As soon as it's in, I'll add it to my review list.

Comment 10 Mamoru TASAKA 2006-08-10 14:33:15 UTC
Currently, I cannot submit new FE review request:
I mailed this problem to bugzilla-owner and
am waiting for reply.

Once I am able to sumbit new request, I will make the new
bug block this one. 

Comment 11 Mamoru TASAKA 2006-08-10 14:39:37 UTC
Okay. I suceeded in submitting new FE review request (efont-unicode-bdf)
Now, this bug is blocked by bug 202032.

Comment 12 Paul F. Johnson 2006-08-10 15:06:18 UTC
Just chewing over the spec - I've not built it yet (my remote machine connection
is down)

The whole font section in %prep - is this actually required for building the
package?

The calls to aclocal et al. Normally, the configure system calls them (unless
the configure scripts have been messed with and need regenerating). The website
certainly says that it's just a matter of running configure/make/make install

On the make line, is there a reason for not using _smp_mflags? If there is (for
example, it won't build), could you please add a comment to that effect?

%{_mandir}/man*/*

No. If it needs man1/package-name, then call it that. There are not normally
that many man files to install and using /man*/* takes an incorrect ownership. A
quick way if there are many files are to glob them together viz

%{_mandir}/man1/foo*
%{_mandir}/man4/bar*

Can you please check them and if just upload a new spec file?

Comment 13 Mamoru TASAKA 2006-08-10 15:26:10 UTC
A. >The whole font section in %prep - is this actually required for building the
package?

not in build stage, however, make install requires these fonts.

B. aclocal and so on...
A patch is for Makefile.am. This is for removing sticky bit on /usr/bin/jfbterm
binary. So perhaps aclocal and so on is needed, I think.

C. man entry
Split. new spec file and srpm is now uploaded.
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/jfbterm.spec
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/jfbterm-0.4.7-2.src.rpm

Comment 14 Mamoru TASAKA 2006-08-10 15:28:42 UTC
Oh:
D. _smp_mflags

I didn't try it. Now I added this and succeeded in building on
2.6.17-1.2157_FC5smp (_smp_mflags is -j3).

This fix is included in 0.4.7-2.

Comment 15 Patrice Dumas 2006-08-10 16:06:50 UTC
> A patch is for Makefile.am. This is for removing sticky bit on /usr/bin/jfbterm
> binary. So perhaps aclocal and so on is needed, I think.

It would be better to patch Makefile.in instead if possible (I
haven't looked at that precise example, but from your explanation
it seems so).
And, if possible, you could report upstream that making the setuid
bit should be in a specific make target, or it should be checked
the the user running make is root, since otherwise it prevents
staged installs (like we do for rpms).


Comment 16 Mamoru TASAKA 2006-08-10 16:44:15 UTC
(In reply to comment #15)
> It would be better to patch Makefile.in instead if possible (I
> haven't looked at that precise example, but from your explanation
> it seems so).

Okay, another attempt to fix Makefile by patching against Makefile.in
is done on
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/jfbterm-0.4.7-3.src.rpm
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/jfbterm.spec

This removed the necessity of automake, aclocal.
Note: the previous spec file is preserved on
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/jfbterm-0.4.7-2.spec

Comment 17 Mamoru TASAKA 2006-08-10 16:46:21 UTC
(In reply to comment #12)

> The whole font section in %prep - is this actually required for building the
> package?
> 

In jfbterm-0.4.7-3, copying fonts section is moved after %{__make}.

Comment 18 Mamoru TASAKA 2006-08-10 19:18:36 UTC
Created attachment 133971 [details]
build log of jfbterm in mock 

Build log of jfbterm-0.4.7-3.fc5 in mock on fc5 system.
Note that efont-unicode-bdf is installed from LOCAL.

rpmlint shows no errors.
jfbterm-0.4.7-3.fc5.i386.rpm:

jfbterm-0.4.7-3.fc5.src.rpm:

jfbterm-debuginfo-0.4.7-3.fc5.i386.rpm:

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

Comment 20 Mamoru TASAKA 2006-08-15 05:46:23 UTC
bug 202032 (efont-unicode-bdf):

Change in efont-unicode-bdf (font path change) affects jfbterm packaging.
Please check bug 202032 before this.

Comment 21 Paul F. Johnson 2006-08-17 21:01:48 UTC
#20 : 

This is probably why I'm seeing 

cp: cannot stat `/usr/share/fonts/japanese/misc/b16.pcf.gz': No such file or
directory

Can you fix the spec file and just upload that?

Comment 22 Mamoru TASAKA 2006-08-18 01:27:35 UTC
(In reply to comment #21)
> #20 : 
> cp: cannot stat `/usr/share/fonts/japanese/misc/b16.pcf.gz': No such file or
> directory

Yes, this is because I changed the directory in efont-unicode-bdf, is it
okay?
The spec of jfbterm with efont-unicode-bdf > 0.4.2-5 is:
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/jfbterm.spec
(0.4.7-4)
This should fix the error on the comment #20.

Comment 23 Paul F. Johnson 2006-08-20 10:29:58 UTC
Builds fine normally and rpmlint is clean. I am a tad concerned with this
snippet though and have asked for advice on it. There may be both a security and
SELinux implication with it.

8-->
%{__cat} > 60-jfbterm.perms <<EOF
# permission definitions
<console> 0660 /dev/tty0    0660 root
<console> 0600 /dev/console 0600 root
EOF

%{__mkdir_p} -m 755 %{buildroot}%{_sysconfdir}/security/console.perms.d
%{__install} -m 644 60-jfbterm.perms \
   %{buildroot}%{_sysconfdir}/security/console.perms.d/
<--8

As for the spec file...

Good
Follows FE dist tags
No duplicates in the BRs
In american english and with correct permissions
No ownership conflicts
Includes docs
Set out logically
It looks good.

Unsure
Are you installing fonts to %{_datadir}/fonts/%{name}? if you are, you'll need
to do what you did with fonts package this relies on with mkfontdir

Very unsure
Security implications (detailed above)

Builds cleanly in mock

Comment 24 Mamoru TASAKA 2006-08-20 12:11:42 UTC
Before fixing spec file:

(In reply to comment #23)
> Builds fine normally and rpmlint is clean. I am a tad concerned with this
> snippet though and have asked for advice on it. There may be both a security and
> SELinux implication with it.
> 
> 8-->
> %{__cat} > 60-jfbterm.perms <<EOF
> # permission definitions
> <console> 0660 /dev/tty0    0660 root
> <console> 0600 /dev/console 0600 root
> EOF
> 
> %{__mkdir_p} -m 755 %{buildroot}%{_sysconfdir}/security/console.perms.d
> %{__install} -m 644 60-jfbterm.perms \
>    %{buildroot}%{_sysconfdir}/security/console.perms.d/
> <--8
> 
> 
> Very unsure
> Security implications (detailed above)

This application (/usr/bin/jfbterm) needs device access right for
/dev/console and /dev/tty0. So usual compilation of jfbterm
sets sticky bit on /usr/bin/jfbterm, with the permission 4755 like
/usr/bin/kon (in kon2-0.3.9b-26.2.1 rpm) With stilly bit, installing
60-jfbterm.perms is not necessary.
Note: kon cannot deal with frame buffer.
Note: pam has /etc/security/console.perms.d/50-default.perms

Original packager (Hideki Machida) and me concluded that it may be
better that we use console.perms method than use sticky bit.
What do you think of this? Umm. I don't know well about SELINUX....

> Are you installing fonts to %{_datadir}/fonts/%{name}? 
jfbterm requires some fonts (in install stage and on the "real use"), 
however, these fonts are actually the copies of fonts in other
packages (in fonts-japanese, xorg-x11-fonts-XXXXX, and fonts-japanese)

Would it be better that I use only the symlink against that fonts?
Doing so requires a bit of trick on install stage.

Comment 25 Paul F. Johnson 2006-08-20 14:02:52 UTC
Okay, I've had the advice back and it seems like there is nothing untoward
security wise.

If the package is making a copy of fonts, I'd prefer a symlink rather than
bloating the system up with needless copies. A more favourable solution though
would be to have the application just use the fonts already there.

Comment 26 Mamoru TASAKA 2006-08-20 15:48:27 UTC
(In reply to comment #25)
> Okay, I've had the advice back and it seems like there is nothing untoward
> security wise.
> 

Okay, then I use console.perms method.

> If the package is making a copy of fonts, I'd prefer a symlink rather than
> bloating the system up with needless copies. A more favourable solution though
> would be to have the application just use the fonts already there.
Umm... It seems that I have to install fonts required to /usr/share/fonts/jfbterm .
So, I changed so that requires fonts are installed as symlinks to
that directory.

The fixed spec and srpm are:
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/jfbterm.spec
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/jfbterm-0.4.7-6.src.rpm

rpmlint says than symlink should be relative (not absolute), whic made
spec file a bit complex. This spec file (0.4.7-6) leaves rpmlint messages:

W: jfbterm dangling-relative-symlink /usr/share/fonts/jfbterm/shnmk16.pcf.gz
../japanese/misc/shnmk16.pcf.gz
W: jfbterm dangling-relative-symlink /usr/share/fonts/jfbterm/shnm8x16r.pcf.gz
../japanese/misc/shnm8x16r.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-14.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-14.pcf.gz
W: jfbterm dangling-relative-symlink /usr/share/fonts/jfbterm/b16.pcf.gz
../japanese/efont-unicode-bdf/b16.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-9.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-9.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-7.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-7.pcf.gz
W: jfbterm dangling-relative-symlink /usr/share/fonts/jfbterm/8x16.pcf.gz
../../X11/fonts/misc/8x16.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-5.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-5.pcf.gz
W: jfbterm dangling-relative-symlink /usr/share/fonts/jfbterm/hanglg16.pcf.gz
../../X11/fonts/misc/hanglg16.pcf.gz
W: jfbterm dangling-relative-symlink /usr/share/fonts/jfbterm/gb16fs.pcf.gz
../../X11/fonts/misc/gb16fs.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/jisksp16-1990.pcf.gz ../japanese/misc/jisksp16-1990.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-8.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-8.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-1.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-1.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-2.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-2.pcf.gz
W: jfbterm dangling-relative-symlink
/usr/share/fonts/jfbterm/8x13-ISO8859-15.pcf.gz
../../X11/fonts/misc/8x13-ISO8859-15.pcf.gz

.... I cannot help but leave this because the "real" fonts are
in other packages. I added some font packages to Require.



Comment 27 Paul F. Johnson 2006-08-20 15:58:06 UTC
Can you not do
        
        ln -s %{buildroot}%{_datadir}/fonts/jfbterm/shnmk16.pcf.gz
        %{_datadir}/fonts/japanese/misc/shnmk16.pcf.gz
        
?
        
That should avoid the dangling problem (that said, the dangling isn't
that big a worry from what I can see)

Comment 28 Mamoru TASAKA 2006-08-20 16:07:38 UTC
ln <source> <dest>

You mean 
ln -s %{_datadir}/fonts/japanese/misc/shnmk16.pcf.gz \
   %{buildroot}%{_datadir}/fonts/jfbterm/shnmk16.pcf.gz ?

Anyway, %{_datadir}/fonts/japanese/misc/shnmk16.pcf.gz is not in
jfbterm but in fonts-japanese and this cause
dangling-symlink (not dangling-"relative"-symlink).
Also, rpmlint warn about "symlink-should-be-absolute"
(although I think that absolute symlink can be allowed......)

Comment 29 Paul F. Johnson 2006-08-20 20:16:26 UTC
It's built cleanly in mock and rpmlint said nothing when I ran the rpms past it,
so that's find.

Review time

consistent use of macros
no ownership problems
documentation included
no dupes in the installed rpms
builds cleanly in mock
no devel, so no problems with pkgconfig, .so/la etc
software works when run
no security problems (according to advice)
installed rpm has no missing requires

I'm happy for this to go

APPROVED

Comment 30 Mamoru TASAKA 2006-08-21 02:58:41 UTC
(In reply to comment #29)
> I'm happy for this to go
> 
> APPROVED

Thank you!!

* Finally released as 0.4.7-7 to fix the mixed use of %{build_root}
  and $RPM_BUILD_ROOT and the compilation problem on ppc
  (now 0.4.7-7 can be built on i386, x86_64, ppc).
http://buildsys.fedoraproject.org/build-status/job.psp?uid=14387
* SyncNeeded is requiested for FE-5.

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