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

Issue 748501 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug

Blocking:
issue 495204



Sign in to add a comment

Host and target toolchains writing to same output dir when building chrome/win on a non-win host

Project Member Reported by thakis@chromium.org, Jul 25 2017

Issue description

I tried my build-chrome/win-on-linux branch again after a long time. `gn gen` errors out because both the host and the target toolchain are called clang_x64 (in build/toolchain/linux and in build/toolchain/win):

thakis@thakis:~/src/chrome/src$ GYP_MSVS_VERSION=2017 gn gen out/gnwin
ERROR at //build/toolchain/gcc_toolchain.gni:86:3: Duplicate toolchain.
  toolchain(target_name) {
  ^-----------------------
Two or more toolchains write to the same directory:
  //out/gnwin/clang_x64/

This can be fixed by making sure that distinct toolchains have
distinct names.

See //build/toolchain/win/BUILD.gn:55:3: Previous toolchain.
  toolchain(target_name) {
  ^-----------------------


I think this has always been broken (which explains a few weird effects I've seen), but before https://codereview.chromium.org/2483713003 gn didn't diagnose this problem.

There are a handful of references to //build/toolchain/win:clang_x64 (https://cs.chromium.org/search/?q=toolchain.win.clang+file:%5C.gn&sq=package:chromium&type=cs); in particular there's a use in libyuv which is in a different repo. If that wasn't the case, just renaming win:clang_x64 to win:win_clang_x64 would be an easy fix, and it's what we did for android.

Since libyuv references win:clang_x64, one way to get that working would be do introduce a variable win_clang_x64_toolchain that's set to "//build/toolchain/win:clang_x64" in src.git, then make libyuv refer to that variable instead, and then do the renaming.

An alternative would be to give toolchains an tool_out_dir variable that allows controlling where the toolchain's output goes independently of the toolchain's name.


I think the first approach might be simpler, so I'll try and do that. (+brettw for opinions on gn things.)
 

Comment 1 by thakis@chromium.org, Jul 25 2017

Owner: thakis@chromium.org
Status: started (was: Available)
https://chromium-review.googlesource.com/c/584811/
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 25 2017

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

commit 88ef4818de0b999a15969413c0b490a1f14bcd16
Author: Nico Weber <thakis@chromium.org>
Date: Tue Jul 25 15:57:12 2017

win: Rename clang_x64 to win_clang_x64 in build/toolchain/win, step 1/5.

In cross builds of chrome/win, the host and target toolchains currently
have the same name.  To fix this, rename clang_x64 to win_clang_x64.
Because the toolchain name is also referenced in libyuv, this requires
a five-sided change:

1* Introduce variable containing the toolchain name in src.git
2. Change libyuv to refer to the variable
3. Rename toolchain in src.git (including in the newly introduced var)
4. Let libyuv refer to the new name directly
5. Remove variable again

(See also https://codereview.chromium.org/2463143002)

Bug:  748501 
Change-Id: Ia54b2f3f6cb37524c698ad8e3b411d77db612188
Reviewed-on: https://chromium-review.googlesource.com/584811
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489318}
[add] https://crrev.com/88ef4818de0b999a15969413c0b490a1f14bcd16/build/toolchain/win/clang_name.gni

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 25 2017

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

commit 94d731c12e243640231fcebfe14109c1a897465d
Author: Nico Weber <thakis@chromium.org>
Date: Tue Jul 25 19:40:46 2017

win: Rename clang_x64 to win_clang_x64 in build/toolchain/win, step 3/5.

In cross builds of chrome/win, the host and target toolchains currently
have the same name.  To fix this, rename clang_x64 to win_clang_x64.
Because the toolchain name is also referenced in libyuv, this requires
a five-sided change:

1. Introduce variable containing the toolchain name in src.git
2. Change libyuv to refer to the variable
3* Rename toolchain in src.git (including in the newly introduced var)
4. Let libyuv refer to the new name directly
5. Remove variable again

(See also https://codereview.chromium.org/2463143002)

TBR=pastarmovj,wfh,dschuff

Bug:  748501 
Change-Id: Ifad35c6d2d0c29404637421a36a2a6bfeaa113c5
Reviewed-on: https://chromium-review.googlesource.com/585151
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489403}
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/BUILD.gn
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/DEPS
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/build/config/BUILDCONFIG.gn
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/build/toolchain/win/BUILD.gn
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/build/toolchain/win/clang_name.gni
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/cloud_print/BUILD.gn
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/cloud_print/virtual_driver/win/port_monitor/BUILD.gn
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/components/nacl/broker/BUILD.gn
[modify] https://crrev.com/94d731c12e243640231fcebfe14109c1a897465d/courgette/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 25 2017

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

commit 9f072955094ffe6fbab3e864286ab6b1dcdf8f8c
Author: Nico Weber <thakis@chromium.org>
Date: Tue Jul 25 21:01:33 2017

win: Rename clang_x64 to win_clang_x64 in build/toolchain/win, step 5/5.

In cross builds of chrome/win, the host and target toolchains currently
have the same name.  To fix this, rename clang_x64 to win_clang_x64.
Because the toolchain name is also referenced in libyuv, this requires
a five-sided change:

1. Introduce variable containing the toolchain name in src.git
2. Change libyuv to refer to the variable
3. Rename toolchain in src.git (including in the newly introduced var)
4. Let libyuv refer to the new name directly
5* Remove variable again

(See also https://codereview.chromium.org/2463143002)

Bug:  748501 
Change-Id: I8fe3557fb6a200116ba4c4d6dc3338f895fd3757
Reviewed-on: https://chromium-review.googlesource.com/585592
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489426}
[modify] https://crrev.com/9f072955094ffe6fbab3e864286ab6b1dcdf8f8c/DEPS
[delete] https://crrev.com/35312169018b50666fd155db3107066c39625f92/build/toolchain/win/clang_name.gni

Comment 7 by thakis@chromium.org, Jul 25 2017

Status: Fixed (was: Started)

Sign in to add a comment