This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 489803 - (libserial) Review Request: libserial - C++ library to access serial ports on POSIX systems
Review Request: libserial - C++ library to access serial ports on POSIX systems
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
http://sourceforge.net/projects/libse...
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2009-03-11 18:25 EDT by Alex Orlandi
Modified: 2013-02-19 05:57 EST (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-02-19 05:57:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Alex Orlandi 2009-03-11 18:25:35 EDT
Spec URL: http://nyrk.awardspace.com/SRPMS/libserial.spec
SRPM URL: http://nyrk.awardspace.com/SRPMS/libserial-0.5.2-1.fc9.src.rpm
Description: libserial (http://libserial.sourceforge.net/) is a collection of C++ classes which allow the serial port on POSIX systems to be accessed like an iostream object. Special functions are provided for setting various parameters of the serial port such as the baud rate, character size, flow control and others. 

This is my first package so I'm seeking a sponsor.
The package has been built following the fedora project guidelines and has been tested with koji (http://koji.fedoraproject.org/koji/tasks?state=closed&owner=nyrk71)
Comment 1 Alex Orlandi 2009-03-11 18:53:56 EDT
Further information:

Before testing with koji, I checked the builds using rpmlint.

As required, I subscribed to
   fedora-devel-announce@redhat.com
and 
   fedora-package-review@redhat.com mailing lists

I already have a FAS account, too (nyrk71) and I'm often in the #fedora channel  on irc.freenode.net with the registered nick: nyrk71.
Comment 2 Kevin Fenzi 2009-03-11 19:23:37 EDT
I can try and review this this weekend and see about sponsoring you... 
If anyone else cares to do so before then, feel free. ;)
Comment 3 Ralf Corsepius 2009-03-12 01:16:55 EDT
Some comments (leaving a formal review to Kevin):

- BR: gzip and BR: libtool are superfluous
Please remove them.


- Explicitly gzip'ing man-pages is a mistake.
rpm automatically compresses man-pages to the compression format _it_ prefers by itself.

Please remove the gzip ../man/.. line


- Package installs a man3 man-page called "todo.3"
This is a) too general and b) hardly useful.
Please remove this man-page.



- Fedora specs are supposed not to set Vendor:
Please remove this.


- I for one prefer packages which encapsulate their headers in a package-specific subdir of /usr/include, instead to put them directly into /usr/include.

=> Proposal: Install the headers into /usr/include/libserial
(%configure ... --includedir=%{_includedir}/libserial)

[Note: This is just my personal preference and is not a must.]
Comment 4 Alex Orlandi 2009-03-12 06:05:14 EDT
Thanks Ralf for your informal review.

While waiting for the formal review from Kevin, I add other hints coming from an other informal review (thanks to musuruan)

1 - remove from %install the line:
echo RPM_BUILD_ROOT $RPM_BUILD_ROOT 
(it had just a debug purpose)

2 - there are some residual "libserial" string instead of  %{name} 

3 - when copying the man pages, user "cp -a" to preserve attributes (such as timestamp)

4 - write a better changelog
Comment 5 Alex Orlandi 2009-03-12 18:43:10 EDT
Released a revision of the spec file.
Applied all the hints coming from informal reviews, except C++ headers installation under /usr/include/libserial instead of /usr/include/ (not mandatory)

URLs for spec and SRPM files:
Spec URL: http://nyrk.awardspace.com/Fedora/libserial/0.5.2-2.fc9/libserial.spec
SRPM URL: http://nyrk.awardspace.com/Fedora/libserial/0.5.2-2.fc9/libserial-0.5.2-2.fc9.src.rpm

