Bug 547832 - Review Request: procServ - A process server with telnet console and log access
Summary: Review Request: procServ - A process server with telnet console and log access
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Martin Gieseking
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-12-15 19:15 UTC by Ralph Lange
Modified: 2010-09-25 05:34 UTC (History)
6 users (show)

Fixed In Version: procServ-2.5.1-3.fc14
Clone Of:
Environment:
Last Closed: 2010-09-23 04:58:26 UTC
Type: ---
Embargoed:
martin.gieseking: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Ralph Lange 2009-12-15 19:15:26 UTC
Spec URL: http://pubweb.bnl.gov/~rlange/procserv/procServ.spec
SRPM URL: http://pubweb.bnl.gov/~rlange/procserv/procServ-2.5.0-1.fc12.src.rpm

Description: procServ is a wrapper that starts an arbitrary command as a child process in the background, connecting its standard input and output to a TCP port for telnet access. It supports logging, child restart (manual or automatic on exit), and more.

This is my first fedora package, so I will need a sponsor.

Comment 1 Fabian Affolter 2009-12-24 10:04:22 UTC
rpmlint output is not clean
  [fab@localhost SRPMS]$ rpmlint procServ-2.5.0-1.fc12.src.rpm
  procServ.src: E: description-line-too-long procServ is a wrapper that starts an
  arbitrary command as a child process in the background, connecting its standard 
  input and output to a TCP port for telnet access. It supports logging, child  
  restart (manual or automatic on exit), and more.
  1 packages and 0 specfiles checked; 1 errors, 0 warnings.

Comment 2 Ralph Lange 2009-12-24 14:19:22 UTC
Fixed. (By breaking up the description into separate lines.)

New URLs are

Spec URL: http://pubweb.bnl.gov/~rlange/procserv/procServ.spec
SRPM URL: http://pubweb.bnl.gov/~rlange/procserv/procServ-2.5.0-2.fc12.src.rpm

Thanks and happy holidays!

Comment 3 Fabian Affolter 2009-12-24 23:33:14 UTC
I did a quick test without autoconf, just used '%configure --docdir=%{_defaultdocdir}/%{name}-%{version}' and the doc stuff is in the right place.  If this really works the way it should there are no BR needed and the %install section only needs 'make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"'

- Please add '%{?_smp_mflags}' to make
  https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make

- A blank line between the changelog entries would be nice ;-)

- Without further investigation I would say that '%attr(755,root,root)' is not needed for '%{_bindir}/procServ'

Comment 4 Ralph Lange 2009-12-26 21:51:05 UTC
Fixed.

New URLs are

Spec URL: http://pubweb.bnl.gov/~rlange/procserv/procServ.spec
SRPM URL: http://pubweb.bnl.gov/~rlange/procserv/procServ-2.5.0-3.fc12.src.rpm

- I was confusing the package build from release tar with the build from repository (which needs the autotools), so I followed your suggestions. I kept the BuildRoot prep in %install, though, as many of my "customers" are still running older RHEL/SL and Fedora releases and will package locally using that spec.

- I also added the parallel make flags,

- nice blank lines in the changelog,

- and removed the %attr, which indeed was not needed.

Thanks again!

Comment 5 Ralph Lange 2009-12-29 19:12:01 UTC
A bit more background (as suggested in PackageMaintainers/HowToGetSponsored)

