New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 807986 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

install-sysroot.py still requires on gyp

Project Member Reported by yangguo@chromium.org, Feb 1 2018

Issue description

build/linux/sysroot_scripts/install-sysroot.py imports gyp_chromium, which imports gyp.

That causes failures such as this one: https://logs.chromium.org/v/?s=v8%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8955799053464237104%2F%2B%2Fsteps%2Fgclient_runhooks%2F0%2Fstdout

Background is that V8 wants to remove gyp, including DEPS entry. That does not work because install-sysroot is run as a gclient hook.
 
Cc: timbrown@chromium.org thomasanderson@chromium.org
Yeah, this needs to be rewritten to use gclient conditionals instead.

timbrown@, thomasanderson@, do one of you want to take a stab at doing so?
Owner: ----
Status: Available (was: Assigned)
Cc: dpranke@chromium.org
Labels: OS-Linux
Owner: thomasanderson@chromium.org
Status: Assigned (was: Available)
dpranke: you mean using the 'condition' property like this?
https://cs.chromium.org/chromium/src/DEPS?rcl=abf2fa9f6f05b24614db2f72e74fb516f5193de1&l=808

install-sysroot.py looks at GYP_DEFINES to find the target_arch.  I don't see how gclient conditionals could provide this.  Actually, when I run 'gclient runhooks', target_arch isn't set at all, so that code isn't even doing anything most of the time.  The only case I can think of where a non-host-arch sysroot is downloaded is on the Linux Arm bot, and I think that sets up the target_arch for GYP_DEFINES manually.

I see two options:
1. Get rid of the target_arch code in install-sysroot and add a step to the Linux arm bot that downloads the right sysroot.
2. Stop using the gyp module in install-sysroot and just parse GYP_DEFINES manually.
dpranke wdyt?
Sorry, I guess I wasn't clear enough. The script should stop looking at GYP_DEFINES, and we'd write conditional hooks to install each sysroot explicitly.
That probably requires us to add host_cpu and target_cpu as a builtin vars to gclient as well, then the hooks would just fetch the sysroots based on the settings for target_os and target_cpu.
Thanks for clarifying.  A few more things:
1. I don't think we need to add host_cpu.  Anything that wants that info could get it easily enough.
2. I still don't understand why we need to use conditionals.  gclient provides the ability to inject variables into the arguments:
https://cs.chromium.org/chromium/tools/depot_tools/gclient.py?rcl=1f067b88df42154d71bcadeddec7e830fb7d2979&l=203
So we could just do [ 'build/linux/sysroot_scripts/install-sysroot.py', '--arch={target_cpu}' ]

Also, we would need to update the gclient config on the arm bots.
You'll recall we install up to three sysroots right now. Imagine we're doing a arm32 target build, for example; we'll install x64 (for the host tools), x86 (for the v8 snapshot), and arm32 sysroots.

I suppose we could leave the logic to detect the host arch and whether we need the snapshot in the script, and only need to pass in target_cpu. I was kinda picturing we'd make the script stupider and call it three times, putting the logic in the conditions in DEPS instead. Up to you.
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/c31ae0b4a930023c5fbcf171a11f7f3b788b076a

commit c31ae0b4a930023c5fbcf171a11f7f3b788b076a
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Tue Feb 06 22:54:33 2018

Add target_cpu builtin variable to gclient

This CL adds target_cpu and target_cpu_only variables.  The logic is:

* If target_cpu is defined in .gclient, use it.  Otherwise, initialize it as an
  empty list.
* If not target_cpu_only, add the host arch to the list too.

detect_host_arch.py is copied directly from the Chromium repo.

BUG= 807986 
R=dpranke

Change-Id: I27621cbc81ad6a844648525863b92ffdd3b1d03a
Reviewed-on: https://chromium-review.googlesource.com/902441
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>

[modify] https://crrev.com/c31ae0b4a930023c5fbcf171a11f7f3b788b076a/gclient.py
[add] https://crrev.com/c31ae0b4a930023c5fbcf171a11f7f3b788b076a/detect_host_arch.py

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/2dd54735574b36e3b3ee041a7792644c6d6315d5

commit 2dd54735574b36e3b3ee041a7792644c6d6315d5
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Wed Feb 07 02:53:36 2018

