Bug 289701 - Review Request: qca-ossl - OpenSSL plugin for the Qt Cryptographic Architecture v2
Summary: Review Request: qca-ossl - OpenSSL plugin for the Qt Cryptographic Architectu...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 289681
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-09-13 18:24 UTC by Aurelien Bompard
Modified: 2007-11-30 22:12 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-11-02 08:07:31 UTC
Type: ---
Embargoed:
lemenkov: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Aurelien Bompard 2007-09-13 18:24:45 UTC
Spec URL: http://gauret.free.fr/fichiers/rpms/fedora/psi/qca-ossl.spec
SRPM URL: http://gauret.free.fr/fichiers/rpms/fedora/psi/qca2-2.0.0-0.1.beta7.fc7.src.rpm
Description: 
This is a plugin to provide SSL/TLS capability to programs that use the Qt
Cryptographic Architecture (QCA).  QCA is a library providing an easy API
for several cryptographic algorithms to Qt programs.  This package only
contains the TLS plugin.

This package is a rename of the existing, Fedora-included qca-tls, but for qca2.
It should be rather easy to review.

Comment 1 Rex Dieter 2007-09-13 20:11:24 UTC
Initial comments:

0. SRPM URL appears wrong.  qca2 != qca-ossl ??

1.  configure (like qca2), I suggest using something like:
unset QTDIR
./configure \
  --no-separate-debug-info

2.  use %{_qt4_plugindir} everwhere instead of
%{_qt4_prefix}/plugins/
or
$QTDIR/plugins

Comment 2 Rex Dieter 2007-09-13 20:12:58 UTC
3.  naming...  qca2-ossl?  (alternatively, either retire existing qca-1.x or 
make a compat pkg compat-qca or qca1)

Comment 3 Aurelien Bompard 2007-09-13 21:33:16 UTC
I was wondering about whether to rename qca 1.x to compat-qca or to add qca 2.x
as qca2. Renaming to compat-qca would force me to update the buildrequires in
depending packages, so I'd rather go with qca2, but I don't have any real
preference. Is there some kind of "best practice" about that ?

Comment 4 Aurelien Bompard 2007-09-13 21:56:38 UTC
(1) and (2) fixed. Sorry for the URL typo...
Spec URL: http://gauret.free.fr/fichiers/rpms/fedora/psi/qca-ossl.spec
SRPM URL:
http://gauret.free.fr/fichiers/rpms/fedora/psi/qca-ossl-0.1-1.20070706.fc7.src.rpm

Comment 5 Aurelien Bompard 2007-09-13 21:58:38 UTC
Darn I'm tired... SRPM URL is:
http://gauret.free.fr/fichiers/rpms/fedora/psi/qca-ossl-0.1-2.20070706.fc7.src.rpm

Comment 6 Aurelien Bompard 2007-10-16 06:19:07 UTC
ping ? Psi 0.11 is out and I need qca-ossl to upgrade...

Comment 7 Peter Lemenkov 2007-10-16 21:32:18 UTC
I'll pick up the review.
Wait a little...

Comment 8 Aurelien Bompard 2007-10-23 18:29:34 UTC
Did you get any chance to free up some time for this review ?

Comment 9 Peter Lemenkov 2007-10-25 10:12:57 UTC
Sorry for the delay. BTW why you still not build qca2?

Formal review:

- rpmlint not silent - then checking SRPM-package it shouts:

[petro@host-12-109 SPECS]$ rpmlint ../SRPMS/qca-ossl-0.1-2.20070706.fc7.src.rpm 
qca-ossl.src:35: E: configure-without-libdir-spec
qca-ossl.src:56: W: macro-in-%changelog date

The last warning should be easily omitted. But what about first one? Looks like
qt4-buildsustem handles all such things by itself but can you prove that on
x86_64 libqca-ossl.so placed in the right place? 

+ package meets naming and packaging guidelines
+ specfile is properly named
+ package meets Packaging/Guidelines
+ package licensed with a Fedora approved license and meets the Licensing
Guidelines.
+ spec file written in American English
+ spec file legible
+ sources match the upstream source:
908eaeed2c0f873bb7cf602814041559 
/usr/src/redhat/SOURCES/qca-ossl-0.1-20070706.tar.bz2
+ package builds file (x86)
+ all build dependencies listed in BuildRequires
+ package doesn't need to call ldconfig in %post and %postun
+ package owns all created directories
+ package not contains any duplicate files in the %files listing. 
+ permissions on files sets properly
+ package has a %clean section
+ package uses macros consistently 
+ package contains code or permissable content
+ header files are in a -devel package
+ package contains no static libs
+ package does not contain any .la libtool archives
+ not a GUI app
+ all filenames in rpm packages are valid UTF-8

