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

Issue 634446 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Aug 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

GN toolchain_args should be a scope rather than a function

Project Member Reported by brettw@chromium.org, Aug 4 2016

Issue description

Currently in a toolchain args overrides are:
  toolchain_args() {
    foo = 1
    bar = "baz"
  }

We're transitioning this to be a scope type:
  toolchain_args = {
    foo = 1
    bar = "baz"
  }

which will allow the gcc_toolchain template to forward values from the invoker without it having to know about all build args ever overridden in the entire build.
 
Labels: Build-Tools-GN
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 5 2016

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

commit 22227896dc68aa88ba1836cd53fd261ba21bef88
Author: brettw <brettw@chromium.org>
Date: Fri Aug 05 19:04:35 2016

Update GN toolchain_args to be a variable.

This makes toolchain_args on a toolchain definition a variable instead of a function call.

The function call is kept for backwards compatibility (for now) and it just sets the variable.

forward_variables_from now accepts a first parameter of any type that evaluates to a scope, which allows things like "invoker.toolchain_args" to be used there.

Updates "gn format" to format scope assignments without extra indents.

BUG= 634446 

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

[modify] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/args.cc
[modify] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/command_format.cc
[modify] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/command_format_unittest.cc
[add] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/format_test_data/067.gn
[add] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/format_test_data/067.golden
[modify] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/function_forward_variables_from.cc
[modify] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/function_forward_variables_from_unittest.cc
[modify] https://crrev.com/22227896dc68aa88ba1836cd53fd261ba21bef88/tools/gn/function_toolchain.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 9 2016

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

commit 3c38c5cdab9e6fc35852ae79be5351be2bd6d49e
Author: brettw <brettw@chromium.org>
Date: Tue Aug 09 22:21:02 2016

Use new toolchain_args variable in GN.

toolchain_args is changing from a function call to a scope that can be passed around. Pipe these args through the various toolchain templates. This adds complexity in some cases, but it eliminates the need for the toolchain templates to know about every build flag they might ever be called with.

BUG= 634446 

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

[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/android/BUILD.gn
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/cc_wrapper.gni
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/cros/BUILD.gn
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/gcc_toolchain.gni
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/goma.gni
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/linux/BUILD.gn
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/mac/BUILD.gn
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/nacl/BUILD.gn
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/nacl_toolchain.gni
[modify] https://crrev.com/3c38c5cdab9e6fc35852ae79be5351be2bd6d49e/build/toolchain/win/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 10 2016

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

commit 72d6d06aef7e5284e20e30c685ec35557d9e38a7
Author: dpranke <dpranke@chromium.org>
Date: Wed Aug 10 03:02:39 2016

Fix v8_current_cpu setting for CrOS builds after r410853.

In r410853 we changed how the GN toolchains move arguments around,
but we missed one reference to the v8_toolchain_cpu in the CrOS
toolchain definitions. This CL fixes that.

TBR=brettw@chromium.org, stevenjb@chromium.org
BUG= 634446 

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

[modify] https://crrev.com/72d6d06aef7e5284e20e30c685ec35557d9e38a7/build/toolchain/cros/BUILD.gn

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 10 2016

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

commit fcd8d221139567226dc07b8b94ebeae1f2b94134
Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com>
Date: Wed Aug 10 19:38:45 2016

Stop pointing to "gn help toolchain" in that very help message.

toolchain_args became a variable in commit 22227896 ("Update GN
toolchain_args to be a variable"), but the help message was copy-pasted
from when it was a separate function and referenced the toolchain
function's help message.

Remove that reference now that it is pointing to itself.

R=brettw@chromium.org,dpranke@chromium.org
BUG= 634446 

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

[modify] https://crrev.com/fcd8d221139567226dc07b8b94ebeae1f2b94134/tools/gn/function_toolchain.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 11 2016

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

commit 5c2b9e90ae933ea112d9ca2f07c99ebf1f90d3b9
Author: nacl-deps-roller <nacl-deps-roller@chromium.org>
Date: Thu Aug 11 23:27:10 2016

Roll src/native_client/ ac69e80b3..105a5e3e1 (2 commits).

https://chromium.googlesource.com/native_client/src/native_client.git/+log/ac69e80b3ce9..105a5e3e176e

$ git log ac69e80b3..105a5e3e1 --date=short --no-merges --format='%ad %ae %s'
2016-08-11 brettw Update NaCl GN toolchain definitions.
2016-08-11 dschuff Use prebuilt CMake for target libs as well as host libs

BUG= 634446 

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

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

[modify] https://crrev.com/5c2b9e90ae933ea112d9ca2f07c99ebf1f90d3b9/DEPS

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 12 2016

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

commit ca677516fc34e7a5004ba013f99499403d5d9994
Author: brettw <brettw@chromium.org>
Date: Fri Aug 12 19:43:12 2016

Remove GN back-compat toolchain code for NaCl.

Now that the NaCl side of the toolchain update has rolled in (this was
https://codereview.chromium.org/2235083002/), the compatibility shim can be
removed.

BUG= 634446 

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

[modify] https://crrev.com/ca677516fc34e7a5004ba013f99499403d5d9994/build/toolchain/gcc_toolchain.gni

Comment 8 by brettw@chromium.org, Aug 12 2016

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 12 2016

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

commit b68d6d77e73624422dce58601eafad9d8e5d4f14
Author: brettw <brettw@chromium.org>
Date: Fri Aug 12 21:12:07 2016

Remove GN back-compat toolchain args code.

All callers of toolchain_args as a function have been update to use the new
variable. The shim code can be removed.

BUG= 634446 

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

[modify] https://crrev.com/b68d6d77e73624422dce58601eafad9d8e5d4f14/tools/gn/function_toolchain.cc
[modify] https://crrev.com/b68d6d77e73624422dce58601eafad9d8e5d4f14/tools/gn/functions.cc
[modify] https://crrev.com/b68d6d77e73624422dce58601eafad9d8e5d4f14/tools/gn/functions.h

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 19 2016

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

commit fe6025a7bcbba301acfe8a049c44b17f0b19efd5
Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com>
Date: Wed Oct 19 17:06:35 2016

gn docs: Update toolchain example in the language reference

Since https://codereview.chromium.org/2219083002 the syntax for
declaring toolchain arguments has changed, so update the
example:
- Use toolchain_args = {} instead of toolchain_args() {}
- Use current_cpu instead of toolchain_cpu.

R=brettw@chromium.org,dpranke@chromium.org
BUG= 634446 

Review-Url: https://chromiumcodereview.appspot.com/2430153004
Cr-Commit-Position: refs/heads/master@{#426228}

[modify] https://crrev.com/fe6025a7bcbba301acfe8a049c44b17f0b19efd5/tools/gn/docs/language.md

Sign in to add a comment