New issue
Advanced search Search tips
Starred by 6 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 692601
issue 724550

Blocking:
issue 721585



Sign in to add a comment
link

Issue 723856: Moving checkouts without rebuilding

Reported by wychen@chromium.org, May 17 2017 Project Member

Issue description

When doing a btrfs snapshot on a checkout and trying to reuse the artifact for instantaneous incremental build, the workflow almost works as intended, but not exactly. Reference for the workflow: https://groups.google.com/a/chromium.org/d/msg/chromium-dev/MZNPbkJfEh0/f9eFIafPEgAJ and issue 721585.

When a checkout was moved or copied to a new path, ninja would no longer work properly because there are many hard coded absolute paths in the generated ninja files.

It works if the checkout is copied, and the original one is intact so that these absolute paths still refer to valid files with the same contents. However, any changes in the original checkout could break the copied one.

"gn gen" updates these hard coded paths to the correct ones, but then ninja would rebuild many things, which is arguably unnecessary. Since we haven't reached "deterministic build independent from checkout path" tracked in  issue 439949 , rebuilding might be the correct behavior, since the resulting binary could be different. We probably don't care about the absolute paths in the output most of the time, so we probably shouldn't block this on  issue 439949 .

In short, directly rebuilding without running "gn gen" first would get the instantaneous rebuild, but if the two copies diverge, the copied one could break. Doing "gn gen" first fixes the issue, but artifact reuse is not ideal. I think one way forward is to decrease the absolute paths in the generated ninja files, so that artifacts can be reused more. Completely removing absolute paths might not be possible, or cost effective.
 

Comment 1 by wychen@chromium.org, May 17 2017

Blocking: 721585

Comment 2 by torne@chromium.org, May 17 2017

Yeah. This is another problem that I mostly avoid by my usage pattern of snapshotting: because I don't keep snapshots around and only create them for temporary work separate from my main checkout, I rarely touch the original checkout during the time that the snapshot actually exists, and so there's few problems caused by things referring to the old location and it's not really necessary to regenerate the ninja files until I actually change a gn file.

In the cases where I do use multiple checkouts in parallel I typically have them *all* be snapshots of the same original checkout, which similarly works okay in most cases.

It would still be great to reduce the number of cases where we depend on absolute paths if that's feasible but I've not really looked into it.

Comment 3 by wychen@chromium.org, May 18 2017

Some numbers for reference.

For Linux Release build, build all from scratch requires 54K steps for ninja. After snapshot and "gn gen", rebuilding target "chrome" requires 9K steps, and rebuilding all requires 17K steps.

Comment 4 by wychen@chromium.org, May 19 2017

Rerunning "gn gen" touches some ninja files, even when the contents remain the same. As a result, rebuilding all requires 362 steps, but it only takes <5 seconds.

Comment 5 by mar...@chromium.org, May 19 2017

Oh, so do you think 'gn gen' shouldn't change mtime if the content hasn't changed? Then it's a GN bug/feature request.

Comment 6 by torne@chromium.org, May 19 2017

Why does the mtime of ninja files cause anything to actually get rebuilt? That seems like a weird dependency.

Comment 7 by wychen@chromium.org, May 19 2017

The following files are touched by a noop "gn gen".

out-linux-gn/Release/build.ninja
out-linux-gn/Release/build.ninja.d
out-linux-gn/Release/newlib_pnacl/toolchain.ninja
out-linux-gn/Release/newlib_pnacl_nonsfi/toolchain.ninja
out-linux-gn/Release/clang_newlib_x64/toolchain.ninja
out-linux-gn/Release/glibc_x64/toolchain.ninja
out-linux-gn/Release/irt_x64/toolchain.ninja
out-linux-gn/Release/nacl_bootstrap_x64/toolchain.ninja
out-linux-gn/Release/toolchain.ninja

These toolchain.ninja files were updated because the BUILD.gn file runs an external python script to get toolchain versions. These didn't cause rebuild, so they don't really matter.

The one causing rebuild is build.ninja. I guess it's a special root-level ninja file, directly generated by GN, not by some BUILD.gn files.

Comment 8 by wychen@chromium.org, May 19 2017

Blockedon: 724550

Comment 9 by sergeybe...@chromium.org, May 22 2017

Components: -Infra
I don't believe it's an infra issue, removing the component. I couldn't find the appropriate component for gn / ninja; please help triage.

Comment 10 by wychen@chromium.org, May 22 2017

Components: Build

