Re: RFS: packagekit
Hi!
>>
http://mentors.debian.net/debian/pool/main/p/packagekit/packagekit_0.6.7-1.dsc
>
> A review as promised...
>
> The copyright situation is much more complex than what you present in
> debian/copyright. Please look at each file and fully document the
> license situation. It is possible to have a per-binary-package
> copyright file, which you might want to do since the libraries are
> licensed differently.
I updated the license file. Should be okay now.
> Why do you need to build in a subdirectory? Seems like it complicates
> the debian/rules file for no benefit. You can drop many of the flags
> from DEB_BASIC_AUTOTOOLS_FLAGS by using dh_auto_configure --
> $(DEB_BASIC_AUTOTOOLS_FLAGS) $(PK_CONFIGURE_FLAGS) instead. Same for
> make vs dh_auto_install.
Changed. I built in a subdirectory because I thought maybe the scripts
would create their modified file versions there so I could easily cleanup
the sources. This is not the case, so I dropped the out-of-source build as
you said.
> Why do you move the upstream helper scripts to /usr/lib?
They're scripts and should not be in /usr/share/PackageKit. In this
directory we only have documentation of PK.
> Not sure that patch removal stuff is a good idea. Better to fix
> upstream to not include generated files in the tarball.
I asked upstream to do this. As soon as the problem is fixed I will remove
the patch removal.
> Can the ln -s can be achieved with dh_link instead?
Changed.
> The watch file isn't quite as specific as it could be, I would replace
> (.*) with ([\d\.]+) and drop the blank line.
Okay, done.
> Why is most of 01_set_defaults.patch needed? Shouldn't we only need to
> change the iceweasel thing?
The patch sets the optimal settings for the APT(cc) backends for Debian.
The other settings are set to good values which Kubuntu uses too.
> 02_update_aptcc.patch is a pretty big patch, has it been applied
upstream
> yet?
Yes. Will be included in the next PK release.
> For the URLs in 00_set_vendor.patch I would suggest creating new pages
> in the PackageKit namespace on the wiki focused on what PackageKit
> needs from them and based on what other distros have in the equivalent
> pages. Then make those pages CategoryPermalink.
I'll do this. Can copy the Ubuntu page with instructions how to add new
repositories? It's a pretty nice page.
> I wonder if the s/emblem-favorite/distributor-logo/ part of
> 00_set_vendor.patch should be forwarded upstream.
Don't know... Fedora does not change this setting, so I think it's okay
since Richard is the maintainer of the Fedora packaging and could easily
change it upstream too.
> The browser plugin package naming has changed recently, they should no
> longer be called mozilla-* but browser-plugin-* or xul-ext-* IIRC.
Okay, changed.
> Is the smart backend useful in Debian? I can't see smart in Debian.
See http://labix.org/smart . We do have a smart package in Debian Sid.
> The upstream README duplicates the information from debian/control,
> drop it from debian/docs.
Done.
> Please add explanations to README.Debian about why things aren't
shipped.
Added.
> Would it be a good idea to turn on unit tests? That would help ensure
> the package works on all arches.
Unit tests always failed in pbuilder and while running a local debuild. I
think they're broken at time, cause Fedora has them disabled too.
I'll enable them if they do not block the build process.
> Which features of PackageKit in Fedora will not work in Debian?
>
> An automated warning:
>
> configure: WARNING: Distro upgrade notification not supported
Strange... ALL features of PackageKit the backend supports will work on
Debian. (And the APT backends do support most of the PK features) Distro
upgrade notifications have to be done by KPackageKit or GNOME-PackageKit.
But anyway I don't think it's a good idea to let PackageKit do
distro-upgrades, so APT will notify about a new distro release and do the
upgrade instead. (PackageKit won't interrupt APT working as usual)
> The package FTBFS in pbuilder:
>
> [...]
>
> ../configure: line 3493: syntax error near unexpected token `0.6.7'
> ../configure: line 3493: `GOBJECT_INTROSPECTION_CHECK(0.6.7)'
> make[1]: *** [config.status] Error 2
> make[1]: *** Waiting for unfinished jobs....
> lib/packagekit-glib2/Makefile.am:234: HAVE_INTROSPECTION does not
> appear in AM_CONDITIONAL
> lib/packagekit-glib2/Makefile.am:241: addprefix $(srcdir: non-POSIX
> variable name
> lib/packagekit-glib2/Makefile.am:241: (probably a GNU make extension)
> make[1]: *** [../Makefile.in] Error 1
> make[1]: Leaving directory `/tmp/buildd/packagekit-0.6.7/build'
> dh_auto_build: make -j2 returned exit code 2
> make: *** [build] Error 2
> dpkg-buildpackage: error: debian/rules build gave error exit status 2
Fixed.
Thanks!
Reply to: