Bug 1376848 - Review Request: captagent - HOMER SIPCapture agent
Summary: Review Request: captagent - HOMER SIPCapture agent
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
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-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2016-09-16 15:17 UTC by Daniel Pocock
Modified: 2021-08-29 00:45 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-08-29 00:45:54 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Daniel Pocock 2016-09-16 15:17:03 UTC
Spec URL: https://fedrtc.org/homer-packages/captagent.spec
SRPM URL: https://fedrtc.org/homer-packages/captagent-6.2.0.2-1.fc21.src.rpm
Description: captures SIP and related protocols and forwards to the HOMER node for analysis
Fedora Account System Username: pocock

Comment 1 Daniel Pocock 2016-09-16 15:59:25 UTC
Also in Debian now:

https://packages.qa.debian.org/c/captagent.html

Comment 2 Björn 'besser82' Esser 2016-10-25 09:32:51 UTC
Taken!  =)

***

First look at spec-file:


> Source0:	%name-%version.tar.gz

This tarball comes from github?  Please use github-style source urls:
https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Git_Hosting_Services


> Requires(post): systemd
> Requires(preun): systemd
> Requires(postun): systemd
>
> … snip …
>
> %post
> %systemd_post %name.service
>
> %preun
> %systemd_preun %name.service

Please use proper macros for this:
https://fedoraproject.org/wiki/Packaging:Scriptlets?rd=Packaging:ScriptletSnippets#Systemd


> %prep
> %setup -q

Please use new `%autosetup` macro instead of `%setup -q`.


> %build
> autoreconf -if
> %configure
> make

Please use `%make_build`-macro instead of plain `make`.


> %files
> %doc COPYING

Please package license-files as `%license` instead of `%doc`.  The soure-tarball contains an AUTHORS-file, which is part of the license…


> %{_libdir}/%name/modules/*.so

This way the built rpm does not properly own the directories below %{_libdir}.
See:  https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership


Please fix those directly obvious issues, properly bump revision and I'll take a detailed review of the package.

Comment 3 Björn 'besser82' Esser 2016-10-25 10:19:00 UTC
Besides my previous comment, the package FTBFS:


> Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.H5Q6Wj
> + umask 022
> + cd /builddir/build/BUILD
> + cd captagent-6.2.0.2
> + autoreconf -if
> /var/tmp/rpm-tmp.H5Q6Wj: line 31: autoreconf: command not found
> RPM build errors:
> error: Bad exit status from /var/tmp/rpm-tmp.H5Q6Wj (%build)
>     Bad exit status from /var/tmp/rpm-tmp.H5Q6Wj (%build)
> Child return code was: 1

You need to explicitly add:

BuildRequires: autoconf
BuildRequires: automake
BuildRequires: libtool

For full logs, see scratch-build on Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16194917

Comment 4 Daniel Pocock 2016-11-02 09:06:47 UTC
Björn, thanks for reviewing this spec file

I'm happy to make these changes, but will all of these things be compatible with RHEL7 / EPEL7 as well?  I'm hoping to use the same spec file for both Fedora and EPEL RPMs.

If some of the changes are not good for RHEL7/EPEL builds, are they essential for Fedora?

Comment 5 Björn 'besser82' Esser 2016-11-02 09:42:26 UTC
(In reply to Daniel Pocock from comment #4)
> Björn, thanks for reviewing this spec file
> 
> I'm happy to make these changes, but will all of these things be compatible
> with RHEL7 / EPEL7 as well?  I'm hoping to use the same spec file for both
> Fedora and EPEL RPMs.
> 
> If some of the changes are not good for RHEL7/EPEL builds, are they
> essential for Fedora?

You're welcome.  Those changes would be the same for EPEL7; there is no need to have some conditionals-magic for my suggeseted changes.

BuildRequires on auto(conf|make) and libtool are needed on any release of Fedora or EPEL, those `%make_*`-macros are available on EPEL >= 6, using systemd-macros is mandatory starting with EPEL7 [1], %autosetup works with all maintained releases of Fedora and EPEL (with a little exception on EPEL5 [2]), `%license` translates to `%doc` on EPEL <= 6 [3] and changes for github-style source packaging works fine for Fedora and EPEL >= 5.  For EPEL <= 6 the might apply some other conditional changes [4] not needed for Fedora or EPEL >= 7.


[1]  https://fedoraproject.org/wiki/EPEL:SysVInitScript#EPEL_SysV_Initscripts
[2]  https://fedoraproject.org/wiki/EPEL:Packaging#.25autosetup
[3]  https://fedoraproject.org/wiki/EPEL:Packaging#Previously_required_boilerplate
[4]  https://fedoraproject.org/wiki/EPEL:Packaging

Comment 6 Package Review 2020-07-10 00:55:15 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

Comment 7 Package Review 2020-11-13 00:46:46 UTC
This is an automatic action taken by review-stats script.

The ticket reviewer failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we reset the status and the assignee of this ticket.

Comment 8 Package Review 2021-08-29 00:45:54 UTC
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.


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