Comment 11 by bugdroid1@chromium.org, May 23 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/56e4ad939b3c06e2915faea0be14d34473bc3602

commit 56e4ad939b3c06e2915faea0be14d34473bc3602
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Tue May 23 20:30:23 2017

Add messages in gclient-new-workdir.py

When the repo is copied in copy-on-write mode, print informative
messages afterwards to help users.

Also, skip "git clean" step in copy-on-write mode.

Bug: 721585, 723856
Change-Id: I3235398c960d59a4cf44cfe7dffc79ed008a5190
Reviewed-on: https://chromium-review.googlesource.com/512262
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>

[modify] https://crrev.com/56e4ad939b3c06e2915faea0be14d34473bc3602/gclient-new-workdir.py

Comment 12 by thakis@chromium.org, May 23 2017

Re comment 4: That's a regression being discussed in issue 692601 that agrieve added.

Comment 13 by wychen@chromium.org, May 23 2017

Blockedon: 692601

Comment 14 by bugdroid1@chromium.org, May 24 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/04a56818d510da7570c25f9e63217752eb25d17e

commit 04a56818d510da7570c25f9e63217752eb25d17e
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Wed May 24 14:53:05 2017

Reduce absolute paths in generated ninja files in angle

BUG=chromium:723856

Change-Id: I7481d3335111c697f77aa0ea673614e79b4a86f6
Reviewed-on: https://chromium-review.googlesource.com/513588
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>

[modify] https://crrev.com/04a56818d510da7570c25f9e63217752eb25d17e/src/vulkan_support/BUILD.gn

Comment 15 by bugdroid1@chromium.org, May 24 2017

Project Member
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/56a1e6a1095567809cfb45f450dfbe104cad5aaf

commit 56a1e6a1095567809cfb45f450dfbe104cad5aaf
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Wed May 24 20:16:55 2017

Reduce absolute paths in generated ninja files in skia

BUG=chromium:723856

Change-Id: Ifa065daddfe1d775e843307587e4007ad6dae6d4
Reviewed-on: https://skia-review.googlesource.com/17802
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

[modify] https://crrev.com/56a1e6a1095567809cfb45f450dfbe104cad5aaf/third_party/spirv-tools/BUILD.gn

Comment 16 by bugdroid1@chromium.org, May 25 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/65b4b99159119398e7fec4dafdbed897153efcc7

commit 65b4b99159119398e7fec4dafdbed897153efcc7
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Thu May 25 00:26:43 2017

Roll src/third_party/skia/ 176f19cce..6533159f2 (9 commits)

https://skia.googlesource.com/skia.git/+log/176f19cce532..6533159f2c83

$ git log 176f19cce..6533159f2 --date=short --no-merges --format='%ad %ae %s'
2017-05-24 reed add stage for gaussian alpha to rgba for shadows
2017-05-24 scroggo Revert "Add animated webp images to DM testing"
2017-05-24 fmalita Revert "SkShaderBase"
2017-05-24 kjlubick Enable chromecast gpu perf
2017-05-24 wychen Reduce absolute paths in generated ninja files in skia
2017-05-24 fmalita SkShaderBase
2017-05-24 egdaniel Allow for non opaque colors for src-over lcd fallback case in gpu
2017-05-23 halcanary SkStream: DynamicMemoryWStream gets writeToAndReset
2017-05-24 fmalita SkSTArenaAlloc

Created with:
  roll-dep src/third_party/skia
BUG=723856


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=brianosman@chromium.org

Change-Id: I3d62ca9c34310acbb1f50fd6efd9732d4b703da3
Reviewed-on: https://chromium-review.googlesource.com/514367
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474492}
[modify] https://crrev.com/65b4b99159119398e7fec4dafdbed897153efcc7/DEPS

Comment 17 by tfarina@chromium.org, May 25 2017

Owner: wychen@chromium.org
Status: Started (was: Untriaged)

Comment 19 by wychen@chromium.org, May 25 2017

Status update:

For Linux Release build, building all from scratch requires 54K steps for ninja. After snapshot and "gn gen", steps required for rebuilding target "chrome" reduced from 9K to 7K, and rebuilding all from 17K to 14K.

Comment 20 by bugdroid1@chromium.org, May 26 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a5b43da4d706751276a46486256ad6677014868

commit 0a5b43da4d706751276a46486256ad6677014868
Author: ynovikov <ynovikov@chromium.org>
Date: Fri May 26 00:40:52 2017

Roll ANGLE 9e3bd31..ff77c35

