Bug 542458 - (libqt4pas) Review Request: libqt4pas - Wrapper library for Qt4
Review Request: libqt4pas - Wrapper library for Qt4
Status: CLOSED DUPLICATE of bug 987731
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2009-11-29 15:48 EST by matthias
Modified: 2013-08-06 23:33 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-09-02 11:08:39 EDT
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 matthias 2009-11-29 15:48:18 EST
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 15:20:03 EST
(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 16:50:13 EST
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 16:27:35 EST
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 16:32:44 EST
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 17:19:55 EST
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 10:11:41 EDT
ping?  any news? :)
Comment 7 matthias 2010-04-09 09:37:18 EDT
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 10:32:09 EDT
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 10:35:59 EDT
The right name for this lib is libqt4pas. Libqt4intf was the previous upstream name.
Comment 10 Rex Dieter 2010-04-09 10:53:12 EDT
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 13:46:59 EDT
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 11:53:46 EDT
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 11:55:03 EDT
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 07:36:03 EDT
Changed. Thanks!

http://listaller.nlinux.org/factory/rpmbuild/libqtpas4.spec
Comment 15 Rex Dieter 2010-05-11 17:00:14 EDT
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 17:59:50 EDT
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 11:08:39 EDT
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-06 23:33:36 EDT

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

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