Bug 542458 (libqt4pas)

Summary: Review Request: libqt4pas - Wrapper library for Qt4
Product: [Fedora] Fedora Reporter: matthias
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: alagunambiwelkin, fedora-package-review, i, matthias, notting, rdieter
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-09-02 15:08:39 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 201449    

Description matthias 2009-11-29 20:48:18 UTC
Spec URL: http://listaller.nlinux.org/factory/rpmbuild/libqtintf4.spec
SRPM URL: http://listaller.nlinux.org/factory/rpmbuild/libqt4intf5-1.72_Qt4.5.2-6.1.src.rpm
Description: The libqt4intf library allows applications which use Free Pascal and Lazarus to be compiled for Qt4. The binding can be used with Lazarus >= 0.9.28 or without the LCL in "pure Pascal" programs. It is also possible for other non-C++ languages to link against libqt4intf.so access Qt4.

I'm new to RPM packaging, I hope this package isn't too buggy.

Comment 1 Alagunambi Welkin 2009-12-29 20:20:03 UTC
(In reply to comment #0)
Hi,

Just a quick suggestions

################################################################################
rpmlint on libqtintf4.spec throwed this,
################################################################################
libqtintf4.spec:24: W: setup-not-quiet
libqtintf4.spec: E: no-cleaning-of-buildroot %install
libqtintf4.spec: E: no-cleaning-of-buildroot %clean
libqtintf4.spec: E: no-buildroot-tag
0 packages and 1 specfiles checked; 3 errors, 1 warnings.
################################################################################rpmlint on libqt4intf5.src.rpm
################################################################################
libqt4intf5.src: E: invalid-spec-name
libqt4intf5.src:24: W: setup-not-quiet
libqt4intf5.src: E: no-cleaning-of-buildroot %install
libqt4intf5.src: E: no-cleaning-of-buildroot %clean
libqt4intf5.src: E: no-buildroot-tag
libqt4intf5.src: W: non-standard-group System/Libraries
libqt4intf5.src: W: no-version-in-last-changelog
libqt4intf5.src: W: invalid-license LGPL v2+
libqt4intf5.src: W: no-url-tag
1 packages and 0 specfiles checked; 4 errors, 5 warnings.
################################################################################
Few suggested corrections:
################################################################################
#) Change the release form "0" to "1"
#) Change COPYING.TXT it is not mentioning which version of LGPL used
#) In SPEC Try to use "%{name}" and "%{version}" like macros, which will be easy to update
#) In Group change "System/Libraries" to "Development/Libraries" refer to /usr/share/doc/rpm-[version]/GROUPS
#) add "-q" along with your %setup to make it quiet: Error 1
#) echo will not be effective here
#) uncomment the "rm -rf %{buildroot}" in %clean: Error 3
#) Add "rm -rf %{buildroot}" in %{install}: Error 2
#) Error 4 can be ignored unless if you want this package to be on EPEL
#) Add your project url from where project source can be downloaded in URL tag
#) The availability of source in internet is *must*
################################################################################
For your reference
################################################################################
http://fedoraproject.org/wiki/Packaging/NamingGuidelines
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Spec_file_name
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#GPL_and_LGPL
https://fedoraproject.org/wiki/Packaging/RPMMacros
https://fedoraproject.org/wiki/PackagingDrafts/Changelog
https://fedoraproject.org/wiki/Packaging/SourceURL
https://fedoraproject.org/wiki/PackagingGuidelines#Shared_Libraries
https://fedoraproject.org/wiki/Archive:PackagingDrafts/BuildRootHandling
################################################################################

Please do rpmlint to your spec and *.rpm files before uploading


Alagunambi Welkin

Comment 2 matthias 2009-12-29 21:50:13 UTC
Okay, I fixed everything. Lintian is now silent, except for "E: no-buildroot-tag
"

You can find the updated specfile and the new src.rpm here:
http://listaller.nlinux.org/factory/rpmbuild/libqt4intf5-1.72_Qt4.5.2-1.src.rpm
http://listaller.nlinux.org/factory/rpmbuild/libqtintf4.spec

Comment 3 Rex Dieter 2010-01-20 21:27:35 UTC
A couple of initial comments/questions, then I'll do a formal review...

1.  SHOULD: please come up with a better package summary.  
Wrapper library for Qt4
is not very descriptive.

2.  Why the
BuildRequires:	phonon-backend-xine
?

Comment 4 Rex Dieter 2010-01-20 21:32:44 UTC
and some small simplifications and cosmetics would help, like

3.  MUST use the following scriptlets instead:
%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

4.  MUST: instead of /usr/%_lib  use %{_libdir}

5.  SHOULD: no need to quote stuff in %files

Comment 5 matthias 2010-01-20 22:19:55 UTC
There is a new release now which has a qmake-file. I will update the package.
But the new version of libqt4pas will then be usable with Lazarus 0.9.30, which is still in development.

Comment 6 Rex Dieter 2010-04-08 14:11:41 UTC
ping?  any news? :)

