Bug 1986844 - Review Request: ircii - Popular Unix Irc client
Summary: Review Request: ircii - Popular Unix Irc client
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2021-07-28 13:06 UTC by Aníbal Monsalve Salazar
Modified: 2023-04-30 13:22 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Aníbal Monsalve Salazar 2021-07-28 13:06:53 UTC
Spec URL: http://v7w.com/anibal/ircii/ircii.spec
SRPM URL: http://v7w.com/anibal/ircii/ircii-20210314-1.src.rpm
Description: This is a popular Internet Relay Chat (IRC) client. It is a program used to connect to IRC servers around the globe so that the user can ``chat'' with others.
Fedora Account System Username: anibal

Web pages: 
  https://accounts.fedoraproject.org/user/anibal/
  https://fedoraproject.org/wiki/User:Anibal

Other email addresses: 
  anibal
  anibal

Comment 1 Gustavo Costa 2021-07-28 19:40:05 UTC
Hi Aníbal. I'm not a packager yet, but I have some things to point out about your spec file:

* The %{?dist} tag should be used in the Release field [1]

Release:        1%{?dist}

* The upstream points to ircii.warped.com to get the source code [2]

Source:         http://ircii.warped.com/%{name}-%{version}.tar.gz

* Group, Vendor, and Packager fields should be not used [3]

* Since gcc, make, ncurses, and openssl is used to build ircii, you have to use them

BuildRequires:  gcc
BuildRequires:  make
BuildRequires:  ncurses-devel
BuildRequires:  openssl-devel

* There are macros for configure and make build, you can use them

%configure --mandir=%{buildroot}/%{_mandir}
%make_build

Note that "--mandir=%{buildroot}/%{_mandir}" is used because the upstream expect this [4]

* In the build section, you can use the make_install macro

%make_install

* By using the macros above, the files will be installed in the right place (prefix=/usr) [5]. You should use macros like _bindir and datadir to install the files. You can also use glob

%{_bindir}/irc
%{_bindir}/irc-%{version}
%{_bindir}/ircbug
%{_bindir}/ircflush
%{_libexecdir}/ircio
%{_libexecdir}/wserv
%{_datadir}/irc/*
%{_mandir}/man1/irc.1*
%{_mandir}/man1/ircII.1*
%{_mandir}/man1/ircbug.1*


* You have to use the changelog field to report your changes in the spec file [6]

%changelog
* Wed Jul 28 2021 Anibal Monsalve <anibal> - 20210314-1
- Initial package


References
----------

1. https://docs.fedoraproject.org/en-US/packaging-guidelines/DistTag/
2. "You can fetch ircII from the following places: America [main site] (San Jose, CA) http://ircii.warped.com/" http://www.eterna.com.au/ircii
3. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections
4. "@mandir@ tends to be $datarootdir which already has $DESTDIR" http://ircii.warped.com/ircii-current/ircii/Makefile.in
5. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_no_files_or_directories_under_srv_usrlocal_or_homeuser
6. https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs

The final spec file: https://gist.githubusercontent.com/xfgusta/148cc68fdbcfa70d2bc582d404c7f3fa/raw/9c0361a8f0e862c7fb4255f16a0b381187d2d451/ircii.spec

Comment 2 Aníbal Monsalve Salazar 2021-07-29 05:36:06 UTC
Gustavo,

Thank you for the review.

New spec and srpm files are at:

http://v7w.com/anibal/ircii/ircii.spec
http://v7w.com/anibal/ircii/ircii-20210314-1.fc34.src.rpm

Please review them.

Cheers,

Aníbal

Comment 3 Gustavo Costa 2021-07-30 20:51:45 UTC
* Use http://ircii.warped.com instead of ftp://ircii.warped.com

Source0:        http://ircii.warped.com/%{name}-%{version}.tar.gz

* The make_install macro should be used in the install section

%install
%make_install

* Do not use

/usr/bin/irc
/usr/bin/irc-20210314
/usr/bin/ircbug
/usr/bin/ircflush
/usr/libexec/ircio
/usr/libexec/wserv

use macros like %{_bindir} for /usr/bin and %{_libexecdir} for /usr/libexec instead:

%{_bindir}/irc
%{_bindir}/irc-%{version}
%{_bindir}/ircbug
%{_bindir}/ircflush
%{_libexecdir}/ircio
%{_libexecdir}/wserv

You can also use glob instead of listing various files

%{_datadir}/irc/*

For manpages, you have to use glob:

%{_mandir}/man1/irc.1*
%{_mandir}/man1/ircII.1*
%{_mandir}/man1/ircbug.1*

* Do not use the clean section

* You have to install the license file

%license doc/Copyright

---

You can see all these changes here: https://gist.githubusercontent.com/xfgusta/148cc68fdbcfa70d2bc582d404c7f3fa/raw/bb96e238385be88f09aba9a18acfb0c76db24c93/ircii.spec

Note that there are other things you have to fix, but I don't know how to.

ircii.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/irc-20210314
ircii.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/ircflush
ircii.x86_64: E: incorrect-fsf-address /usr/bin/ircbug

I'm also not sure if openssl and ncurses are required to run ircii, although they are used to build it.

Comment 4 Troy Curtis 2021-08-01 17:06:14 UTC
Also note that once the package is approved, you'll need to seek sponsorship into the package group. See https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join#Get_Sponsored for more details. 

Gustavo has some great suggestions. Additionally, you should prefer https over http, so Source0 should be https://ircii.warped.com/%{name}-%{version}.tar.gz

Comment 5 Aníbal Monsalve Salazar 2021-08-28 01:02:57 UTC
Thank you Troy and Gustavo. I've been busy with other stuff.

Comment 6 Robby Callicotte 2021-10-17 22:05:35 UTC
Greetings,

This is an unofficial review... I am not a packager.

"rpmlint -i ircii" states the following:

ircii.spec:17: W: setup-not-quiet
Use the -q option to the %setup macro to avoid useless build output from
unpacking the sources.

ircii.spec:20: W: rpm-buildroot-usage %build %configure --mandir=%{buildroot}/%{_mandir}
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it may
break short circuit builds.

ircii.spec: W: no-%install-section
The spec file does not contain an %install section.  Even if some packages
don't directly need it, section markers may be overridden in rpm's
configuration to provide additional "under the hood" functionality.  Add the
section, even if empty.


As Gustavo stated previously, the macro form (%{_mandir}, %{_bindir}, etc) should be used in the %files section.

Stylistically, I believe that the "Name" tag in the spec should be listed before anything else in the preamble except for use of %global definitions.


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