Bug 469808 - Review Request: pstreams-devel - POSIX Process Control in C++
Summary: Review Request: pstreams-devel - POSIX Process Control in C++
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Till Maas
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 469811
TreeView+ depends on / blocked
 
Reported: 2008-11-04 05:53 UTC by Rakesh Pandit
Modified: 2010-05-12 14:20 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-11-08 05:18:42 UTC
Type: ---
Embargoed:
opensource: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Rakesh Pandit 2008-11-04 05:53:22 UTC
Description:
PStreams class is like a C++ wrapper for the POSIX.2 functions
popen(3) and pclose(3), using C++ iostreams instead of C's stdio
library.


SPEC: http://rakesh.fedorapeople.org/spec/pstreams-devel.spec
SRPM: http://rakesh.fedorapeople.org/srpm/pstreams-devel-0.6.0-1.fc10.src.rpm

Needed for pdf2djvu package.

Comment 1 Aaron S. Hawley 2008-11-06 07:27:56 UTC
What's the significance of doxygen as a build requirement?

Comment 2 Rakesh Pandit 2008-11-06 08:25:42 UTC
- removed, mistakenly wrote it. - Thanks.

Updated:
http://rakesh.fedorapeople.org/spec/pstreams-devel.spec
http://rakesh.fedorapeople.org/srpm/pstreams-devel-0.6.0-2.fc10.src.rpm

Comment 3 Till Maas 2008-11-07 12:11:42 UTC
You should use either %{buildroot} or RPM_BUILD_ROOT, but not both in the same spec file.

The install is missing a "-p" to preserve the timestamps.

Comment 5 Till Maas 2008-11-07 13:24:18 UTC
There are some issues left:

- license is not included in %doc:
COPYING.LIB

You should also include in %doc:
AUTHORS ChangeLog

You need the BR doxygen, because you can create additional documentation with "make docs", this is also missing in the rpm. I would at least include the html documentation in %doc.

Comment 6 Till Maas 2008-11-07 13:34:16 UTC
[OK] rpmlint output:
silent
[OK] Spec in %{name}.spec format

[OK] license allowed:LGPLv3
[OK] license matches shortname in License: tag
[NOT OK] license in tarball and included in %doc: COPYING.LIB

[OK] package is code or permissive content:
{N/A} patches sent to upstream and commented
[OK] Source0 is a working URL
{OK} Sourceforge URL is Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
<N/A> SourceX / PatchY prefixed with %{name}

[OK] Source0 matches Upstream:
a12c2793937c2ea1bee04827d7b2ca63  pstreams-0.6.0.tar.gz


[OK] Package builds on all platforms:
[N/A] ExcludeArch bugs are filed and commented:
[TODO] BuildRequires are complete (mock builds)
(OK) No file dependencies outside of /etc /bin /sbin /usr/bin /usr/sbin 

[N/A] %find_lang used for locales

[N/A] Every (sub)package containing libraries runs ldconfig
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
[OK] .h (header) files are in -devel subpackage
[N/A] .a (static libraries) are in -static subpackage
[N/A] contains .pc (pkgconfig) files and has Requires: pkgconfig
(N/A) .pc files are in -devel subpackage
[N/A] contains .so.X(.Y) files and .so is in -devel
[N/A] -devel subpackage has Requires: %{name} = %{version}-%{release}
The -devel package is the main apge here.
[N/A] .la files (libtool) are not included

[N/A] Has GUI and includes %{name}.desktop
[N/A] .desktop file installed with desktop-file-install in %install

[OK] Prefix: /usr not used (not relocatable)

[OK] Owns all created directories
[OK] no duplicates in %files
[OK] %defattr(-,root,root,-) is in every %files section
[OK] Does not own files or dirs from other packages
[OK] included filenames are in UTF-8

[OK] %clean is rm -rf %{buildroot} or $RPM_BUILD_ROOT 
[OK] %install starts with rm -rf %{buildroot} or $RPM_BUILD_ROOT 

[OK] Consistent macro usage

[OK] large documentation is -doc subpackage
[OK] %doc does not affect runtime

{OK} no pre-built binaries (.a, .so*, executable)

{OK} well known BuildRoot
It's kind of this one:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

{OK} PreReq not used

{N/A} RPM_OPT_FLAGS honoured

{N/A} Useful debuginfo generated

{OK} no duplication of system libraries

{N/A} no rpath

{OK} Timestamps preserved with cp and install

{N/A} Uses parallel make (%{?_smp_mflags})

{OK} Requires(pre,post) style notation not used

{OK} only writes to tmp /var/tmp $TMPDIR %{_tmppath} %{_builddir} (and %{buildroot} on %install and %clean)

{OK} no Conflicts

{OK} nothing installed in /srv

{OK} Changelog in allowed format

{OK} does not use Scriptlets

<OK> Architecture independent packages have: BuildArch: noarch
<OK> Sane Provides: and Requires:

{OK} Follows Naming Guidelines

Comment 8 Till Maas 2008-11-07 14:27:48 UTC
[OK] license in tarball and included in %doc: COPYING.LIB

