Bug 333451 - Review Request: PackageKit - Package management with dbus and policykit
Review Request: PackageKit - Package management with dbus and policykit
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 333471
  Show dependency treegraph
 
Reported: 2007-10-15 23:47 EDT by Robin Norwood
Modified: 2007-11-30 17:12 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-05 15:57:07 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)
docs cleanup for SPEC file (3.66 KB, text/x-rpm-spec)
2007-10-24 02:25 EDT, Parag AN(पराग)
no flags Details

  None (edit)
Description Robin Norwood 2007-10-15 23:47:09 EDT
Spec URL: http://people.redhat.com/rnorwood/rpms/PackageKit.spec
SRPM: http://people.redhat.com/rnorwood/rpms/PackageKit-0.1.0-1.fc8.src.rpm
Description: 
PackageKit is a D-Bus abstraction layer that allows the session user
to manage packages in a secure way using a cross-distro,
cross-architecture API.

There are a few rpmlint errors:
"""
PackageKit.i386: E: non-executable-script /usr/lib/python2.5/site-packages/packagekit/backend.py 0644
"""
fixed upstream

"""
PackageKit.i386: E: non-executable-script /usr/lib/python2.5/site-packages/packagekit/frontend.py 0644
"""
fixed upstream

"""
PackageKit.i386: W: file-not-utf8 /usr/share/doc/packagekit/reference.html
PackageKit.i386: W: non-standard-dir-in-var db
"""

These two issues aren't fixed yet.  I'll look into them later.
Comment 1 Parag AN(पराग) 2007-10-16 00:09:04 EDT
mock build failed with following log
checking for SQLITE... configure: error: Package requirements (sqlite3) were not
met:

No package 'sqlite3' found


Comment 2 Robin Norwood 2007-10-16 00:25:20 EDT
Thanks for the quick feedback - new srpm and spec file are here now:

Spec URL: http://people.redhat.com/rnorwood/rpms/PackageKit.spec
SRPM: http://people.redhat.com/rnorwood/rpms/PackageKit-0.1.0-1.fc8.src.rpm
Comment 3 Parag AN(पराग) 2007-10-16 01:18:03 EDT
Kindly update release tag each time you give here updated SPEC and SRPM.

You may like to use make as
make %{?_smp_mflags}

Above SRPM gave me mock build error
You need to add 
BuildRequires: NetworkManager-glib-devel, PolicyKit-devel, libtool
Comment 4 Parag AN(पराग) 2007-10-16 01:23:39 EDT
Also,
  I can't see any man file getting installed as can be seen from build.log
    File not found by glob:
