Merge lp:~unity-api-team/click/drop_packagekit into lp:click/devel

Proposed by Antti Kaijanmäki on 2016-02-05
Status: Merged
Merged at revision: 608
Proposed branch: lp:~unity-api-team/click/drop_packagekit
Merge into: lp:click/devel
Diff against target: 26 lines (+3/-2)
2 files modified
debian/control (+1/-1)
debian/packagekit-check (+2/-1)
To merge this branch: bzr merge lp:~unity-api-team/click/drop_packagekit
Reviewer Review Type Date Requested Status
Alejandro J. Cura (community) Approve on 2016-02-05
Colin Watson 2016-02-05 Disapprove on 2016-02-05
Review via email: mp+285225@code.launchpad.net

Commit message

As PK 1.0 does not support plugins anymore, drop the click plugin.

Description of the change

As PK 1.0 does not support plugins anymore, drop the click plugin.

This should land together with the PK 1.0 transition.

To post a comment you must log in.
Colin Watson (cjwatson) wrote :

No, this makes click largely unusable in the phone context. The correct approach is lp:~mvo/click/native-dbus to implement a D-Bus API of click's very own instead of piggybacking on PK; that needs finishing off and landing.

review: Disapprove
Antti Kaijanmäki (kaijanmaki) wrote :

> No, this makes click largely unusable in the phone context. The correct
> approach is lp:~mvo/click/native-dbus to implement a D-Bus API of click's very
> own instead of piggybacking on PK; that needs finishing off and landing.

We are forcing packagekit 0.x on the phone context. Therefore the PK plugin will continue to work there, but is not getting build for the main.

Alejandro J. Cura (alecu) wrote :

Hi Colin,

we took a look at mvo's proposed branch, but we are reluctant of adding a new dbus service running as root just for this, mostly when click will be deprecated when the phone moves to snappy.
So, we decided to keep the older version of PackageKit in the phone overlay, both for vivid and xenial based phones. And to ship the new version of PackageKit in the desktop where there's no use for click's packagekit plugin.

review: Approve
Colin Watson (cjwatson) wrote :

Well, I'd like to be on record that I think this is a bad idea, because in my experience this kind of divergence always lasts much longer than originally intended, so I'd prefer not to change my vote from Disapprove. However, it's up to you if you want to land it over my objections, since I'm not in charge of this project any more.

On a technical level, this branch is incorrect because it will break dual landings: it will have the effect of disabling PK even if an appropriate version of PK is available in the archive. To fix this, I would suggest restoring the build-dependency on libpackagekit-glib2-dev, but changing it to read "libpackagekit-glib2-dev (>= 0.8.10) | base-files, libpackagekit-glib2-dev (<< 1.0.0)" instead.

Please also fix the indentation so that it isn't confusing (i.e. don't have inner levels at the same level as outer levels). It would be simplest for it to just read:

  if dpkg --compare-versions "$packagekit_version" ge 0.8.10 && \
     dpkg --compare-versions "$packagekit_version" lt 1.0.0; then
          echo yes
  else
          echo no
  fi

Antti Kaijanmäki (kaijanmaki) wrote :

> On a technical level, this branch is incorrect because it will break dual
> landings:

Thanks! I will address these ASAP.

Alejandro J. Cura (alecu) wrote :

Thanks Colin for your concerns and attention to detail.

I'd like to add that in addition to the risk of introducing security issues by adding a new root service that gets called by userspace code, we believe it's a lot more work adding the new dbus interface because we'd have to modify and retest the phone parts that are used to install click packages: the click scope and the updates page in system settings of the top of my head. And like click itself we are short of maintainers there.

With a change like this we are minimizing the possible disruptions to the phone stack.

606. By Antti Kaijanmäki on 2016-02-09

return libpackagekit-glib2-dev build-dep

607. By Antti Kaijanmäki on 2016-02-09

simplify debian/packagekit-check

Antti Kaijanmäki (kaijanmaki) wrote :

> To fix this, I would suggest
> restoring the build-dependency on libpackagekit-glib2-dev, but changing it to
> read "libpackagekit-glib2-dev (>= 0.8.10) | base-files, libpackagekit-
> glib2-dev (<< 1.0.0)" instead.

done.