Add target_cpu and target_cpu_only to gclient base config

BUG= 807986 
R=dpranke

Change-Id: I6b7e94d489a228c5dde23a9b6c7d6f58ce73b180
Reviewed-on: https://chromium-review.googlesource.com/905285
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>

[modify] https://crrev.com/2dd54735574b36e3b3ee041a7792644c6d6315d5/recipes/recipe_modules/gclient/config.py

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 7 2018

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

commit 40f270043c0d69d8d491cd8e8534321577a41278
Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Feb 07 04:02:36 2018

Roll src/third_party/depot_tools/ e117e46a6..2b51c930e (9 commits; 2 trivial rolls)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/e117e46a6894..2b51c930e009

$ git log e117e46a6..2b51c930e --date=short --no-merges --format='%ad %ae %s'
2018-02-07 tandrii Revert "bot_update: allow rebasing the patch onto an older revision."
2018-02-06 iannucci [luci-auth] Add statically-linked luci-auth CLI tool.
2018-02-06 iannucci [prpc] Add pRPC CLI tool to depot_tools.
2018-02-06 tandrii bot_update: optimize checkout of solution's main repo.
2018-02-06 thomasanderson Add target_cpu builtin variable to gclient
2018-02-06 oprypin bot_update: allow rebasing the patch onto an older revision.
2018-02-05 mmoss Make flattened recursedeps inherit parent conditionals.

Created with:
  roll-dep src/third_party/depot_tools
BUG= 807986 , 783607 


The AutoRoll server is located here: https://depot-tools-chromium-roll.skia.org

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.


TBR=agable@chromium.org

Change-Id: I2db4866326042b36e2d166db3cfd0740dcb639ec
Reviewed-on: https://chromium-review.googlesource.com/905932
Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#534909}
[modify] https://crrev.com/40f270043c0d69d8d491cd8e8534321577a41278/DEPS

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 7 2018

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

commit 625881822778702836a3026c636eb97ce1bf3fea
Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Feb 07 06:00:49 2018

Roll src/third_party/depot_tools/ 2b51c930e..6034966c9 (2 commits; 1 trivial rolls)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/2b51c930e009..6034966c95cf

$ git log 2b51c930e..6034966c9 --date=short --no-merges --format='%ad %ae %s'
2018-02-06 thomasanderson Add target_cpu and target_cpu_only to gclient base config

Created with:
  roll-dep src/third_party/depot_tools
BUG= 807986 


The AutoRoll server is located here: https://depot-tools-chromium-roll.skia.org

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.


TBR=agable@chromium.org

Change-Id: Ifb905d579212a3573ea1dd49419e258cc9d22801
Reviewed-on: https://chromium-review.googlesource.com/905690
Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#534927}
[modify] https://crrev.com/625881822778702836a3026c636eb97ce1bf3fea/DEPS

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 9 2018

Project Member

Comment 15 by bugdroid1@chromium.org, Feb 9 2018

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

commit c19b7703a1182ad135fad48cc0a1db220ab1c5e5
Author: Michael Achenbach <machenbach@chromium.org>
Date: Fri Feb 09 10:09:47 2018

Update V8 DEPS.

Rolling v8/build: https://chromium.googlesource.com/chromium/src/build/+log/3ba6ca6..c1972dd

Rolling v8/third_party/android_tools: https://chromium.googlesource.com/android_tools/+log/c9f9bbf..9a70d48

Rolling v8/third_party/catapult: https://chromium.googlesource.com/catapult/+log/b4a4bed..e653c4b

Rolling v8/tools/clang: https://chromium.googlesource.com/chromium/src/tools/clang/+log/357315f..7c6255f

This also ports https://crrev.com/c/907673 for adjusting V8's DEPS file to the sysroot changes
rolling in v8/build.

TBR=yangguo@chromium.org

Bug:  chromium:807986 
Change-Id: If55378330ae8797798abe1a98a858a0f57cf7fbe
Reviewed-on: https://chromium-review.googlesource.com/910193
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: v8 autoroll <v8-autoroll@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51202}
[modify] https://crrev.com/c19b7703a1182ad135fad48cc0a1db220ab1c5e5/DEPS

Project Member

Comment 16 by bugdroid1@chromium.org, Feb 9 2018

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

