After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 677394 - Add "Install updates and restart" to the user-menu
Add "Install updates and restart" to the user-menu
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 649281 677367 (view as bug list)
Depends on: 679084
Blocks:
 
 
Reported: 2012-06-04 14:47 UTC by Richard Hughes
Modified: 2012-08-23 16:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GsmManager: add a command for showing a reboot dialog (12.00 KB, patch)
2012-06-04 17:36 UTC, Giovanni Campagna
needs-work Details | Review
UserMenu: show "Install Updates & Restart" when appropriate (4.36 KB, patch)
2012-06-04 17:37 UTC, Giovanni Campagna
needs-work Details | Review
UserMenu: show "Install Updates & Restart" when appropriate (4.29 KB, patch)
2012-06-27 14:23 UTC, Giovanni Campagna
none Details | Review
UserMenu: show "Install Updates & Restart" when appropriate (3.44 KB, patch)
2012-07-06 17:09 UTC, Giovanni Campagna
reviewed Details | Review
UserMenu: show "Install Updates & Restart" when appropriate (3.38 KB, patch)
2012-07-06 17:19 UTC, Giovanni Campagna
committed Details | Review

Description Richard Hughes 2012-06-04 14:47:30 UTC
(Sorry for the kinda-duplicate, the original bug got lost in yesterdays backup restore)

As part of http://freedesktop.org/wiki/Software/systemd/OSUpgrade and https://live.gnome.org/GnomeOS/Design/Whiteboards/SoftwareUpdates I've added the required bits to PackageKit in git master and would now like to add the UI bits to js/ui/userMenu.js

The basics are thus:

If the /var/lib/PackageKit/prepared-update file exists then we need to offer
the user to:
 - _Install updates and restart
 - _Suspend
rather than just:
 - _Suspend

If the user clicks "Install updates and restart" then we just need to execute "pkexec /usr/libexec/pk-trigger-offline-update" and then restart the computer the usual way. This will cause the computer to reboot and then update the system with all the pre-prepared updates. The computer will then automatically reboot when the updates are applied into the new system. In the future we'll be using btrfs snapshotting too, so it's a lot safer than just trying to update
random packages at runtime whilst the session is running.

I'm not exactly a javascript expert, so if anyone can give me a few pointers (or write the patch for me ;) then I'd appreciate it very much. Thanks!

Richard.
Comment 1 Giovanni Campagna 2012-06-04 17:35:58 UTC
It is possible that because the db was down, two of us started working at the same time, but I'm posting what I got anyway.
It needs a patch to gnome-session to request the reboot without bypassing session management.

Btw, I think a DBus API would be cleaner than an inotify watch and a pkexec helper, but I understand you don't want to keep packagekitd permanently alive.
Comment 2 Giovanni Campagna 2012-06-04 17:36:39 UTC
Created attachment 215567 [details] [review]
GsmManager: add a command for showing a reboot dialog

Before OS upgrades, applications may request a reboot. Add the
necessary infrastructure for showing a confirmation dialog, both
in fallback mode and in gnome-shell.

(patch for gnome-session)
Comment 3 Giovanni Campagna 2012-06-04 17:37:37 UTC
Created attachment 215568 [details] [review]
UserMenu: show "Install Updates & Restart" when appropriate

When PackageKit signals that it prepared an update, offer an option
to reboot and apply it, using a helper that will setup the next
reboot and then calling to gnome-session.