Comment 7 matthias 2010-04-09 13:37:18 UTC
Upstream changed the name of this library, so I made this package:
Spec URL: http://listaller.nlinux.org/factory/rpmbuild/libqtpas4.spec
Source tarball: http://listaller.nlinux.org/factory/rpmbuild/libqt4pas_2.1Qt4.5.3.tar.gz

Comment 8 Rex Dieter 2010-04-09 14:32:09 UTC
hrm, I see 2 package names used here, libqt4inf and libqt4pas , which is it?

I'll assume comment #7 is what is under scrutiny here.  Please confirm, and we can adjust this the bug summary to match.

Comment 9 matthias 2010-04-09 14:35:59 UTC
The right name for this lib is libqt4pas. Libqt4intf was the previous upstream name.

Comment 10 Rex Dieter 2010-04-09 14:53:12 UTC
1 MUST: missing upstream project URL: tag

2 MUST: Source: tags should contain full URLs (to at least be able to verify validity of upstream sources)  If a full URL cannot be provided, please include a comment in the .spec explaining why.
See https://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source
 
3 MUST:  Version tag is strange, 2.1Qt4.5.3 , I'd suggest using simply
Version: 2.1
I don't see much purpose encoding Qt or qt version here.  
See https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Version

4 MUST: many of the BuildRequires: are superfluous or not needed, I'd suggest trimming them down to simply:
BuildRequires: libxml2-devl
BuildRequires: qt4-devel
See https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRequires

5 MUST: Requires can be reduced to just:
Requires: qt4%{?_isa} >= %{_qt4_version}
See https://fedoraproject.org/wiki/Packaging/Guidelines#Requires

6 MUST: scripts can be adjusted to:
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
See https://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries

7 SHOULD: can use %{qt4_qmake} macro instead of qmake-qt4 (but that's up to you).

8 SHOULD: in %build, use
make %{?_smp_mflags}
if parallel make isn't supported, please add comment in .spec saying why

9 MUST:
Summary:        Qt4 interface bindings
is vague, please adjust to mention Pascal, maybe something like
Summary: Qt4 interface bindings for Pascal

10 MUST: -devel subpkg requires should be tightened to:
Requires:       %{name} = %{version}-%{release}
See https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

11 MUST: Group: tags invalid (or missing), I suggest
Group: System Environment/Libraries
and for -devel subpkg
Group: Development/Libraries

12 MUST: License tag invalid, use
License: LGPLv2+
instead.  See https://fedoraproject.org/wiki/Licensing#Good_Licenses

13 SHOULD: use of %{?dist} is highly recommended, I'd suggest using
Release: 1%{?dist}
Seehttps://fedoraproject.org/wiki/Packaging/NamingGuidelines#Using_the_.25.7B.3Fdist.7D_Tag

Comment 11 matthias 2010-04-09 17:46:59 UTC
Many thanks for this detailed description!
I use Debian as primary workstation and Fedora on a second machine. Cause I changed the specs on Debian this time and hadn't checked rpmlint, I've overseen those issues.
-> All changed

Comment 12 Rex Dieter 2010-04-21 15:53:46 UTC
7.  I made a typo, correct macro is %{_qt4_qmake}  .  sorry.

but I won't consider that a blocker.


I should note that whenever making any changes (even for review), please document them in the %changelog , and increment Release when doing so.  


The .spec now looks clean and conformant, APPROVED.

Comment 13 Rex Dieter 2010-04-21 15:55:03 UTC
ok, one more small suggestion, I'd recommend using the %{_datadir} macro, instead of hard-coding /usr/share
(unless there's a compelling reason to do otherwise, if so, please document that in a comment in the .spec).

Comment 14 matthias 2010-05-07 11:36:03 UTC
Changed. Thanks!

http://listaller.nlinux.org/factory/rpmbuild/libqtpas4.spec

Comment 15 Rex Dieter 2010-05-11 21:00:14 UTC
A couple of minor issues,
Source + Source0
%qt4_qmake vs %_qt4_qmake

fixed, so I could do a koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2181345

Comment 16 Rex Dieter 2010-08-09 21:59:50 UTC
Haven't heard back in awhile, please follow the next steps of the process, 

http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

Comment 17 Rex Dieter 2011-09-02 15:08:39 UTC
No response marking as stalled review per,
http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

please to ping back and re-open if you want to pursue this further.

Comment 18 Christopher Meng 2013-08-07 03:33:36 UTC

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