Just FIY looking at the delta.affinix.com site I found that they released new
versions for qca2 and qca-ossl.

http://delta.affinix.com/download/qca/2.0/qca-2.0.0.tar.bz2
http://delta.affinix.com/download/qca/2.0/test4/qca-ossl-0.1-20070904.tar.bz2

Maybe you should update these two packages?

Comment 10 Aurelien Bompard 2007-10-25 18:07:07 UTC
I've already updated and build qca2 to the final version:
http://koji.fedoraproject.org/koji/buildinfo?buildID=21840

Concerning the absence of %_libdir, the rpm will fail to build if the .so is
placed anywhere else than _libdir, because of the %files section. Right now I
can't test it on x86_64, but I suppose it will work if the configure detects the
qt_dir in /usr/lib64/qt. Else the build will fail in the buildsystem, and I'll
fix it.

Updated version:
* Thu Oct 25 2007 Aurelien Bompard <abompard> 0.1-3.20070904
- update to 20070904

http://gauret.free.fr/fichiers/rpms/fedora/psi/qca-ossl.spec
http://gauret.free.fr/fichiers/rpms/fedora/psi/qca-ossl-0.1-3.20070904.fc7.src.rpm

Comment 11 Peter Lemenkov 2007-10-26 10:00:53 UTC
 * Wrote atest.cpp:
#include<openssl/opensslv.h>
int main()
{
  unsigned long x = OPENSSL_VERSION_NUMBER;
  if(x >= 0x00907000) return 0; else return 1;
}

 * Wrote atest.pro:
CONFIG  += console
CONFIG  -= qt app_bundle
SOURCES += atest.cpp
LIBS += -lssl -lcrypto

 * [/usr/lib/qt4/bin/qmake-qt4 atest.pro]
 * returned: 0
 * [/usr/bin/gmake]
g++ -c -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -Wall -W  -I/usr/lib/qt4/mkspecs/linux-g++ -I. -I.
-o atest.o atest.cpp
g++  -o atest atest.o     -lssl -lcrypto
 * returned: 0
 * [./atest]
 * returned: 0
 * [/usr/bin/gmake distclean]
rm -f atest.o
rm -f *~ core *.core
rm -f atest 
rm -f Makefile
 * returned: 0
 * DEFINES += OSSL_097
 * LIBS += -lssl -lcrypto
 -> yes

 * extra += target.path=/usr/lib/qt4/plugins/crypto
INSTALLS += target


Good, your configure finished.  Now run /usr/bin/gmake.

