Bug 465928 - Review Request: fbterm - a fast terminal emulator for linux with frame buffer device
Summary: Review Request: fbterm - a fast terminal emulator for linux with frame buffer...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-10-07 07:27 UTC by Ding-Yi Chen
Modified: 2008-10-30 03:15 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-30 03:15:35 UTC
Type: ---
Embargoed:
panemade: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
Patch to fix headers (295 bytes, patch)
2008-10-16 07:18 UTC, Parag AN(पराग)
no flags Details | Diff

Description Ding-Yi Chen 2008-10-07 07:27:48 UTC
Spec URL: http://dchen.fedorapeople.org/files/rpms/fbterm.spec
SRPM URL: http://dchen.fedorapeople.org/files/rpms/fbterm-1.1-0.fc9.src.rpm

Description:
FbTerm is a fast terminal emulator for linux with frame buffer device. 
Features include: 
- mostly as fast as terminal of linux kernel while accelerated scrolling
  is enabled on framebuffer device 
- select font with fontconfig and draw text with freetype2, same as 
  Qt/Gtk+ based GUI apps 
- dynamicly create/destroy up to 10 windows initially running default
  shell 
- record scrollback history for every window 
- auto-detect text encoding with current locale, support double width 
  scripts like  Chinese, Japanese etc 
- switch between configurable additional text encodings with hot keys
  on the fly 
- copy/past selected text between windows with mouse when gpm server 
  is running

Comment 1 Parag AN(पराग) 2008-10-10 04:38:04 UTC
rpmlint gave me
fbterm.i386: E: setuid-binary /usr/bin/fbterm root 04755
The file is setuid, this may be dangerous, especially if this  file is setuid
root.

fbterm.i386: E: non-standard-executable-perm /usr/bin/fbterm 04755
A standard executable should have permission set to 0755. If you get this
message, it means that you have a wrong executable permissions in some files
included in your package.

also, I don't see any need of following in SPEC
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig

package failed to build on dist-f10 see
http://koji.fedoraproject.org/koji/taskinfo?taskID=872504

Can you update SRPM with above fixes?

Comment 2 Ding-Yi Chen 2008-10-16 06:19:06 UTC
Hi,
Still not buildable with F-10, other issues are fixed.

SPEC: http://dchen.fedorapeople.org/files/rpms/fbterm.spec
SRPM: http://dchen.fedorapeople.org/files/rpms/fbterm-1.1-1.fc9.src.rpm

Regards,
Ding-Yi Chen

Comment 3 Parag AN(पराग) 2008-10-16 07:16:04 UTC
Thanks to David Woodhouse for his help on IRC to fix F10 build issue. Actually problem is fbterm.cpp is using kernel header file 
#include <linux/signalfd.h>
which should be
#include <sys/signalfd.h>

Comment 4 Parag AN(पराग) 2008-10-16 07:18:46 UTC
Created attachment 320522 [details]
Patch to fix headers

Comment 5 Ding-Yi Chen 2008-10-16 07:58:03 UTC
Thanks for the patch, it works.
The revised SPEC and SRPM:
SPEC: http://dchen.fedorapeople.org/files/rpms/fbterm.spec
SRPM: http://dchen.fedorapeople.org/files/rpms/fbterm-1.1-2.fc9.src.rpm

Regards,

Comment 6 Parag AN(पराग) 2008-10-16 09:12:38 UTC
Review:
+ package builds in mock (rawhide i386).
koji build => http://koji.fedoraproject.org/koji/taskinfo?taskID=883692
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
f97c7a403fa0895349809c8d18355cbc  fbterm-1.1.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc files present.
+ BuildRequires are proper.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Not a GUI App.


Suggestions:-
1) I see following in build.log
configure: WARNING:     gpm.h dosn't exist! gpm mouse support will be disabled!
If you want you can add that support.
==> you can do that by adding BR:gpm-devel in SPEC

2) drop line in %build
CFLAGS="-D__GNUC__" ; export CFLAGS

3) defattr usage should be
%defattr(-,root,root,-)

4) Drop unnecessary file INSTALL from %docs

Make sure to fix above issues before committing to CVS.

APPROVED.

Comment 7 Ding-Yi Chen 2008-10-17 00:52:37 UTC
Thanks so much for helping me to fix the issues.

The revise spec and srpm which address your latest comments are at:
SPEC: http://dchen.fedorapeople.org/files/rpms/fbterm.spec
SRPM: http://dchen.fedorapeople.org/files/rpms/fbterm-1.1-3.fc9.src.rpm

Regards,
Ding-Yi Chen

Comment 8 Ding-Yi Chen 2008-10-17 04:03:50 UTC
New Package CVS Request
=======================
Package Name: fbterm
Short Description: a fast terminal emulator for linux with frame buffer device
Owners: dchen
Branches: F-9 F-10 EL-5
InitialCC:

Comment 9 Kevin Fenzi 2008-10-19 22:47:56 UTC
cvs done.

Comment 10 Parag AN(पराग) 2008-10-29 06:36:52 UTC
can we close this if package is built for all requested branches?


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