New issue
Advanced search Search tips

Issue 906559 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Nov 20
Cc:
agrieve@chromium.org
brucedaw...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Change to win_toolchain.json broke external projects using Chromium's build/

Project Member Reported by cwallez@google.com, Nov 19 
Rencently Dawn's standalone Windows builds started failing like this build https://ci.chromium.org/p/dawn/builders/luci.dawn.try/win-clang-dbg-x64/b8929442480635862816

with

Traceback (most recent call last):
  File "e:/b/swarm_slave/w/ir/cache/builder/dawn/build/vs_toolchain.py", line 471, in <module>
    sys.exit(main())
  File "e:/b/swarm_slave/w/ir/cache/builder/dawn/build/vs_toolchain.py", line 467, in main
    return commands[sys.argv[1]](*sys.argv[2:])
  File "e:/b/swarm_slave/w/ir/cache/builder/dawn/build/vs_toolchain.py", line 285, in CopyDlls
    x64_runtime, x86_runtime = vs_runtime_dll_dirs
ValueError: too many values to unpack
ERROR at //build/toolchain/win/BUILD.gn:43:3: Script returned non-zero exit code.
  exec_script("../../vs_toolchain.py",
  ^----------

This is because win_toolchain.json gained an extra directory for ARM64. I see that vs_toolchain.py was updated to support it, but the update to win_toolchain.py happened without us rolling anything into Dawn, which means it is somehow updated out of band.

Shouldn't both win_toolchain.json and vs_toolchain.py be updated at the same time so we don't have breakage because of mismatches?

+CC folks on the CL updating vs_toolchain.py because I don't know how win_toolchain.json is created.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 19 

The following revision refers to this bug:
  https://dawn.googlesource.com/dawn/+/15d8cb765f748d817e79e3d267806c715100997b

commit 15d8cb765f748d817e79e3d267806c715100997b
Author: Corentin Wallez <cwallez@chromium.org>
Date: Mon Nov 19 10:13:31 2018

Roll Chromium's build system to fix Windows builds

build/win_toolchain.json was updated out of band so we need to roll all
Chromium's build support repos to pick up the respective change to
build/vs_toolchain.py

In the meantime, LASTCHANGE-related functionality changed and requires a
new hook to run before we can call `gn gen`.

TBR=kainino@chromium.org
TBR=senorblanco@chromium.org
BUG= chromium:906559 

Change-Id: Ic5d297b5700193ca72b02874073e6c553f06e04f
Reviewed-on: https://dawn-review.googlesource.com/c/2560
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/15d8cb765f748d817e79e3d267806c715100997b/DEPS

Comment 2 by brucedaw...@chromium.org, Nov 19 

Sorry about this. The asynchrony was known when I made these changes but I forgot how many projects beyond Chromium consume depot_tools.

The basic problem is that depot_tools' win_toolchain\get_toolchain_if_necessary.py generates the .json file, and build\vs_toolchain.py consumes it. Therefore it was important to land the build\vs_toolchain.py change before landing the depot_tools change. This worked for Chromium, but not for other projects. Merging over the build\vs_toolchain.py change is the correct fix.

This is a one-time break.

Comment 3 by cwallez@chromium.org, Nov 20

Status: Fixed (was: Available)
Thanks for the explanation, if it's a one-time break it is probably ok to close this issue. I'm thinking we should probably have a mailing list for people using Chromium's build system so we can send PSAs for changes like this.
►

Sign in to add a comment