Bug 614036

Summary: Review Request: stdair - C++ Standard Airline IT Library - FE-NEEDSPONSOR
Product: [Fedora] Fedora Reporter: Son Nguyen Kim <nguyenkims>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: clacombe, denis.arnaud_fedora, fedora-package-review, martin.gieseking, nguyenkims, notting, randyn3lrx
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-05-08 17:13:54 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Son Nguyen Kim 2010-07-13 10:39:22 EDT
Spec URL: https://downloads.sourceforge.net/project/stdair/stdair_0_1_0.spec
SRPM URL: https://downloads.sourceforge.net/project/stdair/fedora_12/stdair-0.1.0-1.fc12.src.rpm
===============================================
Description:
That is my first package, and I am seeking a sponsor.
===============================================
STDAIR aims at providing a clean API, and the corresponding C++
implementation, for the basis of Airline IT Business Object Model (BOM),
that is, to be used by several other Open Source projects, such as RMOL, 
Air-Sched, Travel-CCM, OpenTREP, etc.
Comment 1 Randall "Randy" Berry 2010-07-13 11:32:12 EDT
I am not a sponsor so I can't sponsor you. But there are a few things I noticed.

1) The spec file should be named as the package name without version. i.e. stdair.spec not stdair_0_1_0.spec

2) Links to both files should be a direct download path without having to go through sourceforge's mirror pages.
Comment 2 Randall "Randy" Berry 2010-07-13 11:38:03 EDT
Additionally, Fedora 12 is half way through its life cycle. Consider building and submitting this package built against Fedora 13 and/or rawhide.
Comment 3 Son Nguyen Kim 2010-07-15 04:44:02 EDT
(In reply to comment #2)
> Additionally, Fedora 12 is half way through its life cycle. Consider building
> and submitting this package built against Fedora 13 and/or rawhide.    
Thanks for your remarks!
I don't have fedora 13 on my machine but i think there is no problem for installing on fedora 13.
SPEC and rpm source now are moved to:
SPEC file: http://subversion.assembla.com/svn/stdair/stdair.spec
RPM Source: http://subversion.assembla.com/svn/stdair/stdair-0.1.0-1.fc12.src.rpm
Comment 4 Martin Gieseking 2010-08-28 03:49:07 EDT
Here are some more comments:

- the package doesn't build in mock because of missing 
  BuildRequires:soci-mysql-devel

- The tarball bundles library libextracppunit. It's not permitted to link
  against bundled libraries in Fedora. Instead, remove it and use the
  corresponding library provided by a separate package. extracc isn't part of
  Fedora yet, but already under review (bug #616881).
  Also see https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

- replace %define with %global
  https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define 

- As far as I see, there are no info files. Hence, drop Requires(post) and
  Requires(preun).

- since you can't add any new packages to Fedora < 12, remove the %if 
  statements. It's sufficient to use the following for all targets: 
  BuildArch:      noarch
  BuildRequires:  tex(latex)

- it's not necessary to remove INSTALL in %prep
  
- I recommend to replace
    rm -f $RPM_BUILD_ROOT%{_libdir}/lib%{name}.la
    rm -f $RPM_BUILD_ROOT%{_libdir}/libextracppunit.la
    rm -rf %{mydocs} && mkdir -p %{mydocs}
  with
    rm -f $RPM_BUILD_ROOT%{_libdir}/*.la
    mkdir -p %{mydocs}

  (you don't need to remove %{mydocs} as it doesn't exist here yet)

- move the man3 manpages to the devel package as it contains information for
  developers

- drop AUTHORS, ChangeLog, NEWS, and README from the doc package
  (files must not be added multiple times, COPYING is an exception here)
Comment 5 Son Nguyen Kim 2010-08-29 13:25:20 EDT
Hello Martin, 

Thank you very much for your suggestions !

Here's the updated SPEC  and SRPM file after taking changing the SPEC file as you suggested, except the libextracppunit is not still removed. It permit someone who does not have libextracppunit can nevertheless use stdair. I'll remove it when libextracppunit is passed the review.

SPEC file : http://subversion.assembla.com/svn/stdair/stdair.spec
SRPM file: http://subversion.assembla.com/svn/stdair/stdair-0.3.0-1.fc13.src.rpm
Comment 6 Martin Gieseking 2010-08-29 14:40:45 EDT
(In reply to comment #5)
> Thank you very much for your suggestions !

Hi Son, you're welcome. 


> the libextracppunit is not still removed. It permit
> someone who does not have libextracppunit can nevertheless use stdair. I'll
> remove it when libextracppunit is passed the review.

Unfortunately, that's not possible. Your package can't be approved as long as it bundles a library. Thus, you should prepare your package as if extracc was already available. As a consequence, extracc blocks stdair, and you have to wait until it's approved.

Concerning your current spec file, you should replace %{?el5:FOO} and %{?rhel:BAR} with FOO and BAR, respectively. And completely drop %{?el5:BuildRequires: tetex-latex}

The virtual package tex(latex) ensures that the correct packages are referenced (currently texlive-latex in Fedora, and tetex-latex in EPEL <= 5).
Explicitly adding the BuildRoot tag and cleaning %{buildroot} also doesn't hurt. So if you plan to maintain this package for EPEL too, just add the corresponding lines as requested by the EPEL guidelines. Fedora also still accepts them.

You can drop
  rm -f $RPM_BUILD_ROOT%{_libdir}/lib%{name}.la
  rm -f $RPM_BUILD_ROOT%{_libdir}/libextracppunit.la
The preceding rm statement already removes all .la files.
Comment 7 Son Nguyen Kim 2010-08-29 17:48:01 EDT
Thanks! The new specification and source RPM are:
SpecURL: http://subversion.assembla.com/svn/stdair/stdair.spec
SRPM Spec: http://subversion.assembla.com/svn/stdair/stdair-0.3.0-2.fc13.src.rpm

> Unfortunately, that's not possible. Your package can't be approved as long as
> it bundles a library. Thus, you should prepare your package as if extracc was
> already available. As a consequence, extracc blocks stdair, and you have to
> wait until it's approved.

Yes, of course, I did not intend to violate the guideline (https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries). I know the person who submitted the review request for extracc (https://bugzilla.redhat.com/show_bug.cgi?id=616881), namely Christophe. He is very busy and may not have time to further maintain the extracc package. Therefore, I may well maintain extracc as well, especially since it has been approved by Mamoru Tasaka (https://bugzilla.redhat.com/show_bug.cgi?id=616881#c3). So, I would submit extracc and stdair at the same time.

Besides, I indeed intend to maintain those package for EPEL. That's why I kept some %{?el5:FOO} and %{?rhel:BAR} macros.
Comment 8 Martin Gieseking 2010-08-30 02:33:22 EDT
(In reply to comment #7)
> I know the
> person who submitted the review request for extracc
> (https://bugzilla.redhat.com/show_bug.cgi?id=616881), namely Christophe. He is
> very busy and may not have time to further maintain the extracc package.
> Therefore, I may well maintain extracc as well, especially since it has been
> approved by Mamoru Tasaka
> (https://bugzilla.redhat.com/show_bug.cgi?id=616881#c3). So, I would submit
> extracc and stdair at the same time.

OK, if Christophe isn't interested in extracc any longer (he should leave a comment about that in his review request), you can take his package once you're sponsored.

> Besides, I indeed intend to maintain those package for EPEL. That's why I kept
> some %{?el5:FOO} and %{?rhel:BAR} macros.

That's fine, but you don't need these macros. Just add the BuildRoot tag and the %clean section, and clean $RPM_BUILD_ROOT at the beginning of %install. These additional lines work in EPEL and in Fedora. So, there's no need to distinguish between EPEL and Fedora here.

BTW, are you still looking for a sponsor? If so, I'm willing to sponsor you. As a member of the packager group, you will be allowed to approve other people's packages. Thus, you should do two or three informal reviews before, in order to familiarize with the review process and to show an understanding of the Fedora guidelines.
Comment 9 Christophe LACOMBE 2010-09-08 11:40:01 EDT
I am not a sponsor so I can't sponsor you. But there are a few things I
noticed.

1) The devel rpm do not install any header, while it is specified in your spec file. This might come from a wrong defined variable pkginclude_HEADERS.

2) As suggested by martin, add the dependency to extracc-devel, then you can remove the one to cppunit-devel as it is part of the extracc spec file.
Comment 10 Son Nguyen Kim 2010-09-09 16:49:40 EDT
Thanks, Christophe !
Here are the updated SPEC file and RPM SRC file:

RPM SRC file: http://subversion.assembla.com/svn/stdair/stdair-0.4.0-1.fc13.src.rpm
SPEC file: http://subversion.assembla.com/svn/stdair/stdair.spec

Now, stdair does not contain extracc or extracppunit. Stdair has been also tested with chain builds in mock and it worked.
Comment 11 Son Nguyen Kim 2011-05-05 15:37:50 EDT
As I have no longer time to be responsible for that package, I hand over that responsibility to Denis Arnaud, who is one of the upstream maintainers. I've talked to him, and he agrees to take back StdAir from me
Comment 12 Martin Gieseking 2011-05-06 06:17:21 EDT
OK, in this case, Denis should open a new review request ticket and close this one as a duplicate.
Comment 13 Denis Arnaud 2011-05-08 17:13:54 EDT

*** This bug has been marked as a duplicate of bug 702987 ***