[OK] BuildRequires are complete (mock builds)
http://koji.fedoraproject.org/koji/taskinfo?taskID=920922


{NOT OK} Timestamps preserved with cp and install
If you use make install here, then the timestamp is not preserved.

You can use this patch to preserve it:
https://sourceforge.net/tracker2/download.php?group_id=48695&atid=453894&file_id=300540&aid=2234202

If you add the patch, please mention in a comment in the spec file, that it is already submitted upstream and point to this location (the tracker for this patch):
https://sourceforge.net/tracker2/?func=detail&atid=453894&aid=2234202&group_id=48695

Comment 10 Rakesh Pandit 2008-11-07 14:40:32 UTC
I clashed with your updated. Wait I will updated.

Comment 12 Till Maas 2008-11-07 15:00:11 UTC
{OK} Timestamps preserved with cp and install

Thank you for your fast responses, now everything is good, this package is now APPROVED.

Comment 13 Rakesh Pandit 2008-11-07 15:05:17 UTC
Thanks

New Package CVS Request
=======================
Package Name: pstreams-devel
Short Description: POSIX Process Control in C++
Owners: rakesh
Branches: F-8 F-9 F-10
InitialCC:
Cvsextras Commits: yes

Comment 14 Kevin Fenzi 2008-11-07 21:31:14 UTC
cvs done.

Comment 15 Fedora Update System 2008-11-08 05:05:44 UTC
pstreams-devel-0.6.0-6.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/pstreams-devel-0.6.0-6.fc8

Comment 16 Fedora Update System 2008-11-08 05:06:52 UTC
pstreams-devel-0.6.0-6.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/pstreams-devel-0.6.0-6.fc9

Comment 17 Fedora Update System 2008-11-08 05:07:16 UTC
pstreams-devel-0.6.0-6.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/pstreams-devel-0.6.0-6.fc10

Comment 18 Susi Lehtola 2008-11-08 13:23:38 UTC
According to the Package Naming Conventions, shouldn't this package be named pstreams instead of pstreams-devel? The dash is a separator which shouldn't be used in the base name of the package.

http://fedoraproject.org/wiki/Packaging/NamingGuidelines#General_Naming

Of course, pstreams should then provide pstreams-devel = %{version}-%{release} .

Comment 19 Till Maas 2008-11-08 13:47:00 UTC
(In reply to comment #18)
> According to the Package Naming Conventions, shouldn't this package be named
> pstreams instead of pstreams-devel? The dash is a separator which shouldn't be
> used in the base name of the package.

The naming guidelines demand that the dash[1] is used as a separator in the base name of packages and there are only some exceptions that allow to use the underscore instead. Also the review guidelines contain a MUST item that demands header files being in a -devel package:

- MUST: Header files must be in a -devel package.

Btw. debian also uses their devel naming scheme for pstreams:
http://packages.debian.org/sid/libpstreams-dev


> Of course, pstreams should then provide pstreams-devel = %{version}-%{release}

What is the technical advantage of this? I would expect header files to be in a -devel package.


[1] some examples:
bitmap-fonts
bodhi-client
bridge-utils

Comment 20 Susi Lehtola 2008-11-08 14:14:38 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > According to the Package Naming Conventions, shouldn't this package be named
> > pstreams instead of pstreams-devel? The dash is a separator which shouldn't be
> > used in the base name of the package.
> 
> The naming guidelines demand that the dash[1] is used as a separator in the
> base name of packages and there are only some exceptions that allow to use the
> underscore instead. Also the review guidelines contain a MUST item that demands
> header files being in a -devel package:

Right.

> > Of course, pstreams should then provide pstreams-devel = %{version}-%{release}
> 
> What is the technical advantage of this? I would expect header files to be in a
> -devel package.

Well, the name confuses me a bit since I'd expect there to be a package named pstreams. Of course, one can add Provides: pstreams to pstreams-devel as well.

> [1] some examples:
> bitmap-fonts
> bodhi-client
> bridge-utils

OK. Well, maybe it isn't as bad as I first thought: I find the following packages in F9 Everything SRPMS with names containing -devel:

gnome-devel-docs
sblim-cmpi-devel
xorg-x11-proto-devel
xorg-x11-xtrans-devel

Comment 21 Fedora Update System 2008-12-11 07:57:59 UTC
pstreams-devel-0.6.0-6.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2008-12-11 08:04:40 UTC
pstreams-devel-0.6.0-6.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2008-12-11 08:05:31 UTC
pstreams-devel-0.6.0-6.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 24 Jonathan Wakely 2010-05-12 14:04:31 UTC
I'm not sure the best way to announce this, but I've released a new version of pstreams, including the spec file and patch from the Fedora package

Comment 25 Rakesh Pandit 2010-05-12 14:20:12 UTC
Because it is already available in fedora, two three ways in which you can help here are:

a) In case you want latest release to fill a bug and provide a patch to spec (or additional patches if interested)
b) In case you got a bug in already existing package fill a bug (and help in solving it if interested)
c) In case you want to maintain package, get yourself sponsorship (details on fedora wiki) and request for co-maintainer-ship on pkgdb


Thanks,


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