> It would be simplest for it to just read:
>
> if dpkg --compare-versions "$packagekit_version" ge 0.8.10 && \
> dpkg --compare-versions "$packagekit_version" lt 1.0.0; then
> echo yes
> else
> echo no
> fi

done.

Antti Kaijanmäki (kaijanmaki) wrote :

> "libpackagekit-glib2-dev (>= 0.8.10) | base-files, libpackagekit-glib2-dev (<< 1.0.0)"

Actually, does this also get satisfied on a system (near future xenial) that only has libpackagekit-glib2-dev > 1.0.0 available? We naturally want click to build there but without the plugin.

Colin Watson (cjwatson) wrote :

Sorry, this was a silly typo - apparently I got distracted half-way through typing those dependencies. I meant to say:

  libpackagekit-glib2-dev (>= 0.8.10) | base-files, libpackagekit-glib2-dev (<< 1.0.0) | base-files

This is logically equivalent to "(libpackagekit-glib2-dev (>= 0.8.10) & libpackagekit-glib2-dev (<< 1.0.0)) | base-files", but expanded out by Boolean algebra into a form permitted in dependencies.

608. By Antti Kaijanmäki on 2016-02-10

fix build-deps

Antti Kaijanmäki (kaijanmaki) wrote :

> Sorry, this was a silly typo - apparently I got distracted half-way through
> typing those dependencies.

np. should be fixed now :)

Antti Kaijanmäki (kaijanmaki) wrote :

@cjwatson: anything else that needs amending?

Colin Watson (cjwatson) wrote :

Assuming it builds sensibly, I think it's technically OK at this point.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2015-12-15 12:04:40 +0000
3+++ debian/control 2016-02-10 12:48:11 +0000
4@@ -3,7 +3,7 @@
5 Priority: optional
6 Maintainer: Colin Watson <cjwatson@ubuntu.com>
7 Standards-Version: 3.9.5
8-Build-Depends: debhelper (>= 9~), dh-autoreconf, intltool, python3:any (>= 3.2), python3-all:any, python3-setuptools, python3-apt, python3-debian, python3-gi, python3:any (>= 3.3) | python3-mock, pep8, python3-pep8, pyflakes, python3-sphinx, pkg-config, valac, gobject-introspection (>= 0.6.7), libgirepository1.0-dev (>= 0.6.7), libglib2.0-dev (>= 2.34), gir1.2-glib-2.0, libjson-glib-dev (>= 0.10), libgee-0.8-dev, libpackagekit-glib2-dev (>= 0.7.2), python3-coverage, python3-six, dh-systemd (>= 1.3)
9+Build-Depends: debhelper (>= 9~), dh-autoreconf, intltool, python3:any (>= 3.2), python3-all:any, python3-setuptools, python3-apt, python3-debian, python3-gi, python3:any (>= 3.3) | python3-mock, pep8, python3-pep8, pyflakes, python3-sphinx, pkg-config, valac, gobject-introspection (>= 0.6.7), libgirepository1.0-dev (>= 0.6.7), libglib2.0-dev (>= 2.34), gir1.2-glib-2.0, libjson-glib-dev (>= 0.10), libgee-0.8-dev, libpackagekit-glib2-dev (>= 0.8.10) | base-files, libpackagekit-glib2-dev (<< 1.0.0) | base-files, python3-coverage, python3-six, dh-systemd (>= 1.3)
10 Vcs-Bzr: https://code.launchpad.net/~click-hackers/click/devel
11 Vcs-Browser: http://bazaar.launchpad.net/~click-hackers/click/devel/files
12 X-Python-Version: >= 2.7
13
14=== modified file 'debian/packagekit-check'
15--- debian/packagekit-check 2013-09-09 09:56:42 +0000
16+++ debian/packagekit-check 2016-02-10 12:48:11 +0000
17@@ -2,7 +2,8 @@
18 set -e
19
20 packagekit_version="$(dpkg-query -f '${Version}\n' -W libpackagekit-glib2-dev || echo 0)"
21-if dpkg --compare-versions "$packagekit_version" ge 0.8.10; then
22+if dpkg --compare-versions "$packagekit_version" ge 0.8.10 && \
23+ dpkg --compare-versions "$packagekit_version" lt 1.0.0; then
24 echo yes
25 else
26 echo no

Subscribers

People subscribed via source and target branches

to all changes: