Bug 812574 - (BitchX) Review Request: BitchX - IRC chat client
Review Request: BitchX - IRC chat client
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-14 21:24 EDT by Dan Mashal
Modified: 2012-07-23 19:11 EDT (History)
4 users (show)

See Also:
Fixed In Version: BitchX-1.2-5.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-05-11 06:25:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rdieter: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Dan Mashal 2012-04-14 21:24:43 EDT
Spec URL: http://vicodan.fedoraproject.org/BitchX.spec
SRPM URL: http://vicodan.fedoraproject.org/BitchX-1.2-1.fc17.src.rpm
Description: BitchX: The ultimate IRC client
Comment 1 Dan Mashal 2012-04-14 21:26:00 EDT
Sorry correct URLs are here:

Spec URL: http://vicodan.fedorapeople.org/BitchX.spec
SRPM URL: http://vicodan.fedorapeople.org/BitchX-1.2-1.fc17.src.rpm
Description: BitchX: The ultimate IRC client
Comment 2 Rex Dieter 2012-04-14 21:34:47 EDT
I can do the review/sponsorship.
Comment 3 Peter Lemenkov 2012-04-15 13:22:17 EDT
Why this review request is restricted?
Comment 4 Rex Dieter 2012-04-16 13:37:16 EDT
marking unrestricted
Comment 5 Rex Dieter 2012-04-16 13:54:59 EDT
scratch build:
http://vicodan.fedorapeople.org/BitchX-1.2-1.fc17.src.rpm

rpmlint:
BitchX.x86_64: E: devel-dependency ncurses-devel
BitchX.x86_64: W: incoherent-version-in-changelog 1.2 ['1.2-1.fc17', '1.2-1']
BitchX.x86_64: W: invalid-url URL: http://www.BitchX.org/ <urlopen error [Errno -2] Name or service not known>
BitchX.x86_64: W: spurious-executable-perm /usr/lib64/bx/help/6_Functions/open.bz
... (bazillions of these spurious-executable-perm warnings)
...
BitchX.x86_64: E: script-without-shebang /usr/lib64/bx/script/auto_resume
BitchX.x86_64: W: no-manual-page-for-binary BitchX-1.2c01-svn
BitchX.x86_64: W: no-manual-page-for-binary scr-bx



naming: ok

1.  MUST remove .spec tags:
Vendor: BitchX
Packager: Dan Mashal <vicodan@fedoraproject.org>

See:
https://fedoraproject.org/wiki/Packaging/Guidelines#Tags


2.  MUST remove seemingly erroneous
Requires: ncurses ncurses-devel openssl xmlsec1-openssl-devel
runtime shlib dependencies should get calculated automatically by rpm, no need to list these by hand.  -devel runtime deps are almost certainly wrong here too.

license: NOT ok
3.  licensecheck. reveals many sources to be GPLv2+ and many without license attribution at all.  Top level directory contains a COPYRIGHT file containing BSD licence.   Can probably infer that all files without contrary licensing details are indeed BSD.  So, you have a choice, to either use
MUST: change to License: GPLv2+
or
MUST: change to License: BSD and GPLv2+
depending on how pedantic you want to be.

SHOULD: poke upstream to add license attribution to all source files (or at least the .c, .h ones).  For example,
/*
 * wserv.c - little program to be a pipe between a screen or
 * xterm window to the calling ircII process.
 *
 * Written by Troy Rollo, Copyright 1992 Troy Rollo
 * Finished by Matthew Green, Copyright 1993 Matthew Green
 * Support for 4.4BSD by Jeremy Nelson, Copyright 1997 EPIC Software Labs
 *
 * Works by opening up the unix domain socket that ircII bind's
 * before calling wserv, and which ircII also deleted after the
 * connection has been made.
 */

4.  As this is a GUI app, 
MUST: add a .desktop file so it appears in DE menus
See: https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files


macros: ok

scriptlets: n/a


So, looks like we've 4 MUST fix items in need of some love and fixing before I can approve this.
Comment 6 Dan Mashal 2012-04-16 15:44:45 EDT
Rex,

Please re-review. 

Per our IRC conversation this is not a GUI app. 

Removed:

1) Vendora and Packager tags from spec file
2) Removed -devel requirements from spec file
3) Updated License 

Also please note at this time BitchX.org is down so I have updated the spec file to use fedorapeople.org.

Thanks,
Dan
Comment 7 Rex Dieter 2012-04-17 11:03:19 EDT
OK, for documentation purposes, this latest .spec has some other issues or gressed in a couple of ways:

2. still not completely fixed, still has unnecessary Requires:
5. source tarball has precompiled .o files
6. BuildRequires: compat-gcc34 added, not needed (or used) here
Comment 8 Dan Mashal 2012-04-18 04:41:48 EDT
Removed Requires: tag from spec.
Removed .o files from tarball
Removed compat-gcc34 from BuildRequires.