+ make
/usr/lib/qt4/bin/moc -DOSSL_097 -DQT_NO_DEBUG -DQT_PLUGIN -DQT_CORE_LIB
-DQT_SHARED -I../../../../lib/qt4/mkspecs/linux-g++ -I.
-I../../../../include/QtCore -I../../../../include/QtCore -I../../../../include
-I../../../../include/QtCrypto -I. -I. qca-ossl.cpp -o qca-ossl.moc
g++ -c -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -Wall -W -D_REENTRANT -fPIC -DOSSL_097
-DQT_NO_DEBUG -DQT_PLUGIN -DQT_CORE_LIB -DQT_SHARED
-I../../../../lib/qt4/mkspecs/linux-g++ -I. -I../../../../include/QtCore
-I../../../../include/QtCore -I../../../../include
-I../../../../include/QtCrypto -I. -I. -o qca-ossl.o qca-ossl.cpp
qca-ossl.cpp:5263: error: conflicting return type specified for ‘virtual bool
opensslQCAPlugin::MyTLSContext::waitForResultsReady(int)’
/usr/include/QtCrypto/qcaprovider.h:520: error:   overriding ‘virtual void
QCA::TLSContext::waitForResultsReady(int)’
qca-ossl.cpp:6147: error: conflicting return type specified for ‘virtual bool
opensslQCAPlugin::MyMessageContext::waitForFinished(int)’
/usr/include/QtCrypto/qcaprovider.h:674: error:   overriding ‘virtual void
QCA::MessageContext::waitForFinished(int)’
qca-ossl.cpp:6459: error: expected class-name before ‘{’ token
qca-ossl.cpp:6466: error: ISO C++ forbids declaration of ‘Context’ with no type
qca-ossl.cpp:6466: error: expected ‘;’ before ‘*’ token
qca-ossl.cpp:6471: error: expected `;' before ‘QStringList’
qca-ossl.cpp:6459: warning: ‘class opensslQCAPlugin::opensslInfoContext’ has
virtual functions but non-virtual destructor
qca-ossl.cpp: In constructor
‘opensslQCAPlugin::opensslInfoContext::opensslInfoContext(QCA::Provider*)’:
qca-ossl.cpp:6462: error: class ‘opensslQCAPlugin::opensslInfoContext’ does not
have any field named ‘InfoContext’
qca-ossl.cpp: In member function ‘virtual QCA::Provider::Context*
opensslProvider::createContext(const QString&)’:
qca-ossl.cpp:6574: error: cannot convert ‘opensslQCAPlugin::opensslInfoContext*’
to ‘QCA::Provider::Context*’ in return
qca-ossl.moc: At global scope:
qca-ossl.moc:544: error: ‘InfoContext’ has not been declared
qca-ossl.moc: In member function ‘virtual void*
opensslQCAPlugin::opensslInfoContext::qt_metacast(const char*)’:
qca-ossl.moc:558: error: ‘InfoContext’ has not been declared
qca-ossl.moc: In member function ‘virtual int
opensslQCAPlugin::opensslInfoContext::qt_metacall(QMetaObject::Call, int, void**)’:
qca-ossl.moc:563: error: ‘InfoContext’ has not been declared
make: *** [qca-ossl.o] Error 1
error: Bad exit status from /var/tmp/rpm-tmp.89396 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.89396 (%build)
[petro@host-12-109 SPECS]$ rpm -q openssl
openssl-0.9.8b-15.fc7
[petro@host-12-109 SPECS]$ 

Comment 12 Aurelien Bompard 2007-10-26 21:09:37 UTC
Which version of qca2-devel do you have installed ? Please make sure you have
the qca2-2.0.0 final, the beta7 will cause this error.

Comment 13 Peter Lemenkov 2007-10-27 17:31:00 UTC
Ok, I updated oqa2 and now qca-ossl builds fine. So...

* rpmlint not silent but this "error" may be omitted

[petro@host-12-109 SPECS]$ rpmlint ../SRPMS/qca-ossl-0.1-3.20070904.fc7.src.rpm 
qca-ossl.src:35: E: configure-without-libdir-spec

+ package meets naming and packaging guidelines
+ specfile is properly named
+ package meets Packaging/Guidelines
+ package licensed with a Fedora approved license and meets the Licensing
Guidelines.
+ COPYING file included in %doc 
+ spec file written in American English
+ spec file legible
+ sources match the upstream source:
85713bfebcb8ed4fddf6f6bc8dd930a3
/usr/src/redhat/SOURCES/qca-ossl-0.1-20070904.tar.bz2
- wrong source in spec file
must me http://delta.affinix.com/download/qca/2.0/test4/qca-ossl-0.1-%{date}.tar.bz2

instead of 

http://delta.affinix.com/download/qca/2.0/beta7/qca-ossl-0.1-%{date}.tar.bz2
+ package builds file (x86)
+ all build dependencies listed in BuildRequires
+ package doesn't need to call ldconfig in %post and %postun
+ package owns all created directories
+ package not contains any duplicate files in the %files listing. 
+ permissions on files sets properly
+ package has a %clean section
+ package uses macros consistently 
+ package contains code or permissable content
+ header files are in a -devel package
+ package contains no static libs
+ package does not contain any .la libtool archives
+ not a GUI app
+ all filenames in rpm packages are valid UTF-8

Just fix the Source0 field and this package is APPROVED.

Comment 14 Aurelien Bompard 2007-10-27 18:28:01 UTC
Good catch ! I've updated the SRPM and I'll import the fixed one in CVS.
Thanks a lot for reviewing !

New Package CVS Request
=======================
Package Name: qca-ossl
Short Description: OpenSSL plugin for the Qt Cryptographic Architecture v2
Owners: abompard
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

Comment 15 Fedora Update System 2007-11-05 15:04:11 UTC
qca-ossl-0.1-4.20070904.fc7 has been pushed to the Fedora 7 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.