https://chromium.googlesource.com/angle/angle.git/+log/9e3bd31..ff77c35

BUG=None,449754,chromium:723856,chromium:723069,722684,chromium:699479
TBR=jmadill@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2908513003
Cr-Commit-Position: refs/heads/master@{#474852}

[modify] https://crrev.com/0a5b43da4d706751276a46486256ad6677014868/DEPS

Comment 21 by bugdroid1@chromium.org, May 30 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c

commit 5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c
Author: wychen <wychen@chromium.org>
Date: Tue May 30 08:05:10 2017

Reduce absolute paths in generated ninja files

After this change, the number of steps required for rebuilding all
after snapshot and "gn gen" is reduced to 711 (Linux Release build).

BUG=723856

Review-Url: https://codereview.chromium.org/2905523002
Cr-Commit-Position: refs/heads/master@{#475458}

[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/build/config/nacl/rules.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/build/toolchain/gcc_toolchain.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/chrome/renderer/BUILD.gn
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/extensions/BUILD.gn
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/headless/BUILD.gn
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/mojo/public/js/BUILD.gn
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/mojo/public/tools/bindings/BUILD.gn
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/ppapi/native_client/BUILD.gn
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/ppapi/native_client/nacl_test_data.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/remoting/host/BUILD.gn
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/remoting/tools/build/remoting_localize.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/services/catalog/public/tools/catalog.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/testing/libfuzzer/fuzzer_test.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/tools/generate_library_loader/generate_library_loader.gni
[modify] https://crrev.com/5e1f7259d0638f865ed7f5a6c1ecabf5b9968e2c/ui/vector_icons/vector_icons.gni

Comment 22 by bugdroid1@chromium.org, Jun 6 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/src/native_client.git/+/f88070127d32714df2714d94c9494fce951993d8

commit f88070127d32714df2714d94c9494fce951993d8
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Tue Jun 06 01:13:12 2017

Reduce absolute paths in generated ninja files in NaCl

BUG=chromium:723856
NOTRY=true

Change-Id: Ic9d0cca3fbc9b10edacd9de10e6d6e6ef5311f3e
Reviewed-on: https://chromium-review.googlesource.com/513515
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>

[modify] https://crrev.com/f88070127d32714df2714d94c9494fce951993d8/src/trusted/service_runtime/linux/BUILD.gn

Comment 23 by bugdroid1@chromium.org, Jun 6 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c438f78ca087f2b992bbf76beb25043ab0c3c0bd

commit c438f78ca087f2b992bbf76beb25043ab0c3c0bd
Author: nacl-deps-roller@chromium.org <nacl-deps-roller@chromium.org>
Date: Tue Jun 06 05:40:26 2017

Roll src/native_client/ b1f13a7bd..f88070127 (1 commit)

https://chromium.googlesource.com/native_client/src/native_client.git/+log/b1f13a7bdc72..f88070127d32

$ git log b1f13a7bd..f88070127 --date=short --no-merges --format='%ad %ae %s'
2017-05-24 wychen Reduce absolute paths in generated ninja files in NaCl

Created with:
  roll-dep src/native_client
BUG=723856


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_dbg_32_ng,linux_nacl_sdk_build
TBR=mseaborn@chromium.org

Change-Id: Id3200f7a8fd3b2cf3456f8aeeb39ed5efd60efc8
Reviewed-on: https://chromium-review.googlesource.com/525140
Reviewed-by: <nacl-deps-roller@chromium.org>
Commit-Queue: <nacl-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477203}
[modify] https://crrev.com/c438f78ca087f2b992bbf76beb25043ab0c3c0bd/DEPS

Comment 24 by bugdroid1@chromium.org, Jun 7 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bf8ffab8059f83d6fd93485184627fbe3fce2e2

commit 6bf8ffab8059f83d6fd93485184627fbe3fce2e2
Author: wychen <wychen@chromium.org>
Date: Wed Jun 07 08:51:52 2017

Eliminate absolute paths in ninja files for Linux Release build

After this change, the number of steps required for rebuilding all
in Linux Release build after snapshot and "gn gen" is reduced to 384,
the same as touching OUT_DIR/build.ninja.

BUG=723856

Review-Url: https://codereview.chromium.org/2909253002
Cr-Commit-Position: refs/heads/master@{#477584}

[modify] https://crrev.com/6bf8ffab8059f83d6fd93485184627fbe3fce2e2/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/6bf8ffab8059f83d6fd93485184627fbe3fce2e2/third_party/google_input_tools/closure.gni

Sign in to add a comment