Simple Chrome workflow should copy Chrome's D-Bus permission files |
|||||||
Issue description
The chromeos-chrome ebuild's src_install step currently installs D-Bus permissions for org.chromium.LibCrosService.conf:
# Copy org.chromium.LibCrosService.conf, the D-Bus config file for the
# D-Bus service exported by Chrome.
insinto /etc/dbus-1/system.d
DBUS="${CHROME_ROOT}"/src/chromeos/dbus/services
doins "${DBUS}"/org.chromium.LibCrosService.conf
The Simple Chrome workflow doesn't do this, which is problematic because it means that developers trying to install ToT Chrome on older OS images might see Chrome not have permission to own newly-added D-Bus service names.
chromite's deploy_chrome script (which is used by both Simple Chrome and the ebuild) ought to install *.conf from this directory; then we can remove this from the ebuild (and moving stuff from the ebuild to deploy_chrome is a long-term goal, in any case).
Unfortunately, I don't see any support in deploy_chrome.py or chrome_util.py for installing source files (as opposed to files from the output dir). I'm also not sure that deploy_chrome supports installing to /etc/dbus-1/system.d rather than to /opt/google/chrome.
One approach might be:
a) change the Chrome gn build to copy src/chromeos/dbus/services/*.conf to the output dir
b) update chromite to copy these files to /opt/google/chrome/dbus
c) update /etc/dbus-1/system.conf to read /opt/google/chrome/dbus in addition to /etc/dbus-1/system.d
d) update the ebuild to not copy these files
I don't know if there's trickiness around synchronizing these changes.
Setting this to available in case it catches Justin's interest. :-)
,
Mar 15 2017
Is there already code in deploy_chrome for copying files from the source tree? I didn't see it in a quick pass through the files. I think it's currently geared toward copying files from the output dir. I suggested adding a new /opt/google/chrome/dbus directory for these since I didn't see support for copying to dests outside of /opt/google/chrome and since we'll end up with stale files when things are removed (but since we mostly just use these config files to add permissions, that may not cause problems for developers).
,
Mar 15 2017
We might not be copying from src. You may need (or want) to copy the files to the out/ directory as part of the build step to avoid making assumptions about the src path.
,
Mar 15 2017
Moving from ebuild script to chromite python code generally sounds good. Are we going to move all doins (and dosym/dodir) from chromeos-chrome ebuild to deploy_chrome?
,
Mar 15 2017
I'm happy to take a look at this, it'll make it much easier to test changes for crbug.com/692246 .
,
Mar 15 2017
Thanks, and let me know if you get stuck! /etc/dbus-1/system.conf looks like it's installed by the sys-apps/dbus package (determined by running "equery-lumpy b /etc/dbus-1/system.conf" in my chroot), so you may need to apply a patch in that package's ebuild to add the additional path. (I think that we should probably move Chrome's D-Bus config files to /opt/google/chrome since devs play various games with symlinking that directory to another partition to get more free space if they're installing an unstripped version of Chrome. It could get confusing if Simple Chrome additionally installs files to /etc/dbus-1.)
,
Mar 15 2017
Re comment #4 - I would love to move as much as possible to deploy_chrome. In particular, some of the files used for test should definitely be moved so that when we get VM Tests working in Simple Chrome they use the latest Chrome files and not whatever is on the chromeos image. +achuith@
,
Mar 15 2017
Great! But let's use other bugs to track the other stuff to move. :-)
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/toolchain-utils/+/cdd7d45be9b171835027165ed0474bf8dbc04a9d commit cdd7d45be9b171835027165ed0474bf8dbc04a9d Author: Justin TerAvest <teravest@chromium.org> Date: Fri Mar 17 12:46:46 2017 Remove reference to chromeos.gyp. chromeos.gyp is used here to detect a path to a valid chrome browser checkout. I'd like to get rid of chromeos.gyp, so this changes the logic to use BUILD.gn, which was added to the source in June 2014. BUG= chromium:701617 TEST=None Change-Id: I16f1705874bb8acafca0d3c38211f5e192e00a33 Reviewed-on: https://chromium-review.googlesource.com/455279 Commit-Ready: Justin TerAvest <teravest@chromium.org> Tested-by: Justin TerAvest <teravest@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: Ting-Yuan Huang <laszio@chromium.org> [modify] https://crrev.com/cdd7d45be9b171835027165ed0474bf8dbc04a9d/build_chrome_browser.py
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54ee225d47dd22fe7d2a2cb75bde7d27d790bf49 commit 54ee225d47dd22fe7d2a2cb75bde7d27d790bf49 Author: teravest <teravest@chromium.org> Date: Fri Mar 17 14:18:16 2017 Remove chromeos/chromeos.gyp. This migrates the variables in chromeos.gyp to BUILD.gn, making the source easier to read. This also lets us remove a use of exec_script, so chromeos/BUILD.gn no longer has to be whitelisted to use it. BUG= 701617 Review-Url: https://codereview.chromium.org/2753883002 Cr-Commit-Position: refs/heads/master@{#457759} [modify] https://crrev.com/54ee225d47dd22fe7d2a2cb75bde7d27d790bf49/.gn [modify] https://crrev.com/54ee225d47dd22fe7d2a2cb75bde7d27d790bf49/chromeos/BUILD.gn [delete] https://crrev.com/3c10af5a884d33dc190ce36474db8e17db2ff51c/chromeos/chromeos.gyp
,
Mar 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/822d58e6b22506cbd4787b58d8ccb421409b5b65 commit 822d58e6b22506cbd4787b58d8ccb421409b5b65 Author: teravest <teravest@chromium.org> Date: Mon Mar 20 16:31:58 2017 Copy D-Bus service files to build output. This is done so that chromite can copy these files from the build_output to remove this step from an ebuild in ChromeOS. This also makes "Simple Chrome" installs more closely match standard Chrome installs on ChromeOS. I've confirmed that dbus/org.chromium.LibCrosService.conf and dbus/org.chromium.LivenessService.conf are created in the output directory when performing a chrome build for a chromeos board. BUG= 701617 Review-Url: https://codereview.chromium.org/2764513002 Cr-Commit-Position: refs/heads/master@{#458083} [modify] https://crrev.com/822d58e6b22506cbd4787b58d8ccb421409b5b65/chromeos/BUILD.gn [add] https://crrev.com/822d58e6b22506cbd4787b58d8ccb421409b5b65/chromeos/dbus/services/README.md
,
Mar 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/d407f485a58520d7b4259d18ef92786232166386 commit d407f485a58520d7b4259d18ef92786232166386 Author: Justin TerAvest <teravest@chromium.org> Date: Tue Mar 21 02:30:15 2017 Copy D-Bus service files in deploy_chrome. This allows D-Bus service file changes to be tested with deploy_chrome, which lets us make changes without breaking the Simple Chrome workflow. I've made this optional so that deploy_chrome will still succeed for older Chrome builds that don't write these service files to the build output. BUG= chromium:701617 TEST=deploy_chrome, files appeared in /opt/google/chrome/dbus Change-Id: I2f0b9daded88e434006a9397227b1089a56f9c4a Reviewed-on: https://chromium-review.googlesource.com/457260 Commit-Ready: Justin TerAvest <teravest@chromium.org> Tested-by: Justin TerAvest <teravest@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/d407f485a58520d7b4259d18ef92786232166386/lib/chrome_util.py
,
Mar 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/7c4d0fb591d43e1596c47671e184d806ece90cb8 commit 7c4d0fb591d43e1596c47671e184d806ece90cb8 Author: Justin TerAvest <teravest@chromium.org> Date: Wed Mar 29 18:42:17 2017 Add /opt/google/chrome/dbus to include dirs. This allows D-Bus service files to be copied to the output of the Chrome build and managed by deploy_chrome instead of requiring an update to an ebuild file for every new D-Bus service file. This new file must have a ".conf" extension to be included; D-Bus ignored an attempted chromium.xml. BUG= chromium:701617 TEST=Installed package, service files in /opt/google/chrome/dbus work Change-Id: I0ce8bac696be2a96e1d0ae10d2106bb6918fa47c Reviewed-on: https://chromium-review.googlesource.com/461330 Commit-Ready: Justin TerAvest <teravest@chromium.org> Tested-by: Justin TerAvest <teravest@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [add] https://crrev.com/7c4d0fb591d43e1596c47671e184d806ece90cb8/chromeos-base/chromeos-chrome/files/chrome.conf [modify] https://crrev.com/7c4d0fb591d43e1596c47671e184d806ece90cb8/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/ecb7ceadcc21d0758e9c44ab903c6ec9fb1af9c0 commit ecb7ceadcc21d0758e9c44ab903c6ec9fb1af9c0 Author: Justin TerAvest <teravest@chromium.org> Date: Thu Mar 30 22:48:16 2017 Remove doins for LibCrosService. org.chromium.LibCrosService.conf should now be available in /opt/google/chrome/dbus, which is included by /etc/dbus-1/system.d/chrome.conf. This means that we no longer need this explicit copy step in the ebuild. BUG= chromium:701617 TEST=None Change-Id: I7d81e7d97f7d9033546bef33ff62e25edc835da9 Reviewed-on: https://chromium-review.googlesource.com/463586 Commit-Ready: Justin TerAvest <teravest@chromium.org> Tested-by: Justin TerAvest <teravest@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/ecb7ceadcc21d0758e9c44ab903c6ec9fb1af9c0/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild
,
Apr 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/0c0d01f3b42c68535d400f0a8b44bd31d7148941 commit 0c0d01f3b42c68535d400f0a8b44bd31d7148941 Author: Dan Erat <derat@chromium.org> Date: Mon Apr 03 21:39:50 2017 Revert "Remove doins for LibCrosService." This reverts commit ecb7ceadcc21d0758e9c44ab903c6ec9fb1af9c0. Reason for revert: It looks like the Chrome-provided version of this config file isn't being loaded, resulting in Chrome aborting at startup after failing to take ownership of the service name: http://crbug.com/707317 Original change's description: > Remove doins for LibCrosService. > > org.chromium.LibCrosService.conf should now be available in > /opt/google/chrome/dbus, which is included by > /etc/dbus-1/system.d/chrome.conf. This means that we no longer need > this explicit copy step in the ebuild. > > BUG= chromium:701617 > TEST=None > > Change-Id: I7d81e7d97f7d9033546bef33ff62e25edc835da9 > Reviewed-on: https://chromium-review.googlesource.com/463586 > Commit-Ready: Justin TerAvest <teravest@chromium.org> > Tested-by: Justin TerAvest <teravest@chromium.org> > Reviewed-by: Dan Erat <derat@chromium.org> > Reviewed-by: Mike Frysinger <vapier@chromium.org> > TBR=derat@chromium.org,vapier@chromium.org,teravest@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. BUG= chromium:701617 Change-Id: I69c02b2c42a211b35cef2d3d7dcdd35d7005b187 Reviewed-on: https://chromium-review.googlesource.com/466510 Reviewed-by: Dan Erat <derat@chromium.org> Commit-Queue: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Trybot-Ready: Dan Erat <derat@chromium.org> [modify] https://crrev.com/0c0d01f3b42c68535d400f0a8b44bd31d7148941/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild
,
Apr 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f721f93701a353deb099f775cbe5131b2377aa3c commit f721f93701a353deb099f775cbe5131b2377aa3c Author: teravest <teravest@chromium.org> Date: Tue Apr 04 18:19:36 2017 Roll DEPS for chromite. This is needed to pick up changes to deploy_chrome (https://chromium-review.googlesource.com/457260). The chromeos-base/chromeos-chrome ebuild runs deploy_chrome from the Chrome checkout, so we need to roll deps to use the correct version. BUG= 701617 Review-Url: https://codereview.chromium.org/2797703003 Cr-Commit-Position: refs/heads/master@{#461772} [modify] https://crrev.com/f721f93701a353deb099f775cbe5131b2377aa3c/DEPS
,
Apr 7 2017
Justin figured out that some of the brokenness devs have been seeing is due to them having correct D-Bus configs on their devices, but dbus-daemon not having been restarted to use them. deploy_chrome should probably compare the on-device D-Bus config files in /opt/google/chrome/dbus against the ones that it's about to install. If they've changed, it should reboot the device. _DisableRootfsVerification already has code for rebooting. There are mentions online about sending SIGHUP to dbus-daemon to trigger a "partial reload", but there are also mentions saying that you can't. It's unclear whether it'll do a full permissions reload or not, but that may be a simpler option if it works. All we really need for this is for it to allow new permissions, really.
,
Apr 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/fac210ee0821219f5cf57501c9ee4bf6e45b54f0 commit fac210ee0821219f5cf57501c9ee4bf6e45b54f0 Author: Justin TerAvest <teravest@chromium.org> Date: Fri Apr 14 06:54:07 2017 deploy_chrome: Reboot if D-Bus files changed. D-Bus service files are read and configured at boot, and can be updated when deploy_chrome is run. To prevent users from having to deal with mysterious bugs, we reboot the system after copying files on any changes to /opt/google/chrome/dbus. BUG= chromium:701617 TEST=Manually tested with and without modifying D-Bus files. Change-Id: I2fd95f6cea4838c2335ef5e56830fb2466f02009 Reviewed-on: https://chromium-review.googlesource.com/477151 Commit-Ready: Justin TerAvest <teravest@chromium.org> Tested-by: Justin TerAvest <teravest@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/fac210ee0821219f5cf57501c9ee4bf6e45b54f0/scripts/deploy_chrome.py
,
Apr 14 2017
chromite with the deploy_chrome change was rolled into Chrome DEPS today: https://codereview.chromium.org/2820693003/
,
May 30 2017
,
Aug 1 2017
,
Jan 22 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by steve...@chromium.org
, Mar 15 2017