Please let me know if any other issues.
Comment 9 Rex Dieter 2012-04-19 07:59:44 EDT
On other thing I mentioned in irc, but not explicitly here.  Whenever making changes, please bump Release.
Comment 10 Rex Dieter 2012-04-19 08:05:07 EDT
7.  MUST %{_libdir}/bx/ dir is unowned, replace
%{_libdir}/bx/*
with
%{_libdir}/bx/
(or equivalent)

5.  is seems not yet fixed, latest srpm I just tried,
http://vicodan.fedorapeople.org/BitchX-1.2-1.fc17.src.rpm
still includes a bunch of .o files
Comment 11 Dan Mashal 2012-04-19 14:26:37 EDT
Updated spec files per your request, tarball source RPM, change log, release version.
Comment 12 Rex Dieter 2012-04-24 21:03:44 EDT
Scratch builds fail, looks like we're missing
BuildRequires: openssl-devel

after adding that, still getting an odd failure on i686:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4020520

I'll have to look deeper.
Comment 13 Dan Mashal 2012-04-26 01:59:05 EDT
Try one of these..

From the original BitchX.spec:

BuildRequires: ncurses ncurses-devel openssl xmlsec1-openssl-devel glib2-devel compat-gcc-34 compat-gcc-34-c++
Requires: ncurses openssl glibc
Comment 14 Dan Mashal 2012-04-26 02:06:35 EDT
I just built with the latest source and spec on Fedora 17 i686.

http://vicodan.fedorapeople.org/BitchX-1.2-4.fc17.i686.rpm
http://vicodan.fedorapeople.org/BitchX-1.2-4.fc17.srpm

Also make sure you have the latest source (it has changed)

http://vicodan.fedorapeople.org/BitchX1.2.tar.gz
Comment 16 kevin_redhat 2012-04-26 02:57:15 EDT
The build failure is trying to link x86-64 .o files into a x86 build.  Note for example that wterm.o is *not* being built, so it must already have been present - was that the older .tar.gz which included some .o files? Dan seems to have fixed that.
Comment 17 Rex Dieter 2012-04-26 09:11:36 EDT
Per comment #12,

still missing
BuildRequires: openssl-devel

and fwiw, no need for 
BuildRequires: ncurses openssl

after adding the missing BuildRequires to your latest src.rpm, here's another try at a koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4024686
Comment 18 Rex Dieter 2012-04-26 09:14:01 EDT
ok, sans my .spec typo (BR: openssl-openssl, haha), another one:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4024692
Comment 19 Rex Dieter 2012-04-26 15:43:37 EDT
ok, I think all my MUST fix items have been resolved, let's consider this good and APPROVED.

I'll leave it to you to add
BuildRequires: openssl-devel
so that it builds ok.

next step is to request scm access,
https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner
Comment 20 Dan Mashal 2012-04-26 17:01:08 EDT
New Package SCM Request
=======================
Package Name: BitchX 
Short Description: The Ultimate IRC client.
Owners: vicodan
Branches: f15 f16 f17
InitialCC: rdieter
Comment 21 Gwyn Ciesla 2012-04-26 22:13:04 EDT
Git done (by process-git-requests).
Comment 22 Dan Mashal 2012-04-26 22:17:42 EDT
Thanks Jon!
Comment 23 Fedora Update System 2012-04-27 01:02:47 EDT
BitchX-1.2-4.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/BitchX-1.2-4.fc17
Comment 24 Fedora Update System 2012-04-27 01:07:27 EDT
BitchX-1.2-4.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/BitchX-1.2-4.fc16
Comment 25 Fedora Update System 2012-04-27 01:07:37 EDT
BitchX-1.2-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/BitchX-1.2-4.fc15
Comment 26 Fedora Update System 2012-04-30 21:40:34 EDT
BitchX-1.2-5.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/BitchX-1.2-5.fc16
Comment 27 Fedora Update System 2012-04-30 21:40:43 EDT
BitchX-1.2-5.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/BitchX-1.2-5.fc17
Comment 28 Fedora Update System 2012-04-30 21:40:52 EDT
BitchX-1.2-5.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/BitchX-1.2-5.fc15
Comment 29 Fedora Update System 2012-05-01 17:33:14 EDT
Package BitchX-1.2-5.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing BitchX-1.2-5.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-7046/BitchX-1.2-5.fc17
then log in and leave karma (feedback).
Comment 30 Fedora Update System 2012-05-11 06:25:22 EDT
BitchX-1.2-5.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 31 Fedora Update System 2012-05-11 06:28:21 EDT
BitchX-1.2-5.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 32 Fedora Update System 2012-05-26 04:01:11 EDT
BitchX-1.2-5.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 33 Dan Mashal 2012-07-23 17:39:25 EDT
Package Change Request
======================
Package Name: BitchX
New Branches: el5 el6
Owners: vicodan
Comment 34 Gwyn Ciesla 2012-07-23 19:11:25 EDT
Git done (by process-git-requests).

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