New issue
Advanced search Search tips

Issue 887888 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 2
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

arm.gni should not depend on catapult

Project Member Reported by yangguo@chromium.org, Sep 21

Issue description

One of the use cases of V8 is to build a static library as dependency for Node.js. For this, we only check out the bare necessity [0].

V8's BUILD.gn imports /build/config/arm.gni [1], which since recently imports /third_party/catapult/devil/devil_arm.gni. This means that in order to build V8 in this use case, we have to check out catapult as well.

Could we change this?

[0] https://cs.chromium.org/chromium/src/v8/tools/node/fetch_deps.py
[1] https://cs.chromium.org/chromium/src/v8/BUILD.gn?q=v8/build.gn&l=6
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/4988ecbb5d57f0e82e984dda545f512e4b99398e

commit 4988ecbb5d57f0e82e984dda545f512e4b99398e
Author: Yang Guo <yangguo@chromium.org>
Date: Fri Sep 21 11:17:53 2018

[node] check out catapult as dependency

R=ahaas@chromium.org

Bug:  chromium:887888 
Change-Id: I69edac2289ae6c00aeba82edcd780861568165ac
Reviewed-on: https://chromium-review.googlesource.com/1238178
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56130}
[modify] https://crrev.com/4988ecbb5d57f0e82e984dda545f512e4b99398e/tools/node/fetch_deps.py

Cc: thestig@chromium.org
Does https://chromium-review.googlesource.com/c/chromium/src/+/1227604 cover your use case?
Since we don't support Node.js on Android on our integration bots, this would work for us, thanks.

I'd love to have a general guideline to prohibit src/build to depend on anything outside src/build unless behind a flag or unless explicitly whitelisted...
turns out that devil no longer needs to be able to build the host arm targets at all. removing that entirely, along w/ this dependency.

should we need it again, we should be able to reimplement this using a specific toolchain.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 28

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/5b5abe5b654e1191737385ef2554ba8673f7c6f5

commit 5b5abe5b654e1191737385ef2554ba8673f7c6f5
Author: John Budorick <jbudorick@chromium.org>
Date: Fri Sep 28 16:41:22 2018

devil: remove host arm build rules.

Bug:  chromium:887888 
Change-Id: Idbc8f721cb823d3869cf9ceec8da7c0a5ccf33b9
Reviewed-on: https://chromium-review.googlesource.com/1249147
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>

[modify] https://crrev.com/5b5abe5b654e1191737385ef2554ba8673f7c6f5/devil/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 29

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

commit f0ac64f08cdcc2d964a60be45db795505784d931
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Sat Sep 29 19:17:06 2018

Roll src/third_party/catapult d525ef309fca..98289bcecf60 (5 commits)

https://chromium.googlesource.com/catapult.git/+log/d525ef309fca..98289bcecf60


git log d525ef309fca..98289bcecf60 --date=short --no-merges --format='%ad %ae %s'
2018-09-28 simonhatch@chromium.org Dashboard - Add missing index for querying last alert.
2018-09-28 chiniforooshan@chromium.org Telemetry: cpu_per_frame metrics in TBMv2
2018-09-28 manojgupta@google.com benchmark_runner: Emit all stories in a benchmark.
2018-09-28 dtu@chromium.org [pinpoint] Don't set difference_count for try jobs.
2018-09-28 jbudorick@chromium.org devil: remove host arm build rules.


Created with:
  gclient setdep -r src/third_party/catapult@98289bcecf60

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:890325 ,chromium:760553, chromium:888551 , chromium:888114 , chromium:890041 , chromium:887888 
TBR=sullivan@chromium.org

Change-Id: I25e06429f53579449a9ebd2d1ba948ff6409f4e7
Reviewed-on: https://chromium-review.googlesource.com/1252266
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#595322}
[modify] https://crrev.com/f0ac64f08cdcc2d964a60be45db795505784d931/DEPS

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76a1ed51d47ddc3b10485fe2572240d3b3098405

commit 76a1ed51d47ddc3b10485fe2572240d3b3098405
Author: John Budorick <jbudorick@chromium.org>
Date: Tue Oct 02 04:30:07 2018

Remove references to devil_arm.gni.

Bug:  887888 
Change-Id: I51557e114a1385e75476bae20dd8843c383c1b26
Reviewed-on: https://chromium-review.googlesource.com/1253103
Commit-Queue: John Budorick <jbudorick@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595722}
[modify] https://crrev.com/76a1ed51d47ddc3b10485fe2572240d3b3098405/build/config/arm.gni

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/3941d933684b49a978092011f58069e08b51d8cb

commit 3941d933684b49a978092011f58069e08b51d8cb
Author: John Budorick <jbudorick@chromium.org>
Date: Tue Oct 02 18:50:39 2018

devil: remove devil_arm.gni.

Bug:  chromium:887888 
Change-Id: I76ca3a37fac246f599c8532421ffd4fda40c210d
Reviewed-on: https://chromium-review.googlesource.com/c/1256927
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>

[delete] https://crrev.com/a115d8de1c986bb73971deac71ee74a5721c32de/devil/devil_arm.gni

Status: Fixed (was: Started)
The src/build subtree no longer references devil as of https://chromium.googlesource.com/chromium/src/build/+/fa903a459ceccbca3d026647b9affcfa20188d23
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 2

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

commit db8ad72a1fc3de2e8db748989c9dd695fb58ed82
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Tue Oct 02 22:27:22 2018

Roll src/third_party/catapult a115d8de1c98..cb192dee19d4 (2 commits)

https://chromium.googlesource.com/catapult.git/+log/a115d8de1c98..cb192dee19d4


git log a115d8de1c98..cb192dee19d4 --date=short --no-merges --format='%ad %ae %s'
2018-10-02 sadrul@chromium.org pinpoint: Include the metric name in the report.
2018-10-02 jbudorick@chromium.org devil: remove devil_arm.gni.


Created with:
  gclient setdep -r src/third_party/catapult@cb192dee19d4

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:887888 
TBR=sullivan@chromium.org

Change-Id: I2efdf81816c6a740372cbb95eeabed8612cc605c
Reviewed-on: https://chromium-review.googlesource.com/c/1257608
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#596008}
[modify] https://crrev.com/db8ad72a1fc3de2e8db748989c9dd695fb58ed82/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 4

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/26f6e418ffbc8bdfb99156664cc90fe0b0ba1d99

commit 26f6e418ffbc8bdfb99156664cc90fe0b0ba1d99
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu Oct 04 08:06:06 2018

[build] Remove catapult dependency for node.js

This undoes the workaround from https://crrev.com/c/1223426.

Bug:  chromium:887888 
Change-Id: Id7a68354b1f1020d7d001ba4120be8a11f896067
Reviewed-on: https://chromium-review.googlesource.com/c/1260942
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56364}
[modify] https://crrev.com/26f6e418ffbc8bdfb99156664cc90fe0b0ba1d99/DEPS
[modify] https://crrev.com/26f6e418ffbc8bdfb99156664cc90fe0b0ba1d99/tools/node/fetch_deps.py
[modify] https://crrev.com/26f6e418ffbc8bdfb99156664cc90fe0b0ba1d99/tools/node/update_node.py

Status: Verified (was: Fixed)
Thanks!

Sign in to add a comment