Replacing hard-coded clang version by a common variable.

Replacing hard-coded clang version by a common variable.

This variable will be used in the next CL to add ASAN support on mac.

BUG=

Committed: https://crrev.com/a913c3da4c4c6e00cb26775908b51fd0f79b77f8
Cr-Commit-Position: refs/heads/master@{#391345}

[aizatsky, 2016-04-25 20:24:43]

Description was changed from

==========
Replacing hard-coded clang version by a common variable.

This variable will be used in the next CL to add ASAN support on mac.

BUG=
==========

to

==========
Replacing hard-coded clang version by a common variable.

This variable will be used in the next CL to add ASAN support on mac.

BUG=
==========

[aizatsky, 2016-04-25 20:24:43]

aizatsky@chromium.org changed reviewers:
+ brettw@chromium.org

[aizatsky, 2016-04-29 22:30:02]

ping.

[brettw, 2016-05-02 22:35:10]

Sorry! This fell off my list.

[brettw, 2016-05-03 16:55:35]

lgtm

https://codereview.chromium.org/1921853002/diff/1/build/toolchain/toolchain.gni
File build/toolchain/toolchain.gni (right):

https://codereview.chromium.org/1921853002/diff/1/build/toolchain/toolchain.g...
build/toolchain/toolchain.gni:16: clang_version = "3.9.0"
Can this be an an is_clang condition so it's not defined in a non-clang build?

[aizatsky, 2016-05-03 17:21:24]

PTAL.

https://codereview.chromium.org/1921853002/diff/1/build/toolchain/toolchain.gni
File build/toolchain/toolchain.gni (right):

https://codereview.chromium.org/1921853002/diff/1/build/toolchain/toolchain.g...
build/toolchain/toolchain.gni:16: clang_version = "3.9.0"
On 2016/05/03 16:55:35, brettw wrote:
> Can this be an an is_clang condition so it's not defined in a non-clang build?

Done. Like this?

[brettw, 2016-05-03 18:03:25]

https://codereview.chromium.org/1921853002/diff/20001/build/toolchain/toolcha...
File build/toolchain/toolchain.gni (right):

https://codereview.chromium.org/1921853002/diff/20001/build/toolchain/toolcha...
build/toolchain/toolchain.gni:21: # Clang compiler version. Clang files are
placed at version-dependent paths.
I was thinking:
  declare_args() {
    ...

    if (is_clang) {
      # Clang compiler version. Clang files are placed at version-dependent
paths.
      clang_version = "3.9.0"
    }
  }

So this won't even be declared as a build arg, or defined at all, unless you're
actually using clang.

[aizatsky, 2016-05-03 18:07:31]

https://codereview.chromium.org/1921853002/diff/20001/build/toolchain/toolcha...
File build/toolchain/toolchain.gni (right):

https://codereview.chromium.org/1921853002/diff/20001/build/toolchain/toolcha...
build/toolchain/toolchain.gni:21: # Clang compiler version. Clang files are
placed at version-dependent paths.
On 2016/05/03 18:03:25, brettw wrote:
> I was thinking:
>   declare_args() {
>     ...
> 
>     if (is_clang) {
>       # Clang compiler version. Clang files are placed at version-dependent
> paths.
>       clang_version = "3.9.0"
>     }
>   }
> 
> So this won't even be declared as a build arg, or defined at all, unless
you're
> actually using clang.

Done. I didn't know conditionals are allowed in declare_args.

[aizatsky, 2016-05-03 18:08:11]

The CQ bit was checked by aizatsky@chromium.org

[aizatsky, 2016-05-03 18:08:11]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1921853002/#ps40001
(title: "wrapping clang_version in conditional.")

[commit-bot: I haz the power, 2016-05-03 18:08:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1921853002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1921853002/40001

[brettw, 2016-05-03 18:08:36]

lgtm

[commit-bot: I haz the power, 2016-05-03 18:19:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-03 18:19:33]

Try jobs failed on following builders:
  win_clang on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[aizatsky, 2016-05-03 18:34:00]

On 2016/05/03 18:19:33, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   win_clang on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

Brett,

Windows bot fails because win file doesn't include toolchain.gni
(https://code.google.com/p/chromium/codesearch#chromium/src/build/config/win/B...)

Do you think clang_version should be defined elsewhere or should I include
toolchain.gni in win/BUILD.gn?

[brettw, 2016-05-03 19:09:11]

> Do you think clang_version should be defined elsewhere or should I include
> toolchain.gni in win/BUILD.gn?

Just import the file.

[aizatsky, 2016-05-03 19:34:27]

The CQ bit was checked by aizatsky@chromium.org

[aizatsky, 2016-05-03 19:34:27]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1921853002/#ps60001
(title: "importing toolchain.gni")

[commit-bot: I haz the power, 2016-05-03 19:34:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1921853002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1921853002/60001

[commit-bot: I haz the power, 2016-05-03 20:35:50]

Description was changed from

==========
Replacing hard-coded clang version by a common variable.

This variable will be used in the next CL to add ASAN support on mac.

BUG=
==========

to

==========
Replacing hard-coded clang version by a common variable.

This variable will be used in the next CL to add ASAN support on mac.

BUG=
==========

[commit-bot: I haz the power, 2016-05-03 20:35:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-03 20:37:06]

Description was changed from

==========
Replacing hard-coded clang version by a common variable.

This variable will be used in the next CL to add ASAN support on mac.

BUG=
==========

to

==========
Replacing hard-coded clang version by a common variable.

This variable will be used in the next CL to add ASAN support on mac.

BUG=

Committed: https://crrev.com/a913c3da4c4c6e00cb26775908b51fd0f79b77f8
Cr-Commit-Position: refs/heads/master@{#391345}
==========

[commit-bot: I haz the power, 2016-05-03 20:37:07]

Patchset 4 (id:??) landed as
https://crrev.com/a913c3da4c4c6e00cb26775908b51fd0f79b77f8
Cr-Commit-Position: refs/heads/master@{#391345}