GNOME Bugzilla – Bug 677394
Add "Install updates and restart" to the user-menu
Last modified: 2012-08-23 16:20:05 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.
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.
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)
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)
Also see bug 677367
*** Bug 677367 has been marked as a duplicate of this bug. ***
(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?
Ping? (At least for the gnome-session patch, as it breaks DBus API, so the earlier it gets in the better)
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?
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?
(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?
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.
Hm, I thought DBus handled enums natively. Guess not.
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 ?
The gnome-session patch should be in a gnome-session bug, to attract the attention of gnome-session maintainers...
I've filed bug 679084 againts gnome-session now
All the bits are in rawhide now to test this, I'm just waiting for some UI to trigger all the magic </hint> :)
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.
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?
(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
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.
Review of attachment 218195 [details] [review]: Looks fine to me.
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.
Attachment 218195 [details] pushed as 3483179 - UserMenu: show "Install Updates & Restart" when appropriate
*** Bug 649281 has been marked as a duplicate of this bug. ***