Bug 465928 - Review Request: fbterm - a fast terminal emulator for linux with frame buffer device
Review Request: fbterm - a fast terminal emulator for linux with frame buffer...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-07 03:27 EDT by Ding-Yi Chen
Modified: 2008-10-29 23:15 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-29 23:15:35 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


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

  None (edit)
Description Ding-Yi Chen 2008-10-07 03:27:48 EDT
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 00:38:04 EDT
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 02:19:06 EDT
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 03:16:04 EDT
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 03:18:46 EDT
Created attachment 320522 [details]
Patch to fix headers
Comment 5 Ding-Yi Chen 2008-10-16 03:58:03 EDT
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 05:12:38 EDT
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-16 20:52:37 EDT
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 00:03:50 EDT
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 18:47:56 EDT
cvs done.
Comment 10 Parag AN(पराग) 2008-10-29 02:36:52 EDT
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.