commit 29e3b8e9e47d70a60e421d5140ab73ab4bd59fcf
Author: Yang Guo <yangguo@chromium.org>
Date: Fri Feb 09 10:42:47 2018

[deps] fix hooks for node.

R=machenbach@chromium.org
NOTREECHECKS=true
NOTRY=true

Bug:  chromium:807986 
Change-Id: Iffcd9fb943c59e218e70c61491efcadce5a7497c
Reviewed-on: https://chromium-review.googlesource.com/911049
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51205}
[modify] https://crrev.com/29e3b8e9e47d70a60e421d5140ab73ab4bd59fcf/DEPS

Beep boop.  I am bugdroid.  Cross-referencing CL that refers to this bug because thomasanderson@ totally didn't put the wrong bug# on the CL.

https://chromium.googlesource.com/chromium/src/+/a07b9feb203a4f0e1465adbfcd1e9883728b9c8b

Remove reliance on GYP_DEFINES from install-sysroot.py

BUG=905285
R=dpranke
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_arm

Change-Id: I08d7a19cf89ebf1132d1fd27d88970099f94ed0e
Reviewed-on: https://chromium-review.googlesource.com/907673
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535624}
Project Member

Comment 18 by bugdroid1@chromium.org, Feb 9 2018

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

commit 76c42f50780b60a8fa1e2793e1fd3087048d5e89
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Fri Feb 09 18:55:13 2018

Remove gyp includes from install-sysroot.py

BUG= 807986 
R=dpranke
CC=machenbach

Change-Id: I998ae84df8445d0f10770011bdee50bd94cc0114
Reviewed-on: https://chromium-review.googlesource.com/911892
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535768}
[modify] https://crrev.com/76c42f50780b60a8fa1e2793e1fd3087048d5e89/build/linux/sysroot_scripts/install-sysroot.py

Status: Fixed (was: Assigned)
Should be fixed now % updating some bots in webrtc
Project Member

Comment 21 by bugdroid1@chromium.org, Feb 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/0a45ad26403f4aeb3eb793813474c50a219f89e9

commit 0a45ad26403f4aeb3eb793813474c50a219f89e9
Author: Oleh Prypin <oprypin@webrtc.org>
Date: Mon Feb 12 08:05:47 2018

WebRTC: Add target_cpu gclient config for ARM builders

From https://chromium-review.googlesource.com/c/chromium/tools/build/+/907173

Bug:  chromium:807986 
Change-Id: I97ece20607dd6f8276dcd16fccd6a8ef197dab55
Reviewed-on: https://chromium-review.googlesource.com/911769
Reviewed-by: Patrik Höglund <phoglund@chromium.org>
Commit-Queue: Oleh Prypin <oprypin@chromium.org>

[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_linux32_debug__arm_.json
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_linux32_release__arm_.json
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/tryserver_webrtc_linux32_arm_rel.json
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipe_modules/webrtc/builders.py
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/tryserver_webrtc_linux32_arm_dbg.json
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_linux64_debug__arm_.json
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/tryserver_webrtc_linux_arm64_dbg.json
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/tryserver_webrtc_linux_arm64_rel.json
[modify] https://crrev.com/0a45ad26403f4aeb3eb793813474c50a219f89e9/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_linux64_release__arm_.json

Project Member

Comment 22 by bugdroid1@chromium.org, Feb 12 2018

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

commit b8a7d9d09f8c4b47043e09966e3bcd0856416f0d
Author: Mirko Bonadei <mbonadei@webrtc.org>
Date: Mon Feb 12 08:37:00 2018

Updating usage of install-sysroot.py to stop relying on GYP_DEFINES.

The --running-as-hook flag has been removed in
https://chromium-review.googlesource.com/c/chromium/src/+/907673.

This CL mirrors the changes done in the Chromium src/DEPS file.

Bug:  chromium:807986 
Change-Id: Ib952eb0dbd8149e4f8bdfa2323cb8f23e1d63e0b
Reviewed-on: https://webrtc-review.googlesource.com/51760
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Oleh Prypin <oprypin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21979}
[modify] https://crrev.com/b8a7d9d09f8c4b47043e09966e3bcd0856416f0d/DEPS

Sign in to add a comment