Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
626443004:
Update webtry setup docs to reflect new installation procedure (Closed)
DescriptionUpdate webtry setup docs to reflect new installation procedure
BUG=skia:
Committed: https://skia.googlesource.com/skia/+/f0e2154b45ab2bb0d7fa5a9d38955260ce42831c
Patch Set 1 #
Total comments: 6
Patch Set 2 : Fix doc formatting / numbering #
Total comments: 10
Messages
Total messages: 11 (2 generated)
humper@google.com changed reviewers: + mtklein@google.com, tfarina@chromium.org
ptal; I just checked that this procedure now works from scratch.
https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.md File experimental/webtry/README.md (left): https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.m... experimental/webtry/README.md:84: sudo debootstrap --variant=minbase wheezy /srv/chroot/webtry Isn't debootstrap necessary anymore? https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.m... experimental/webtry/README.md:95: I have been trying to keep two blank lines between sections. I think it is clear to read that way. https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.md File experimental/webtry/README.md (right): https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.m... experimental/webtry/README.md:49: 2. sudo apt-get install git schroot debootstrap If debootstrap is not needed anymore, could you update this command line? See question below.
https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.md File experimental/webtry/README.md (left): https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.m... experimental/webtry/README.md:84: sudo debootstrap --variant=minbase wheezy /srv/chroot/webtry On 2014/10/02 17:16:13, tfarina wrote: > Isn't debootstrap necessary anymore? Nope -- the setup script does it. https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.m... experimental/webtry/README.md:95: On 2014/10/02 17:16:13, tfarina wrote: > I have been trying to keep two blank lines between sections. I think it is clear > to read that way. Done. https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.md File experimental/webtry/README.md (right): https://codereview.chromium.org/626443004/diff/1/experimental/webtry/README.m... experimental/webtry/README.md:49: 2. sudo apt-get install git schroot debootstrap On 2014/10/02 17:16:13, tfarina wrote: > If debootstrap is not needed anymore, could you update this command line? See > question below. It's still needed, but the webtry_setup.sh script uses it. Probably I should have webtry_setup install schroot and debootstrap too; fewer steps is better.
On 2014/10/02 17:12:37, humper wrote: > ptal; I just checked that this procedure now works from scratch. hard to argue with that. lgtm.
The CQ bit was checked by humper@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/626443004/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as f0e2154b45ab2bb0d7fa5a9d38955260ce42831c
Message was sent while issue was closed.
https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... File experimental/webtry/README.md (left): https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:84: sudo debootstrap --variant=minbase wheezy /srv/chroot/webtry OK. This is done by webtry_setup.sh with: sudo debootstrap --variant=minbase wheezy ${CHROOT_JAIL} https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... File experimental/webtry/README.md (right): https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:50: 2. sudo apt-get install git schroot debootstrap As you noted, schroot and debootstrap are also installed by webtry_setup.sh -> https://skia.googlesource.com/skia/+/master/experimental/webtry/setup/webtry_... https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:52: 3. Add the following to the /etc/schroot/minimal/fstab: Does this needs to happen before we call webtry_setup.sh? https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:78: sudo bash -c 'echo 60 > /proc/sys/net/ipv4/tcp_keepalive_time' s/bash/sh?
Message was sent while issue was closed.
https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... File experimental/webtry/README.md (right): https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:54: none /run/shm tmpfs rw,nosuid,nodev,noexec 0 0 Should we do this in the script as well rather than by hand? We write a line like this to '/srv/chroot/webtry_gyp/etc/fstab' at line 32: https://skia.googlesource.com/skia/+/master/experimental/webtry/setup/webtry_...
Message was sent while issue was closed.
https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... File experimental/webtry/README.md (left): https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:84: sudo debootstrap --variant=minbase wheezy /srv/chroot/webtry On 2014/10/02 17:56:27, tfarina wrote: > OK. This is done by webtry_setup.sh with: > > sudo debootstrap --variant=minbase wheezy ${CHROOT_JAIL} Acknowledged. https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... File experimental/webtry/README.md (right): https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:50: 2. sudo apt-get install git schroot debootstrap On 2014/10/02 17:56:27, tfarina wrote: > As you noted, schroot and debootstrap are also installed by webtry_setup.sh -> > https://skia.googlesource.com/skia/+/master/experimental/webtry/setup/webtry_... Done. https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:52: 3. Add the following to the /etc/schroot/minimal/fstab: On 2014/10/02 17:56:27, tfarina wrote: > Does this needs to happen before we call webtry_setup.sh? Not really -- the setup scripts just use regular chroot, since the schroot environment isn't really set up yet :) https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:54: none /run/shm tmpfs rw,nosuid,nodev,noexec 0 0 On 2014/10/02 18:55:27, tfarina wrote: > Should we do this in the script as well rather than by hand? > > We write a line like this to '/srv/chroot/webtry_gyp/etc/fstab' at line 32: > https://skia.googlesource.com/skia/+/master/experimental/webtry/setup/webtry_... Well, line 32 only runs once, when the chroot is very first set up. Since the setup script can be run multiple times (to update the source code), I didn't want to do it that way. We *could* say that webtry_setup.sh is used for one-time-only setup, and after that you run the continue_install.sh script to update skia; that would be OK. If you think that's worthwhile, feel free to file a bug and I'll do it. https://codereview.chromium.org/626443004/diff/20001/experimental/webtry/READ... experimental/webtry/README.md:78: sudo bash -c 'echo 60 > /proc/sys/net/ipv4/tcp_keepalive_time' On 2014/10/02 17:56:27, tfarina wrote: > s/bash/sh? Done. |