procServ origins as a tool for the open source accelerator and physics control system software EPICS (http://www.aps.anl.gov/epics). In that context it is mainly used to run "soft" EPICS I/O processes in the background, while giving access to the console (stdin/stdout) of the process through a local telnet port.
As the tool is mature, quite generic, and already being used to "daemonify" a number of other command line applications, it has been moved to SourceForge, and people have asked me to package it and get it included into the distribution.

The package is small, with minimal dependencies.
I was able to successfully do koji --scratch builds for all default architectures on fc11 and fc12.

I am the current upstream author. I am working in accelerator controls, being actively involved in the development of the EPICS software package that this tool relates to.

Comment 6 Ralph Lange 2010-01-20 20:34:18 UTC
Update providing a better description.

New URLs are

Spec URL: http://pubweb.bnl.gov/~rlange/procserv/procServ.spec
SRPM URL: http://pubweb.bnl.gov/~rlange/procserv/procServ-2.5.0-4.fc12.src.rpm

I am still looking for a sponsor.

This is a very small simplistic package, just a binary and doc, hardly any external dependencies.

I would very much appreciate if someone was willing to review and upload the package.

Thanks a lot!

Comment 7 Ralph Lange 2010-03-24 15:12:26 UTC
Update for a new upstream version.

New URLs are

Spec URL: http://pubweb.bnl.gov/~rlange/procserv/procServ.spec
SRPM URL: http://pubweb.bnl.gov/~rlange/procserv/procServ-2.5.1-1.fc12.src.rpm

I am still looking for a sponsor.

I would very much appreciate if someone was willing to review and upload the
package.

Thanks a lot!

Comment 8 Michael Schwendt 2010-07-23 13:45:32 UTC
Just a brief look at the spec file:


> Summary: A process server with telnet console and log access

Most packages try to be even more concise, e.g.:

  Summary: Process server with telnet console and log access


> %configure --docdir=%{_defaultdocdir}/%{name}-%{version}
> [...]
> %files
> %defattr(-,root,root,-)
> %doc AUTHORS COPYING ChangeLog NEWS README procServ.html procServ.pdf procServ.txt
>

Caution! This is a packaging pitfall.

This definition of --docdir here conflicts with the %doc line. What happens is that any files installed into the --docdir are killed by %doc, because %doc creates its target directory from scratch. You would not notice if "make install" installs additional documentation.

Correct would be:

%configure --docdir=%{_defaultdocdir}/%{name}-%{version}
[...]
%files
%defattr(-,root,root,-)
%{_defaultdocdir}/%{name}-%{version}/

Comment 9 Ralph Lange 2010-07-23 14:48:15 UTC
Thank you for the review and especially for pointing out that pitfall. I never realized %doc was replacing anything that was in the target location.

I changed/fixed both issues and re-uploaded source RPM and spec.

New URLs are

Spec URL: http://pubweb.bnl.gov/~rlange/procserv/procServ.spec
SRPM URL: http://pubweb.bnl.gov/~rlange/procserv/procServ-2.5.1-2.fc12.src.rpm

And, of course: I am still looking for a sponsor.
I would very much appreciate if someone was willing to review and upload the
package.

Again: Thanks a lot!

Comment 10 Martin Gieseking 2010-08-31 07:47:39 UTC
Hi Ralph,

your package looks almost fine now. Just a couple of things to consider:
- please adapt the URL in Source0 according to
  https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

- in the %changelog section, replace %doc with %%doc to make rpmlint happy
  (see rpmlint output below)

- I recommend to be a bit more specific in %files and replace
  %{_mandir}/man1/procServ.* with
  %{_mandir}/man1/procServ.1*
  
The latter might seem a bit nit-picking, but it helps to avoid adding files in wrong places. I already came across packages with man3 files in man1 folders. When using an explicit suffix, something like that won't happen by accident in future releases.

BTW, if you don't have a sponsor yet, I'm willing to sponsor you.

$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
procServ.src: W: spelling-error %description -l en_US conserver -> conserve, conserves, conserved
procServ.src: W: spelling-error %description -l en_US localhost -> local host, local-host, localism
procServ.src:48: W: macro-in-%changelog %doc
procServ.x86_64: W: spelling-error %description -l en_US conserver -> conserve, conserves, conserved
procServ.x86_64: W: spelling-error %description -l en_US localhost -> local host, local-host, localism
3 packages and 0 specfiles checked; 0 errors, 5 warnings.

Comment 11 Ralph Lange 2010-09-02 20:34:52 UTC
Hello Martin,

thanks a lot for your review and the additional hints.

I fixed all issues as you suggested.
The two spelling-error warnings remain, both of which are true false-positives: the package I recommend is named "conserver", and I mention "localhost" as found in /etc/hosts - changing the spelling would be misleading.

I do not have a sponsor yet, so I accept and highly appreciate your offer.
Thank you very much!

New URLs:
Spec  http://pubweb.bnl.gov/~rlange/procserv/procServ.spec
SRPM  http://pubweb.bnl.gov/~rlange/procserv/procServ-2.5.1-3.fc13.src.rpm

Comment 12 Martin Gieseking 2010-09-03 07:13:03 UTC
(In reply to comment #11)
Hi Ralph,

> The two spelling-error warnings remain, both of which are true false-positives:

Yes, that's OK. The spell checker isn't perfect. It sometimes makes a lot of noise. You can just ignore the false positive warnings.
 
> I do not have a sponsor yet, so I accept and highly appreciate your offer.
> Thank you very much!

You're welcome. :)

However, before I can approve you, you should show that you're familiar with the packaging guidelines. This is important because you're allowed to formally review and approve other packager's submissions once you've been sponsored. 
To show your (basic) understanding of the guidelines, and to become familiar with reviewing packages, I encourage you to do two or three informal reviews. I'll contact you privately for further details.

Comment 13 Martin Gieseking 2010-09-09 20:02:14 UTC
Removed FE-NEEDSPONSOR as I've sponsored Ralph.

Comment 14 Martin Gieseking 2010-09-10 06:10:46 UTC
Hi Ralph,

welcome to the packager group. I hope you'll enjoy maintaining and reviewing packages for Fedora.

Your procServ package is ready now, so we can finish here. The next step is to request a git repository with the distro branches you're planning to maintain:
https://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure


$ rpmlint  /var/lib/mock/fedora-13-x86_64/result/*.rpm
procServ.src: W: spelling-error %description -l en_US conserver -> conserve, conserves, conserved
procServ.src: W: spelling-error %description -l en_US localhost -> local host, local-host, localism
procServ.x86_64: W: spelling-error %description -l en_US conserver -> conserve, conserves, conserved
procServ.x86_64: W: spelling-error %description -l en_US localhost -> local host, local-host, localism
3 packages and 0 specfiles checked; 0 errors, 4 warnings.

The above spelling errors can be ignored.


---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum procServ-2.5.1.tar.gz*
    a6d3131361189458fc276ca4323efe46  procServ-2.5.1.tar.gz
    a6d3131361189458fc276ca4323efe46  procServ-2.5.1.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
    Koji scratch build:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2458825

[.] MUST: If the package does not successfully compile, build or work on an architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: Packages storing shared library files must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: .so files must go in a -devel package.
[.] MUST: devel packages must require the base package. 
[+] MUST: Packages must NOT contain any .la libtool archives.
[.] MUST: Packages containing GUI applications must include a %{name}.desktop file.
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[.] SHOULD: If the source package does not include license text(s) ...
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described. 
    - seems to work as expected but I tested only superficially

[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin ...
[+] SHOULD: your package should contain man pages for binaries/scripts. 

----------------
Package APPROVED
----------------

Comment 15 Ralph Lange 2010-09-10 13:31:30 UTC
New Package SCM Request
=======================
Package Name: procServ
Short Description: Process server with telnet console and log access
Owners: ralphlange
Branches: f13 f14 el6
InitialCC:

Comment 16 Kevin Fenzi 2010-09-10 18:43:29 UTC
Git done (by process-git-requests).

Comment 17 Fedora Update System 2010-09-14 15:30:35 UTC
procServ-2.5.1-3.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/procServ-2.5.1-3.fc13

Comment 18 Fedora Update System 2010-09-14 15:30:42 UTC
procServ-2.5.1-3.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/procServ-2.5.1-3.fc14

Comment 19 Fedora Update System 2010-09-15 05:23:19 UTC
procServ-2.5.1-3.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update procServ'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/procServ-2.5.1-3.fc13

Comment 20 Fedora Update System 2010-09-23 04:58:21 UTC
procServ-2.5.1-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 21 Fedora Update System 2010-09-25 05:34:06 UTC
procServ-2.5.1-3.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.


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