Bug 582864 - Review Request: qtlockedfile - QFile extension with advisory locking functions
Summary: Review Request: qtlockedfile - QFile extension with advisory locking functions
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kalev Lember
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 581220
TreeView+ depends on / blocked
 
Reported: 2010-04-16 01:55 UTC by Orcan Ogetbil
Modified: 2014-10-06 12:23 UTC (History)
5 users (show)

Fixed In Version: qtlockedfile-2.4-2.fc12
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-04-25 13:56:53 UTC
kalevlember: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Orcan Ogetbil 2010-04-16 01:55:32 UTC
Spec URL: http://6mata.com:8014/review/qtlockedfile.spec
SRPM URL: http://6mata.com:8014/review/qtlockedfile-2.4-1.fc12.src.rpm
Description: 
This class extends the QFile class with inter-process file locking capabilities.
If an application requires that several processes should access the same file,
QtLockedFile can be used to easily ensure that only one process at a time is
writing to the file, and that no process is writing to it while others are
reading it.

scratch builds look fine:
F-13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2118318
F-14: http://koji.fedoraproject.org/koji/taskinfo?taskID=2118323

rpmlint says:
qtlockedfile.x86_64: W: spelling-error Summary(en_US) QFile -> Q File, File, Filler
   This can be ignored
qtlockedfile.src:39: W: configure-without-libdir-spec
   This does not use autotools configure, hence this warning can be ignored.

Comment 1 Kalev Lember 2010-04-16 14:06:09 UTC
Fedora review qtlockedfile-2.4-1.fc12.src.rpm 2010-04-16

+ OK
! needs attention

rpmlint output:
qtlockedfile.i686: W: spelling-error Summary(en_US) QFile -> Q File, File, Filled
qtlockedfile.i686: W: unused-direct-shlib-dependency /usr/lib/libQtSolutions_LockedFile-2.4.so.1.0.0 /usr/lib/libQtGui.so.4
qtlockedfile.i686: W: unused-direct-shlib-dependency /usr/lib/libQtSolutions_LockedFile-2.4.so.1.0.0 /lib/libm.so.6
qtlockedfile.src: W: spelling-error Summary(en_US) QFile -> Q File, File, Filled
qtlockedfile.src:39: W: configure-without-libdir-spec
4 packages and 0 specfiles checked; 0 errors, 5 warnings.

! Most of the rpmlint warnings above are harmless and can be ignored, but the unused-direct-shlib-dependency libQtGui.so.4 is something that
 drags in unneeded qt-x11 package. If it's easy to fix it might be worth looking into.

+ The package is named according to the Package Naming Guidelines.
+ Spec file name matches the base package name
+ The package follows the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains license files (LGPL_EXCEPTION.txt, LICENSE.GPL3, and LICENSE.LGPL)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  ac8f848f59038a414f3ab4f4cc08e99c  qtlockedfile-2.4_1-opensource.tar.gz
  ac8f848f59038a414f3ab4f4cc08e99c  Download/qtlockedfile-2.4_1-opensource.tar.gz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file MUST handle locales properly
+ ldconfig is properly called in %post and %postun
+ Package does not bundle copies of system libraries
n/a Does not use Prefix: /usr
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set and %files has %defattr
+ Consistent use of macros
+ Package contains code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
+ Header files are in -devel
n/a Static libraries should be in -static
+ Library files that end in .so must go in a -devel package
+ -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a Packages containing GUI apps must include %{name}.desktop file
+ Packages must not own files or directories owned by other packages
+ Filenames must be valid UTF-8


I'll wait with approving this package until you've replied about the unused-direct-shlib-dependency libQtGui.so.4, but besides that everything looks good.

Comment 2 Orcan Ogetbil 2010-04-16 16:45:39 UTC
Hi Kalev, thanks for the review. The QtCore linkage can be removed easily by adding
   echo "QT -= gui" >> src/qtlockedfile.pri
to %prep. But I don't know how to remove the linkage to the math library. I don't see a "-lm" in the build log. Any ideas?

Comment 3 Kalev Lember 2010-04-16 18:52:16 UTC
I think it's enough to fix overlinking with QtGui, which would pull in some very large deps.

libm is part of glibc, so overlinking with libm wouldn't pull in any additional deps and is thus fine in my opinion.

Comment 4 Orcan Ogetbil 2010-04-17 02:12:14 UTC
I removed the libQtGui linkage:

Spec URL: http://6mata.com:8014/review/qtlockedfile.spec
SRPM URL: http://6mata.com:8014/review/qtlockedfile-2.4-2.fc12.src.rpm

Notes: A proper .pro file may be written (and submitted upstream) to set the installation locations correctly, so we can simply use "make install" but I need to learn qmake to do this. I don't have much time for it these days. Maybe I'll do this over the summer.

Comment 5 Kalev Lember 2010-04-17 08:31:10 UTC
Looks good.

APPROVED

Comment 6 Orcan Ogetbil 2010-04-17 08:58:17 UTC
Thank you for the review

New Package CVS Request
=======================
Package Name: qtlockedfile
Short Description: QFile extension with advisory locking functions
Owners: oget
Branches: F-11 F-12 F-13
InitialCC:

Comment 7 Orcan Ogetbil 2010-04-17 21:12:00 UTC
By the way, I think we need to write .cmake files, at least for qtsingleapplication, so that we can link to these library(s) from clementine, which uses cmake, without too much pain. cmake does not understand these .prf files.

Anyone comfortable writing .cmake files? What is a good simple example?

Comment 8 Kevin Fenzi 2010-04-18 01:49:19 UTC
CVS done (by process-cvs-requests.py).

Comment 9 Kalev Lember 2010-04-19 09:16:20 UTC
(In reply to comment #7)
> By the way, I think we need to write .cmake files, at least for
> qtsingleapplication, so that we can link to these library(s) from clementine,
> which uses cmake, without too much pain. cmake does not understand these .prf
> files.

I doubt qtsingleapplication's upstream is interested in supporting anything else besides qmake. Because of that reason, it's probably not worth it to add cmake support to qtsingleapplication.

Instead, I think clementine's cmake buildsystem should get proper qtsingleapplication detection as it's upstreamable in there, and fall back to bundled copy if system qtsingleapplication wasn't found.

Comment 10 Fedora Update System 2010-04-20 09:59:08 UTC
qtlockedfile-2.4-2.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/qtlockedfile-2.4-2.fc13

Comment 11 Fedora Update System 2010-04-20 09:59:40 UTC
qtlockedfile-2.4-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/qtlockedfile-2.4-2.fc12

Comment 12 Fedora Update System 2010-04-20 10:00:18 UTC
qtlockedfile-2.4-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/qtlockedfile-2.4-2.fc11

Comment 13 Fedora Update System 2010-04-21 02:17:44 UTC
qtlockedfile-2.4-2.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update qtlockedfile'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/qtlockedfile-2.4-2.fc12

Comment 14 Fedora Update System 2010-04-21 02:22:56 UTC
qtlockedfile-2.4-2.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update qtlockedfile'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/qtlockedfile-2.4-2.fc13

Comment 15 Fedora Update System 2010-04-21 02:24:43 UTC
qtlockedfile-2.4-2.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update qtlockedfile'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/qtlockedfile-2.4-2.fc11

Comment 16 Fedora Update System 2010-04-25 13:56:46 UTC
qtlockedfile-2.4-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2010-04-27 02:15:32 UTC
qtlockedfile-2.4-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2010-04-27 02:27:00 UTC
qtlockedfile-2.4-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 19 Fabio Alessandro Locati 2014-09-28 16:27:12 UTC
Package Change Request
======================
Package Name: qtlockedfile
New Branches: el6 epel7
Owners: fale

Comment 20 Fabio Alessandro Locati 2014-10-04 13:08:29 UTC
epel7 is already done. sorry for the wrong copy&paste

Comment 21 Gwyn Ciesla 2014-10-06 12:23:56 UTC
Git done (by process-git-requests).


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