/var/tmp/PackageKit-0.1.0-2.fc8-root-mockbuild/usr/share/man/man1/*.1.gz
Comment 5 Parag AN(पराग) 2007-10-16 01:34:31 EDT
With above changes I got mock build successfully for i386 but build.log showed me
warning: File listed twice: /etc/dbus-1/system.d/PackageKit.conf
Comment 6 Parag AN(पराग) 2007-10-16 01:40:43 EDT
COPYING seems like GPLv2, you still want to have license tag as GPL+?

I think my comments are over now, You can update SPEC and provide new SRPM for
further review.
Comment 7 Robin Norwood 2007-10-16 13:41:04 EDT
Thanks for the feedback - applied your fixes here:

http://people.redhat.com/rnorwood/rpms/PackageKit-0.1.0-2.fc8.src.rpm
http://people.redhat.com/rnorwood/rpms/PackageKit.spec

Also, there's still some rpmlint output:

PackageKit.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/packagekit/backend.py 0644
PackageKit.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/packagekit/frontend.py 0644
PackageKit.i386: W: file-not-utf8 /usr/share/doc/packagekit/reference.html
PackageKit.i386: W: non-standard-dir-in-var db


I'm working with upstream to fix these issues - they should all be fixed in the
next release.

Finally, the license should be GPLv2+ - most of the source files state this
explicitly, a few are unclear.  This should also be fixed in the next upstream
version.
Comment 8 Parag AN(पराग) 2007-10-16 23:24:43 EDT
Thanks.

1) I found problem in mockbuild =>
RPM build errors:
    File not found by glob:
/var/tmp/PackageKit-0.1.0-2.fc8-root-mockbuild/usr/share/man/man1/*.1.gz
This is happening because of missing BR: docbook-utils in SPEC.

2) After adding missing BR when I mock build it went fine. But rpmlint complained
PackageKit.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/packagekit/backend.py 0644
This text file contains a shebang or is located in a path dedicated for
executables, but lacks the executable bits and cannot thus be executed.  If
the file is meant to be an executable script, add the executable bits,
otherwise remove the shebang or move the file elsewhere.

PackageKit.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/packagekit/frontend.py 0644
This text file contains a shebang or is located in a path dedicated for
executables, but lacks the executable bits and cannot thus be executed.  If
the file is meant to be an executable script, add the executable bits,
otherwise remove the shebang or move the file elsewhere.
===> Once you fix this in upstream. Kindly provide new SRPM for review with
fixes mentioned in this comment.

PackageKit.i386: W: file-not-utf8 /usr/share/doc/packagekit/reference.html
The character encoding of this file is not UTF-8.  Consider converting it
in the specfile for example using iconv(1).
==> You need to use iconv command on that file. Not needed in SPEC as you are
upstream you can release new tarball with this fix.


PackageKit.i386: W: non-standard-dir-in-var db
Your package is creating a non-standard subdirectory in /var. The standard
directories are:
account, lib, cache, crash, games, lock, log, opt, run, spool, state,
tmp, yp, www, ftp
==> This looks fake warning as /var/db is already owned by filesystem package.
Comment 9 Tim Lauridsen 2007-10-17 04:35:33 EDT
Robin:
Adding this to %install
chmod +x $RPM_BUILD_ROOT%{python_sitelib}/packagekit/*.py
should silence the E: non-executable-script errors.

It can be removed when next upstream tarball is released with the right file
permissions.
Comment 10 Robin Norwood 2007-10-17 10:43:12 EDT
Tim - thanks - I'm going to see today when Hughsie wants to release 0.1.1 - if
it's soon enough (like in a day or two), I might just wait for that.
Comment 11 Robin Norwood 2007-10-19 10:45:30 EDT
The next PK upstream release should be early next week.  I'll wait for that,
which fixes the above issues, adds some nice new functionality, and fixes
several more bugs.  Then I'll resubmit it for review.
Comment 12 Robin Norwood 2007-10-23 11:41:11 EDT
Ok - new version:

http://people.redhat.com/rnorwood/rpms/PackageKit-0.1.1-1.fc8.src.rpm
http://people.redhat.com/rnorwood/rpms/PackageKit.spec

Should fix all of the existing issues, as well as going to the latest version.
Comment 13 Parag AN(पराग) 2007-10-23 23:06:05 EDT
Got following in build.log
chmod: cannot access
`/var/tmp/PackageKit-0.1.1-1.fc8-root-mockbuild/usr/share/PackageKit/helpers/yum/yumBackend.py':
No such file or directory
Comment 14 Parag AN(पराग) 2007-10-24 02:23:18 EDT
Installed PackageKit after solving above problem but then found that package
PackageKit installed 
/usr/share/doc/packagekit/pk-reference.html
but can't find who owns /usr/share/doc/packagekit ? and why to create one more
docs directory for PackageKit?
I did some workaround for docs files here.
Comment 15 Parag AN(पराग) 2007-10-24 02:25:30 EDT
Created attachment 236001 [details]
docs cleanup for SPEC file
Comment 16 Robin Norwood 2007-10-24 16:50:45 EDT
o Included your docs cleanup changes
o It looks like the build is generating
/usr/share/doc/packagekit/pk-reference.html and copying it to the other doc
directory.  Changed the specfile to just remove the one in
/usr/share/doc/packagekit and leave the other.
o I'm mystified by the chmod error in comment #13 - I don't get that error, and
if I leave out the chmod command I get:

PackageKit.i386: E: script-without-shebang
/usr/share/PackageKit/helpers/yum/yumBackend.py

from rpmlint.

I'll leave it out for now.

New SRPM and spec file:

http://people.redhat.com/rnorwood/rpms/PackageKit.spec
http://people.redhat.com/rnorwood/rpms/PackageKit-0.1.1-2.fc8.src.rpm
Comment 17 Matthias Clasen 2007-10-25 01:11:35 EDT
Here is what I get when trying to build this srpm in mock:

checking for python... no
checking for python2... no
checking for python2.5... no
checking for python2.4... no
checking for python2.3... no
checking for python2.2... no
checking for python2.1... no
checking for python2.0... no
checking for python1.6... no
checking for python1.5... no
configure: error: no suitable Python interpreter found
error: Bad exit status from /var/tmp/rpm-tmp.7108 (%build)
Comment 18 Parag AN(पराग) 2007-10-25 01:16:17 EDT
mclasen,
  Are you using old rawhide(say one month old) by any chance? I also got same
error. But when I mock build on latest rawhide I got it working fine on my local
machine.
Comment 19 Tim Lauridsen 2007-10-25 03:20:46 EDT
(In reply to comment #17)
> Here is what I get when trying to build this srpm in mock:
> 
> checking for python... no
> checking for python2... no
> checking for python2.5... no
> checking for python2.4... no
> checking for python2.3... no
> checking for python2.2... no
> checking for python2.1... no
> checking for python2.0... no
> checking for python1.6... no
> checking for python1.5... no
> configure: error: no suitable Python interpreter found
> error: Bad exit status from /var/tmp/rpm-tmp.7108 (%build)
> 

I think 'BuildRequires: python-devel' is needed in the spec because
it contains python code. 
Comment 20 Parag AN(पराग) 2007-10-25 04:25:50 EDT
looks like my local rawhide setup gone wrong with last week's updates. Did a
koji build then and found this package is really missing 
BR:python-devel
Comment 22 Matthias Clasen 2007-10-26 00:05:24 EDT
The configure option changed, you must use --with-default-backend=yum now.
Comment 24 Parag AN(पराग) 2007-10-26 00:51:23 EDT
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
ad915a2cd3d5028fa6e399e0947aecdc  PackageKit-0.1.1.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is small so no need of -doc subpackage.
+ BuildRequires are proper.
+ Compiler flags are honoured correctly.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ packagekit.pc files are present.
+ -devel,-libs subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ ldconfig scriptlets are used.
+ package PackageKit-0.1.0-4.fc8 ->
  Provides: config(PackageKit) = 0.1.0-4.fc8
  Requires: /bin/sh /usr/bin/python PackageKit-libs = 0.1.1-4.fc9
config(PackageKit) = 0.1.1-4.fc9 dbus >= 0.90 libc.so.6 libc.so.6(GLIBC_2.0)
libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libdbus-1.so.3 libdbus-glib-1.so.2
libdl.so.2 libglib-2.0.so.0 libgmodule-2.0.so.0 libgobject-2.0.so.0
libgthread-2.0.so.0 libnm_glib.so.0 libpackagekit.so.2 libpolkit-dbus.so.2
libpolkit-grant.so.2 libpolkit.so.2 libpthread.so.0 libpthread.so.0(GLIBC_2.0)
librt.so.1 libsqlite3.so.0 python(abi) = 2.5 rtld(GNU_HASH) yum >= 3.2.6
+ package PackageKit-devel-0.1.0-4.fc8 ->
  Requires: PackageKit = 0.1.1-4.fc9 dbus-devel >= 0.90 libpackagekit.so.2
pkgconfig sqlite-devel
+ package PackageKit-libs-0.1.0-4.fc8 ->
  Provides: libpackagekit.so.2 libpk_backend_dummy.so libpk_backend_test_fail.so
libpk_backend_test_nop.so libpk_backend_test_spawn.so
libpk_backend_test_succeed.so libpk_backend_test_thread.so libpk_backend_yum.so
  Requires: dbus >= 0.90 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1.3)
libc.so.6(GLIBC_2.3.4) libdbus-1.so.3 libdbus-glib-1.so.2 libdl.so.2
libglib-2.0.so.0 libgmodule-2.0.so.0 libgobject-2.0.so.0 libgthread-2.0.so.0
libnm_glib.so.0 libpackagekit.so.2 libpthread.so.0 librt.so.1 rtld(GNU_HASH)
+ Not a GUI app.

APPROVED.

Comment 25 Parag AN(पराग) 2007-10-26 00:56:39 EDT
argh. Sorry for above mistake.
I was already prepared for review but then I modified it according to latest
SRPM and koji build log but mistakenly pasted old review text.

Heres correct provides and requires:
+ package PackageKit-0.1.1-4.fc8 ->
  Provides: config(PackageKit) = 0.1.1-4.fc8
  Requires: /bin/sh /usr/bin/python PackageKit-libs = 0.1.1-4.fc8
config(PackageKit) = 0.1.1-4.fc8 dbus >= 0.90 libc.so.6 libc.so.6(GLIBC_2.0)
libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libdbus-1.so.3 libdbus-glib-1.so.2
libdl.so.2 libglib-2.0.so.0 libgmodule-2.0.so.0 libgobject-2.0.so.0
libgthread-2.0.so.0 libnm_glib.so.0 libpackagekit.so.2 libpolkit-dbus.so.2
libpolkit-grant.so.2 libpolkit.so.2 libpthread.so.0 libpthread.so.0(GLIBC_2.0)
librt.so.1 libsqlite3.so.0 python(abi) = 2.5 rtld(GNU_HASH) yum >= 3.2.6
+ package PackageKit-devel-0.1.1-4.fc8 ->
  Requires: PackageKit = 0.1.1-4.fc8 dbus-devel >= 0.90 libpackagekit.so.2
pkgconfig sqlite-devel
+ package PackageKit-libs-0.1.1-4.fc8 ->
  Provides: libpackagekit.so.2 libpk_backend_dummy.so libpk_backend_test_fail.so
libpk_backend_test_nop.so libpk_backend_test_spawn.so
libpk_backend_test_succeed.so libpk_backend_test_thread.so libpk_backend_yum.so
  Requires: dbus >= 0.90 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1.3)
libc.so.6(GLIBC_2.3.4) libdbus-1.so.3 libdbus-glib-1.so.2 libdl.so.2
libglib-2.0.so.0 libgmodule-2.0.so.0 libgobject-2.0.so.0 libgthread-2.0.so.0
libnm_glib.so.0 libpackagekit.so.2 libpthread.so.0 librt.so.1 rtld(GNU_HASH)

Comment 26 Mamoru TASAKA 2007-10-26 02:32:14 EDT
Would you check what owns the following directories?
---------------------------------------------------------------
%{_localstatedir}/run/PackageKit/
%{_libdir}/packagekit-backend/
---------------------------------------------------------------
Comment 27 Mamoru TASAKA 2007-10-26 02:41:03 EDT
Also:
- According to this packaging, we can install PackageKit-libs only
  and we don't have to install PackageKit itself.
  In this case license text and other documents are not installed, which
  is not proper.
Comment 28 Parag AN(पराग) 2007-10-26 02:52:38 EDT
thanks for having a look on this package Mamoru.

You are right.
directory is not owned 
/usr/lib/packagekit-backend
and missed my eyes following rpm view
drwxr-xr-x    2 root    root                0 Oct 26 11:59 /var/lib/PackageKit
-rw-r--r--    1 root    root             3072 Oct 26 11:59
/var/lib/PackageKit/transactions.db
-rw-r--r--    1 root    root                2 Oct 26 11:59
/var/run/PackageKit/job_count.dat

so directory /var/run/PackageKit is not owned.

For Documentation thing I agree with you. -libs should install docs.
Comment 29 Parag AN(पराग) 2007-10-26 02:55:04 EDT
Mamoru,
Do you see any other issues in this packaging?
Comment 30 Mamoru TASAKA 2007-10-26 03:06:46 EDT
Well, one more thing
- It doesn't seem that PackageKit-devel should require sqlite-devel.
Comment 31 Robin Norwood 2007-10-26 13:44:53 EDT
Ok.  All of the above comments should be fixed with version 0.1.1-5:

http://people.redhat.com/rnorwood/rpms/PackageKit.spec
http://people.redhat.com/rnorwood/rpms/PackageKit-0.1.1-5.fc8.src.rpm
Comment 32 Jakub 'Livio' Rusinek 2007-10-26 14:14:01 EDT
You could replace PackageKit in Source0 with %{name}.

PS: I'm starting with packaging.
Comment 33 Mamoru TASAKA 2007-10-26 22:43:05 EDT
Seems okay to me.
Comment 34 Parag AN(पराग) 2007-10-26 23:26:28 EDT
New version looks ok now.
Comment 35 Robin Norwood 2007-10-27 00:15:06 EDT
New Package CVS Request
=======================
Package Name: PackageKit
Short Description: Package management with dbus and policykit
Owners: rnorwood
Branches: 
InitialCC: 
Cvsextras Commits: no
Comment 36 Mamoru TASAKA 2007-11-01 14:35:57 EDT
Please close this bug when rebuild is done.

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