|
|
Chromium Code Reviews|
Created:
8 years, 3 months ago by Sergey Ulanov Modified:
8 years, 3 months ago CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, rmsousa+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionFix debian compatibility issues in the chrome-remote-desktop dpk.
This fixes some issues I have installing dpk on debian (actually
debian-based mint linux):
- update-manager may not be installed by default, and we don't strictly need
it, so removing it from dependencies.
- The init script depends on bash-specific features, so updating shebang to
bash instead of sh.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=156139
Patch Set 1 #
Total comments: 2
Messages
Total messages: 7 (0 generated)
http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... File remoting/host/installer/linux/debian/control (right): http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... remoting/host/installer/linux/debian/control:15: Depends: xvfb-randr, python (>= 2.6), ${shlibs:Depends}, ${misc:Depends}, ${python:Depends} The reason we depend on update-notifier is that the postinst script creates a file inside /var/lib/update-notifier/user.d in order to notify the user that they need to restart their virtual desktop. At minimum, we should have "update-notifier" as "Recommends:" if we remove it from "Depends:". In that case, we must ensure that our postinst script runs with exit-code 0 if update-manager isn't actually installed. Better still: we should find out what package "xxx" is the Debian/Linux-Mint equivalent of Ubuntu's update-manager. Change our package so that the notification works on those other distros as well. Then update the Depends: to "update-notifier | xxx".
http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... File remoting/host/installer/linux/debian/control (right): http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... remoting/host/installer/linux/debian/control:15: Depends: xvfb-randr, python (>= 2.6), ${shlibs:Depends}, ${misc:Depends}, ${python:Depends} On 2012/09/07 18:21:59, Lambros wrote: > The reason we depend on update-notifier is that the postinst script creates a > file inside /var/lib/update-notifier/user.d in order to notify the user that > they need to restart their virtual desktop. postinst script already handles the case when this directory doesn't exist. > > At minimum, we should have "update-notifier" as "Recommends:" if we remove it > from "Depends:". In that case, we must ensure that our postinst script runs > with exit-code 0 if update-manager isn't actually installed. I don't think adding it in Recommends list is the right thing to do. From Debian docs: "The Recommends field should list packages that would be found together with this one in all but unusual installations." I don't see why we need to worry about these update nags anyway - it's only a way to annoy users. E.g. does Xorg or Gnome nag you to restart once they are updated? Even linux kernel packages don't have update-manager dependency - how is our package special? > > Better still: we should find out what package "xxx" is the Debian/Linux-Mint > equivalent of Ubuntu's update-manager. Change our package so that the > notification works on those other distros as well. Then update the Depends: to > "update-notifier | xxx". AFAIK mintupdate which is used on mint doesn't have similar notifications system, and even if it did, I think supporting it would be waste of time anyway.
On 2012/09/07 20:31:50, sergeyu wrote: > http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... > File remoting/host/installer/linux/debian/control (right): > > http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... > remoting/host/installer/linux/debian/control:15: Depends: xvfb-randr, python (>= > 2.6), ${shlibs:Depends}, ${misc:Depends}, ${python:Depends} > On 2012/09/07 18:21:59, Lambros wrote: > > The reason we depend on update-notifier is that the postinst script creates a > > file inside /var/lib/update-notifier/user.d in order to notify the user that > > they need to restart their virtual desktop. > > postinst script already handles the case when this directory doesn't exist. OK, just wanted to be sure. > > > > > At minimum, we should have "update-notifier" as "Recommends:" if we remove it > > from "Depends:". In that case, we must ensure that our postinst script runs > > with exit-code 0 if update-manager isn't actually installed. > > I don't think adding it in Recommends list is the right thing to do. From Debian > docs: > "The Recommends field should list packages that would be found together with > this one in all but unusual installations." > > I don't see why we need to worry about these update nags anyway - it's only a > way to annoy users. E.g. does Xorg or Gnome nag you to restart once they are > updated? Even linux kernel packages don't have update-manager dependency - how > is our package special? linux-image and firefox (and a few others) do use update-notifier to nag the user to reboot (or restart the browser), similar to what we've done. However, they don't seem to express any dependency on update-notifier, so I guess you're right, it's OK for us to do the same here. We're basically now saying: if you have update-notifier on your system, we'll use it to nag you about updates; but if you don't, we won't, and the package will still install and run just fine. > > > > > Better still: we should find out what package "xxx" is the Debian/Linux-Mint > > equivalent of Ubuntu's update-manager. Change our package so that the > > notification works on those other distros as well. Then update the Depends: > to > > "update-notifier | xxx". > > AFAIK mintupdate which is used on mint doesn't have similar notifications > system, and even if it did, I think supporting it would be waste of time anyway. Yeah, not worth the effort at this point. OK, lgtm, but let's wait for Jamie.
On 2012/09/08 00:27:08, Lambros wrote: > On 2012/09/07 20:31:50, sergeyu wrote: > > > http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... > > File remoting/host/installer/linux/debian/control (right): > > > > > http://codereview.chromium.org/10917125/diff/1/remoting/host/installer/linux/... > > remoting/host/installer/linux/debian/control:15: Depends: xvfb-randr, python > (>= > > 2.6), ${shlibs:Depends}, ${misc:Depends}, ${python:Depends} > > On 2012/09/07 18:21:59, Lambros wrote: > > > The reason we depend on update-notifier is that the postinst script creates > a > > > file inside /var/lib/update-notifier/user.d in order to notify the user that > > > they need to restart their virtual desktop. > > > > postinst script already handles the case when this directory doesn't exist. > OK, just wanted to be sure. > > > > > > > > > At minimum, we should have "update-notifier" as "Recommends:" if we remove > it > > > from "Depends:". In that case, we must ensure that our postinst script runs > > > with exit-code 0 if update-manager isn't actually installed. > > > > I don't think adding it in Recommends list is the right thing to do. From > Debian > > docs: > > "The Recommends field should list packages that would be found together with > > this one in all but unusual installations." > > > > I don't see why we need to worry about these update nags anyway - it's only a > > way to annoy users. E.g. does Xorg or Gnome nag you to restart once they are > > updated? Even linux kernel packages don't have update-manager dependency - how > > is our package special? > linux-image and firefox (and a few others) do use update-notifier to nag the > user to reboot (or restart the browser), similar to what we've done. However, > they don't seem to express any dependency on update-notifier, so I guess you're > right, it's OK for us to do the same here. > > We're basically now saying: if you have update-notifier on your system, we'll > use it to nag you about updates; but if you don't, we won't, and the package > will still install and run just fine. > > > > > > > > > Better still: we should find out what package "xxx" is the Debian/Linux-Mint > > > equivalent of Ubuntu's update-manager. Change our package so that the > > > notification works on those other distros as well. Then update the Depends: > > to > > > "update-notifier | xxx". > > > > AFAIK mintupdate which is used on mint doesn't have similar notifications > > system, and even if it did, I think supporting it would be waste of time > anyway. > Yeah, not worth the effort at this point. > > OK, lgtm, but let's wait for Jamie. If update-notifier is included in the Depends: line, and isn't installed, won't it be installed automatically? I think that's the ideal scenario. The problem with not being able to notify the user is that our auto-update won't work if we change the Python script, so long-lived desktops will drift gradually out-of-date. We're not asking the user to reboot, just log out so it's not so annoying. Also, if the user dismisses the prompt, it won't come back.
On 2012/09/09 16:21:35, Jamie wrote: > If update-notifier is included in the Depends: line, and isn't installed, won't > it be installed automatically? I think that's the ideal scenario. It is installed automatically only when apt-get is used to install the package, but we want the user to be able download DPK instead of adding apt repo manually - that's the way it works for chrome and it's much easier than explaining to the user how to add apt repo. Even if it was installed automatically I don't think it's right thing to do. E.g. Mint Linux uses it's own update manager (called mintupdate) - we don't want to install update-manager alongside. > The problem > with not being able to notify the user is that our auto-update won't work if we > change the Python script, so long-lived desktops will drift gradually > out-of-date. Yes, but that's also a problem with a lot of other packages, e.g. Gnome and Xorg may also be update while user is logged in, yet none of them depend on update-manager. Besides, it's not really a problem if user updates the host but doesn't restart it - the host should still keep working. If it works there is not reason to nag the user about restarting it. Also the user will normally see when the package is updated (updates are not silent by default in Ubuntu), so he/she will still know when the package has been updated. > We're not asking the user to reboot, just log out so it's not so > annoying. Also, if the user dismisses the prompt, it won't come back. I'm not trying to remove this prompt - in fact most users will still see it as update-manager is installed by default on Ubuntu. I just want to make it possible to use this package without update-manager.
Okay, that all seems reasonable, so lgtm. I do think we should do our best to display a notification to the user, so if mint is popular enough, we should integrate with its notification system as well. Not in this CL though. On 2012/09/10 18:46:46, sergeyu wrote: > On 2012/09/09 16:21:35, Jamie wrote: > > If update-notifier is included in the Depends: line, and isn't installed, > won't > > it be installed automatically? I think that's the ideal scenario. > > It is installed automatically only when apt-get is used to install the package, > but we want the user to be able download DPK instead of adding apt repo manually > - that's the way it works for chrome and it's much easier than explaining to the > user how to add apt repo. Even if it was installed automatically I don't think > it's right thing to do. E.g. Mint Linux uses it's own update manager (called > mintupdate) - we don't want to install update-manager alongside. > > > The problem > > with not being able to notify the user is that our auto-update won't work if > we > > change the Python script, so long-lived desktops will drift gradually > > out-of-date. > > Yes, but that's also a problem with a lot of other packages, e.g. Gnome and Xorg > may also be update while user is logged in, yet none of them depend on > update-manager. Besides, it's not really a problem if user updates the host but > doesn't restart it - the host should still keep working. If it works there is > not reason to nag the user about restarting it. Also the user will normally see > when the package is updated (updates are not silent by default in Ubuntu), so > he/she will still know when the package has been updated. > > > We're not asking the user to reboot, just log out so it's not so > > annoying. Also, if the user dismisses the prompt, it won't come back. > > I'm not trying to remove this prompt - in fact most users will still see it as > update-manager is installed by default on Ubuntu. I just want to make it > possible to use this package without update-manager. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||