(patch for gnome-shell)
Comment 4 Florian Müllner 2012-06-04 19:01:07 UTC
Also see bug 677367
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-06-05 04:22:47 UTC
*** Bug 677367 has been marked as a duplicate of this bug. ***
Comment 6 Milan Bouchet-Valat 2012-06-05 08:52:49 UTC
(In reply to comment #0)
> If the user clicks "Install updates and restart" then we just need to execute
> "pkexec /usr/libexec/pk-trigger-offline-update" and then restart the computer
> the usual way. This will cause the computer to reboot and then update the
> system with all the pre-prepared updates. The computer will then automatically
> reboot when the updates are applied into the new system.
I'm confused. Is that "Install updates and restart", or "Restart and install updates (an restart again)"?

> In the future we'll be
> using btrfs snapshotting too, so it's a lot safer than just trying to update
> random packages at runtime whilst the session is running.
Nice idea, but it seems to me you can do that without restarting, and even without logging out, since the user's files are on a different path from system files.

Also, is that only for updates requiring a reboot, or for all updates?
Comment 7 Giovanni Campagna 2012-06-27 12:12:59 UTC
Ping?

(At least for the gnome-session patch, as it breaks DBus API, so the earlier it gets in the better)
Comment 8 Jasper St. Pierre (not reading bugmail) 2012-06-27 13:59:22 UTC
Review of attachment 215568 [details] [review]:

Your patch is literally the same as Florian's patch.

::: js/ui/userMenu.js
@@ +589,3 @@
+        this._haveUpdates = this._updatesFile.query_exists(null);
+
+        if (!this._installUpdatesItem)

???

@@ +711,3 @@
+        item.connect('activate', Lang.bind(this, this._onInstallUpdatesActivate));
+        this.menu.addMenuItem(item);
+        this._installUpdatesItem = item;

Call this._updateInstallUpdates here?
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-06-27 14:01:02 UTC
Review of attachment 215567 [details] [review]:

I don't understand the purpose of this - don't we already have the restart API?

::: gnome-session/org.gnome.SessionManager.xml
@@ +251,2 @@
     <method name="Shutdown">
+      <arg name="action" direction="in" type="u">

Make this an enum?
Comment 10 Giovanni Campagna 2012-06-27 14:23:34 UTC
(In reply to comment #8)
> Review of attachment 215568 [details] [review]:
> 
> Your patch is literally the same as Florian's patch.

It's similar, but not identical: mine shows a Reboot dialog (and depends on gnome-session for that), his shows the usual Power Off one.

> ::: js/ui/userMenu.js
> @@ +589,3 @@
> +        this._haveUpdates = this._updatesFile.query_exists(null);
> +
> +        if (!this._installUpdatesItem)
> 
> ???

Boh! Killed :)

> @@ +711,3 @@
> +        item.connect('activate', Lang.bind(this,
> this._onInstallUpdatesActivate));
> +        this.menu.addMenuItem(item);
> +        this._installUpdatesItem = item;
> 
> Call this._updateInstallUpdates here?

It is called when the menu is opened, through updateHaveShutdown.

(In reply to comment #9)
> Review of attachment 215567 [details] [review]:
> 
> I don't understand the purpose of this - don't we already have the restart API?

No, just Logout and Shutdown.

> ::: gnome-session/org.gnome.SessionManager.xml
> @@ +251,2 @@
>      <method name="Shutdown">
> +      <arg name="action" direction="in" type="u">
> 
> Make this an enum?

How so?
Comment 11 Giovanni Campagna 2012-06-27 14:23:55 UTC
Created attachment 217409 [details] [review]
UserMenu: show "Install Updates & Restart" when appropriate

When PackageKit signals that it prepared an update, offer an option
to reboot and apply it, using a helper that will setup the next
reboot and then calling to gnome-session.

Rebased.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-06-27 14:24:49 UTC
Hm, I thought DBus handled enums natively. Guess not.
Comment 13 Matthias Clasen 2012-06-28 00:46:48 UTC
Review of attachment 215567 [details] [review]:

This is an api change in gnome-session. There's at least one caller beyond gnome-shell, namely the media-keys plugin in gsd.
Can we not just add a Reboot method and avoid the api break ?
Comment 14 Matthias Clasen 2012-06-28 00:54:38 UTC
The gnome-session patch should be in a gnome-session bug, to attract the attention of gnome-session maintainers...
Comment 15 Matthias Clasen 2012-06-28 19:41:00 UTC
I've filed bug 679084 againts gnome-session now
Comment 16 Richard Hughes 2012-07-06 13:54:45 UTC
All the bits are in rawhide now to test this, I'm just waiting for some UI to trigger all the magic </hint> :)
Comment 17 Giovanni Campagna 2012-07-06 17:09:39 UTC
Created attachment 218192 [details] [review]
UserMenu: show "Install Updates & Restart" when appropriate

When PackageKit signals that it prepared an update, offer an option
to reboot and apply it, using a helper that will setup the next
reboot and then calling to gnome-session.

Second try, without the API break. Message changed "Reboot and install updates",
which more closely represents what will happen.
Comment 18 Jasper St. Pierre (not reading bugmail) 2012-07-06 17:15:17 UTC
Review of attachment 218192 [details] [review]:

Mostly fine.

::: js/ui/userMenu.js
@@ +526,3 @@
+        this._updatesFile = Gio.File.new_for_path('/var/lib/PackageKit/prepared-update');
+        this._updatesMonitor = this._updatesFile.monitor(Gio.FileMonitorFlags.NONE,
+                                                         null);

This can go on the same line.

@@ +723,3 @@
         this.menu.addMenuItem(item);
 
+        item = new PopupMenu.PopupMenuItem(_("Restart and install updates"));

This string should be "Install Updates & Restart", if anything. I couldn't find any design page giving the correct string with a cursory look.

@@ +788,3 @@
+    _onInstallUpdatesActivate: function() {
+        Main.overview.hide();
+        Util.spawn(['pkexec', '/usr/libexec/pk-trigger-offline-update']);

ew.

Are we going to have a standard policy to permit that?
Comment 19 Giovanni Campagna 2012-07-06 17:18:01 UTC
(In reply to comment #18)
> Review of attachment 218192 [details] [review]:
> 
> @@ +788,3 @@
> +    _onInstallUpdatesActivate: function() {
> +        Main.overview.hide();
> +        Util.spawn(['pkexec', '/usr/libexec/pk-trigger-offline-update']);
> 
> ew.
> 
> Are we going to have a standard policy to permit that?

Yes, it's shipped with PackageKit
Comment 20 Giovanni Campagna 2012-07-06 17:19:14 UTC
Created attachment 218195 [details] [review]
UserMenu: show "Install Updates & Restart" when appropriate

When PackageKit signals that it prepared an update, offer an option
to reboot and apply it, using a helper that will setup the next
reboot and then calling to gnome-session.
Comment 21 Jasper St. Pierre (not reading bugmail) 2012-07-06 17:25:20 UTC
Review of attachment 218195 [details] [review]:

Looks fine to me.
Comment 22 Matthias Clasen 2012-07-07 04:43:02 UTC
Review of attachment 218195 [details] [review]:

::: js/ui/userMenu.js
@@ +585,3 @@
+    _updateInstallUpdates: function() {
+        let haveUpdates = this._updatesFile.query_exists(null);
+        this._installUpdatesItem.actor.visible = let haveUpdates && this._haveShutdown;

There's an extra 'let' here.
After removing that, the patch works as expected.
Comment 23 Giovanni Campagna 2012-07-07 16:27:33 UTC
Attachment 218195 [details] pushed as 3483179 - UserMenu: show "Install Updates & Restart" when appropriate
Comment 24 Allan Day 2012-08-23 16:20:05 UTC
*** Bug 649281 has been marked as a duplicate of this bug. ***