Waiting for a formal review.
Comment 6 Alex Orlandi 2009-03-12 18:47:45 EDT
- rpmlint check OK on both libserial.spec and libserial-0.5.2-2.fc9.src.rpm
- koji build result: OK (http://koji.fedoraproject.org/koji/taskinfo?taskID=1238812)
Comment 7 Kevin Fenzi 2009-03-15 21:30:28 EDT
Hey Alex. I would be happy to review this and look at sponsoring you. 

Do you have any other reviews you have submitted? Any 'pre-reviews' of pending reviews to show you know the guidelines?

Look for a full review of this package in a bit.
Comment 8 Kevin Fenzi 2009-03-15 21:51:45 EDT
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
See below - License
See below - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
7a3766e354c31513ff6d7859a4b2c1b8  libserial-0.5.2.tar.gz
7a3766e354c31513ff6d7859a4b2c1b8  libserial-0.5.2.tar.gz.orig
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
OK - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Headers/static libs in -devel subpackage. 
OK - Spec has needed ldconfig in post and postun
OK - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - .la files are removed. 

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
OK - Package obey's FHS standard (except for 2 exceptions)
OK - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should have dist tag

Issues: 

1. It's not entirely clear what the license should be here. ;) 
Some of the source files have: 
"Copyright: See COPYING file that comes with this distribution"
which in fedora implies the License tag should be "GPL+"
SerialPort.cpp and SerialPort.h say GPLv2+

It's probibly best to talk to upstream and add a header to all the 
source files with their intended license. 

2. Non blocker, but there are some doxygen warnings you might 
want to mention to upsteam: 

Warning: Tag `CGI_NAME' at line 573 of file doxygen.conf has become obsolete.
To avoid this warning please update your configuration file using "doxygen -u"
...(and a few more)
Comment 9 Alex Orlandi 2009-03-18 06:43:31 EDT
(In reply to comment #7)
> Hey Alex. I would be happy to review this and look at sponsoring you. 
> 
> Do you have any other reviews you have submitted? Any 'pre-reviews' of pending
> reviews to show you know the guidelines?

Hi Kevin,
thank you for reviewing and for looking at sponsoring me.

For now the only "public" package I've done is libserial, so I don't have any other pending review but I should have another one soon because I would like to submit an application.

In the meanwhile I can make some informal pre-reviews of pending packages to give a clearer evidence fo the understanding of the guidelines; I will inform you of my informal pre-reviews asap.

Thank you very much for your time.
Comment 10 Alex Orlandi 2009-03-18 06:49:46 EDT
(In reply to comment #8)
> OK - Package meets naming and packaging guidelines
> [...cut..]
> Issues: 
> 
> 1. It's not entirely clear what the license should be here. ;) 
> Some of the source files have: 
> "Copyright: See COPYING file that comes with this distribution"
> which in fedora implies the License tag should be "GPL+"
> SerialPort.cpp and SerialPort.h say GPLv2+
> 
> It's probibly best to talk to upstream and add a header to all the 
> source files with their intended license. 

I talked to upstream that says:
"Thanks for pointing out the licensing issue. The original intention was to
use GPLv2. I will fix the headers in the file as soon as I return so they
are consistent with GPLv2. However, please feel free to patch the files for
Fedora as you find fit to make them appear as distributed under GPLv2"


> 2. Non blocker, but there are some doxygen warnings you might 
> want to mention to upsteam: 
> 
> Warning: Tag `CGI_NAME' at line 573 of file doxygen.conf has become obsolete.
> To avoid this warning please update your configuration file using "doxygen -u"
> ...(and a few more)  

I'll try to patch it in the next spec file; in the meanwhile I notify the warnings to upstream.
Comment 11 Kevin Fenzi 2009-03-18 23:50:26 EDT
Well, we really don't want to patch in licensing info. 
Did upstream say when they might do a new release? Or perhaps you could use a snapshot if they have made changes in their version control system already?
Comment 12 Alex Orlandi 2009-03-20 18:20:30 EDT
(In reply to comment #11)
> Well, we really don't want to patch in licensing info. 
Yes, of course.

> Did upstream say when they might do a new releas

A new release of 0.6.0 is planned within April '09 (for now just 0.6.0_RC1 is available)

> Or perhaps you could use a snapshot if they have made changes in their version control system already?  

I checkout the svn repository but, as far as license statements is concerned, neither tags neither  trunk have relevant differences: the only two files containing license statements are just SerailPort.cpp and SerialPort.h.

I can try to ask upstream to add a license header to all the source files in the new release of the library 0.6.0.
Comment 13 Kevin Fenzi 2009-03-20 19:39:37 EDT
That sounds great. Thanks for talking with upstream on this.
Comment 14 Kevin Fenzi 2009-05-09 22:45:17 EDT
Any word from upstream?
Comment 15 Alex Orlandi 2009-05-11 05:13:48 EDT
I'm sorry but I did not receive any word from upstream.

I proposed to join to the libserial project to contribute to make testing and/or to help to  fix license issues: the maintainer seemed to agree but, at the end of the story, I have not been included . Besides, the maintainer told me that there would have been a new release for the end of April but, as far as I can see, there's no relevant activity on the project.

As we have decided to use other libraries to develop our project (i.e. gammagrape), it is not so important to me to have libserial included in Fedora.

In any case, if you think that could be useful to include libserial package in Fedora, I will be glad to follow this issue. In this case, as soon as version 0.6.0 will be released (I can't say when), I will package it and I will submit for further review (hoping that upstream will have fixed license issues).

Otherwise, if nobody else is interested in libserial to be included in Fedora, this package review can be closed.

[OT]: There's a couple of programs I'd like to package in Fedora: as soon as I package them, if you don't mind and if you have time, I will ask you for the review.
Comment 16 Kevin Fenzi 2009-05-11 12:16:16 EDT
:( 

Well, it doesn't matter to me, if you would like to keep persuing including this package thats fine. If you would prefer to just drop it thats fine with me too. 

I'd be happy to help review any other submissions you have.
Comment 17 Alex Orlandi 2009-05-11 13:08:57 EDT
I'm glad to keep pursuing including libserial package in Fedora (hoping it will be useful for somebody else).

Unfortunately, we can't rely too much on the fact that the upstream will fix blocking issues in a short time: if this is not a problem, it's ok for me.
Comment 18 Kevin Fenzi 2010-03-23 23:03:33 EDT
Removing NEEDSPONSOR, as I have sponsored Alex.
Comment 19 Tim Niemueller 2011-07-20 19:51:25 EDT
Is this package going to be added anytime soon?
Comment 20 Alex Orlandi 2011-07-21 16:06:06 EDT
Hi Tim,
I would be glad to add this package but it seems the situation is a little bit stucked because of licensing issue.
It is quite clear that the upstream's intention is to release the library under GPL2, as I inquired the project's owner and he answered:
"Thanks for pointing out the licensing issue. The original intention was to use GPLv2. I will fix the headers in the file as soon as I return so they are consistent with GPLv2. However, please feel free to patch the files for Fedora as you find fit to make them appear as distributed under GPLv2."

From one side, as correctly pointed out from Kevin, we  can't patch the file adding licensing header in the source, from the other side, it seems the upstream is not interested in releasing a new version (considering that in 2009 they said they would have released a new version within a couple of month but 2 years have passed).

Reading the FAQ https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F
it seems there could be a way out, i.e. to use the "strictest license" but it remains the doubt about the license of the source files that do not contain license info in the header.

Any suggestions?
Comment 21 Tim Niemueller 2011-10-11 18:34:07 EDT
Poke the maintainer again...
Comment 22 Miroslav Suchý 2012-12-16 08:29:21 EST
Ping? Any progress here? Or we can close this review?
Comment 23 Tim Niemueller 2012-12-16 09:18:47 EST
The COPYING and file licenses seem now to agree on GPLv2+. The "See COPYING" file is still there, but this has the header suggestion "at your option, any later version". So I'd think we're good to go with GPLv2+. In any case, it might be a good idea to prepare a patch adding license headers and send it upstream, so that they just have to accept, apply, and re-release it.
Comment 24 Miroslav Suchý 2013-02-19 05:57:05 EST
Stalled Review. Closing per:
https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
If you ever want to continue with this review, please reopen or
submit new review.

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